[libcamera-devel,01/12] libcamera: controls: Rename ControlValueType to ControlType

Message ID 20190928152238.23752-2-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Improve the application-facing controls API
Related show

Commit Message

Laurent Pinchart Sept. 28, 2019, 3:22 p.m. UTC
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@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(-)

Comments

Jacopo Mondi Sept. 29, 2019, 8:52 a.m. UTC | #1
Hi Laurent,

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@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...

In any case
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>  };
>
>  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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Sept. 29, 2019, 11:24 a.m. UTC | #2
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@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@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@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

Patch

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,
 };
 
 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;