[{"id":28454,"web_url":"https://patchwork.libcamera.org/comment/28454/","msgid":"<170506056491.3227220.14430535097223904641@ping.linuxembedded.co.uk>","date":"2024-01-12T11:56:04","subject":"Re: [libcamera-devel] [PATCH v3 5/7] apps: lc-compliance: Test for\n\tmandatory controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2023-12-30 16:29:10)\n> Test for mandatory controls and properties to be supported by\n> a Camera.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/apps/lc-compliance/meson.build | 1 +\n> .../lc-compliance/tests/controls_test.cpp | 98 +++++++++++++++++++\n> 2 files changed, 99 insertions(+)\n> create mode 100644 src/apps/lc-compliance/tests/controls_test.cpp\n> \n> diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build\n> index ae8c6f4db51b..c927dd12e9f4 100644\n> --- a/src/apps/lc-compliance/meson.build\n> +++ b/src/apps/lc-compliance/meson.build\n> @@ -15,6 +15,7 @@ lc_compliance_sources = files([\n> 'helpers/capture.cpp',\n> 'main.cpp',\n> 'tests/capture_test.cpp',\n> + 'tests/controls_test.cpp'\n> ])\n> \n> lc_compliance_includes = ([\n> diff --git a/src/apps/lc-compliance/tests/controls_test.cpp b/src/apps/lc-compliance/tests/controls_test.cpp\n> new file mode 100644\n> index 000000000000..e9bdf6fdb7e3\n> --- /dev/null\n> +++ b/src/apps/lc-compliance/tests/controls_test.cpp\n> @@ -0,0 +1,98 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas On Board Oy\n> + *\n> + * controls_test.cpp - Test controls and properties\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"environment.h\"\n> +\n> +using namespace libcamera;\n> +\n> +std::array controlMaps = {\n> + &controls::controls,\n> + &properties::properties,\n> +};\n> +\n> +class ControlTest : public testing::TestWithParam\n> +{\n> +public:\n> + static std::string nameParameters(const testing::TestParamInfo &info);\n> +\n> +protected:\n> + void SetUp() override;\n> + void TearDown() override;\n> +\n> + std::shared_ptr camera_;\n> +};\n> +\n> +void ControlTest::SetUp()\n> +{\n> + Environment *env = Environment::get();\n> +\n> + camera_ = env->cm()->get(env->cameraId());\n> +\n> + ASSERT_EQ(camera_->acquire(), 0);\n> +}\n> +\n> +void ControlTest::TearDown()\n> +{\n> + if (!camera_)\n> + return;\n> +\n> + camera_->release();\n> + camera_.reset();\n> +}\n> +\n> +std::string ControlTest::nameParameters(const testing::TestParamInfo &info)\n> +{\n> + const ControlIdMap *idMap = info.param;\n> + if (idMap == &controls::controls)\n> + return \"controls\";\n> + else if (idMap == &properties::properties)\n> + return \"properties\";\n> +\n> + return \"vendor\";\n> +}\n> +\n> +/* Test that mandatory controls and properties are supported by a camera. */\n> +TEST_P(ControlTest, RequiredControls)\n> +{\n> + auto controlMap = GetParam();\n> +\n> + for (const auto &[id, ctrl] : *controlMap) {\n> + if (!ctrl->required())\n> + continue;\n> +\n> + if (controlMap == &controls::controls) {\n> + const auto it = camera_->controls().find(ctrl);\n> +\n> + if (it == camera_->controls().end())\n> + FAIL() << \"Mandatory control \\\"\" << ctrl->name()\n> + << \"\\\" not supported\" << std::endl;\n> + } else if (controlMap == &properties::properties) {\n> + bool found = camera_->properties().contains(id);\n> +\n> + if (!found)\n> + FAIL() << \"Mandatory property \\\"\" << ctrl->name()\n> + << \"\\\" not supported\" << std::endl;\n> + }\n> + }\n> +}\n> +\n> +/*\n> + * Use a Value-Parametrized Test case so that vendors can easily test vendor\n> + * control lists by expading 'controlMaps'.\n> + */\n\ns/expading/expanding/\n\n\nWhere does this happen? I can't see a list of mandatory controls in this\ntest. Are any vendor controls even possible to be made Mandatory? I\ncan't see how vendor controls could be mandatory because they couldn't\napply to all cameras...\n\n\n\n> +INSTANTIATE_TEST_SUITE_P(ControlsTest, ControlTest,\n> + testing::ValuesIn(controlMaps),\n> + ControlTest::nameParameters);\n> -- \n> 2.41.0\n>","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 7E799C323E\n\tfor ;\n\tFri, 12 Jan 2024 11:56:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E96A1628B6;\n\tFri, 12 Jan 2024 12:56:08 +0100 (CET)","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 6F28961E17\n\tfor ;\n\tFri, 12 Jan 2024 12:56:07 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 616B97FC;\n\tFri, 12 Jan 2024 12:55:01 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705060568;\n\tbh=nIYVnTol4/SfZP83qjyPjofJPVL3mG882Bfkd+1F7rU=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=SF3+kwC1O7IqZP7AKThbviVHEB8p9M1lswEBhIm2n4laIOm0VNkVBT2SvJEi3khf2\n\ty9wHzp4YrZ3nx4Ibz/whmF2LVPQWIi1OkW5uknmba9uoTGVSE/o37BCmrzgssOjMaV\n\tLArUzY4GhFVzLlagnl444UJzj54lOgI+ac9kgD9qrIAMUwdsG4gaXPdT2cFbLU/iVC\n\t41Mt58xa3HF2DNhQ5HeL4W5OTVMGZ5hVSkmUk2RdTXWFn8OOL4/xmEmV5NW2pBh1ku\n\taRBuHM0vVUPG573FLGL1HNygCSn5ZzOhiZIPNsEzNusqqMUeu5OL5lUaXfhf92Kvoc\n\t7rSc1OT6cCNmg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705060501;\n\tbh=nIYVnTol4/SfZP83qjyPjofJPVL3mG882Bfkd+1F7rU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=JVdDAsi8c7ssBO0GH5zKijnq0WJtM6WfUL514uQCl/m6Izzt6uXU5dZR1rtYEzzP2\n\tozILrkXPJYcjTvP21aJgUJsqAinIp3FdH99tD868lkntmHrILd/HiqqIb6QzScSB/+\n\tGvjO12BKqJ4bS3ruseJbBtUN4khds9VhPzZ/tBK8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JVdDAsi8\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20231230162912.827669-6-jacopo.mondi@ideasonboard.com>","References":"<20231230162912.827669-1-jacopo.mondi@ideasonboard.com>\n\t<20231230162912.827669-6-jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 12 Jan 2024 11:56:04 +0000","Message-ID":"<170506056491.3227220.14430535097223904641@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 5/7] apps: lc-compliance: Test for\n\tmandatory controls","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Jacopo Mondi ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28455,"web_url":"https://patchwork.libcamera.org/comment/28455/","msgid":"","date":"2024-01-12T12:23:13","subject":"Re: [libcamera-devel] [PATCH v3 5/7] apps: lc-compliance: Test for\n\tmandatory controls","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Fri, Jan 12, 2024 at 11:56:04AM +0000, Kieran Bingham wrote:\n> Quoting Jacopo Mondi via libcamera-devel (2023-12-30 16:29:10)\n> > Test for mandatory controls and properties to be supported by\n> > a Camera.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/apps/lc-compliance/meson.build | 1 +\n> > .../lc-compliance/tests/controls_test.cpp | 98 +++++++++++++++++++\n> > 2 files changed, 99 insertions(+)\n> > create mode 100644 src/apps/lc-compliance/tests/controls_test.cpp\n> >\n> > diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build\n> > index ae8c6f4db51b..c927dd12e9f4 100644\n> > --- a/src/apps/lc-compliance/meson.build\n> > +++ b/src/apps/lc-compliance/meson.build\n> > @@ -15,6 +15,7 @@ lc_compliance_sources = files([\n> > 'helpers/capture.cpp',\n> > 'main.cpp',\n> > 'tests/capture_test.cpp',\n> > + 'tests/controls_test.cpp'\n> > ])\n> >\n> > lc_compliance_includes = ([\n> > diff --git a/src/apps/lc-compliance/tests/controls_test.cpp b/src/apps/lc-compliance/tests/controls_test.cpp\n> > new file mode 100644\n> > index 000000000000..e9bdf6fdb7e3\n> > --- /dev/null\n> > +++ b/src/apps/lc-compliance/tests/controls_test.cpp\n> > @@ -0,0 +1,98 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Ideas On Board Oy\n> > + *\n> > + * controls_test.cpp - Test controls and properties\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"environment.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +std::array controlMaps = {\n> > + &controls::controls,\n> > + &properties::properties,\n> > +};\n> > +\n> > +class ControlTest : public testing::TestWithParam\n> > +{\n> > +public:\n> > + static std::string nameParameters(const testing::TestParamInfo &info);\n> > +\n> > +protected:\n> > + void SetUp() override;\n> > + void TearDown() override;\n> > +\n> > + std::shared_ptr camera_;\n> > +};\n> > +\n> > +void ControlTest::SetUp()\n> > +{\n> > + Environment *env = Environment::get();\n> > +\n> > + camera_ = env->cm()->get(env->cameraId());\n> > +\n> > + ASSERT_EQ(camera_->acquire(), 0);\n> > +}\n> > +\n> > +void ControlTest::TearDown()\n> > +{\n> > + if (!camera_)\n> > + return;\n> > +\n> > + camera_->release();\n> > + camera_.reset();\n> > +}\n> > +\n> > +std::string ControlTest::nameParameters(const testing::TestParamInfo &info)\n> > +{\n> > + const ControlIdMap *idMap = info.param;\n> > + if (idMap == &controls::controls)\n> > + return \"controls\";\n> > + else if (idMap == &properties::properties)\n> > + return \"properties\";\n> > +\n> > + return \"vendor\";\n> > +}\n> > +\n> > +/* Test that mandatory controls and properties are supported by a camera. */\n> > +TEST_P(ControlTest, RequiredControls)\n> > +{\n> > + auto controlMap = GetParam();\n> > +\n> > + for (const auto &[id, ctrl] : *controlMap) {\n> > + if (!ctrl->required())\n> > + continue;\n> > +\n> > + if (controlMap == &controls::controls) {\n> > + const auto it = camera_->controls().find(ctrl);\n> > +\n> > + if (it == camera_->controls().end())\n> > + FAIL() << \"Mandatory control \\\"\" << ctrl->name()\n> > + << \"\\\" not supported\" << std::endl;\n> > + } else if (controlMap == &properties::properties) {\n> > + bool found = camera_->properties().contains(id);\n> > +\n> > + if (!found)\n> > + FAIL() << \"Mandatory property \\\"\" << ctrl->name()\n> > + << \"\\\" not supported\" << std::endl;\n> > + }\n> > + }\n> > +}\n> > +\n> > +/*\n> > + * Use a Value-Parametrized Test case so that vendors can easily test vendor\n> > + * control lists by expading 'controlMaps'.\n> > + */\n>\n> s/expading/expanding/\n>\n>\n> Where does this happen? I can't see a list of mandatory controls in this\n> test. Are any vendor controls even possible to be made Mandatory? I\n\nThe series is based on \"[PATCH 0/2] libcamera: Add support for required\ncontrols\" where mandatory controls and properties are flagged in the\ncontrols/property definition itself\n\n> can't see how vendor controls could be mandatory because they couldn't\n> apply to all cameras...\n\nNo, but the comment was there to explain why I chose to go for\nValue-Parametrized test. I could have simply iterated tests on\ncontrols::controls and properties::properties, but using 'controlMaps'\na vendor can easily add its own properties/control vector there. The\nuse case I was thinking about is mostly to validate properties and\ncontrols that comes from a sensor, not the ph as vendors are in\ncontrol of it.\n\n>\n>\n>\n> > +INSTANTIATE_TEST_SUITE_P(ControlsTest, ControlTest,\n> > + testing::ValuesIn(controlMaps),\n> > + ControlTest::nameParameters);\n> > --\n> > 2.41.0\n> >","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 34881C323E\n\tfor ;\n\tFri, 12 Jan 2024 12:23:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70D83628B6;\n\tFri, 12 Jan 2024 13:23:17 +0100 (CET)","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 D9EBC61E17\n\tfor ;\n\tFri, 12 Jan 2024 13:23:16 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9068EE62;\n\tFri, 12 Jan 2024 13:22:10 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705062197;\n\tbh=6hTth/tmDmVVCoUP/5fUJ6JH+5EwL9GQnS8Dopf0Okg=;\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=vl31/MlhWVim/DD1/bSFrb3yNUSxok42r8HAzIYrMtwkEEp7024Cx+JwXxXiJ/K7n\n\tMI9XpiSH9VjXnA+pS+GcqDAqS+LOLiNewpbmRICsLGZcTSvK0MDmcIebvvMHHJyyfQ\n\t9S0eQzTb18cvZMUmhOgjL67rQJOl0vUy+O7dp1R0VHOYxzU5cUcfYbo9Iq5PmVQr6T\n\tN+xCIjLn3qTULp1a5xYhku8Ve7/1FBFGUwvRmZw3NPUHnoU0PvJ5BtOFNkHZB8tayy\n\tDc5nrpH6WopVwsTh4vWw5fZz5n7HvOa1yUTS38LHoSUduRk1Yin31v0sy1sKqm6ZI3\n\tqHknHiE3aYWPA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705062130;\n\tbh=6hTth/tmDmVVCoUP/5fUJ6JH+5EwL9GQnS8Dopf0Okg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=o9FQwRL5h3TiLD364YCtlgvSkh1i8g4+hPVA/RjbNfRZtWNBMgVAYx36Z0+RAf3j1\n\tsgL+6iOZq2hAArtBB36r7xgSwM0VPSC4lQHzuMFRu7y725C60R3WcEMI7bnQPY/3G3\n\t3riv+2/uHv7YRJc3/UlhMHoQksJZ9ZprLFmMoW4U="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"o9FQwRL5\"; dkim-atps=neutral","Date":"Fri, 12 Jan 2024 13:23:13 +0100","To":"Kieran Bingham ","Message-ID":"","References":"<20231230162912.827669-1-jacopo.mondi@ideasonboard.com>\n\t<20231230162912.827669-6-jacopo.mondi@ideasonboard.com>\n\t<170506056491.3227220.14430535097223904641@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<170506056491.3227220.14430535097223904641@ping.linuxembedded.co.uk>","Subject":"Re: [libcamera-devel] [PATCH v3 5/7] apps: lc-compliance: Test for\n\tmandatory controls","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]