[{"id":25382,"web_url":"https://patchwork.libcamera.org/comment/25382/","msgid":"","date":"2022-10-11T14:01:53","subject":"Re: [libcamera-devel] [PATCH v5 9/9] [RFC] test:\n\tgenerated_serializer: Test skipHeader enums and flags","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Tue, Oct 11, 2022 at 07:58:59PM +0900, Paul Elder via libcamera-devel wrote:\n> Add fields to the test struct to test serialization/deserialization of\n> scoped enums and flags of said scoped enums that are defined in a C++\n> header, which are thus designated by [skipHeader] in mojom.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> New in v5\n> \n> RFC because this touches core stuff. This can be detached from the rest\n> of the series.\n> ---\n> include/libcamera/ipa/core.mojom | 2 ++\n> include/libcamera/ipa/ipa_interface.h | 7 +++++++\n> .../generated_serializer_test.cpp | 11 +++++++++++\n> .../include/libcamera/ipa/test.mojom | 4 ++++\n> 4 files changed, 24 insertions(+)\n> \n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index 1ff674b0..3438af93 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -84,6 +84,8 @@ module libcamera;\n> [skipSerdes, skipHeader] struct ControlList {};\n> [skipSerdes, skipHeader] struct SharedFD {};\n> \n> +[skipHeader, scopedEnum] enum TestEnum {};\n> +\n\nThis particular part isn't very nice. Same for the change to\nipa_interface.h below. We should either find a suitable enum that IPA\nmodules (will) really need, or try to test this feature using test.mojom\nand/or vimc.mojom.\n\n> [skipHeader] struct Point {\n> \tint32 x;\n> \tint32 y;\n> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> index 8884f0ed..96fb8344 100644\n> --- a/include/libcamera/ipa/ipa_interface.h\n> +++ b/include/libcamera/ipa/ipa_interface.h\n> @@ -27,6 +27,13 @@ namespace libcamera {\n> * tag must be #included here.\n> */\n> \n> +enum class TestEnum {\n> +\tTestEnumValueA,\n> +\tTestEnumValueB,\n> +\tTestEnumValueC,\n> +\tTestEnumValueD,\n> +};\n> +\n> class IPAInterface\n> {\n> public:\n> diff --git a/test/serialization/generated_serializer/generated_serializer_test.cpp b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> index 4670fe46..6f01c3d4 100644\n> --- a/test/serialization/generated_serializer/generated_serializer_test.cpp\n> +++ b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> @@ -11,6 +11,8 @@\n> \n> #include \"test.h\"\n> \n> +#include \n> +\n> #include \"test_ipa_interface.h\"\n> #include \"test_ipa_serializer.h\"\n> \n> @@ -59,6 +61,9 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> \t\tt.s3 = \"lorem ipsum\";\n> \t\tt.i = 58527;\n> \t\tt.c = ipa::test::IPAOperationInit;\n> +\t\tt.ex = TestEnum::TestEnumValueA;\n> +\t\tt.exf |= TestEnum::TestEnumValueB;\n> +\n> \t\tt.e = ipa::test::ErrorFlags::Error1;\n> \n> \t\tFlags flags;\n> @@ -87,6 +92,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> \n> \t\tTEST_SCOPED_ENUM_EQUALITY(t, u, e);\n> \t\tTEST_SCOPED_ENUM_EQUALITY(t, u, f);\n> +\t\tTEST_SCOPED_ENUM_EQUALITY(t, u, ex);\n> +\t\tTEST_SCOPED_ENUM_EQUALITY(t, u, exf);\n> \n> \t\t/* Test vector of generated structs */\n> \t\tstd::vector v = { t, u };\n> @@ -113,6 +120,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> \n> \t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], e);\n> \t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], f);\n> +\t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], ex);\n> +\t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], exf);\n> \n> \t\tTEST_FIELD_EQUALITY(v[1], w[1], s1);\n> \t\tTEST_FIELD_EQUALITY(v[1], w[1], s2);\n> @@ -122,6 +131,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> \n> \t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], e);\n> \t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], f);\n> +\t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], ex);\n> +\t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], exf);\n> \n> \t\treturn TestPass;\n> \t}\n> diff --git a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> index 91c31642..1f1ba8dc 100644\n> --- a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> +++ b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> @@ -2,6 +2,8 @@\n> \n> module ipa.test;\n> \n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> enum IPAOperationCode {\n> \tIPAOperationNone,\n> \tIPAOperationInit,\n> @@ -28,6 +30,8 @@ struct TestStruct {\n> \tIPAOperationCode c;\n> \tErrorFlags e;\n> \t[flags] ErrorFlags f;\n> +\tlibcamera.TestEnum ex;\n> +\t[flags] libcamera.TestEnum exf;\n> };\n> \n> interface IPATestInterface {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 29257C0DA4\n\tfor ;\n\tTue, 11 Oct 2022 14:02:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F44062D77;\n\tTue, 11 Oct 2022 16:02:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56991603F3\n\tfor ;\n\tTue, 11 Oct 2022 16:02:00 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 96E96907;\n\tTue, 11 Oct 2022 16:01:59 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665496921;\n\tbh=lNL0uCkylTMcC75Kxx0gbw4zGQl9dlvZxLM0mQtl5nA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=S56vDkOCmZFxCL6fNTmRhgvAlZc0s5kTYzvnJDUL1gcGq60rIuIqYQRf7Y3J3NuhM\n\tJSnQ2IBKmlfrDe604LHvDvsO6t4mP3I/PidOEA3J1NOH+N4TCvBj3h26SER/TQ01sP\n\tM/BlLawv0MOWSPQQDuLojE4eV5KtLoGLIIns4nSm6GA6GrlIyIcmzw1iJXKXeF9EKA\n\tUqOTP2hEChyzBm9pEuA2OStmFmsSUT6u0L8qGhrBHmpLPLZBxsuwPtoUxHKXMTkupT\n\tq3PSvHcp+lP4TRIgqLKUXsByP8QczcmSB7ITAoStAyzckpuHCRlSYhuigJpQIWaLoG\n\t3/T6hGQlidkpw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1665496919;\n\tbh=lNL0uCkylTMcC75Kxx0gbw4zGQl9dlvZxLM0mQtl5nA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XU1k/Df4ilw/YQKnokDNBOekYsNnbgaOApcznyQ+oSfoC0MCTwipMI3YG5Gh1awQI\n\t/zGGfqrCoi60mmOc9ajG1NuonLLLQmW5iSRSEqymafNh8NxbdHOIqzz7Bai9ljldZM\n\tTJ+OFsJXNqAk3GB0+zFlCUU3v2VhX4vqU4EUbja8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XU1k/Df4\"; dkim-atps=neutral","Date":"Tue, 11 Oct 2022 17:01:53 +0300","To":"Paul Elder ","Message-ID":"","References":"<20221011105859.457567-1-paul.elder@ideasonboard.com>\n\t<20221011105859.457567-10-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221011105859.457567-10-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 9/9] [RFC] test:\n\tgenerated_serializer: Test skipHeader enums and flags","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25390,"web_url":"https://patchwork.libcamera.org/comment/25390/","msgid":"<20221012023002.GB2921939@pyrite.rasen.tech>","date":"2022-10-12T02:30:02","subject":"Re: [libcamera-devel] [PATCH v5 9/9] [RFC] test:\n\tgenerated_serializer: Test skipHeader enums and flags","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Laurent,\n\nOn Tue, Oct 11, 2022 at 05:01:53PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Tue, Oct 11, 2022 at 07:58:59PM +0900, Paul Elder via libcamera-devel wrote:\n> > Add fields to the test struct to test serialization/deserialization of\n> > scoped enums and flags of said scoped enums that are defined in a C++\n> > header, which are thus designated by [skipHeader] in mojom.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > New in v5\n> > \n> > RFC because this touches core stuff. This can be detached from the rest\n> > of the series.\n> > ---\n> > include/libcamera/ipa/core.mojom | 2 ++\n> > include/libcamera/ipa/ipa_interface.h | 7 +++++++\n> > .../generated_serializer_test.cpp | 11 +++++++++++\n> > .../include/libcamera/ipa/test.mojom | 4 ++++\n> > 4 files changed, 24 insertions(+)\n> > \n> > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> > index 1ff674b0..3438af93 100644\n> > --- a/include/libcamera/ipa/core.mojom\n> > +++ b/include/libcamera/ipa/core.mojom\n> > @@ -84,6 +84,8 @@ module libcamera;\n> > [skipSerdes, skipHeader] struct ControlList {};\n> > [skipSerdes, skipHeader] struct SharedFD {};\n> > \n> > +[skipHeader, scopedEnum] enum TestEnum {};\n> > +\n> \n> This particular part isn't very nice. Same for the change to\n> ipa_interface.h below.\n\nYeah that's what I thought :/\n\n> We should either find a suitable enum that IPA modules (will) really need,\n\nAny ideas? :)\n\n> or try to test this feature using test.mojom and/or vimc.mojom.\n\nThat won't work (that's why this part of the series was delayed). The\nenums that are skipHeader-ed need to be accessbile from\n{pipeline}_ipa_interface.h, but those are generated from\n{pipeline}.mojom. Which means that either 1) the headers that defined\nthose enums need to be #included in the global ipa_interface.h (so *all*\nIPAs have access to them) or 2) we add a new header that will\nautomatically be imcluded in the generated {pipeline}_ipa_interface.h,\nwhich imo defeats the whole purpose of {pipeline}.mojom.\n\nSo we need a suitable enum that IPA modules will really need.\n\n\nPaul\n\n> \n> > [skipHeader] struct Point {\n> > \tint32 x;\n> > \tint32 y;\n> > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > index 8884f0ed..96fb8344 100644\n> > --- a/include/libcamera/ipa/ipa_interface.h\n> > +++ b/include/libcamera/ipa/ipa_interface.h\n> > @@ -27,6 +27,13 @@ namespace libcamera {\n> > * tag must be #included here.\n> > */\n> > \n> > +enum class TestEnum {\n> > +\tTestEnumValueA,\n> > +\tTestEnumValueB,\n> > +\tTestEnumValueC,\n> > +\tTestEnumValueD,\n> > +};\n> > +\n> > class IPAInterface\n> > {\n> > public:\n> > diff --git a/test/serialization/generated_serializer/generated_serializer_test.cpp b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> > index 4670fe46..6f01c3d4 100644\n> > --- a/test/serialization/generated_serializer/generated_serializer_test.cpp\n> > +++ b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> > @@ -11,6 +11,8 @@\n> > \n> > #include \"test.h\"\n> > \n> > +#include \n> > +\n> > #include \"test_ipa_interface.h\"\n> > #include \"test_ipa_serializer.h\"\n> > \n> > @@ -59,6 +61,9 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > \t\tt.s3 = \"lorem ipsum\";\n> > \t\tt.i = 58527;\n> > \t\tt.c = ipa::test::IPAOperationInit;\n> > +\t\tt.ex = TestEnum::TestEnumValueA;\n> > +\t\tt.exf |= TestEnum::TestEnumValueB;\n> > +\n> > \t\tt.e = ipa::test::ErrorFlags::Error1;\n> > \n> > \t\tFlags flags;\n> > @@ -87,6 +92,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > \n> > \t\tTEST_SCOPED_ENUM_EQUALITY(t, u, e);\n> > \t\tTEST_SCOPED_ENUM_EQUALITY(t, u, f);\n> > +\t\tTEST_SCOPED_ENUM_EQUALITY(t, u, ex);\n> > +\t\tTEST_SCOPED_ENUM_EQUALITY(t, u, exf);\n> > \n> > \t\t/* Test vector of generated structs */\n> > \t\tstd::vector v = { t, u };\n> > @@ -113,6 +120,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > \n> > \t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], e);\n> > \t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], f);\n> > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], ex);\n> > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], exf);\n> > \n> > \t\tTEST_FIELD_EQUALITY(v[1], w[1], s1);\n> > \t\tTEST_FIELD_EQUALITY(v[1], w[1], s2);\n> > @@ -122,6 +131,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > \n> > \t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], e);\n> > \t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], f);\n> > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], ex);\n> > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], exf);\n> > \n> > \t\treturn TestPass;\n> > \t}\n> > diff --git a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> > index 91c31642..1f1ba8dc 100644\n> > --- a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> > +++ b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> > @@ -2,6 +2,8 @@\n> > \n> > module ipa.test;\n> > \n> > +import \"include/libcamera/ipa/core.mojom\";\n> > +\n> > enum IPAOperationCode {\n> > \tIPAOperationNone,\n> > \tIPAOperationInit,\n> > @@ -28,6 +30,8 @@ struct TestStruct {\n> > \tIPAOperationCode c;\n> > \tErrorFlags e;\n> > \t[flags] ErrorFlags f;\n> > +\tlibcamera.TestEnum ex;\n> > +\t[flags] libcamera.TestEnum exf;\n> > };\n> > \n> > interface IPATestInterface {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 777ABBD16B\n\tfor ;\n\tWed, 12 Oct 2022 02:30:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23EE662D83;\n\tWed, 12 Oct 2022 04:30:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DD3361F61\n\tfor ;\n\tWed, 12 Oct 2022 04:30:10 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DD0004DB;\n\tWed, 12 Oct 2022 04:30:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665541812;\n\tbh=/yGIltNMRvOFJD6RYfSiP1RYuFYgGQiZs8CSyLkFQTU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=l44aJyJs2XpXOfYiHdnpFjiGGhv9JL/d5vXSJTmCadgzWPMQ0grZwudFhfSvo/mUl\n\tAjXzF/bEnaVOE8EWIdzkzgNjwGqxhxtPqYk0IsWD4ZF1aaeD7LKvhuslff/UEOMVyF\n\tAeu8K64aEeW+fA4L0orucWObJkTj6rMSUFtW21zYj/+53jJD6gPfIx6uNfOZyCZWeJ\n\tPCXG9n9Ew7NvWne3/q8H6be5CMykU/2Z9ILdwGPfEbDRZi3hMzSmoVDG/yeZ24pBLw\n\t14L/ztcjQoHy1uaMQ4zFVy3sB3tNJdXN0zap4eo6WrZSz6RpEWDiWekI0x8grEId50\n\t8disX0IVqsF9w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1665541810;\n\tbh=/yGIltNMRvOFJD6RYfSiP1RYuFYgGQiZs8CSyLkFQTU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=M/9HOZYpZu+RrCKRI0x/3qSjb1sTbkkma0NsfaS3EK3P++6AsrWW8iM2IiYrB3/W/\n\txCK6OF3tneuUQ44IYq2RtPTyhOmni0OLTEPBYuyBpjs81c9SRY/hREgZ4V2EV3Xe2C\n\t6C5GZMYsmZFhBSdYfegOMjV9beqMt6vR/e/MYr24="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"M/9HOZYp\"; dkim-atps=neutral","Date":"Wed, 12 Oct 2022 11:30:02 +0900","To":"Laurent Pinchart ","Message-ID":"<20221012023002.GB2921939@pyrite.rasen.tech>","References":"<20221011105859.457567-1-paul.elder@ideasonboard.com>\n\t<20221011105859.457567-10-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 9/9] [RFC] test:\n\tgenerated_serializer: Test skipHeader enums and flags","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25424,"web_url":"https://patchwork.libcamera.org/comment/25424/","msgid":"","date":"2022-10-16T02:06:14","subject":"Re: [libcamera-devel] [PATCH v5 9/9] [RFC] test:\n\tgenerated_serializer: Test skipHeader enums and flags","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, Oct 12, 2022 at 11:30:02AM +0900, paul.elder@ideasonboard.com wrote:\n> On Tue, Oct 11, 2022 at 05:01:53PM +0300, Laurent Pinchart wrote:\n> > On Tue, Oct 11, 2022 at 07:58:59PM +0900, Paul Elder via libcamera-devel wrote:\n> > > Add fields to the test struct to test serialization/deserialization of\n> > > scoped enums and flags of said scoped enums that are defined in a C++\n> > > header, which are thus designated by [skipHeader] in mojom.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > \n> > > ---\n> > > New in v5\n> > > \n> > > RFC because this touches core stuff. This can be detached from the rest\n> > > of the series.\n> > > ---\n> > > include/libcamera/ipa/core.mojom | 2 ++\n> > > include/libcamera/ipa/ipa_interface.h | 7 +++++++\n> > > .../generated_serializer_test.cpp | 11 +++++++++++\n> > > .../include/libcamera/ipa/test.mojom | 4 ++++\n> > > 4 files changed, 24 insertions(+)\n> > > \n> > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> > > index 1ff674b0..3438af93 100644\n> > > --- a/include/libcamera/ipa/core.mojom\n> > > +++ b/include/libcamera/ipa/core.mojom\n> > > @@ -84,6 +84,8 @@ module libcamera;\n> > > [skipSerdes, skipHeader] struct ControlList {};\n> > > [skipSerdes, skipHeader] struct SharedFD {};\n> > > \n> > > +[skipHeader, scopedEnum] enum TestEnum {};\n> > > +\n> > \n> > This particular part isn't very nice. Same for the change to\n> > ipa_interface.h below.\n> \n> Yeah that's what I thought :/\n> \n> > We should either find a suitable enum that IPA modules (will) really need,\n> \n> Any ideas? :)\n\nWell... :-)\n\n> > or try to test this feature using test.mojom and/or vimc.mojom.\n> \n> That won't work (that's why this part of the series was delayed). The\n> enums that are skipHeader-ed need to be accessbile from\n> {pipeline}_ipa_interface.h, but those are generated from\n> {pipeline}.mojom. Which means that either 1) the headers that defined\n> those enums need to be #included in the global ipa_interface.h (so *all*\n> IPAs have access to them) or 2) we add a new header that will\n> automatically be imcluded in the generated {pipeline}_ipa_interface.h,\n> which imo defeats the whole purpose of {pipeline}.mojom.\n\nIndeed, neither are nice.\n\nCould we use the mojo import directive ? It's meant to import other\n.mojom files, but maybe we could use it to specify .h files that need to\nbe included ?\n\n> So we need a suitable enum that IPA modules will really need.\n> \n> > > [skipHeader] struct Point {\n> > > \tint32 x;\n> > > \tint32 y;\n> > > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > > index 8884f0ed..96fb8344 100644\n> > > --- a/include/libcamera/ipa/ipa_interface.h\n> > > +++ b/include/libcamera/ipa/ipa_interface.h\n> > > @@ -27,6 +27,13 @@ namespace libcamera {\n> > > * tag must be #included here.\n> > > */\n> > > \n> > > +enum class TestEnum {\n> > > +\tTestEnumValueA,\n> > > +\tTestEnumValueB,\n> > > +\tTestEnumValueC,\n> > > +\tTestEnumValueD,\n> > > +};\n> > > +\n> > > class IPAInterface\n> > > {\n> > > public:\n> > > diff --git a/test/serialization/generated_serializer/generated_serializer_test.cpp b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> > > index 4670fe46..6f01c3d4 100644\n> > > --- a/test/serialization/generated_serializer/generated_serializer_test.cpp\n> > > +++ b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> > > @@ -11,6 +11,8 @@\n> > > \n> > > #include \"test.h\"\n> > > \n> > > +#include \n> > > +\n> > > #include \"test_ipa_interface.h\"\n> > > #include \"test_ipa_serializer.h\"\n> > > \n> > > @@ -59,6 +61,9 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > > \t\tt.s3 = \"lorem ipsum\";\n> > > \t\tt.i = 58527;\n> > > \t\tt.c = ipa::test::IPAOperationInit;\n> > > +\t\tt.ex = TestEnum::TestEnumValueA;\n> > > +\t\tt.exf |= TestEnum::TestEnumValueB;\n> > > +\n> > > \t\tt.e = ipa::test::ErrorFlags::Error1;\n> > > \n> > > \t\tFlags flags;\n> > > @@ -87,6 +92,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > > \n> > > \t\tTEST_SCOPED_ENUM_EQUALITY(t, u, e);\n> > > \t\tTEST_SCOPED_ENUM_EQUALITY(t, u, f);\n> > > +\t\tTEST_SCOPED_ENUM_EQUALITY(t, u, ex);\n> > > +\t\tTEST_SCOPED_ENUM_EQUALITY(t, u, exf);\n> > > \n> > > \t\t/* Test vector of generated structs */\n> > > \t\tstd::vector v = { t, u };\n> > > @@ -113,6 +120,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > > \n> > > \t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], e);\n> > > \t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], f);\n> > > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], ex);\n> > > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[0], w[0], exf);\n> > > \n> > > \t\tTEST_FIELD_EQUALITY(v[1], w[1], s1);\n> > > \t\tTEST_FIELD_EQUALITY(v[1], w[1], s2);\n> > > @@ -122,6 +131,8 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n> > > \n> > > \t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], e);\n> > > \t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], f);\n> > > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], ex);\n> > > +\t\tTEST_SCOPED_ENUM_EQUALITY(v[1], w[1], exf);\n> > > \n> > > \t\treturn TestPass;\n> > > \t}\n> > > diff --git a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> > > index 91c31642..1f1ba8dc 100644\n> > > --- a/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> > > +++ b/test/serialization/generated_serializer/include/libcamera/ipa/test.mojom\n> > > @@ -2,6 +2,8 @@\n> > > \n> > > module ipa.test;\n> > > \n> > > +import \"include/libcamera/ipa/core.mojom\";\n> > > +\n> > > enum IPAOperationCode {\n> > > \tIPAOperationNone,\n> > > \tIPAOperationInit,\n> > > @@ -28,6 +30,8 @@ struct TestStruct {\n> > > \tIPAOperationCode c;\n> > > \tErrorFlags e;\n> > > \t[flags] ErrorFlags f;\n> > > +\tlibcamera.TestEnum ex;\n> > > +\t[flags] libcamera.TestEnum exf;\n> > > };\n> > > \n> > > interface IPATestInterface {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 24E87C0DA4\n\tfor ;\n\tSun, 16 Oct 2022 02:06:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6360862DD8;\n\tSun, 16 Oct 2022 04:06:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 85116603D1\n\tfor ;\n\tSun, 16 Oct 2022 04:06:38 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9CF0130A;\n\tSun, 16 Oct 2022 04:06:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665886000;\n\tbh=rAtrzCvPFtMJl1iwG0PufLpVvV7OgGXEn2P2wz4d9ng=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=WuS8fdISgCWtOhZe2SWCtZ/mlEK9C1wq+VUnGfFr4BoVjPNp9tJ6DPXL4NGamThLK\n\tnXW2cepPmdZ8IfXrSLkW00ezXHUAHrpjszNl2LtuVkiDfovJzMyk+GGcqsSn4upaWB\n\t1ygqTbKYZPE1CKgfv31eS3eWogtDJ99iRoUCFMi2SzfEVRyUnpmGLSdy0TXrdxgEoP\n\tQoj06mPD6MPi+5ahKYbFT0qfuf1eV+rnmzq4YrY7Xroifs2Ud8VWwtDPm/kK8CnR/d\n\t20c/lnvh7Bt9iwMoZttTY7WfddLPa/S5vpKW/2/hHC8njeCft2BpD3mdg8mhgmrKS/\n\tbbVyWrh8msaLA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1665885998;\n\tbh=rAtrzCvPFtMJl1iwG0PufLpVvV7OgGXEn2P2wz4d9ng=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TAbAceq00JhSBA7ZLhIhm5FRXDn1YGJ/K12GIEU+rfZIrGE81EIxhIRbl1L8wRss5\n\t+4E3FDII0QOZnBeslAjM7odO7SLZNB8u0o65KjPsxBR1ngzYf9due9kObLXdkA2UQZ\n\tI6dQBv65zs8LXo+bbRVlD2Yje2OXIxvOj9sCLPj4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TAbAceq0\"; dkim-atps=neutral","Date":"Sun, 16 Oct 2022 05:06:14 +0300","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20221011105859.457567-1-paul.elder@ideasonboard.com>\n\t<20221011105859.457567-10-paul.elder@ideasonboard.com>\n\t\n\t<20221012023002.GB2921939@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221012023002.GB2921939@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v5 9/9] [RFC] test:\n\tgenerated_serializer: Test skipHeader enums and flags","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]