| Message ID | 20190928152238.23752-8-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, 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 Also, it seems ControlInfoMap stays with the same name, which still includes 'Info'. Is this intended ? Anwyay Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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@jmondi.org> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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' ], > > ] > >
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. */ /** - * \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' ], ]
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@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%)