[libcamera-devel] [PATCH v5 9/9] [RFC] test: generated_serializer: Test skipHeader enums and flags
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Oct 16 04:06:14 CEST 2022
Hi Paul,
On Wed, Oct 12, 2022 at 11:30:02AM +0900, paul.elder at ideasonboard.com wrote:
> On Tue, Oct 11, 2022 at 05:01:53PM +0300, Laurent Pinchart wrote:
> > On Tue, Oct 11, 2022 at 07:58:59PM +0900, Paul Elder via libcamera-devel wrote:
> > > Add fields to the test struct to test serialization/deserialization of
> > > scoped enums and flags of said scoped enums that are defined in a C++
> > > header, which are thus designated by [skipHeader] in mojom.
> > >
> > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > >
> > > ---
> > > New in v5
> > >
> > > RFC because this touches core stuff. This can be detached from the rest
> > > of the series.
> > > ---
> > > include/libcamera/ipa/core.mojom | 2 ++
> > > include/libcamera/ipa/ipa_interface.h | 7 +++++++
> > > .../generated_serializer_test.cpp | 11 +++++++++++
> > > .../include/libcamera/ipa/test.mojom | 4 ++++
> > > 4 files changed, 24 insertions(+)
> > >
> > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
> > > index 1ff674b0..3438af93 100644
> > > --- a/include/libcamera/ipa/core.mojom
> > > +++ b/include/libcamera/ipa/core.mojom
> > > @@ -84,6 +84,8 @@ module libcamera;
> > > [skipSerdes, skipHeader] struct ControlList {};
> > > [skipSerdes, skipHeader] struct SharedFD {};
> > >
> > > +[skipHeader, scopedEnum] enum TestEnum {};
> > > +
> >
> > This particular part isn't very nice. Same for the change to
> > ipa_interface.h below.
>
> Yeah that's what I thought :/
>
> > We should either find a suitable enum that IPA modules (will) really need,
>
> Any ideas? :)
Well... :-)
> > or try to test this feature using test.mojom and/or vimc.mojom.
>
> That won't work (that's why this part of the series was delayed). The
> enums that are skipHeader-ed need to be accessbile from
> {pipeline}_ipa_interface.h, but those are generated from
> {pipeline}.mojom. Which means that either 1) the headers that defined
> those enums need to be #included in the global ipa_interface.h (so *all*
> IPAs have access to them) or 2) we add a new header that will
> automatically be imcluded in the generated {pipeline}_ipa_interface.h,
> which imo defeats the whole purpose of {pipeline}.mojom.
Indeed, neither are nice.
Could we use the mojo import directive ? It's meant to import other
.mojom files, but maybe we could use it to specify .h files that need to
be included ?
> So we need a suitable enum that IPA modules will really need.
>
> > > [skipHeader] struct Point {
> > > int32 x;
> > > int32 y;
> > > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> > > index 8884f0ed..96fb8344 100644
> > > --- a/include/libcamera/ipa/ipa_interface.h
> > > +++ b/include/libcamera/ipa/ipa_interface.h
> > > @@ -27,6 +27,13 @@ namespace libcamera {
> > > * tag must be #included here.
> > > */
> > >
> > > +enum class TestEnum {
> > > + TestEnumValueA,
> > > + TestEnumValueB,
> > > + TestEnumValueC,
> > > + TestEnumValueD,
> > > +};
> > > +
> > > class IPAInterface
> > > {
> > > public:
> > > diff --git a/test/serialization/generated_serializer/generated_serializer_test.cpp b/test/serialization/generated_serializer/generated_serializer_test.cpp
> > > index 4670fe46..6f01c3d4 100644
> > > --- a/test/serialization/generated_serializer/generated_serializer_test.cpp
> > > +++ b/test/serialization/generated_serializer/generated_serializer_test.cpp
> > > @@ -11,6 +11,8 @@
> > >
> > > #include "test.h"
> > >
> > > +#include <libcamera/ipa/ipa_interface.h>
> > > +
> > > #include "test_ipa_interface.h"
> > > #include "test_ipa_serializer.h"
> > >
> > > @@ -59,6 +61,9 @@ if (struct1.field != struct2.field) { \
> > > t.s3 = "lorem ipsum";
> > > t.i = 58527;
> > > t.c = ipa::test::IPAOperationInit;
> > > + t.ex = TestEnum::TestEnumValueA;
> > > + t.exf |= TestEnum::TestEnumValueB;
> > > +
> > > t.e = ipa::test::ErrorFlags::Error1;
> > >
> > > Flags<ipa::test::ErrorFlags> flags;
> > > @@ -87,6 +92,8 @@ if (struct1.field != struct2.field) { \
> > >
> > > TEST_SCOPED_ENUM_EQUALITY(t, u, e);
> > > TEST_SCOPED_ENUM_EQUALITY(t, u, f);
> > > + TEST_SCOPED_ENUM_EQUALITY(t, u, ex);
> > > + TEST_SCOPED_ENUM_EQUALITY(t, u, exf);
> > >
> > > /* Test vector of generated structs */
> > > std::vector<ipa::test::TestStruct> v = { t, u };
> > > @@ -113,6 +120,8 @@ if (struct1.field != struct2.field) { \
> > >
> > > TEST_SCOPED_ENUM_EQUALITY(v[0], w[0], e);
> > > TEST_SCOPED_ENUM_EQUALITY(v[0], w[0], f);
> > > + TEST_SCOPED_ENUM_EQUALITY(v[0], w[0], ex);
> > > + TEST_SCOPED_ENUM_EQUALITY(v[0], w[0], exf);
> > >
> > > TEST_FIELD_EQUALITY(v[1], w[1], s1);
> > > TEST_FIELD_EQUALITY(v[1], w[1], s2);
> > > @@ -122,6 +131,8 @@ if (struct1.field != struct2.field) { \
> > >
> > > TEST_SCOPED_ENUM_EQUALITY(v[1], w[1], e);
> > > TEST_SCOPED_ENUM_EQUALITY(v[1], w[1], f);
> > > + TEST_SCOPED_ENUM_EQUALITY(v[1], w[1], ex);
> > > + TEST_SCOPED_ENUM_EQUALITY(v[1], w[1], exf);
> > >
> > > return TestPass;
> > > }
> > > diff --git a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom
> > > index 91c31642..1f1ba8dc 100644
> > > --- a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom
> > > +++ b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom
> > > @@ -2,6 +2,8 @@
> > >
> > > module ipa.test;
> > >
> > > +import "include/libcamera/ipa/core.mojom";
> > > +
> > > enum IPAOperationCode {
> > > IPAOperationNone,
> > > IPAOperationInit,
> > > @@ -28,6 +30,8 @@ struct TestStruct {
> > > IPAOperationCode c;
> > > ErrorFlags e;
> > > [flags] ErrorFlags f;
> > > + libcamera.TestEnum ex;
> > > + [flags] libcamera.TestEnum exf;
> > > };
> > >
> > > interface IPATestInterface {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list