[libcamera-devel,v3,5/7] apps: lc-compliance: Test for mandatory controls

19356 diff mbox series

Test for mandatory controls and properties to be supported by
a Camera.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/apps/lc-compliance/meson.build            |  1 +
 .../lc-compliance/tests/controls_test.cpp     | 98 +++++++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 src/apps/lc-compliance/tests/controls_test.cpp

Quoting Jacopo Mondi via libcamera-devel (2023-12-30 16:29:10)
> Test for mandatory controls and properties to be supported by
> a Camera.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/apps/lc-compliance/meson.build            |  1 +
>  .../lc-compliance/tests/controls_test.cpp     | 98 +++++++++++++++++++
>  2 files changed, 99 insertions(+)
>  create mode 100644 src/apps/lc-compliance/tests/controls_test.cpp
> 
> diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
> index ae8c6f4db51b..c927dd12e9f4 100644
> --- a/src/apps/lc-compliance/meson.build
> +++ b/src/apps/lc-compliance/meson.build
> @@ -15,6 +15,7 @@ lc_compliance_sources = files([
>      'helpers/capture.cpp',
>      'main.cpp',
>      'tests/capture_test.cpp',
> +    'tests/controls_test.cpp'
>  ])
>  
>  lc_compliance_includes = ([
> diff --git a/src/apps/lc-compliance/tests/controls_test.cpp b/src/apps/lc-compliance/tests/controls_test.cpp
> new file mode 100644
> index 000000000000..e9bdf6fdb7e3
> --- /dev/null
> +++ b/src/apps/lc-compliance/tests/controls_test.cpp
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2023, Ideas On Board Oy
> + *
> + * controls_test.cpp - Test controls and properties
> + */
> +
> +#include <array>
> +#include <iostream>
> +
> +#include <libcamera/control_ids.h>
> +#include <libcamera/controls.h>
> +#include <libcamera/property_ids.h>
> +
> +#include <gtest/gtest.h>
> +
> +#include "environment.h"
> +
> +using namespace libcamera;
> +
> +std::array<const ControlIdMap *, 2> controlMaps = {
> +       &controls::controls,
> +       &properties::properties,
> +};
> +
> +class ControlTest : public testing::TestWithParam<const ControlIdMap *>
> +{
> +public:
> +       static std::string nameParameters(const testing::TestParamInfo<ControlTest::ParamType> &info);
> +
> +protected:
> +       void SetUp() override;
> +       void TearDown() override;
> +
> +       std::shared_ptr<Camera> camera_;
> +};
> +
> +void ControlTest::SetUp()
> +{
> +       Environment *env = Environment::get();
> +
> +       camera_ = env->cm()->get(env->cameraId());
> +
> +       ASSERT_EQ(camera_->acquire(), 0);
> +}
> +
> +void ControlTest::TearDown()
> +{
> +       if (!camera_)
> +               return;
> +
> +       camera_->release();
> +       camera_.reset();
> +}
> +
> +std::string ControlTest::nameParameters(const testing::TestParamInfo<ControlTest::ParamType> &info)
> +{
> +       const ControlIdMap *idMap = info.param;
> +       if (idMap == &controls::controls)
> +               return "controls";
> +       else if (idMap == &properties::properties)
> +               return "properties";
> +
> +       return "vendor";
> +}
> +
> +/* Test that mandatory controls and properties are supported by a camera. */
> +TEST_P(ControlTest, RequiredControls)
> +{
> +       auto controlMap = GetParam();
> +
> +       for (const auto &[id, ctrl] : *controlMap) {
> +               if (!ctrl->required())
> +                       continue;
> +
> +               if (controlMap == &controls::controls) {
> +                       const auto it = camera_->controls().find(ctrl);
> +
> +                       if (it == camera_->controls().end())
> +                               FAIL() << "Mandatory control \"" << ctrl->name()
> +                                      << "\" not supported" << std::endl;
> +               } else if (controlMap == &properties::properties) {
> +                       bool found = camera_->properties().contains(id);
> +
> +                       if (!found)
> +                               FAIL() << "Mandatory property \"" << ctrl->name()
> +                                      << "\" not supported" << std::endl;
> +               }
> +       }
> +}
> +
> +/*
> + * Use a Value-Parametrized Test case so that vendors can easily test vendor
> + * control lists by expading 'controlMaps'.
> + */

s/expading/expanding/


Where does this happen? I can't see a list of mandatory controls in this
test. Are any vendor controls even possible to be made Mandatory? I
can't see how vendor controls could be mandatory because they couldn't
apply to all cameras...



> +INSTANTIATE_TEST_SUITE_P(ControlsTest, ControlTest,
> +                        testing::ValuesIn(controlMaps),
> +                        ControlTest::nameParameters);
> -- 
> 2.41.0
>

Hi Kieran

On Fri, Jan 12, 2024 at 11:56:04AM +0000, Kieran Bingham wrote:
> Quoting Jacopo Mondi via libcamera-devel (2023-12-30 16:29:10)
> > Test for mandatory controls and properties to be supported by
> > a Camera.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/apps/lc-compliance/meson.build            |  1 +
> >  .../lc-compliance/tests/controls_test.cpp     | 98 +++++++++++++++++++
> >  2 files changed, 99 insertions(+)
> >  create mode 100644 src/apps/lc-compliance/tests/controls_test.cpp
> >
> > diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
> > index ae8c6f4db51b..c927dd12e9f4 100644
> > --- a/src/apps/lc-compliance/meson.build
> > +++ b/src/apps/lc-compliance/meson.build
> > @@ -15,6 +15,7 @@ lc_compliance_sources = files([
> >      'helpers/capture.cpp',
> >      'main.cpp',
> >      'tests/capture_test.cpp',
> > +    'tests/controls_test.cpp'
> >  ])
> >
> >  lc_compliance_includes = ([
> > diff --git a/src/apps/lc-compliance/tests/controls_test.cpp b/src/apps/lc-compliance/tests/controls_test.cpp
> > new file mode 100644
> > index 000000000000..e9bdf6fdb7e3
> > --- /dev/null
> > +++ b/src/apps/lc-compliance/tests/controls_test.cpp
> > @@ -0,0 +1,98 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2023, Ideas On Board Oy
> > + *
> > + * controls_test.cpp - Test controls and properties
> > + */
> > +
> > +#include <array>
> > +#include <iostream>
> > +
> > +#include <libcamera/control_ids.h>
> > +#include <libcamera/controls.h>
> > +#include <libcamera/property_ids.h>
> > +
> > +#include <gtest/gtest.h>
> > +
> > +#include "environment.h"
> > +
> > +using namespace libcamera;
> > +
> > +std::array<const ControlIdMap *, 2> controlMaps = {
> > +       &controls::controls,
> > +       &properties::properties,
> > +};
> > +
> > +class ControlTest : public testing::TestWithParam<const ControlIdMap *>
> > +{
> > +public:
> > +       static std::string nameParameters(const testing::TestParamInfo<ControlTest::ParamType> &info);
> > +
> > +protected:
> > +       void SetUp() override;
> > +       void TearDown() override;
> > +
> > +       std::shared_ptr<Camera> camera_;
> > +};
> > +
> > +void ControlTest::SetUp()
> > +{
> > +       Environment *env = Environment::get();
> > +
> > +       camera_ = env->cm()->get(env->cameraId());
> > +
> > +       ASSERT_EQ(camera_->acquire(), 0);
> > +}
> > +
> > +void ControlTest::TearDown()
> > +{
> > +       if (!camera_)
> > +               return;
> > +
> > +       camera_->release();
> > +       camera_.reset();
> > +}
> > +
> > +std::string ControlTest::nameParameters(const testing::TestParamInfo<ControlTest::ParamType> &info)
> > +{
> > +       const ControlIdMap *idMap = info.param;
> > +       if (idMap == &controls::controls)
> > +               return "controls";
> > +       else if (idMap == &properties::properties)
> > +               return "properties";
> > +
> > +       return "vendor";
> > +}
> > +
> > +/* Test that mandatory controls and properties are supported by a camera. */
> > +TEST_P(ControlTest, RequiredControls)
> > +{
> > +       auto controlMap = GetParam();
> > +
> > +       for (const auto &[id, ctrl] : *controlMap) {
> > +               if (!ctrl->required())
> > +                       continue;
> > +
> > +               if (controlMap == &controls::controls) {
> > +                       const auto it = camera_->controls().find(ctrl);
> > +
> > +                       if (it == camera_->controls().end())
> > +                               FAIL() << "Mandatory control \"" << ctrl->name()
> > +                                      << "\" not supported" << std::endl;
> > +               } else if (controlMap == &properties::properties) {
> > +                       bool found = camera_->properties().contains(id);
> > +
> > +                       if (!found)
> > +                               FAIL() << "Mandatory property \"" << ctrl->name()
> > +                                      << "\" not supported" << std::endl;
> > +               }
> > +       }
> > +}
> > +
> > +/*
> > + * Use a Value-Parametrized Test case so that vendors can easily test vendor
> > + * control lists by expading 'controlMaps'.
> > + */
>
> s/expading/expanding/
>
>
> Where does this happen? I can't see a list of mandatory controls in this
> test. Are any vendor controls even possible to be made Mandatory? I

The series is based on "[PATCH 0/2] libcamera: Add support for required
controls" where mandatory controls and properties are flagged in the
controls/property definition itself

> can't see how vendor controls could be mandatory because they couldn't
> apply to all cameras...

No, but the comment was there to explain why I chose to go for
Value-Parametrized test. I could have simply iterated tests on
controls::controls and properties::properties, but using 'controlMaps'
a vendor can easily add its own properties/control vector there. The
use case I was thinking about is mostly to validate properties and
controls that comes from a sensor, not the ph as vendors are in
control of it.

>
>
>
> > +INSTANTIATE_TEST_SUITE_P(ControlsTest, ControlTest,
> > +                        testing::ValuesIn(controlMaps),
> > +                        ControlTest::nameParameters);
> > --
> > 2.41.0
> >