[libcamera-devel] [PATCH 2/2] [DNI] test: Test serialization of Rectangle and Size controls

[Jacopo Mondi]

Hi Laurent,
  I start from here to comment on the API

On Sat, Apr 25, 2020 at 11:56:39PM +0300, Laurent Pinchart wrote:
> This patch should be rebased on real controls once available.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/control_ids.yaml               | 18 ++++++++++++++++++
>  test/serialization/control_serialization.cpp |  5 +++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 4befec746a59..b1ae03e5b0ff 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -50,4 +50,22 @@ controls:
>        type: int32_t
>        description: Specify a fixed gain parameter
>
> +  - TheRectangle:
> +      type: Rectangle
> +      description: A Rectangle property
> +
> +  - TheRectangles:
> +      type: Rectangle
> +      description: A Rectangle array property
> +      size: [n]
> +
> +  - TheSize:
> +      type: Size
> +      description: A Size property
> +
> +  - TheSizes:
> +      type: Size
> +      description: A Size array property
> +      size: [n]
> +
>  ...
> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp
> index 2989b52774fb..789e0d83f4e4 100644
> --- a/test/serialization/control_serialization.cpp
> +++ b/test/serialization/control_serialization.cpp
> @@ -45,6 +45,11 @@ protected:
>  		list.set(controls::Brightness, 255);
>  		list.set(controls::Contrast, 128);
>  		list.set(controls::Saturation, 50);
> +		list.set(controls::TheRectangle, Rectangle{ 100, 100, 640, 480 });
> +		list.set(controls::TheRectangles, { Rectangle{ 100, 100, 640, 480 },
> +						    Rectangle{ 200, 200, 1280, 720 } });
> +		list.set(controls::TheSize, Size{ 640, 480 });
> +		list.set(controls::TheSizes, { Size{ 640, 480 }, Size{ 1280, 720 } });

Is this a good idea ? How easy is that get it wrong assuming

		list.set(controls::TheSizes, { 640, 480 }};

does what one expects ? We'll get a type assertion error, which is not
nice to debug.. I'm debated, this is useful, but might be really a
bleeding edge of the API...

>
>  		/*
>  		 * Serialize the control list, this should fail as the control
> --
> Regards,
>
> Laurent Pinchart
>