[libcamera-devel,1/4] utils: gen-controls: Factor out YAML data handling in classes

17062 diff mbox series

The gen-controls.py script handles the data structure produced by the
YAML parser manually through the whole code base. Clean this up by
encapsulating it in Control and ControlEnum classes to model a control
and its enum values respectively, to decouple YAML data handling from
generation.

No functional change intended.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 utils/gen-controls.py | 156 ++++++++++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 50 deletions(-)

Hi Laurent

On Wed, Aug 10, 2022 at 03:29:03AM +0300, Laurent Pinchart via libcamera-devel wrote:
> The gen-controls.py script handles the data structure produced by the
> YAML parser manually through the whole code base. Clean this up by
> encapsulating it in Control and ControlEnum classes to model a control
> and its enum values respectively, to decouple YAML data handling from
> generation.
>
> No functional change intended.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  utils/gen-controls.py | 156 ++++++++++++++++++++++++++++--------------
>  1 file changed, 106 insertions(+), 50 deletions(-)
>
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 46ba439439ec..f6a3ae39cc84 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -14,6 +14,90 @@ import sys
>  import yaml
>
>
> +class ControlEnum(object):
> +    def __init__(self, data):
> +        self.__data = data
> +
> +    @property
> +    def description(self):
> +        """The enum description"""
> +        return self.__data.get('description')
> +
> +    @property
> +    def name(self):
> +        """The enum name"""
> +        return self.__data.get('name')
> +
> +    @property
> +    def value(self):
> +        """The enum value"""
> +        return self.__data.get('value')
> +
> +
> +class Control(object):
> +    def __init__(self, name, data):
> +        self.__name = name
> +        self.__data = data
> +        self.__enum = None
> +
> +        enum_values = data.get('enum')
> +        if enum_values is not None:
> +            self.__enum_values = [ControlEnum(enum) for enum in enum_values]
> +
> +    @property
> +    def description(self):
> +        """The control description"""
> +        return self.__data.get('description')
> +
> +    @property
> +    def enum_values(self):
> +        """The enum values, if the control is an enumeration"""
> +        if self.__enum_values is None:
> +            return
> +        for enum in self.__enum_values:
> +            yield enum
> +
> +    @property
> +    def is_enum(self):
> +        """Is the control an enumeration"""
> +        return self.__data.get('enum') is not None

Can you use "self.__enum_values is not None" ?

> +
> +    @property
> +    def is_draft(self):
> +        """Is the control a draft control"""
> +        return self.__data.get('draft') is not None
> +
> +    @property
> +    def name(self):
> +        """The control name (CameCase)"""

"CamelCase"

> +        return self.__name
> +
> +    @property
> +    def q_name(self):
> +        """The control name, qualified with a namespace"""
> +        ns = 'draft::' if self.is_draft else ''
> +        return ns + self.__name
> +
> +    @property
> +    def type(self):
> +        typ = self.__data.get('type')
> +        size = self.__data.get('size')
> +
> +        if typ == 'string':
> +            return 'std::string'
> +
> +        if size is None:
> +            return typ
> +
> +        if len(size) > 0:
> +            # fixed-sized Span
> +            span_size = reduce(operator.mul, size)
> +            return f"Span<const {typ}, {span_size}>"
> +        else:
> +            # variable-sized Span
> +            return f"Span<const {typ}>"
> +
> +
>  def snake_case(s):
>      return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')
>
> @@ -24,24 +108,6 @@ def format_description(description):
>      return '\n'.join([(line and ' * ' or ' *') + line for line in description])
>
>
> -def get_ctrl_type(ctrl):
> -    ctrl_type = ctrl['type']
> -    ctrl_size_arr = ctrl.get('size')
> -
> -    if ctrl_type == 'string':
> -        return 'std::string'
> -    elif ctrl_size_arr is not None:
> -        if len(ctrl_size_arr) > 0:
> -            # fixed-sized Span
> -            ctrl_span_size = reduce(operator.mul, ctrl_size_arr)
> -            return f"Span<const {ctrl_type}, {ctrl_span_size}>"
> -        else:
> -            # variable-sized Span
> -            return f"Span<const {ctrl_type}>"
> -    else:
> -        return ctrl_type
> -
> -
>  def generate_cpp(controls):
>      enum_doc_start_template = string.Template('''/**
>   * \\enum ${name}Enum
> @@ -67,35 +133,31 @@ ${description}
>      ctrls_map = []
>
>      for ctrl in controls:
> -        name, ctrl = ctrl.popitem()
> -        id_name = snake_case(name).upper()
> -
> -        ctrl_type = get_ctrl_type(ctrl)
> +        id_name = snake_case(ctrl.name).upper()
>
>          info = {
> -            'name': name,
> -            'type': ctrl_type,
> -            'description': format_description(ctrl['description']),
> +            'name': ctrl.name,
> +            'type': ctrl.type,
> +            'description': format_description(ctrl.description),
>              'id_name': id_name,
>          }
>
>          target_doc = ctrls_doc
>          target_def = ctrls_def
> -        if ctrl.get('draft'):
> +        if ctrl.is_draft:
>              target_doc = draft_ctrls_doc
>              target_def = draft_ctrls_def
>
> -        enum = ctrl.get('enum')
> -        if enum:
> +        if ctrl.is_enum:
>              enum_doc = []
>              enum_doc.append(enum_doc_start_template.substitute(info))
>
>              num_entries = 0
> -            for entry in enum:
> +            for enum in ctrl.enum_values:
>                  value_info = {
> -                    'name': name,
> -                    'value': entry['name'],
> -                    'description': format_description(entry['description']),
> +                    'name': ctrl.name,
> +                    'value': enum.name,
> +                    'description': format_description(enum.description),
>                  }
>                  enum_doc.append(enum_doc_value_template.substitute(value_info))
>                  num_entries += 1
> @@ -110,9 +172,9 @@ ${description}
>              }
>              target_doc.append(enum_values_doc.substitute(values_info))
>              target_def.append(enum_values_start.substitute(values_info))
> -            for entry in enum:
> +            for enum in ctrl.enum_values:
>                  value_info = {
> -                    'name': entry['name']
> +                    'name': enum.name
>                  }
>                  target_def.append(enum_values_values.substitute(value_info))
>              target_def.append("};")
> @@ -120,10 +182,7 @@ ${description}
>          target_doc.append(doc_template.substitute(info))
>          target_def.append(def_template.substitute(info))
>
> -        if ctrl.get('draft'):
> -            name = 'draft::' + name
> -
> -        ctrls_map.append('\t{ ' + id_name + ', &' + name + ' },')
> +        ctrls_map.append('\t{ ' + id_name + ', &' + ctrl.q_name + ' },')
>
>      return {
>          'controls_doc': '\n\n'.join(ctrls_doc),
> @@ -146,31 +205,27 @@ def generate_h(controls):
>      id_value = 1
>
>      for ctrl in controls:
> -        name, ctrl = ctrl.popitem()
> -        id_name = snake_case(name).upper()
> +        id_name = snake_case(ctrl.name).upper()
>
>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>
> -        ctrl_type = get_ctrl_type(ctrl)
> -
>          info = {
> -            'name': name,
> -            'type': ctrl_type,
> +            'name': ctrl.name,
> +            'type': ctrl.type,
>          }
>
>          target_ctrls = ctrls
> -        if ctrl.get('draft'):
> +        if ctrl.is_draft:
>              target_ctrls = draft_ctrls
>
> -        enum = ctrl.get('enum')
> -        if enum:
> +        if ctrl.is_enum:
>              target_ctrls.append(enum_template_start.substitute(info))
>
>              num_entries = 0
> -            for entry in enum:
> +            for enum in ctrl.enum_values:
>                  value_info = {
> -                    'name': entry['name'],
> -                    'value': entry['value'],
> +                    'name': enum.name,
> +                    'value': enum.value,
>                  }
>                  target_ctrls.append(enum_value_template.substitute(value_info))
>                  num_entries += 1
> @@ -214,6 +269,7 @@ def main(argv):
>
>      data = open(args.input, 'rb').read()
>      controls = yaml.safe_load(data)['controls']
> +    controls = [Control(*ctrl.popitem()) for ctrl in controls]

Very nice!

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

Thanks
   j

>
>      if args.template.endswith('.cpp.in'):
>          data = generate_cpp(controls)
> --
> Regards,
>
> Laurent Pinchart
>

Hi Jacopo,

On Wed, Aug 10, 2022 at 10:22:54AM +0200, Jacopo Mondi wrote:
> On Wed, Aug 10, 2022 at 03:29:03AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > The gen-controls.py script handles the data structure produced by the
> > YAML parser manually through the whole code base. Clean this up by
> > encapsulating it in Control and ControlEnum classes to model a control
> > and its enum values respectively, to decouple YAML data handling from
> > generation.
> >
> > No functional change intended.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  utils/gen-controls.py | 156 ++++++++++++++++++++++++++++--------------
> >  1 file changed, 106 insertions(+), 50 deletions(-)
> >
> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> > index 46ba439439ec..f6a3ae39cc84 100755
> > --- a/utils/gen-controls.py
> > +++ b/utils/gen-controls.py
> > @@ -14,6 +14,90 @@ import sys
> >  import yaml
> >
> >
> > +class ControlEnum(object):
> > +    def __init__(self, data):
> > +        self.__data = data
> > +
> > +    @property
> > +    def description(self):
> > +        """The enum description"""
> > +        return self.__data.get('description')
> > +
> > +    @property
> > +    def name(self):
> > +        """The enum name"""
> > +        return self.__data.get('name')
> > +
> > +    @property
> > +    def value(self):
> > +        """The enum value"""
> > +        return self.__data.get('value')
> > +
> > +
> > +class Control(object):
> > +    def __init__(self, name, data):
> > +        self.__name = name
> > +        self.__data = data
> > +        self.__enum = None
> > +
> > +        enum_values = data.get('enum')
> > +        if enum_values is not None:
> > +            self.__enum_values = [ControlEnum(enum) for enum in enum_values]
> > +
> > +    @property
> > +    def description(self):
> > +        """The control description"""
> > +        return self.__data.get('description')
> > +
> > +    @property
> > +    def enum_values(self):
> > +        """The enum values, if the control is an enumeration"""
> > +        if self.__enum_values is None:
> > +            return
> > +        for enum in self.__enum_values:
> > +            yield enum
> > +
> > +    @property
> > +    def is_enum(self):
> > +        """Is the control an enumeration"""
> > +        return self.__data.get('enum') is not None
> 
> Can you use "self.__enum_values is not None" ?

Good idea. I'll address this and fix the typo below.

> > +
> > +    @property
> > +    def is_draft(self):
> > +        """Is the control a draft control"""
> > +        return self.__data.get('draft') is not None
> > +
> > +    @property
> > +    def name(self):
> > +        """The control name (CameCase)"""
> 
> "CamelCase"
> 
> > +        return self.__name
> > +
> > +    @property
> > +    def q_name(self):
> > +        """The control name, qualified with a namespace"""
> > +        ns = 'draft::' if self.is_draft else ''
> > +        return ns + self.__name
> > +
> > +    @property
> > +    def type(self):
> > +        typ = self.__data.get('type')
> > +        size = self.__data.get('size')
> > +
> > +        if typ == 'string':
> > +            return 'std::string'
> > +
> > +        if size is None:
> > +            return typ
> > +
> > +        if len(size) > 0:
> > +            # fixed-sized Span
> > +            span_size = reduce(operator.mul, size)
> > +            return f"Span<const {typ}, {span_size}>"
> > +        else:
> > +            # variable-sized Span
> > +            return f"Span<const {typ}>"
> > +
> > +
> >  def snake_case(s):
> >      return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')
> >
> > @@ -24,24 +108,6 @@ def format_description(description):
> >      return '\n'.join([(line and ' * ' or ' *') + line for line in description])
> >
> >
> > -def get_ctrl_type(ctrl):
> > -    ctrl_type = ctrl['type']
> > -    ctrl_size_arr = ctrl.get('size')
> > -
> > -    if ctrl_type == 'string':
> > -        return 'std::string'
> > -    elif ctrl_size_arr is not None:
> > -        if len(ctrl_size_arr) > 0:
> > -            # fixed-sized Span
> > -            ctrl_span_size = reduce(operator.mul, ctrl_size_arr)
> > -            return f"Span<const {ctrl_type}, {ctrl_span_size}>"
> > -        else:
> > -            # variable-sized Span
> > -            return f"Span<const {ctrl_type}>"
> > -    else:
> > -        return ctrl_type
> > -
> > -
> >  def generate_cpp(controls):
> >      enum_doc_start_template = string.Template('''/**
> >   * \\enum ${name}Enum
> > @@ -67,35 +133,31 @@ ${description}
> >      ctrls_map = []
> >
> >      for ctrl in controls:
> > -        name, ctrl = ctrl.popitem()
> > -        id_name = snake_case(name).upper()
> > -
> > -        ctrl_type = get_ctrl_type(ctrl)
> > +        id_name = snake_case(ctrl.name).upper()
> >
> >          info = {
> > -            'name': name,
> > -            'type': ctrl_type,
> > -            'description': format_description(ctrl['description']),
> > +            'name': ctrl.name,
> > +            'type': ctrl.type,
> > +            'description': format_description(ctrl.description),
> >              'id_name': id_name,
> >          }
> >
> >          target_doc = ctrls_doc
> >          target_def = ctrls_def
> > -        if ctrl.get('draft'):
> > +        if ctrl.is_draft:
> >              target_doc = draft_ctrls_doc
> >              target_def = draft_ctrls_def
> >
> > -        enum = ctrl.get('enum')
> > -        if enum:
> > +        if ctrl.is_enum:
> >              enum_doc = []
> >              enum_doc.append(enum_doc_start_template.substitute(info))
> >
> >              num_entries = 0
> > -            for entry in enum:
> > +            for enum in ctrl.enum_values:
> >                  value_info = {
> > -                    'name': name,
> > -                    'value': entry['name'],
> > -                    'description': format_description(entry['description']),
> > +                    'name': ctrl.name,
> > +                    'value': enum.name,
> > +                    'description': format_description(enum.description),
> >                  }
> >                  enum_doc.append(enum_doc_value_template.substitute(value_info))
> >                  num_entries += 1
> > @@ -110,9 +172,9 @@ ${description}
> >              }
> >              target_doc.append(enum_values_doc.substitute(values_info))
> >              target_def.append(enum_values_start.substitute(values_info))
> > -            for entry in enum:
> > +            for enum in ctrl.enum_values:
> >                  value_info = {
> > -                    'name': entry['name']
> > +                    'name': enum.name
> >                  }
> >                  target_def.append(enum_values_values.substitute(value_info))
> >              target_def.append("};")
> > @@ -120,10 +182,7 @@ ${description}
> >          target_doc.append(doc_template.substitute(info))
> >          target_def.append(def_template.substitute(info))
> >
> > -        if ctrl.get('draft'):
> > -            name = 'draft::' + name
> > -
> > -        ctrls_map.append('\t{ ' + id_name + ', &' + name + ' },')
> > +        ctrls_map.append('\t{ ' + id_name + ', &' + ctrl.q_name + ' },')
> >
> >      return {
> >          'controls_doc': '\n\n'.join(ctrls_doc),
> > @@ -146,31 +205,27 @@ def generate_h(controls):
> >      id_value = 1
> >
> >      for ctrl in controls:
> > -        name, ctrl = ctrl.popitem()
> > -        id_name = snake_case(name).upper()
> > +        id_name = snake_case(ctrl.name).upper()
> >
> >          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
> >
> > -        ctrl_type = get_ctrl_type(ctrl)
> > -
> >          info = {
> > -            'name': name,
> > -            'type': ctrl_type,
> > +            'name': ctrl.name,
> > +            'type': ctrl.type,
> >          }
> >
> >          target_ctrls = ctrls
> > -        if ctrl.get('draft'):
> > +        if ctrl.is_draft:
> >              target_ctrls = draft_ctrls
> >
> > -        enum = ctrl.get('enum')
> > -        if enum:
> > +        if ctrl.is_enum:
> >              target_ctrls.append(enum_template_start.substitute(info))
> >
> >              num_entries = 0
> > -            for entry in enum:
> > +            for enum in ctrl.enum_values:
> >                  value_info = {
> > -                    'name': entry['name'],
> > -                    'value': entry['value'],
> > +                    'name': enum.name,
> > +                    'value': enum.value,
> >                  }
> >                  target_ctrls.append(enum_value_template.substitute(value_info))
> >                  num_entries += 1
> > @@ -214,6 +269,7 @@ def main(argv):
> >
> >      data = open(args.input, 'rb').read()
> >      controls = yaml.safe_load(data)['controls']
> > +    controls = [Control(*ctrl.popitem()) for ctrl in controls]
> 
> Very nice!
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> >
> >      if args.template.endswith('.cpp.in'):
> >          data = generate_cpp(controls)