[libcamera-devel] [PATCH 02/10] libcamera: controls: Parse 'values' in gen-controls.py

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Dec 04, 2019 at 02:20:58PM +0100, Jacopo Mondi wrote:
> In preparation to add libcamera Camera definition by re-using the
> control generation framework, augment the gen_controls.py script to
> support parsing the 'values' yaml tag and generate documentation and
> definition of possible values associated with a Control or a Property.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/gen-controls.py | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py
> index 940386cc68c8..ca843cb9b538 100755
> --- a/src/libcamera/gen-controls.py
> +++ b/src/libcamera/gen-controls.py
> @@ -11,6 +11,24 @@ import string
>  import sys
>  import yaml
>  
> +class ValueGenerator:
> +    doc_template = string.Template('''/**
> + * \\def ${name}
> + * \\brief ${description} */''')
> +    def_template = string.Template('''#define ${name} ${value}''')
> +
> +    def __init__(self, value):
> +        self.info = {
> +            'name': value['name'],
> +            'value': value['value'],
> +            'description': value['description']
> +        }
> +
> +    def generate_doc(self):
> +        return self.doc_template.substitute(self.info)
> +
> +    def generate_def(self):
> +        return self.def_template.substitute(self.info)

I think it makes sense to introduce classes in this python script, but
this one seems a bit strange to me as it's really about calling the
above two methods. I would just move the related code to the single
callers below.

I have more comments regarding the yaml format, but I'll reply to the
patch that introduces property_ids.yaml for that, which I think will
influence the code below.

>  
>  def snake_case(s):
>      return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')
> @@ -35,14 +53,23 @@ ${description}
>          description[0] = '\\brief ' + description[0]
>          description = '\n'.join([(line and ' * ' or ' *') + line for line in description])
>  
> +        try:
> +            values = ctrl['values']
> +        except KeyError:
> +            values = ""
> +
>          info = {
>              'name': name,
>              'type': ctrl['type'],
>              'description': description,
>              'id_name': id_name,
> +            'values' : values,
>          }
>  
> +        for v in values:
> +            ctrls_doc.append(ValueGenerator(v).generate_doc())
>          ctrls_doc.append(doc_template.substitute(info))
> +
>          ctrls_def.append(def_template.substitute(info))
>          ctrls_map.append('\t{ ' + id_name + ', &' + name + ' },')
>  
> @@ -66,11 +93,19 @@ def generate_h(controls):
>  
>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>  
> +        try:
> +            values = ctrl['values']
> +        except KeyError:
> +            values = ""
> +
>          info = {
>              'name': name,
>              'type': ctrl['type'],
> +            'values' : values,
>          }
>  
> +        for v in values:
> +            ctrls.append(ValueGenerator(v).generate_def())
>          ctrls.append(template.substitute(info))
>          id_value += 1
>  

-- 
Regards,

Laurent Pinchart