[libcamera-devel] [PATCH v5 3/9] libcamera: yaml_parser: Add getList() function

[Jacopo Mondi]

Hi Laurent,

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)

> +			 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 ?

> +	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 ?
If not

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> +		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
>