[libcamera-devel,2/5] generate fixed- and variable-sized Span Controls
diff mbox series

Message ID 20220401000616.12976-3-Rauch.Christian@gmx.de
State Superseded
Headers show
Series
  • generate and use fixed-sized Span Control types
Related show

Commit Message

Christian Rauch April 1, 2022, 12:06 a.m. UTC
This defines Controls with a 'size' as either variable-sized Span<T> or as
fixed-sized Span<T,N>.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 utils/gen-controls.py | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

--
2.25.1

Comments

Jacopo Mondi April 1, 2022, 1:40 p.m. UTC | #1
Hi Christian

On Fri, Apr 01, 2022 at 01:06:13AM +0100, Christian Rauch via libcamera-devel wrote:
> This defines Controls with a 'size' as either variable-sized Span<T> or as
> fixed-sized Span<T,N>.
>
> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> ---
>  utils/gen-controls.py | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 3f99b5e2..694d761a 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -10,6 +10,7 @@ import argparse
>  import string
>  import sys
>  import yaml
> +from math import prod

nit: keep the inclusion directives sorted please

>
>
>  def snake_case(s):
> @@ -22,6 +23,26 @@ def format_description(description):
>      return '\n'.join([(line and ' * ' or ' *') + line for line in description])
>
>
> +def get_ctrl_type_format(ctrl):
> +    ctrl_type = ctrl['type']
> +    ctrl_size_arr = ctrl.get('size')
> +    ctrl_is_span = ctrl_size_arr is not None
> +    if ctrl_is_span:
> +        ctrl_span_size = prod(ctrl_size_arr) if len(ctrl_size_arr)>0 else None

nit: unless some python coding style requirements prescribes
differently, "if len(ctrl_size_arr) > 0" (ie space between operators)
reads better

> +
> +    if ctrl_type == 'string':
> +        ctrl_type = 'std::string'
> +    elif ctrl_is_span:
> +        if ctrl_span_size:
> +            # fixed-sized Span
> +            ctrl_type = 'Span<const {}, {}>'.format(ctrl_type, span_size)
> +        else:
> +            # variable-sized Span
> +            ctrl_type = 'Span<const {}>'.format(ctrl_type)

This now reads

        ctrl_is_span = ...
        if ctrl_is_span:
                ...

        if ctrl_type == "string":
                ....
        elif ctrl_is_span:
                ...

        return ctrl_type

Should this just be

        ctrl_type = ctrl['type']

        if ctrl_type == 'string':
                return 'std::string'

        ctrl_size = ctrl.get('size')
        if ctrl_size is not None:
                if len(ctrl_size) > 0:
                        span_size = prod(ctrl_size)
                        return 'Span<const {}, {}>'.format(ctrl_type, span_size)
                else
                        return 'Span<const {}>'.format(ctrl_type)

       return ctrl_type

(not-compiled pseudo-code from someone who barely knows python, take
this just as a suggestion)

As a non-native Python speaker I aslo wonder what the difference
between the existing

        'Span<const %s>' % ctrl_type

and
        'Span<const {}>'.format(ctrl_type)

is. Are those equivalent ? I would assume so...

The patch is good though!

> +
> +    return ctrl_type
> +
> +
>  def generate_cpp(controls):
>      enum_doc_start_template = string.Template('''/**
>   * \\enum ${name}Enum
> @@ -50,11 +71,7 @@ ${description}
>          name, ctrl = ctrl.popitem()
>          id_name = snake_case(name).upper()
>
> -        ctrl_type = ctrl['type']
> -        if ctrl_type == 'string':
> -            ctrl_type = 'std::string'
> -        elif ctrl.get('size'):
> -            ctrl_type = 'Span<const %s>' % ctrl_type
> +        ctrl_type = get_ctrl_type_format(ctrl)
>
>          info = {
>              'name': name,
> @@ -135,11 +152,7 @@ def generate_h(controls):
>
>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>
> -        ctrl_type = ctrl['type']
> -        if ctrl_type == 'string':
> -            ctrl_type = 'std::string'
> -        elif ctrl.get('size'):
> -            ctrl_type = 'Span<const %s>' % ctrl_type
> +        ctrl_type = get_ctrl_type_format(ctrl)
>
>          info = {
>              'name': name,
> --
> 2.25.1
>

Patch
diff mbox series

diff --git a/utils/gen-controls.py b/utils/gen-controls.py
index 3f99b5e2..694d761a 100755
--- a/utils/gen-controls.py
+++ b/utils/gen-controls.py
@@ -10,6 +10,7 @@  import argparse
 import string
 import sys
 import yaml
+from math import prod


 def snake_case(s):
@@ -22,6 +23,26 @@  def format_description(description):
     return '\n'.join([(line and ' * ' or ' *') + line for line in description])


+def get_ctrl_type_format(ctrl):
+    ctrl_type = ctrl['type']
+    ctrl_size_arr = ctrl.get('size')
+    ctrl_is_span = ctrl_size_arr is not None
+    if ctrl_is_span:
+        ctrl_span_size = prod(ctrl_size_arr) if len(ctrl_size_arr)>0 else None
+
+    if ctrl_type == 'string':
+        ctrl_type = 'std::string'
+    elif ctrl_is_span:
+        if ctrl_span_size:
+            # fixed-sized Span
+            ctrl_type = 'Span<const {}, {}>'.format(ctrl_type, ctrl_span_size)
+        else:
+            # variable-sized Span
+            ctrl_type = 'Span<const {}>'.format(ctrl_type)
+
+    return ctrl_type
+
+
 def generate_cpp(controls):
     enum_doc_start_template = string.Template('''/**
  * \\enum ${name}Enum
@@ -50,11 +71,7 @@  ${description}
         name, ctrl = ctrl.popitem()
         id_name = snake_case(name).upper()

-        ctrl_type = ctrl['type']
-        if ctrl_type == 'string':
-            ctrl_type = 'std::string'
-        elif ctrl.get('size'):
-            ctrl_type = 'Span<const %s>' % ctrl_type
+        ctrl_type = get_ctrl_type_format(ctrl)

         info = {
             'name': name,
@@ -135,11 +152,7 @@  def generate_h(controls):

         ids.append('\t' + id_name + ' = ' + str(id_value) + ',')

-        ctrl_type = ctrl['type']
-        if ctrl_type == 'string':
-            ctrl_type = 'std::string'
-        elif ctrl.get('size'):
-            ctrl_type = 'Span<const %s>' % ctrl_type
+        ctrl_type = get_ctrl_type_format(ctrl)

         info = {
             'name': name,