[libcamera-devel] [PATCH 09/31] libcamera: controls: Decouple control and value type in ControlList::set()

[Laurent Pinchart]

On Tue, Mar 03, 2020 at 07:56:56PM +0200, Laurent Pinchart wrote:
> On Mon, Mar 02, 2020 at 11:06:15PM +0000, Kieran Bingham wrote:
> > On 29/02/2020 16:42, Laurent Pinchart wrote:
> > > The ControlList::set() method takes a reference to a Control<T>, and
> > > requires the value to be a reference to T. This prevents the set()
> > > method to be used with value types that are convertible to T,
> > 
> > s/method to be used/method from being used/
> > 
> > :gqip ?
> 
> Not an editor command :-) (s/://)
> 
> > > and in particular with std::array or std::vector values types when the
> > 
> > /values/value/
> > 
> > /in particular/particularly/
> > 
> > > Control type will be a Span<> to support compound controls.
> > 
> > /will be a/is a/
> > 
> > > 
> > > Fix this by decoupling the control type and value type in the template
> > > parameters. The compiler will still catch invalid conversions, including
> > > cases where constructor a T from the value type is explicit.
> > 
> > "where constructor a T from" ?
> > 
> > That's confusing, not sure what you actually meant...
> > 
> > "where a constructor T form the value .." ?
> > "where the constructor of type T from the value..." ?
> 
> Oops. I meant "where constructing".

But I like your last suggestion better, I'll use it.

> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > > ---
> > >  include/libcamera/controls.h | 4 ++--
> > >  src/libcamera/controls.cpp   | 2 +-
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index 9f8a9031bd74..9d93064c11b2 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -223,8 +223,8 @@ public:
> > >  		return val->get<T>();
> > >  	}
> > >  
> > > -	template<typename T>
> > > -	void set(const Control<T> &ctrl, const T &value)
> > > +	template<typename T, typename V>
> > > +	void set(const Control<T> &ctrl, const V &value)
> > >  	{
> > >  		ControlValue *val = find(ctrl.id());
> > >  		if (!val)
> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > > index f632d60adc8b..a136ebd2653b 100644
> > > --- a/src/libcamera/controls.cpp
> > > +++ b/src/libcamera/controls.cpp
> > > @@ -735,7 +735,7 @@ bool ControlList::contains(unsigned int id) const
> > >   */
> > >  
> > >  /**
> > > - * \fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value)
> > > + * \fn template<typename T, typename V> void ControlList::set(const Control<T> &ctrl, const V &value)
> > >   * \brief Set the control \a ctrl value to \a value
> > >   * \param[in] ctrl The control
> > >   * \param[in] value The control value

-- 
Regards,

Laurent Pinchart