[libcamera-devel] [PATCH v2 1/2] libcamera: control: initialise control info to ControlTypeNone by default

[Kieran Bingham]

Quoting Tomi Valkeinen via libcamera-devel (2022-09-30 12:14:12)
> Hi,
> 
> On 30/09/2022 14:01, David Plowman wrote:
> > Hi Christian
> > 
> > I have a question about this because it's causing my Python code to
> > break. Not in any major way, we just need to decide what we want to do
> > about it.
> > 
> > The problem is that you can no longer fetch the default value for a
> > control, it causes a runtime error. The code in question is here:
> > https://git.linuxtv.org/libcamera.git/tree/src/py/libcamera/py_helpers.cpp#n57
> > 
> > There are a number of straightforward solutions, such as returning
> > "py::none()", for example. Does that sound like a reasonable thing to
> > do?
> 
> Are we supposed to have uninitialized values? In that case py::none 
> sounds fine. Or is it a bug if a control has an uninitialized default? 
> Then throwing an exception, as we do now, sounds fine.

Yes, I think the impact was we essentially introduced the abiltiy to
'not' specify a default value. There are controls, where it simply
doesn't make sense to have a default.

So I think returning py::none might be the most correct thing to do ?

--
Kieran


> 
>   Tomi
> 
> 
> > Thanks!
> > David
> > 
> > On Tue, 30 Aug 2022 at 21:21, Christian Rauch via libcamera-devel
> > <libcamera-devel at lists.libcamera.org> wrote:
> >>
> >> The default ControlInfo constructor allows to partially initialised the
> >> min/max/def values. Uninitialised values are assigned to 0 by default. This
> >> implicit initialisation makes it impossible to distinguish between and
> >> uninitialised and an explicitly 0-initialised ControlValue.
> >>
> >> Default construct the ControlValue in the ControlInfo default contructor to
> >> explicitly represent uninitialised values by the ControlTypeNone type.
> >>
> >> Signed-off-by: Christian Rauch <Rauch.Christian at gmx.de>
> >> ---
> >>   include/libcamera/controls.h | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> >> index ebc168fc..38d0a3e8 100644
> >> --- a/include/libcamera/controls.h
> >> +++ b/include/libcamera/controls.h
> >> @@ -268,9 +268,9 @@ private:
> >>   class ControlInfo
> >>   {
> >>   public:
> >> -       explicit ControlInfo(const ControlValue &min = 0,
> >> -                            const ControlValue &max = 0,
> >> -                            const ControlValue &def = 0);
> >> +       explicit ControlInfo(const ControlValue &min = {},
> >> +                            const ControlValue &max = {},
> >> +                            const ControlValue &def = {});
> >>          explicit ControlInfo(Span<const ControlValue> values,
> >>                               const ControlValue &def = {});
> >>          explicit ControlInfo(std::set<bool> values, bool def);
> >> --
> >> 2.34.1
> >>
>