[libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add getList() function
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jul 28 12:27:37 CEST 2022
Hi Jacopo,
On Thu, Jul 28, 2022 at 09:33:11AM +0200, Jacopo Mondi wrote:
> On Thu, Jul 28, 2022 at 01:21:43AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Florian Sylvestre <fsylvestre at baylibre.com>
> >
> > Allow to retrieve a YAML list of any already supported types in a
> > std::vector.
> >
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > Tested-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > include/libcamera/internal/yaml_parser.h | 16 +++++++
> > src/libcamera/yaml_parser.cpp | 53 ++++++++++++++++++++++++
> > test/yaml-parser.cpp | 6 +++
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> > index 61f2223223a7..4b3da0f03512 100644
> > --- a/include/libcamera/internal/yaml_parser.h
> > +++ b/include/libcamera/internal/yaml_parser.h
> > @@ -174,6 +174,22 @@ public:
> > return get<T>().value_or(defaultValue);
> > }
> >
> > +#ifndef __DOXYGEN__
> > + template<typename T,
> > + typename std::enable_if_t<
>
> nit: as you're using the enable_if_t helper, I guess the 'typename' keyword
> can be dropped ? (gcc agrees)
You're right. We have many occurrences of this through the code base,
I'll fix them all on top of this series.
> > + std::is_same_v<bool, T> ||
> > + std::is_same_v<double, T> ||
> > + std::is_same_v<int16_t, T> ||
> > + std::is_same_v<uint16_t, T> ||
> > + std::is_same_v<int32_t, T> ||
> > + std::is_same_v<uint32_t, T> ||
> > + std::is_same_v<std::string, T> ||
> > + std::is_same_v<Size, T>> * = nullptr>
> > +#else
> > + template<typename T>
> > +#endif
>
> Why this #ifdef ? There are no overloads to confuse doxygen with,
> right ?
It's to avoid the enable_if appearing in the generated documentation.
Otherwise it gets rendered as
◆ getList()
template<typename T , typename std::enable_if_t< std::is_same_v< bool, T >||std::is_same_v< double, T >||std::is_same_v< int16_t, T >||std::is_same_v< uint16_t, T >||std::is_same_v< int32_t, T >||std::is_same_v< uint32_t, T >||std::is_same_v< std::string, T >||std::is_same_v< Size, T > > * = nullptr>
template< typename T > libcamera::YamlObject::getList< T > ( ) const
which isn't very nice. We do the same in other places, I can reconsider
this, but I'd then like to do it globally.
> > + std::optional<std::vector<T>> getList() 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 4299f5abd38a..cb4f1eb29a77 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -293,6 +293,59 @@ std::optional<Size> YamlObject::get() const
> >
> > #endif /* __DOXYGEN__ */
> >
> > +/**
> > + * \fn template<typename T> YamlObject::getList<T>() const
> > + * \brief Parse the YamlObject as a list of \a T
> > + *
> > + * 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, std::nullopt
> > + * is returned.
> > + *
> > + * \return The YamlObject value as a std::vector<T>, or std::nullopt if parsing
> > + * failed
> > + */
> > +
> > +#ifndef __DOXYGEN__
> > +
> > +template<typename T,
> > + typename std::enable_if_t<
> > + std::is_same_v<bool, T> ||
> > + std::is_same_v<double, T> ||
> > + std::is_same_v<int16_t, T> ||
> > + std::is_same_v<uint16_t, T> ||
> > + std::is_same_v<int32_t, T> ||
> > + std::is_same_v<uint32_t, T> ||
> > + std::is_same_v<std::string, T> ||
> > + std::is_same_v<Size, T>> *>
> > +std::optional<std::vector<T>> YamlObject::getList() const
> > +{
> > + if (type_ != Type::List)
> > + return {};
> > +
> > + std::vector<T> values;
> > + values.reserve(list_.size());
> > +
> > + for (const YamlObject &entry : asList()) {
> > + const auto value = entry.get<T>();
>
> Should we be concerned about the type of value ?
I'm not sure what you mean. Would you rather write
const std::optional<T> value = entry.get<T>();
or am I missing your point ?
> If not
>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > + if (!value)
> > + return {};
> > + values.emplace_back(*value);
> > + }
> > +
> > + return values;
> > +}
> > +
> > +template std::optional<std::vector<bool>> YamlObject::getList<bool>() const;
> > +template std::optional<std::vector<double>> YamlObject::getList<double>() const;
> > +template std::optional<std::vector<int16_t>> YamlObject::getList<int16_t>() const;
> > +template std::optional<std::vector<uint16_t>> YamlObject::getList<uint16_t>() const;
> > +template std::optional<std::vector<int32_t>> YamlObject::getList<int32_t>() const;
> > +template std::optional<std::vector<uint32_t>> YamlObject::getList<uint32_t>() const;
> > +template std::optional<std::vector<std::string>> YamlObject::getList<std::string>() const;
> > +template std::optional<std::vector<Size>> YamlObject::getList<Size>() const;
> > +
> > +#endif /* __DOXYGEN__ */
> > +
> > /**
> > * \fn YamlObject::asDict() const
> > * \brief Wrap a dictionary YamlObject in an adapter that exposes iterators
> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> > index ebb654f2ef9c..bb54e0d7f76a 100644
> > --- a/test/yaml-parser.cpp
> > +++ b/test/yaml-parser.cpp
> > @@ -523,6 +523,12 @@ protected:
> > return TestFail;
> > }
> >
> > + const auto &values = firstElement.getList<uint16_t>();
> > + if (!values || 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") ||
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list