[libcamera-devel,6/9] utils: ipc: Add support for Flags
diff mbox series

Message ID 20220803112150.3040287-7-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • utils: ipc: Add support for enums and Flags
Related show

Commit Message

Paul Elder Aug. 3, 2022, 11:21 a.m. UTC
Add Flags<E> as a supported type in the IPA interface.

It is defined and used in mojom as "Flags_E_t", and the code generator
will convert it to "Flags<E>". It is usable and has been tested in
struct members, function input and output parameters, and Signal
parameters.

This does not add support for returning Flags as direct return values.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 .../core_ipa_interface.h.tmpl                 |  4 +++
 .../definition_functions.tmpl                 | 13 +++++++
 .../module_ipa_interface.h.tmpl               |  4 +++
 .../libcamera_templates/proxy_functions.tmpl  | 10 ++++--
 .../libcamera_templates/serializer.tmpl       |  4 +++
 .../generators/mojom_libcamera_generator.py   | 35 ++++++++++++++++++-
 6 files changed, 66 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Aug. 9, 2022, 1:59 a.m. UTC | #1
Hi Paul,

Thank you for the patch.

On Wed, Aug 03, 2022 at 08:21:47PM +0900, Paul Elder via libcamera-devel wrote:
> Add Flags<E> as a supported type in the IPA interface.
> 
> It is defined and used in mojom as "Flags_E_t", and the code generator
> will convert it to "Flags<E>". It is usable and has been tested in
> struct members, function input and output parameters, and Signal
> parameters.

Hmmm... mojom can parse the "<...>" syntax to some extend, as it
supports "array<T>" for instance. Would it be possible to extend that
with flags ?

> This does not add support for returning Flags as direct return values.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  .../core_ipa_interface.h.tmpl                 |  4 +++
>  .../definition_functions.tmpl                 | 13 +++++++
>  .../module_ipa_interface.h.tmpl               |  4 +++
>  .../libcamera_templates/proxy_functions.tmpl  | 10 ++++--
>  .../libcamera_templates/serializer.tmpl       |  4 +++
>  .../generators/mojom_libcamera_generator.py   | 35 ++++++++++++++++++-
>  6 files changed, 66 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
> index a565b59a..2fd55119 100644
> --- a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
> @@ -30,6 +30,10 @@ static const {{const.kind|name}} {{const.mojom_name}} = {{const.value}};
>  {{funcs.define_enum(enum)}}
>  {% endfor %}
>  
> +{% for flag in flags %}
> +{{funcs.define_flags(flag)}}
> +{% endfor %}
> +
>  {%- for struct in structs_gen_header %}
>  {{funcs.define_struct(struct)}}
>  {% endfor %}
> diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> index 94bb4918..3dbcfca0 100644
> --- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> @@ -16,6 +16,19 @@ enum {{enum.mojom_name}} {
>  };
>  {%- endmacro -%}
>  
> +{#
> + # \brief Generate Flags definition
> + #
> + # \param flags Enum object from which a Flags definition is to be generated
> + #}
> +{%- macro define_flags(flags) -%}
> +enum class {{flags|flags_name}} {
> +{%- for field in flags.fields %}
> +	{{field.mojom_name}} = {{field.numeric_value}},
> +{%- endfor %}
> +};
> +{%- endmacro -%}
> +
>  {#
>   # \brief Generate struct definition
>   #
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> index 415ec283..5e003849 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> @@ -48,6 +48,10 @@ enum class {{cmd_event_enum_name}} {
>  {{funcs.define_enum(enum)}}
>  {% endfor %}
>  
> +{% for flag in flags %}
> +{{funcs.define_flags(flag)}}
> +{% endfor %}
> +
>  {%- for struct in structs_nonempty %}
>  {{funcs.define_struct(struct)}}
>  {% endfor %}
> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> index dc35620f..c5e24323 100644
> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -59,7 +59,9 @@
>  {%- else %}
>  	std::tie({{param.mojom_name}}Buf, std::ignore) =
>  {%- endif %}
> -{%- if param|is_enum %}
> +{%- if param|is_flags %}
> +		IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}
> +{%- elif param|is_enum %}
>  		IPADataSerializer<uint32_t>::serialize(static_cast<uint32_t>({{param.mojom_name}})
>  {%- else %}
>  		IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
> @@ -102,7 +104,9 @@
>   #}
>  {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
>  {{"*" if pointer}}{{param.mojom_name}} =
> -{%- if param|is_enum %}
> +{%- if param|is_flags %}
> +IPADataSerializer<{{param|name_full}}>::deserialize(
> +{%- elif param|is_enum %}
>  static_cast<{{param|name_full}}>(IPADataSerializer<uint32_t>::deserialize(
>  {%- else %}
>  IPADataSerializer<{{param|name}}>::deserialize(
> @@ -130,7 +134,7 @@ IPADataSerializer<{{param|name}}>::deserialize(
>  {%- if param|needs_control_serializer %}
>  	&controlSerializer_
>  {%- endif -%}
> -){{")" if param|is_enum}};
> +){{")" if param|is_enum and not param|is_flags}};
>  {%- endmacro -%}
>  
>  
> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> index 77bae36f..eec75211 100644
> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -34,6 +34,8 @@
>  		std::tie({{field.mojom_name}}, std::ignore) =
>  	{%- if field|is_pod %}
>  			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
> +	{%- elif field|is_flags %}
> +			IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}});
>  	{%- elif field|is_enum %}
>  			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(data.{{field.mojom_name}});
>  	{%- endif %}
> @@ -96,6 +98,8 @@
>  		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
>  		{%- if field|is_pod %}
>  		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}});
> +		{%- elif field|is_flags %}
> +		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field_size}});
>  		{%- else %}
>  		ret.{{field.mojom_name}} = static_cast<{{field|name_full}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
>  		{%- endif %}
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index 753bfc73..22de6fdd 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -47,6 +47,8 @@ _bit_widths = {
>      mojom.DOUBLE: '64',
>  }
>  
> +_flags_re = r'^Flags_(.*)_t$'
> +
>  def ModuleName(path):
>      return path.split('/')[-1].split('.')[0]
>  
> @@ -74,6 +76,8 @@ def GetDefaultValue(element):
>          return element.default
>      if type(element.kind) == mojom.Kind:
>          return '0'
> +    if IsFlags(element):
> +        return ''
>      if mojom.IsEnumKind(element.kind):
>          return f'static_cast<{element.kind.mojom_name}>(0)'
>      if isinstance(element.kind, mojom.Struct) and \
> @@ -223,6 +227,16 @@ def IsEnum(element):
>  def IsFd(element):
>      return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "SharedFD"
>  
> +def IsFlags(element):
> +    if not hasattr(element, 'mojom_name'):
> +        return False
> +    if re.match(_flags_re, element.mojom_name):
> +        return True
> +    if hasattr(element, 'kind'):
> +        if hasattr(element.kind, 'mojom_name'):
> +            return True if re.match(_flags_re, element.kind.mojom_name) else False
> +    return False
> +
>  def IsMap(element):
>      return mojom.IsMapKind(element.kind)
>  
> @@ -251,9 +265,20 @@ def ByteWidthFromCppType(t):
>          raise Exception('invalid type')
>      return str(int(_bit_widths[key]) // 8)
>  
> +def FlagsName(element):
> +    if hasattr(element, 'mojom_name'):
> +        if re.match(_flags_re, element.mojom_name):
> +            return re.sub(_flags_re, lambda match: match.group(1), element.mojom_name)
> +    if hasattr(element, 'kind'):
> +        if hasattr(element.kind, 'mojom_name'):
> +            return re.sub(_flags_re, lambda match: match.group(1), element.kind.mojom_name)
> +    return None
>  
>  # Get the type name for a given element
>  def GetNameForElement(element):
> +    # Flags
> +    if IsFlags(element):
> +        return f'Flags<{FlagsName(element)}>'
>      # structs
>      if (mojom.IsEnumKind(element) or
>          mojom.IsInterfaceKind(element) or
> @@ -311,6 +336,11 @@ def GetFullNameForElement(element):
>  
>      if namespace_str == '':
>          return name
> +
> +    if IsFlags(element):
> +        name = re.match(r'^Flags<(.*)>$', name).group(1)
> +        return f'Flags<{namespace_str}::{name}>'
> +
>      return f'{namespace_str}::{name}'
>  
>  def ValidateZeroLength(l, s, cap=True):
> @@ -401,6 +431,7 @@ class Generator(generator.Generator):
>              'choose': Choose,
>              'comma_sep': CommaSep,
>              'default_value': GetDefaultValue,
> +            'flags_name': FlagsName,
>              'has_default_fields': HasDefaultFields,
>              'has_fd': HasFd,
>              'is_async': IsAsync,
> @@ -408,6 +439,7 @@ class Generator(generator.Generator):
>              'is_controls': IsControls,
>              'is_enum': IsEnum,
>              'is_fd': IsFd,
> +            'is_flags': IsFlags,
>              'is_map': IsMap,
>              'is_plain_struct': IsPlainStruct,
>              'is_pod': IsPod,
> @@ -433,7 +465,8 @@ class Generator(generator.Generator):
>              'cmd_enum_name': '_%sCmd' % self.module_name,
>              'cmd_event_enum_name': '_%sEventCmd' % self.module_name,
>              'consts': self.module.constants,
> -            'enums': self.module.enums,
> +            'enums': [enum for enum in self.module.enums if not IsFlags(enum)],
> +            'flags': [enum for enum in self.module.enums if IsFlags(enum)],
>              'has_array': len([x for x in self.module.kinds.keys() if x[0] == 'a']) > 0,
>              'has_map': len([x for x in self.module.kinds.keys() if x[0] == 'm']) > 0,
>              'has_namespace': self.module.mojom_namespace != '',

Patch
diff mbox series

diff --git a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
index a565b59a..2fd55119 100644
--- a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
+++ b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
@@ -30,6 +30,10 @@  static const {{const.kind|name}} {{const.mojom_name}} = {{const.value}};
 {{funcs.define_enum(enum)}}
 {% endfor %}
 
+{% for flag in flags %}
+{{funcs.define_flags(flag)}}
+{% endfor %}
+
 {%- for struct in structs_gen_header %}
 {{funcs.define_struct(struct)}}
 {% endfor %}
diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
index 94bb4918..3dbcfca0 100644
--- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
+++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
@@ -16,6 +16,19 @@  enum {{enum.mojom_name}} {
 };
 {%- endmacro -%}
 
+{#
+ # \brief Generate Flags definition
+ #
+ # \param flags Enum object from which a Flags definition is to be generated
+ #}
+{%- macro define_flags(flags) -%}
+enum class {{flags|flags_name}} {
+{%- for field in flags.fields %}
+	{{field.mojom_name}} = {{field.numeric_value}},
+{%- endfor %}
+};
+{%- endmacro -%}
+
 {#
  # \brief Generate struct definition
  #
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
index 415ec283..5e003849 100644
--- a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
+++ b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
@@ -48,6 +48,10 @@  enum class {{cmd_event_enum_name}} {
 {{funcs.define_enum(enum)}}
 {% endfor %}
 
+{% for flag in flags %}
+{{funcs.define_flags(flag)}}
+{% endfor %}
+
 {%- for struct in structs_nonempty %}
 {{funcs.define_struct(struct)}}
 {% endfor %}
diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
index dc35620f..c5e24323 100644
--- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
+++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
@@ -59,7 +59,9 @@ 
 {%- else %}
 	std::tie({{param.mojom_name}}Buf, std::ignore) =
 {%- endif %}
-{%- if param|is_enum %}
+{%- if param|is_flags %}
+		IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}
+{%- elif param|is_enum %}
 		IPADataSerializer<uint32_t>::serialize(static_cast<uint32_t>({{param.mojom_name}})
 {%- else %}
 		IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
@@ -102,7 +104,9 @@ 
  #}
 {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
 {{"*" if pointer}}{{param.mojom_name}} =
-{%- if param|is_enum %}
+{%- if param|is_flags %}
+IPADataSerializer<{{param|name_full}}>::deserialize(
+{%- elif param|is_enum %}
 static_cast<{{param|name_full}}>(IPADataSerializer<uint32_t>::deserialize(
 {%- else %}
 IPADataSerializer<{{param|name}}>::deserialize(
@@ -130,7 +134,7 @@  IPADataSerializer<{{param|name}}>::deserialize(
 {%- if param|needs_control_serializer %}
 	&controlSerializer_
 {%- endif -%}
-){{")" if param|is_enum}};
+){{")" if param|is_enum and not param|is_flags}};
 {%- endmacro -%}
 
 
diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl
index 77bae36f..eec75211 100644
--- a/utils/ipc/generators/libcamera_templates/serializer.tmpl
+++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl
@@ -34,6 +34,8 @@ 
 		std::tie({{field.mojom_name}}, std::ignore) =
 	{%- if field|is_pod %}
 			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
+	{%- elif field|is_flags %}
+			IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}});
 	{%- elif field|is_enum %}
 			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(data.{{field.mojom_name}});
 	{%- endif %}
@@ -96,6 +98,8 @@ 
 		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
 		{%- if field|is_pod %}
 		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}});
+		{%- elif field|is_flags %}
+		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field_size}});
 		{%- else %}
 		ret.{{field.mojom_name}} = static_cast<{{field|name_full}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
 		{%- endif %}
diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
index 753bfc73..22de6fdd 100644
--- a/utils/ipc/generators/mojom_libcamera_generator.py
+++ b/utils/ipc/generators/mojom_libcamera_generator.py
@@ -47,6 +47,8 @@  _bit_widths = {
     mojom.DOUBLE: '64',
 }
 
+_flags_re = r'^Flags_(.*)_t$'
+
 def ModuleName(path):
     return path.split('/')[-1].split('.')[0]
 
@@ -74,6 +76,8 @@  def GetDefaultValue(element):
         return element.default
     if type(element.kind) == mojom.Kind:
         return '0'
+    if IsFlags(element):
+        return ''
     if mojom.IsEnumKind(element.kind):
         return f'static_cast<{element.kind.mojom_name}>(0)'
     if isinstance(element.kind, mojom.Struct) and \
@@ -223,6 +227,16 @@  def IsEnum(element):
 def IsFd(element):
     return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "SharedFD"
 
+def IsFlags(element):
+    if not hasattr(element, 'mojom_name'):
+        return False
+    if re.match(_flags_re, element.mojom_name):
+        return True
+    if hasattr(element, 'kind'):
+        if hasattr(element.kind, 'mojom_name'):
+            return True if re.match(_flags_re, element.kind.mojom_name) else False
+    return False
+
 def IsMap(element):
     return mojom.IsMapKind(element.kind)
 
@@ -251,9 +265,20 @@  def ByteWidthFromCppType(t):
         raise Exception('invalid type')
     return str(int(_bit_widths[key]) // 8)
 
+def FlagsName(element):
+    if hasattr(element, 'mojom_name'):
+        if re.match(_flags_re, element.mojom_name):
+            return re.sub(_flags_re, lambda match: match.group(1), element.mojom_name)
+    if hasattr(element, 'kind'):
+        if hasattr(element.kind, 'mojom_name'):
+            return re.sub(_flags_re, lambda match: match.group(1), element.kind.mojom_name)
+    return None
 
 # Get the type name for a given element
 def GetNameForElement(element):
+    # Flags
+    if IsFlags(element):
+        return f'Flags<{FlagsName(element)}>'
     # structs
     if (mojom.IsEnumKind(element) or
         mojom.IsInterfaceKind(element) or
@@ -311,6 +336,11 @@  def GetFullNameForElement(element):
 
     if namespace_str == '':
         return name
+
+    if IsFlags(element):
+        name = re.match(r'^Flags<(.*)>$', name).group(1)
+        return f'Flags<{namespace_str}::{name}>'
+
     return f'{namespace_str}::{name}'
 
 def ValidateZeroLength(l, s, cap=True):
@@ -401,6 +431,7 @@  class Generator(generator.Generator):
             'choose': Choose,
             'comma_sep': CommaSep,
             'default_value': GetDefaultValue,
+            'flags_name': FlagsName,
             'has_default_fields': HasDefaultFields,
             'has_fd': HasFd,
             'is_async': IsAsync,
@@ -408,6 +439,7 @@  class Generator(generator.Generator):
             'is_controls': IsControls,
             'is_enum': IsEnum,
             'is_fd': IsFd,
+            'is_flags': IsFlags,
             'is_map': IsMap,
             'is_plain_struct': IsPlainStruct,
             'is_pod': IsPod,
@@ -433,7 +465,8 @@  class Generator(generator.Generator):
             'cmd_enum_name': '_%sCmd' % self.module_name,
             'cmd_event_enum_name': '_%sEventCmd' % self.module_name,
             'consts': self.module.constants,
-            'enums': self.module.enums,
+            'enums': [enum for enum in self.module.enums if not IsFlags(enum)],
+            'flags': [enum for enum in self.module.enums if IsFlags(enum)],
             'has_array': len([x for x in self.module.kinds.keys() if x[0] == 'a']) > 0,
             'has_map': len([x for x in self.module.kinds.keys() if x[0] == 'm']) > 0,
             'has_namespace': self.module.mojom_namespace != '',