| Message ID | 20220622151918.451635-2-fsylvestre@baylibre.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Florian, On Wed, Jun 22, 2022 at 05:19:14PM +0200, Florian Sylvestre via libcamera-devel wrote: > Allow to retrieve a YAML list of any already supported types in a std::vector. > > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> > --- > include/libcamera/internal/yaml_parser.h | 16 ++++++ > src/libcamera/yaml_parser.cpp | 73 ++++++++++++++++++++++++ > test/yaml-parser.cpp | 6 ++ > 3 files changed, 95 insertions(+) > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h > index 064cf443..ea9189bd 100644 > --- a/include/libcamera/internal/yaml_parser.h > +++ b/include/libcamera/internal/yaml_parser.h > @@ -167,6 +167,22 @@ public: > #endif > T get(const T &defaultValue, bool *ok = nullptr) const; > > +#ifndef __DOXYGEN__ > + template<typename T, > + typename std::enable_if_t< > + std::is_same<bool, T>::value || > + std::is_same<double, T>::value || > + std::is_same<int16_t, T>::value || > + std::is_same<uint16_t, T>::value || > + std::is_same<int32_t, T>::value || > + std::is_same<uint32_t, T>::value || > + std::is_same<std::string, T>::value || > + std::is_same<Size, T>::value> * = nullptr> Should these be is_same_v ? Otherwise, looks good. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > +#else > + template<typename T> > +#endif > + std::vector<T> getList(bool *ok = nullptr) const; > + > DictAdapter asDict() const { return DictAdapter{ dictionary_ }; } > ListAdapter asList() const { return ListAdapter{ list_ }; } > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > index 5c45e44e..95c0038b 100644 > --- a/src/libcamera/yaml_parser.cpp > +++ b/src/libcamera/yaml_parser.cpp > @@ -117,6 +117,23 @@ std::size_t YamlObject::size() const > * \return Value as a bool type > */ > > +/** > + * \fn template<typename T> YamlObject::getList<T>( > + * bool *ok) const > + * \brief Parse the YamlObject as a list of \a T. > + * \param[out] ok The result of whether the parse succeeded > + * > + * This function parses the value of the YamlObject as a list of \a T objects, > + * and returns the value as a \a std::vector<T>. If parsing fails \a ok is set > + * to false and a default empty vector is returned. Otherwise, the > + * YamlObject list of values is returned, and \a ok is set to true. > + * > + * The \a ok pointer is optional and can be a nullptr if the caller doesn't > + * need to know if parsing succeeded. > + * > + * \return Value as a std::vector<T> type > + */ > + > #ifndef __DOXYGEN__ > > template<> > @@ -323,6 +340,62 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const > return Size(width, height); > } > > +template<typename T, > + typename std::enable_if_t< > + std::is_same<bool, T>::value || > + std::is_same<double, T>::value || > + std::is_same<int16_t, T>::value || > + std::is_same<uint16_t, T>::value || > + std::is_same<int32_t, T>::value || > + std::is_same<uint32_t, T>::value || > + std::is_same<std::string, T>::value || > + std::is_same<Size, T>::value> *> > +std::vector<T> YamlObject::getList(bool *ok) const > +{ > + setOk(ok, false); > + > + std::vector<T> defaultValue; > + > + if (type_ != Type::List) > + return defaultValue; > + > + std::vector<T> value = std::vector<T>(list_.size()); > + > + /* > + * Provide somme dummy defaultValue to get() function > + * and rely on the ok variable to know if the parsing > + * was correct. > + */ > + T defaultArgument = T(); > + bool result; > + for (unsigned int i = 0; i < list_.size(); ++i) { > + result = true; > + value[i] = list_[i]->get<T>(defaultArgument, &result); > + > + if (!result) > + return defaultValue; > + } > + > + setOk(ok, true); > + return value; > +} > + > +template std::vector<bool> YamlObject::getList<bool>(bool *ok) const; > + > +template std::vector<double> YamlObject::getList<double>(bool *ok) const; > + > +template std::vector<int16_t> YamlObject::getList<int16_t>(bool *ok) const; > + > +template std::vector<uint16_t> YamlObject::getList<uint16_t>(bool *ok) const; > + > +template std::vector<int32_t> YamlObject::getList<int32_t>(bool *ok) const; > + > +template std::vector<uint32_t> YamlObject::getList<uint32_t>(bool *ok) const; > + > +template std::vector<std::string> YamlObject::getList<std::string>(bool *ok) const; > + > +template std::vector<Size> YamlObject::getList<Size>(bool *ok) const; > + > #endif /* __DOXYGEN__ */ > > /** > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp > index 38f84823..bb96a11b 100644 > --- a/test/yaml-parser.cpp > +++ b/test/yaml-parser.cpp > @@ -524,6 +524,12 @@ protected: > return TestFail; > } > > + std::vector<uint16_t> values = firstElement.getList<uint16_t>(); > + if (values.size() != 2 || values[0] != 1 || values[1] != 2) { > + cerr << "getList() failed to return correct vector" << std::endl; > + return TestFail; > + } > + > auto &secondElement = level2Obj[1]; > if (!secondElement.isDictionary() || > !secondElement.contains("one") || > -- > 2.34.1 >
Hello, On Tue, Jul 12, 2022 at 04:01:38PM +0900, Paul Elder via libcamera-devel wrote: > On Wed, Jun 22, 2022 at 05:19:14PM +0200, Florian Sylvestre via libcamera-devel wrote: > > Allow to retrieve a YAML list of any already supported types in a std::vector. > > > > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> > > --- > > include/libcamera/internal/yaml_parser.h | 16 ++++++ > > src/libcamera/yaml_parser.cpp | 73 ++++++++++++++++++++++++ > > test/yaml-parser.cpp | 6 ++ > > 3 files changed, 95 insertions(+) > > > > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h > > index 064cf443..ea9189bd 100644 > > --- a/include/libcamera/internal/yaml_parser.h > > +++ b/include/libcamera/internal/yaml_parser.h > > @@ -167,6 +167,22 @@ public: > > #endif > > T get(const T &defaultValue, bool *ok = nullptr) const; > > > > +#ifndef __DOXYGEN__ > > + template<typename T, > > + typename std::enable_if_t< > > + std::is_same<bool, T>::value || > > + std::is_same<double, T>::value || > > + std::is_same<int16_t, T>::value || > > + std::is_same<uint16_t, T>::value || > > + std::is_same<int32_t, T>::value || > > + std::is_same<uint32_t, T>::value || > > + std::is_same<std::string, T>::value || > > + std::is_same<Size, T>::value> * = nullptr> > > Should these be is_same_v ? Yes, now that we use C++17, is_same_v is a nice shortcut. > Otherwise, looks good. > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > +#else > > + template<typename T> > > +#endif > > + std::vector<T> getList(bool *ok = nullptr) const; > > + > > DictAdapter asDict() const { return DictAdapter{ dictionary_ }; } > > ListAdapter asList() const { return ListAdapter{ list_ }; } > > > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > > index 5c45e44e..95c0038b 100644 > > --- a/src/libcamera/yaml_parser.cpp > > +++ b/src/libcamera/yaml_parser.cpp > > @@ -117,6 +117,23 @@ std::size_t YamlObject::size() const > > * \return Value as a bool type > > */ > > > > +/** > > + * \fn template<typename T> YamlObject::getList<T>( > > + * bool *ok) const No need for a line wrap. > > + * \brief Parse the YamlObject as a list of \a T. s/.$// > > + * \param[out] ok The result of whether the parse succeeded > > + * > > + * This function parses the value of the YamlObject as a list of \a T objects, > > + * and returns the value as a \a std::vector<T>. If parsing fails \a ok is set > > + * to false and a default empty vector is returned. Otherwise, the > > + * YamlObject list of values is returned, and \a ok is set to true. > > + * > > + * The \a ok pointer is optional and can be a nullptr if the caller doesn't > > + * need to know if parsing succeeded. > > + * > > + * \return Value as a std::vector<T> type > > + */ This should go just before the YamlObject::getList() function (with an additional #ifndef __DOXYGEN__ block for the function itself). > > + > > #ifndef __DOXYGEN__ > > > > template<> > > @@ -323,6 +340,62 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const > > return Size(width, height); > > } > > > > +template<typename T, > > + typename std::enable_if_t< > > + std::is_same<bool, T>::value || > > + std::is_same<double, T>::value || > > + std::is_same<int16_t, T>::value || > > + std::is_same<uint16_t, T>::value || > > + std::is_same<int32_t, T>::value || > > + std::is_same<uint32_t, T>::value || > > + std::is_same<std::string, T>::value || > > + std::is_same<Size, T>::value> *> > > +std::vector<T> YamlObject::getList(bool *ok) const > > +{ > > + setOk(ok, false); > > + > > + std::vector<T> defaultValue; > > + > > + if (type_ != Type::List) > > + return defaultValue; return {}; and the same below, and drop the defaultValue variable. > > + > > + std::vector<T> value = std::vector<T>(list_.size()); This will initialize all the entries in the vector, while you overwrite them all later. I would do std::vector<T> value; value.reserve(list_.size()); > > + > > + /* > > + * Provide somme dummy defaultValue to get() function > > + * and rely on the ok variable to know if the parsing > > + * was correct. I know I usually ask for line to be wrapped, but here you can expand to the 80 columns limit ;-) > > + */ > > + T defaultArgument = T(); Rename this to defaultValue, and you can write T defaultArgument{}; which avoid separate default contruction (T()) and copy construction (=) (it only makes a difference for std::string and Size). > > + bool result; You can move the variable inside the loop. > > + for (unsigned int i = 0; i < list_.size(); ++i) { > > + result = true; bool result; (no need to initialize it) > > + value[i] = list_[i]->get<T>(defaultArgument, &result); And here, to match the reserve() call, value.emplace_back(list_[i]->get<T>(defaultArgument, &result)); This allows using a for range loop: for (const YamlObject &entry : asList()) { bool result; value.emplace_back(entry.get<T>(defaultArgument, &result)); if (!result) return defaultValue; } > > + > > + if (!result) > > + return defaultValue; > > + } > > + > > + setOk(ok, true); > > + return value; > > +} > > + > > +template std::vector<bool> YamlObject::getList<bool>(bool *ok) const; > > + > > +template std::vector<double> YamlObject::getList<double>(bool *ok) const; > > + > > +template std::vector<int16_t> YamlObject::getList<int16_t>(bool *ok) const; > > + > > +template std::vector<uint16_t> YamlObject::getList<uint16_t>(bool *ok) const; > > + > > +template std::vector<int32_t> YamlObject::getList<int32_t>(bool *ok) const; > > + > > +template std::vector<uint32_t> YamlObject::getList<uint32_t>(bool *ok) const; > > + > > +template std::vector<std::string> YamlObject::getList<std::string>(bool *ok) const; > > + You can drop all the blank lines. > > +template std::vector<Size> YamlObject::getList<Size>(bool *ok) const; > > + > > #endif /* __DOXYGEN__ */ > > > > /** > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp > > index 38f84823..bb96a11b 100644 > > --- a/test/yaml-parser.cpp > > +++ b/test/yaml-parser.cpp > > @@ -524,6 +524,12 @@ protected: > > return TestFail; > > } > > > > + std::vector<uint16_t> values = firstElement.getList<uint16_t>(); > > + if (values.size() != 2 || values[0] != 1 || values[1] != 2) { > > + cerr << "getList() failed to return correct vector" << std::endl; > > + return TestFail; > > + } > > + > > auto &secondElement = level2Obj[1]; > > if (!secondElement.isDictionary() || > > !secondElement.contains("one") ||
diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h index 064cf443..ea9189bd 100644 --- a/include/libcamera/internal/yaml_parser.h +++ b/include/libcamera/internal/yaml_parser.h @@ -167,6 +167,22 @@ public: #endif T get(const T &defaultValue, bool *ok = nullptr) const; +#ifndef __DOXYGEN__ + template<typename T, + typename std::enable_if_t< + std::is_same<bool, T>::value || + std::is_same<double, T>::value || + std::is_same<int16_t, T>::value || + std::is_same<uint16_t, T>::value || + std::is_same<int32_t, T>::value || + std::is_same<uint32_t, T>::value || + std::is_same<std::string, T>::value || + std::is_same<Size, T>::value> * = nullptr> +#else + template<typename T> +#endif + std::vector<T> getList(bool *ok = nullptr) const; + DictAdapter asDict() const { return DictAdapter{ dictionary_ }; } ListAdapter asList() const { return ListAdapter{ list_ }; } diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index 5c45e44e..95c0038b 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -117,6 +117,23 @@ std::size_t YamlObject::size() const * \return Value as a bool type */ +/** + * \fn template<typename T> YamlObject::getList<T>( + * bool *ok) const + * \brief Parse the YamlObject as a list of \a T. + * \param[out] ok The result of whether the parse succeeded + * + * This function parses the value of the YamlObject as a list of \a T objects, + * and returns the value as a \a std::vector<T>. If parsing fails \a ok is set + * to false and a default empty vector is returned. Otherwise, the + * YamlObject list of values is returned, and \a ok is set to true. + * + * The \a ok pointer is optional and can be a nullptr if the caller doesn't + * need to know if parsing succeeded. + * + * \return Value as a std::vector<T> type + */ + #ifndef __DOXYGEN__ template<> @@ -323,6 +340,62 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const return Size(width, height); } +template<typename T, + typename std::enable_if_t< + std::is_same<bool, T>::value || + std::is_same<double, T>::value || + std::is_same<int16_t, T>::value || + std::is_same<uint16_t, T>::value || + std::is_same<int32_t, T>::value || + std::is_same<uint32_t, T>::value || + std::is_same<std::string, T>::value || + std::is_same<Size, T>::value> *> +std::vector<T> YamlObject::getList(bool *ok) const +{ + setOk(ok, false); + + std::vector<T> defaultValue; + + if (type_ != Type::List) + return defaultValue; + + std::vector<T> value = std::vector<T>(list_.size()); + + /* + * Provide somme dummy defaultValue to get() function + * and rely on the ok variable to know if the parsing + * was correct. + */ + T defaultArgument = T(); + bool result; + for (unsigned int i = 0; i < list_.size(); ++i) { + result = true; + value[i] = list_[i]->get<T>(defaultArgument, &result); + + if (!result) + return defaultValue; + } + + setOk(ok, true); + return value; +} + +template std::vector<bool> YamlObject::getList<bool>(bool *ok) const; + +template std::vector<double> YamlObject::getList<double>(bool *ok) const; + +template std::vector<int16_t> YamlObject::getList<int16_t>(bool *ok) const; + +template std::vector<uint16_t> YamlObject::getList<uint16_t>(bool *ok) const; + +template std::vector<int32_t> YamlObject::getList<int32_t>(bool *ok) const; + +template std::vector<uint32_t> YamlObject::getList<uint32_t>(bool *ok) const; + +template std::vector<std::string> YamlObject::getList<std::string>(bool *ok) const; + +template std::vector<Size> YamlObject::getList<Size>(bool *ok) const; + #endif /* __DOXYGEN__ */ /** diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 38f84823..bb96a11b 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -524,6 +524,12 @@ protected: return TestFail; } + std::vector<uint16_t> values = firstElement.getList<uint16_t>(); + if (values.size() != 2 || values[0] != 1 || values[1] != 2) { + cerr << "getList() failed to return correct vector" << std::endl; + return TestFail; + } + auto &secondElement = level2Obj[1]; if (!secondElement.isDictionary() || !secondElement.contains("one") ||
Allow to retrieve a YAML list of any already supported types in a std::vector. Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> --- include/libcamera/internal/yaml_parser.h | 16 ++++++ src/libcamera/yaml_parser.cpp | 73 ++++++++++++++++++++++++ test/yaml-parser.cpp | 6 ++ 3 files changed, 95 insertions(+)