[libcamera-devel] [PATCH v3 6/8] test: v4l2_subdevice: Add ListFormat test

[Jacopo Mondi]

Hi Kieran,

On Tue, Feb 26, 2019 at 11:50:37PM +0000, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 26/02/2019 16:26, Jacopo Mondi wrote:
> > Add test to list formats on a v4l2 subdevice.
> >

[snip]

> > +int ListFormatsTest::run()
> > +{
> > +	/* List all formats available on existing "Scaler" pads. */
> > +	std::vector<V4L2SubdeviceFormat> formats = {};
> > +
> > +	formats = scaler_->formats(0);
> > +	if (formats.empty()) {
> > +		cerr << "Failed to list formats on pad 0 of sudevice "
>
> Is the 'list' a verb or a noun in listFormats?
>

I interpreted it as a verb.

> Should it be getFormats()? or enumFormats() in the Subdevice
> implementation as it seems formats() will name clash :)

getFormats() is easily confusable with getFormat(), enumFormats()
might make sense, but recalls the SUBDEV_ENUM_ ioctls, which have a
different behaviour.

As the 'formats()' methods return the 'formats_' field, which is a
vector of formats, according to out policy that getters should just
repeat the name of the field they access, I would keep 'formats()' as
the method name, even if I might have missed which name clash you're
reporting here...

Thanks
  j

>
>
> > +		     << scaler_->deviceNode() << endl;
> > +		return TestFail;
> > +	}
> > +	printFormats(0, formats);
> > +> +	formats = {};
> > +	formats = scaler_->formats(1);
> > +	if (formats.empty()) {
> > +		cerr << "Failed to list formats on pad 1 of sudevice "
>
> s/sudevice/subdevice/ here and above on pad 0.
>
> > +		     << scaler_->deviceNode() << endl;
> > +		return TestFail;
> > +	}
> > +	printFormats(1, formats);
> > +
> > +	/* List format on a non-existing pad, format vector shall be empty. */
> > +	formats = {};
> > +	formats = scaler_->formats(2);
> > +	if (!formats.empty()) {
> > +		cerr << "Listing formats on non-existing pad 2 of subdevice "
> > +		     << scaler_->deviceNode()
> > +		     << " should return an empty format list" << endl;
> > +		return TestFail;
> > +	}
> > +
> > +	return TestPass;
> > +}
> > +
> > +TEST_REGISTER(ListFormatsTest);
> > diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build
> > index a4359fe1bc19..6023d15e1558 100644
> > --- a/test/v4l2_subdevice/meson.build
> > +++ b/test/v4l2_subdevice/meson.build
> > @@ -1,4 +1,5 @@
> >  v4l2_subdevice_tests = [
> > +  [ 'list_formats',             'list_formats.cpp'],
> >    [ 'test_formats',             'test_formats.cpp'],
> >  ]
> >
> >
>
> --
> Regards
> --
> Kieran
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190227/b9cef35b/attachment.sig>