[libcamera-devel] [PATCH 07/12] libcamera: controls: Rename ControlInfo to ControlRange

[Laurent Pinchart]

Hi Jacopo,

On Sun, Sep 29, 2019 at 12:06:36PM +0200, Jacopo Mondi wrote:
> On Sat, Sep 28, 2019 at 06:22:33PM +0300, Laurent Pinchart wrote:
> > The ControlInfo class stores a range of valid values for a control. Its
> > name is vague, as "info" has multiple meanings. Rename it to
> > ControlRange.
> 
> Don't want to start discussing names, just pointing out Range applies
> well as long as this only describes min and max. I expect it to grow
> with more validation information

I'm aware of that, and we can rename the class then. Hopefully it will
give us a bit more time to think about a better name :-)

> Also, it seems ControlInfoMap stays with the same name, which still
> includes 'Info'. Is this intended ?

I think we'll rename that class eventually. A bit more work is needed in
that area.

> Anwyay
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/controls.h                  |  8 +++---
> >  src/libcamera/controls.cpp                    | 28 +++++++++----------
> >  .../{control_info.cpp => control_range.cpp}   | 14 +++++-----
> >  test/controls/meson.build                     |  2 +-
> >  4 files changed, 26 insertions(+), 26 deletions(-)
> >  rename test/controls/{control_info.cpp => control_range.cpp} (75%)
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 854ea3b84267..d3eea643c0ec 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -95,11 +95,11 @@ private:
> >  	Control &operator=(const Control &) = delete;
> >  };
> >
> > -class ControlInfo
> > +class ControlRange
> >  {
> >  public:
> > -	explicit ControlInfo(const ControlValue &min = 0,
> > -			     const ControlValue &max = 0);
> > +	explicit ControlRange(const ControlValue &min = 0,
> > +			      const ControlValue &max = 0);
> >
> >  	const ControlValue &min() const { return min_; }
> >  	const ControlValue &max() const { return max_; }
> > @@ -111,7 +111,7 @@ private:
> >  	ControlValue max_;
> >  };
> >
> > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlInfo>;
> > +using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> >
> >  class ControlList
> >  {
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 6a3bb353792d..51abb4ea7f6f 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -312,42 +312,42 @@ Control<int64_t>::Control(unsigned int id, const char *name)
> >  #endif /* __DOXYGEN__ */
> >
> >  /**
> > - * \class ControlInfo
> > - * \brief Describe the information and capabilities of a Control
> > + * \class ControlRange
> > + * \brief Describe the limits of valid values for a Control
> >   *
> > - * The ControlInfo represents control specific meta-data which is constant on a
> > - * per camera basis. ControlInfo classes are constructed by pipeline handlers
> > - * to expose the controls they support and the metadata needed to utilise those
> > - * controls.
> > + * The ControlRange expresses the constraints on valid values for a control.
> > + * The constraints depend on the object the control applies to, and are
> > + * constant for the lifetime of that object. They are typically constructed by
> > + * pipeline handlers to describe the controls they support.
> 
> to describe the validation criteria/limits of the controls they
> support.
> 
> >   */
> >
> >  /**
> > - * \brief Construct a ControlInfo with minimum and maximum range parameters
> > + * \brief Construct a ControlRange with minimum and maximum range parameters
> >   * \param[in] min The control minimum value
> >   * \param[in] max The control maximum value
> >   */
> > -ControlInfo::ControlInfo(const ControlValue &min,
> > -			 const ControlValue &max)
> > +ControlRange::ControlRange(const ControlValue &min,
> > +			   const ControlValue &max)
> >  	: min_(min), max_(max)
> >  {
> >  }
> >
> >  /**
> > - * \fn ControlInfo::min()
> > + * \fn ControlRange::min()
> >   * \brief Retrieve the minimum value of the control
> >   * \return A ControlValue with the minimum value for the control
> >   */
> >
> >  /**
> > - * \fn ControlInfo::max()
> > + * \fn ControlRange::max()
> >   * \brief Retrieve the maximum value of the control
> >   * \return A ControlValue with the maximum value for the control
> >   */
> >
> >  /**
> > - * \brief Provide a string representation of the ControlInfo
> > + * \brief Provide a string representation of the ControlRange
> >   */
> > -std::string ControlInfo::toString() const
> > +std::string ControlRange::toString() const
> >  {
> >  	std::stringstream ss;
> >
> > @@ -358,7 +358,7 @@ std::string ControlInfo::toString() const
> >
> >  /**
> >   * \typedef ControlInfoMap
> > - * \brief A map of ControlId to ControlInfo
> > + * \brief A map of ControlId to ControlRange
> >   */
> >
> >  /**
> > diff --git a/test/controls/control_info.cpp b/test/controls/control_range.cpp
> > similarity index 75%
> > rename from test/controls/control_info.cpp
> > rename to test/controls/control_range.cpp
> > index 9cf59185e459..06ec3506ee62 100644
> > --- a/test/controls/control_info.cpp
> > +++ b/test/controls/control_range.cpp
> > @@ -2,7 +2,7 @@
> >  /*
> >   * Copyright (C) 2019, Google Inc.
> >   *
> > - * control_info.cpp - ControlInfo tests
> > + * control_range.cpp - ControlRange tests
> >   */
> >
> >  #include <iostream>
> > @@ -15,16 +15,16 @@
> >  using namespace std;
> >  using namespace libcamera;
> >
> > -class ControlInfoTest : public Test
> > +class ControlRangeTest : public Test
> >  {
> >  protected:
> >  	int run()
> >  	{
> >  		/*
> > -		 * Test information retrieval from a control with no minimum
> > -		 * and maximum.
> > +		 * Test information retrieval from a range with no minimum and
> > +		 * maximum.
> >  		 */
> > -		ControlInfo brightness;
> > +		ControlRange brightness;
> >
> >  		if (brightness.min().get<int32_t>() != 0 ||
> >  		    brightness.max().get<int32_t>() != 0) {
> > @@ -36,7 +36,7 @@ protected:
> >  		 * Test information retrieval from a control with a minimum and
> >  		 * a maximum value.
> >  		 */
> > -		ControlInfo contrast(10, 200);
> > +		ControlRange contrast(10, 200);
> >
> >  		if (contrast.min().get<int32_t>() != 10 ||
> >  		    contrast.max().get<int32_t>() != 200) {
> > @@ -48,4 +48,4 @@ protected:
> >  	}
> >  };
> >
> > -TEST_REGISTER(ControlInfoTest)
> > +TEST_REGISTER(ControlRangeTest)
> > diff --git a/test/controls/meson.build b/test/controls/meson.build
> > index f4fc7b947dd9..9f0f005a2759 100644
> > --- a/test/controls/meson.build
> > +++ b/test/controls/meson.build
> > @@ -1,6 +1,6 @@
> >  control_tests = [
> > -    [ 'control_info',   'control_info.cpp' ],
> >      [ 'control_list',   'control_list.cpp' ],
> > +    [ 'control_range',  'control_range.cpp' ],
> >      [ 'control_value',  'control_value.cpp' ],
> >  ]
> >

-- 
Regards,

Laurent Pinchart