[libcamera-devel,v1,1/2] utils: ipc: Support custom parameters to init()
diff mbox series

Message ID 20210304084703.11542-1-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel,v1,1/2] utils: ipc: Support custom parameters to init()
Related show

Commit Message

Paul Elder March 4, 2021, 8:47 a.m. UTC 
Add support to the mojom-based code generator for custom parameters to
init(). Remove the parameter type and count validation as well.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 15 ++++++++++++---
 .../libcamera_templates/proxy_functions.tmpl      | 13 -------------
 utils/ipc/generators/mojom_libcamera_generator.py | 10 +---------
 3 files changed, 13 insertions(+), 25 deletions(-)

Comments

Paul Elder March 4, 2021, 8:48 a.m. UTC | #1 
Hi Naush,

Sorry I forgot to CC you.

Paul

On Thu, Mar 04, 2021 at 05:47:02PM +0900, Paul Elder wrote:
> Add support to the mojom-based code generator for custom parameters to
> init(). Remove the parameter type and count validation as well.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 15 ++++++++++++---
>  .../libcamera_templates/proxy_functions.tmpl      | 13 -------------
>  utils/ipc/generators/mojom_libcamera_generator.py | 10 +---------
>  3 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> index 5e0d3976..8ba1a5cf 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -144,10 +144,19 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>  
>  {{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
>  {
> -{%- if method.mojom_name == "init" %}
> -	{{proxy_funcs.init_thread_body()}}
> -{%- elif method.mojom_name == "stop" %}
> +{%- if method.mojom_name == "stop" %}
>  	{{proxy_funcs.stop_thread_body()}}
> +{%- elif method.mojom_name == "init" %}
> +	{{ method|method_return_value + " _ret = " if method|method_return_value != "void" -}}
> +	ipa_->{{method.mojom_name}}(
> +	{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +	{%- endfor -%}
> +);
> +
> +	proxy_.moveToThread(&thread_);
> +
> +	return {{ "_ret" if method|method_return_value != "void" }};
>  {%- elif method.mojom_name == "start" %}
>  	running_ = true;
>  	thread_.start();
> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> index 40611feb..f2d86b67 100644
> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -19,19 +19,6 @@
>  ){{" override" if override}}
>  {%- endmacro -%}
>  
> -{#
> - # \brief Generate function body for IPA init() function for thread
> - #}
> -{%- macro init_thread_body() -%}
> -	int ret = ipa_->init(settings);
> -	if (ret)
> -		return ret;
> -
> -	proxy_.moveToThread(&thread_);
> -
> -	return 0;
> -{%- endmacro -%}
> -
>  {#
>   # \brief Generate function body for IPA stop() function for thread
>   #}
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index 438e41c6..af123ef2 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -345,15 +345,7 @@ def ValidateInterfaces(interfaces):
>      f_start = f_start[0]
>      f_stop  = f_stop[0]
>  
> -    # Validate parameters to init()
> -    ValidateSingleLength(f_init.parameters, 'input parameter to init()')
> -    ValidateSingleLength(f_init.response_parameters, 'output parameter from init()')
> -    if f_init.parameters[0].kind.mojom_name != 'IPASettings':
> -        raise Exception('init() must have single IPASettings input parameter')
> -    if f_init.response_parameters[0].kind.spec != 'i32':
> -        raise Exception('init() must have single int32 output parameter')
> -
> -    # No need to validate start() as it is customizable
> +    # No need to validate init() and start() as they are customizable
>  
>      # Validate parameters to stop()
>      ValidateZeroLength(f_stop.parameters, 'input parameter to stop()')
> -- 
> 2.27.0
>
Laurent Pinchart March 4, 2021, 8:56 a.m. UTC | #2 
Hi Paul,

Thank you for the patch.

On Thu, Mar 04, 2021 at 05:47:02PM +0900, Paul Elder wrote:
> Add support to the mojom-based code generator for custom parameters to
> init(). Remove the parameter type and count validation as well.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 15 ++++++++++++---
>  .../libcamera_templates/proxy_functions.tmpl      | 13 -------------
>  utils/ipc/generators/mojom_libcamera_generator.py | 10 +---------
>  3 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> index 5e0d3976..8ba1a5cf 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -144,10 +144,19 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>  
>  {{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
>  {
> -{%- if method.mojom_name == "init" %}
> -	{{proxy_funcs.init_thread_body()}}
> -{%- elif method.mojom_name == "stop" %}
> +{%- if method.mojom_name == "stop" %}
>  	{{proxy_funcs.stop_thread_body()}}
> +{%- elif method.mojom_name == "init" %}
> +	{{ method|method_return_value + " _ret = " if method|method_return_value != "void" -}}
> +	ipa_->{{method.mojom_name}}(
> +	{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +	{%- endfor -%}
> +);
> +
> +	proxy_.moveToThread(&thread_);
> +
> +	return {{ "_ret" if method|method_return_value != "void" }};
>  {%- elif method.mojom_name == "start" %}
>  	running_ = true;
>  	thread_.start();
> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> index 40611feb..f2d86b67 100644
> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -19,19 +19,6 @@
>  ){{" override" if override}}
>  {%- endmacro -%}
>  
> -{#
> - # \brief Generate function body for IPA init() function for thread
> - #}
> -{%- macro init_thread_body() -%}
> -	int ret = ipa_->init(settings);
> -	if (ret)
> -		return ret;
> -
> -	proxy_.moveToThread(&thread_);
> -
> -	return 0;
> -{%- endmacro -%}
> -
>  {#
>   # \brief Generate function body for IPA stop() function for thread
>   #}
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index 438e41c6..af123ef2 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -345,15 +345,7 @@ def ValidateInterfaces(interfaces):

Should you drop f_init from right above ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>      f_start = f_start[0]
>      f_stop  = f_stop[0]
>  
> -    # Validate parameters to init()
> -    ValidateSingleLength(f_init.parameters, 'input parameter to init()')
> -    ValidateSingleLength(f_init.response_parameters, 'output parameter from init()')
> -    if f_init.parameters[0].kind.mojom_name != 'IPASettings':
> -        raise Exception('init() must have single IPASettings input parameter')
> -    if f_init.response_parameters[0].kind.spec != 'i32':
> -        raise Exception('init() must have single int32 output parameter')
> -
> -    # No need to validate start() as it is customizable
> +    # No need to validate init() and start() as they are customizable
>  
>      # Validate parameters to stop()
>      ValidateZeroLength(f_stop.parameters, 'input parameter to stop()')
Paul Elder March 4, 2021, 8:59 a.m. UTC | #3 
Hi Laurent,

On Thu, Mar 04, 2021 at 10:56:47AM +0200, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Thu, Mar 04, 2021 at 05:47:02PM +0900, Paul Elder wrote:
> > Add support to the mojom-based code generator for custom parameters to
> > init(). Remove the parameter type and count validation as well.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 15 ++++++++++++---
> >  .../libcamera_templates/proxy_functions.tmpl      | 13 -------------
> >  utils/ipc/generators/mojom_libcamera_generator.py | 10 +---------
> >  3 files changed, 13 insertions(+), 25 deletions(-)
> > 
> > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > index 5e0d3976..8ba1a5cf 100644
> > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > @@ -144,10 +144,19 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >  
> >  {{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> >  {
> > -{%- if method.mojom_name == "init" %}
> > -	{{proxy_funcs.init_thread_body()}}
> > -{%- elif method.mojom_name == "stop" %}
> > +{%- if method.mojom_name == "stop" %}
> >  	{{proxy_funcs.stop_thread_body()}}
> > +{%- elif method.mojom_name == "init" %}
> > +	{{ method|method_return_value + " _ret = " if method|method_return_value != "void" -}}
> > +	ipa_->{{method.mojom_name}}(
> > +	{%- for param in method|method_param_names -%}
> > +		{{param}}{{- ", " if not loop.last}}
> > +	{%- endfor -%}
> > +);
> > +
> > +	proxy_.moveToThread(&thread_);
> > +
> > +	return {{ "_ret" if method|method_return_value != "void" }};
> >  {%- elif method.mojom_name == "start" %}
> >  	running_ = true;
> >  	thread_.start();
> > diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> > index 40611feb..f2d86b67 100644
> > --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> > +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> > @@ -19,19 +19,6 @@
> >  ){{" override" if override}}
> >  {%- endmacro -%}
> >  
> > -{#
> > - # \brief Generate function body for IPA init() function for thread
> > - #}
> > -{%- macro init_thread_body() -%}
> > -	int ret = ipa_->init(settings);
> > -	if (ret)
> > -		return ret;
> > -
> > -	proxy_.moveToThread(&thread_);
> > -
> > -	return 0;
> > -{%- endmacro -%}
> > -
> >  {#
> >   # \brief Generate function body for IPA stop() function for thread
> >   #}
> > diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> > index 438e41c6..af123ef2 100644
> > --- a/utils/ipc/generators/mojom_libcamera_generator.py
> > +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> > @@ -345,15 +345,7 @@ def ValidateInterfaces(interfaces):
> 
> Should you drop f_init from right above ?

Oh yes you're right, both f_init and f_start should be removed. We need
the earlier one to validate presence, but not the reassignment.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Thanks,

Paul

> 
> >      f_start = f_start[0]
> >      f_stop  = f_stop[0]
> >  
> > -    # Validate parameters to init()
> > -    ValidateSingleLength(f_init.parameters, 'input parameter to init()')
> > -    ValidateSingleLength(f_init.response_parameters, 'output parameter from init()')
> > -    if f_init.parameters[0].kind.mojom_name != 'IPASettings':
> > -        raise Exception('init() must have single IPASettings input parameter')
> > -    if f_init.response_parameters[0].kind.spec != 'i32':
> > -        raise Exception('init() must have single int32 output parameter')
> > -
> > -    # No need to validate start() as it is customizable
> > +    # No need to validate init() and start() as they are customizable
> >  
> >      # Validate parameters to stop()
> >      ValidateZeroLength(f_stop.parameters, 'input parameter to stop()')
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
index 5e0d3976..8ba1a5cf 100644
--- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
+++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
@@ -144,10 +144,19 @@  void {{proxy_name}}::recvMessage(const IPCMessage &data)
 
 {{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
 {
-{%- if method.mojom_name == "init" %}
-	{{proxy_funcs.init_thread_body()}}
-{%- elif method.mojom_name == "stop" %}
+{%- if method.mojom_name == "stop" %}
 	{{proxy_funcs.stop_thread_body()}}
+{%- elif method.mojom_name == "init" %}
+	{{ method|method_return_value + " _ret = " if method|method_return_value != "void" -}}
+	ipa_->{{method.mojom_name}}(
+	{%- for param in method|method_param_names -%}
+		{{param}}{{- ", " if not loop.last}}
+	{%- endfor -%}
+);
+
+	proxy_.moveToThread(&thread_);
+
+	return {{ "_ret" if method|method_return_value != "void" }};
 {%- elif method.mojom_name == "start" %}
 	running_ = true;
 	thread_.start();
diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
index 40611feb..f2d86b67 100644
--- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
+++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
@@ -19,19 +19,6 @@ 
 ){{" override" if override}}
 {%- endmacro -%}
 
-{#
- # \brief Generate function body for IPA init() function for thread
- #}
-{%- macro init_thread_body() -%}
-	int ret = ipa_->init(settings);
-	if (ret)
-		return ret;
-
-	proxy_.moveToThread(&thread_);
-
-	return 0;
-{%- endmacro -%}
-
 {#
  # \brief Generate function body for IPA stop() function for thread
  #}
diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
index 438e41c6..af123ef2 100644
--- a/utils/ipc/generators/mojom_libcamera_generator.py
+++ b/utils/ipc/generators/mojom_libcamera_generator.py
@@ -345,15 +345,7 @@  def ValidateInterfaces(interfaces):
     f_start = f_start[0]
     f_stop  = f_stop[0]
 
-    # Validate parameters to init()
-    ValidateSingleLength(f_init.parameters, 'input parameter to init()')
-    ValidateSingleLength(f_init.response_parameters, 'output parameter from init()')
-    if f_init.parameters[0].kind.mojom_name != 'IPASettings':
-        raise Exception('init() must have single IPASettings input parameter')
-    if f_init.response_parameters[0].kind.spec != 'i32':
-        raise Exception('init() must have single int32 output parameter')
-
-    # No need to validate start() as it is customizable
+    # No need to validate init() and start() as they are customizable
 
     # Validate parameters to stop()
     ValidateZeroLength(f_stop.parameters, 'input parameter to stop()')