[libcamera-devel] [PATCH v2 05/13] libcamera: controls: Generate a vector of enumerated values

[Kieran Bingham]

Hi Jacopo,

On 20/10/2020 19:05, Jacopo Mondi wrote:
> For each Control that support enumerated values generate a vector
> of ControlValues which contains the full list of values.
> 
> At the expense of a slight increase in memory occupation this change
> allows the construction of the ControlInfo associated with a Control
> from the values list, defaulting the minimum and maximum values
> reported by the ControlInfo.
> 

Personally I think this is a good way to handle this data.


> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/control_ids.cpp.in |  2 ++
>  utils/gen-controls.py            | 10 ++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> index 056645cfbdfb..ca0b5b22f899 100644
> --- a/src/libcamera/control_ids.cpp.in
> +++ b/src/libcamera/control_ids.cpp.in
> @@ -6,8 +6,10 @@
>   *
>   * This file is auto-generated. Do not edit.
>   */
> +#include <vector>
>  
>  #include <libcamera/control_ids.h>
> +#include <libcamera/controls.h>
>  
>  /**
>   * \file control_ids.h
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 93cb3885c3da..87036fe7dec1 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -100,6 +100,8 @@ ${description}
>  def generate_h(controls):
>      enum_template_start = string.Template('''enum ${name}Values {''')
>      enum_value_template = string.Template('''\t${name} = ${value},''')
> +    enum_list_start = string.Template('''static const std::vector<ControlValue> ${name}List = {''')

'List' appended to the name feels a bit odd.

Seeing the usage in the next patch,:

&controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeList)
&controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeList)
&controls::AeExposureMode, ControlInfo(controls::AeExposureModeList)

Throwing alternatives in to the bikeshed, because that's all it is, this
could be 'Values' or 'Enum' on the end.

To me, List could end up being a list of strings, or floats, or anything.

Values, could also be ... arbitrary numerical values, but matches the
name in the ControlInfo

Enum ... will be integer values ...


Bikeshed colours aside:

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

> +    enum_list_values = string.Template('''\tstatic_cast<int32_t>(${name}),''')
>      template = string.Template('''extern const Control<${type}> ${name};''')
>  
>      ctrls = []
> @@ -140,6 +142,14 @@ def generate_h(controls):
>                  target_ctrls.append(enum_value_template.substitute(value_info))
>              target_ctrls.append("};")
>  
> +            target_ctrls.append(enum_list_start.substitute(info))
> +            for entry in enum:
> +                value_info = {
> +                    'name': entry['name'],
> +                }
> +                target_ctrls.append(enum_list_values.substitute(value_info))
> +            target_ctrls.append("};")
> +
>          target_ctrls.append(template.substitute(info))
>          id_value += 1
>  
> 

-- 
Regards
--
Kieran