utils: ipc: Fix async main interface functions with no parameters
diff mbox series

Message ID 20240412024256.651465-1-paul.elder@ideasonboard.com
State Accepted
Headers show
Series
  • utils: ipc: Fix async main interface functions with no parameters
Related show

Commit Message

Paul Elder April 12, 2024, 2:42 a.m. UTC
If an async main interface function is defined with no parameters, there
would be a compilation error complaining about an extra comma. Fix this.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Umang Jain April 12, 2024, 3:55 a.m. UTC | #1
Hi Paul,

On 12/04/24 8:12 am, Paul Elder wrote:
> If an async main interface function is defined with no parameters, there
> would be a compilation error complaining about an extra comma. Fix this.

Makes sense to me
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl   | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> 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 238cf4a5..01fe9c51 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -175,7 +175,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>   );
>   {% elif method|is_async %}
>   	ASSERT(state_ == ProxyRunning);
> -	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued,
> +	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued
> +	{%- if method|method_param_names|length > 0 -%}, {% endif -%}
>   	{%- for param in method|method_param_names -%}
>   		{{param}}{{- ", " if not loop.last}}
>   	{%- endfor -%}
Stefan Klug April 12, 2024, 8:45 a.m. UTC | #2
Hi Paul,

thanks for the patch.

On Fri, Apr 12, 2024 at 11:42:56AM +0900, Paul Elder wrote:
> If an async main interface function is defined with no parameters, there
> would be a compilation error complaining about an extra comma. Fix this.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 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 238cf4a5..01fe9c51 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -175,7 +175,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>  );
>  {% elif method|is_async %}
>  	ASSERT(state_ == ProxyRunning);
> -	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued,
> +	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued
> +	{%- if method|method_param_names|length > 0 -%}, {% endif -%}
>  	{%- for param in method|method_param_names -%}
>  		{{param}}{{- ", " if not loop.last}}
>  	{%- endfor -%}

I did not test it, but wouldn't it be easier to place the comma before
the param and remove the two ifs like this:

proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued
{%- for param in method|method_param_names -%}
     , {{param}}
{%- endfor -%}


Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Cheers,
Stefan


> -- 
> 2.39.2
>
Laurent Pinchart April 12, 2024, 2:32 p.m. UTC | #3
On Fri, Apr 12, 2024 at 10:45:30AM +0200, Stefan Klug wrote:
> On Fri, Apr 12, 2024 at 11:42:56AM +0900, Paul Elder wrote:
> > If an async main interface function is defined with no parameters, there
> > would be a compilation error complaining about an extra comma. Fix this.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl   | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > 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 238cf4a5..01fe9c51 100644
> > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > @@ -175,7 +175,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >  );
> >  {% elif method|is_async %}
> >  	ASSERT(state_ == ProxyRunning);
> > -	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued,
> > +	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued
> > +	{%- if method|method_param_names|length > 0 -%}, {% endif -%}
> >  	{%- for param in method|method_param_names -%}
> >  		{{param}}{{- ", " if not loop.last}}
> >  	{%- endfor -%}
> 
> I did not test it, but wouldn't it be easier to place the comma before
> the param and remove the two ifs like this:
> 
> proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued
> {%- for param in method|method_param_names -%}
>      , {{param}}
> {%- endfor -%}

This sounds better to me.

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

for that version.

> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

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 238cf4a5..01fe9c51 100644
--- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
+++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
@@ -175,7 +175,8 @@  void {{proxy_name}}::recvMessage(const IPCMessage &data)
 );
 {% elif method|is_async %}
 	ASSERT(state_ == ProxyRunning);
-	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued,
+	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued
+	{%- if method|method_param_names|length > 0 -%}, {% endif -%}
 	{%- for param in method|method_param_names -%}
 		{{param}}{{- ", " if not loop.last}}
 	{%- endfor -%}