cam: capture_script: Make parseRectangles work for non-array

19643 diff mbox series

From: Paul Elder <paul.elder@ideasonboard.com>

parseRectangles currently always parses Rectangle controls as an array
of Rectangles. This causes non-array Rectangle controls to not be parsed
correctly, as when the ControlValue is get()ed, the non-array assertion
will fail.

Set the ControlValue with a single Rectangle in case a single Rectangle
has been specified in the yaml capture script to fix that.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/apps/cam/capture_script.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Quoting Jacopo Mondi (2024-03-08 10:42:18)
> From: Paul Elder <paul.elder@ideasonboard.com>
> 
> parseRectangles currently always parses Rectangle controls as an array
> of Rectangles. This causes non-array Rectangle controls to not be parsed
> correctly, as when the ControlValue is get()ed, the non-array assertion
> will fail.
> 
> Set the ControlValue with a single Rectangle in case a single Rectangle
> has been specified in the yaml capture script to fix that.
> 

I believe this is reasonable, as we've had this in a branch for a while.

I wonder if anything should be done to make this happen at the
ControlValue::set() / template level more automagically - but I suspect
that's not worth it.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/apps/cam/capture_script.cpp | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp
> index 062a7258e414..1215713fac18 100644
> --- a/src/apps/cam/capture_script.cpp
> +++ b/src/apps/cam/capture_script.cpp
> @@ -351,7 +351,10 @@ ControlValue CaptureScript::parseRectangles()
>         }
>  
>         ControlValue controlValue;
> -       controlValue.set(Span<const Rectangle>(rectangles));
> +       if (rectangles.size() == 1)
> +               controlValue.set(rectangles.at(0));
> +       else
> +               controlValue.set(Span<const Rectangle>(rectangles));
>  
>         return controlValue;
>  }
> -- 
> 2.43.2
>

Hi Jacopo

On 08/03/24 4:12 pm, Jacopo Mondi wrote:
> From: Paul Elder <paul.elder@ideasonboard.com>
>
> parseRectangles currently always parses Rectangle controls as an array

nit: s/parseRectangles/parseRectangles()/
> of Rectangles. This causes non-array Rectangle controls to not be parsed
> correctly, as when the ControlValue is get()ed, the non-array assertion
> will fail.
>
> Set the ControlValue with a single Rectangle in case a single Rectangle
> has been specified in the yaml capture script to fix that.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
LGTM

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

> ---
>   src/apps/cam/capture_script.cpp | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp
> index 062a7258e414..1215713fac18 100644
> --- a/src/apps/cam/capture_script.cpp
> +++ b/src/apps/cam/capture_script.cpp
> @@ -351,7 +351,10 @@ ControlValue CaptureScript::parseRectangles()
>   	}
>   
>   	ControlValue controlValue;
> -	controlValue.set(Span<const Rectangle>(rectangles));
> +	if (rectangles.size() == 1)
> +		controlValue.set(rectangles.at(0));
> +	else
> +		controlValue.set(Span<const Rectangle>(rectangles));
>   
>   	return controlValue;
>   }