[PATCH v4 1/3] libcamera: controls: Add enum names and values map to ControlId

[Kieran Bingham]

Quoting Paul Elder (2024-09-16 00:24:18)
> Add to ControlId information about the names and values of enum, in the
> event that the ControlId is an enum type. This allows applications to
> query the ControlId for the names of the enum values, so that they can
> be displayed on a UI, for example. Without this, it was necessary to use
> macros of NameOfControlNameValueMap, which is difficult to use and is
> very inflexible.
> 
> There already exists a map from name -> value in generated code. Reuse
> this and pass it to the ControlId constructor, which in turn generates
> the reverse map. The reverse map is then exposed to applications.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> 


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
> Changes in v4:
> - improve commit message
> - remove forward map, as it won't be used
> - replace enumToString() with simply exposing the reverse map (as a
>   const map reference)
>   - simplify the code accordingly
> 
> Changes in v3:
> - s/nameValueMap/enumStrMap/
> - s/enumName/enumToString/
> 
> No change in v2
> ---
>  include/libcamera/controls.h     | 14 ++++++++------
>  src/libcamera/control_ids.cpp.in |  4 +++-
>  src/libcamera/controls.cpp       | 15 +++++++++++++++
>  3 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 7c2bb2872..96a774ccd 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -8,6 +8,7 @@
>  #pragma once
>  
>  #include <assert.h>
> +#include <map>
>  #include <optional>
>  #include <set>
>  #include <stdint.h>
> @@ -213,14 +214,13 @@ private:
>  class ControlId
>  {
>  public:
> -       ControlId(unsigned int id, const std::string &name, ControlType type)
> -               : id_(id), name_(name), type_(type)
> -       {
> -       }
> +       ControlId(unsigned int id, const std::string &name, ControlType type,
> +                 const std::map<std::string, int32_t> &enumStrMap = {});
>  
>         unsigned int id() const { return id_; }
>         const std::string &name() const { return name_; }
>         ControlType type() const { return type_; }
> +       const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; }
>  
>  private:
>         LIBCAMERA_DISABLE_COPY_AND_MOVE(ControlId)
> @@ -228,6 +228,8 @@ private:
>         unsigned int id_;
>         std::string name_;
>         ControlType type_;
> +       std::map<std::string, int32_t> enumStrMap_;
> +       std::map<int32_t, std::string> reverseMap_;
>  };
>  
>  static inline bool operator==(unsigned int lhs, const ControlId &rhs)
> @@ -256,8 +258,8 @@ class Control : public ControlId
>  public:
>         using type = T;
>  
> -       Control(unsigned int id, const char *name)
> -               : ControlId(id, name, details::control_type<std::remove_cv_t<T>>::value)
> +       Control(unsigned int id, const char *name, const std::map<std::string, int32_t> &enumStrMap = {})
> +               : ControlId(id, name, details::control_type<std::remove_cv_t<T>>::value, enumStrMap)
>         {
>         }
>  
> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> index 05c8fb385..3a2049311 100644
> --- a/src/libcamera/control_ids.cpp.in
> +++ b/src/libcamera/control_ids.cpp.in
> @@ -89,8 +89,10 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {
>         { "{{enum.name}}", {{enum.name}} },
>  {%- endfor %}
>  };
> -{% endif -%}
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", {{ctrl.name}}NameValueMap);
> +{% else -%}
>  extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}");
> +{% endif -%}
>  {%- endfor %}
>  
>  {% if vendor != 'libcamera' %}
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index dba744042..a46c431a1 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -389,7 +389,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \param[in] id The control numerical ID
>   * \param[in] name The control name
>   * \param[in] type The control data type
> + * \param[in] enumStrMap The map from enum names to values (optional)
>   */
> +ControlId::ControlId(unsigned int id, const std::string &name, ControlType type,
> +                    const std::map<std::string, int32_t> &enumStrMap)
> +       : id_(id), name_(name), type_(type), enumStrMap_(enumStrMap)
> +{
> +       for (const auto &pair : enumStrMap_)
> +               reverseMap_[pair.second] = pair.first;
> +}
>  
>  /**
>   * \fn unsigned int ControlId::id() const
> @@ -409,6 +417,12 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \return The control data type
>   */
>  
> +/**
> + * \fn const std::map<int32_t, std::string> &ControlId::enumerators() const
> + * \brief Retrieve the map of enum values to enum names
> + * \return The map of enum values to enum names
> + */
> +
>  /**
>   * \fn bool operator==(unsigned int lhs, const ControlId &rhs)
>   * \brief Compare a ControlId with a control numerical ID
> @@ -459,6 +473,7 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \brief Construct a Control instance
>   * \param[in] id The control numerical ID
>   * \param[in] name The control name
> + * \param[in] enumStrMap The map from enum names to values (optional)
>   *
>   * The control data type is automatically deduced from the template type T.
>   */
> -- 
> 2.39.2
>