[libcamera-devel] [PATCH 01/12] libcamera: controls: Rename ControlValueType to ControlType
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Sep 29 13:24:48 CEST 2019
Hi Jacopo,
On Sun, Sep 29, 2019 at 10:52:26AM +0200, Jacopo Mondi wrote:
> On Sat, Sep 28, 2019 at 06:22:27PM +0300, Laurent Pinchart wrote:
> > The type of a control value is also the type of the control. Shorten the
> > ControlValueType enumeration to ControlType, and rename ControlValue* to
> > ControlType* for better clarity.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > include/libcamera/controls.h | 20 +++++++-------
> > src/libcamera/controls.cpp | 50 +++++++++++++++++-----------------
> > src/libcamera/gen-controls.awk | 2 +-
> > test/controls/control_info.cpp | 4 +--
> > 4 files changed, 38 insertions(+), 38 deletions(-)
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index fbb3a62274c6..ffba880a66ff 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -18,11 +18,11 @@ namespace libcamera {
> >
> > class Camera;
> >
> > -enum ControlValueType {
> > - ControlValueNone,
> > - ControlValueBool,
> > - ControlValueInteger,
> > - ControlValueInteger64,
> > +enum ControlType {
> > + ControlTypeNone,
> > + ControlTypeBool,
> > + ControlTypeInteger,
> > + ControlTypeInteger64,
>
> I wonder if these would not be worth being ControlBool,
> ControlInteger etc...
It's a very good question, and something we may want to think of more
globally. Should enumerated values contain the enum name ? We have
precedents for both in libcamera, and unifying the code with a single
policy would be more consistent. The only issue I have at the moment is
that I'm not sure what the rule should be :-)
> In any case
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > };
> >
> > class ControlValue
> > @@ -33,8 +33,8 @@ public:
> > ControlValue(int value);
> > ControlValue(int64_t value);
> >
> > - ControlValueType type() const { return type_; };
> > - bool isNone() const { return type_ == ControlValueNone; };
> > + ControlType type() const { return type_; };
> > + bool isNone() const { return type_ == ControlTypeNone; };
> >
> > void set(bool value);
> > void set(int value);
> > @@ -47,7 +47,7 @@ public:
> > std::string toString() const;
> >
> > private:
> > - ControlValueType type_;
> > + ControlType type_;
> >
> > union {
> > bool bool_;
> > @@ -59,7 +59,7 @@ private:
> > struct ControlIdentifier {
> > ControlId id;
> > const char *name;
> > - ControlValueType type;
> > + ControlType type;
> > };
> >
> > class ControlInfo
> > @@ -70,7 +70,7 @@ public:
> >
> > ControlId id() const { return ident_->id; }
> > const char *name() const { return ident_->name; }
> > - ControlValueType type() const { return ident_->type; }
> > + ControlType type() const { return ident_->type; }
> >
> > const ControlValue &min() const { return min_; }
> > const ControlValue &max() const { return max_; }
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 727fdbd9450d..9960a30dfa03 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -25,16 +25,16 @@ namespace libcamera {
> > LOG_DEFINE_CATEGORY(Controls)
> >
> > /**
> > - * \enum ControlValueType
> > - * \brief Define the data type of value represented by a ControlValue
> > - * \var ControlValueNone
> > - * Identifies an unset control value
> > - * \var ControlValueBool
> > - * Identifies controls storing a boolean value
> > - * \var ControlValueInteger
> > - * Identifies controls storing an integer value
> > - * \var ControlValueInteger64
> > - * Identifies controls storing a 64-bit integer value
> > + * \enum ControlType
> > + * \brief Define the data type of a Control
> > + * \var ControlTypeNone
> > + * Invalid type, for empty values
> > + * \var ControlTypeBool
> > + * The control stores a boolean value
> > + * \var ControlTypeInteger
> > + * The control stores an integer value
> > + * \var ControlTypeInteger64
> > + * The control stores a 64-bit integer value
> > */
> >
> > /**
> > @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Controls)
> > * \brief Construct an empty ControlValue.
> > */
> > ControlValue::ControlValue()
> > - : type_(ControlValueNone)
> > + : type_(ControlTypeNone)
> > {
> > }
> >
> > @@ -55,7 +55,7 @@ ControlValue::ControlValue()
> > * \param[in] value Boolean value to store
> > */
> > ControlValue::ControlValue(bool value)
> > - : type_(ControlValueBool), bool_(value)
> > + : type_(ControlTypeBool), bool_(value)
> > {
> > }
> >
> > @@ -64,7 +64,7 @@ ControlValue::ControlValue(bool value)
> > * \param[in] value Integer value to store
> > */
> > ControlValue::ControlValue(int value)
> > - : type_(ControlValueInteger), integer_(value)
> > + : type_(ControlTypeInteger), integer_(value)
> > {
> > }
> >
> > @@ -73,7 +73,7 @@ ControlValue::ControlValue(int value)
> > * \param[in] value Integer value to store
> > */
> > ControlValue::ControlValue(int64_t value)
> > - : type_(ControlValueInteger64), integer64_(value)
> > + : type_(ControlTypeInteger64), integer64_(value)
> > {
> > }
> >
> > @@ -86,7 +86,7 @@ ControlValue::ControlValue(int64_t value)
> > /**
> > * \fn ControlValue::isNone()
> > * \brief Determine if the value is not initialised
> > - * \return True if the value type is ControlValueNone, false otherwise
> > + * \return True if the value type is ControlTypeNone, false otherwise
> > */
> >
> > /**
> > @@ -95,7 +95,7 @@ ControlValue::ControlValue(int64_t value)
> > */
> > void ControlValue::set(bool value)
> > {
> > - type_ = ControlValueBool;
> > + type_ = ControlTypeBool;
> > bool_ = value;
> > }
> >
> > @@ -105,7 +105,7 @@ void ControlValue::set(bool value)
> > */
> > void ControlValue::set(int value)
> > {
> > - type_ = ControlValueInteger;
> > + type_ = ControlTypeInteger;
> > integer_ = value;
> > }
> >
> > @@ -115,7 +115,7 @@ void ControlValue::set(int value)
> > */
> > void ControlValue::set(int64_t value)
> > {
> > - type_ = ControlValueInteger64;
> > + type_ = ControlTypeInteger64;
> > integer64_ = value;
> > }
> >
> > @@ -128,7 +128,7 @@ void ControlValue::set(int64_t value)
> > */
> > bool ControlValue::getBool() const
> > {
> > - ASSERT(type_ == ControlValueBool);
> > + ASSERT(type_ == ControlTypeBool);
> >
> > return bool_;
> > }
> > @@ -142,7 +142,7 @@ bool ControlValue::getBool() const
> > */
> > int ControlValue::getInt() const
> > {
> > - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64);
> > + ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
> >
> > return integer_;
> > }
> > @@ -156,7 +156,7 @@ int ControlValue::getInt() const
> > */
> > int64_t ControlValue::getInt64() const
> > {
> > - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64);
> > + ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
> >
> > return integer64_;
> > }
> > @@ -168,13 +168,13 @@ int64_t ControlValue::getInt64() const
> > std::string ControlValue::toString() const
> > {
> > switch (type_) {
> > - case ControlValueNone:
> > + case ControlTypeNone:
> > return "<None>";
> > - case ControlValueBool:
> > + case ControlTypeBool:
> > return bool_ ? "True" : "False";
> > - case ControlValueInteger:
> > + case ControlTypeInteger:
> > return std::to_string(integer_);
> > - case ControlValueInteger64:
> > + case ControlTypeInteger64:
> > return std::to_string(integer64_);
> > }
> >
> > diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk
> > index f3d068123012..a3f291e7071c 100755
> > --- a/src/libcamera/gen-controls.awk
> > +++ b/src/libcamera/gen-controls.awk
> > @@ -92,7 +92,7 @@ function GenerateTable(file) {
> > print "extern const std::unordered_map<ControlId, ControlIdentifier>" > file
> > print "controlTypes {" > file
> > for (i=1; i <= id; ++i) {
> > - printf "\t{ %s, { %s, \"%s\", ControlValue%s } },\n", names[i], names[i], names[i], types[i] > file
> > + printf "\t{ %s, { %s, \"%s\", ControlType%s } },\n", names[i], names[i], names[i], types[i] > file
> > }
> > print "};" > file
> > ExitNameSpace(file)
> > diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp
> > index aa3a65b1e5ef..8cda860b9fe9 100644
> > --- a/test/controls/control_info.cpp
> > +++ b/test/controls/control_info.cpp
> > @@ -26,7 +26,7 @@ protected:
> > ControlInfo info(Brightness);
> >
> > if (info.id() != Brightness ||
> > - info.type() != ControlValueInteger ||
> > + info.type() != ControlTypeInteger ||
> > info.name() != std::string("Brightness")) {
> > cout << "Invalid control identification for Brightness" << endl;
> > return TestFail;
> > @@ -44,7 +44,7 @@ protected:
> > info = ControlInfo(Contrast, 10, 200);
> >
> > if (info.id() != Contrast ||
> > - info.type() != ControlValueInteger ||
> > + info.type() != ControlTypeInteger ||
> > info.name() != std::string("Contrast")) {
> > cout << "Invalid control identification for Contrast" << endl;
> > return TestFail;
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list