[libcamera-devel] [PATCH] controls: Report required control names

[Kieran Bingham]

The V4L2 controls which are optional, recommended, and mandatory report
errors if they are not found by CameraSensor::validateSensorDriver().

These errors report hex values for the controls, which can easily lead
to confusion and incorrect assumption about which control needs to be
investigated.

This can be seen occuring already in issues such as [0] and is generally
an unfriendly result to report to users in a warning or error message.

Adapt the tables of controls to store both the id and name of the
controls to report a user friendly message that can ease diagnosis of
any CameraSensor validation issues.

 [0] https://github.com/raspberrypi/linux/issues/4500

Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

---
I'm aware of course of the checkstyle changes suggested on V4L2_CID, but
I thought it was better to keep those macros 'scoped' here, and indented
accordingly.
---
 src/libcamera/camera_sensor.cpp | 45 ++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 7a3864150c89..1409da1b071c 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -168,19 +168,26 @@ int CameraSensor::validateSensorDriver()
 {
 	int err = 0;
 
+	struct v4l2_cid {
+		const uint32_t id;
+		const char *name;
+	};
+
+	#define V4L2_CID(x) { x, #x }
+
 	/*
 	 * Optional controls are used to register optional sensor properties. If
 	 * not present, some values will be defaulted.
 	 */
-	static constexpr uint32_t optionalControls[] = {
-		V4L2_CID_CAMERA_SENSOR_ROTATION,
+	static constexpr v4l2_cid optionalControls[] = {
+		V4L2_CID(V4L2_CID_CAMERA_SENSOR_ROTATION),
 	};
 
 	const ControlIdMap &controls = subdev_->controls().idmap();
-	for (uint32_t ctrl : optionalControls) {
-		if (!controls.count(ctrl))
+	for (auto &ctrl : optionalControls) {
+		if (!controls.count(ctrl.id))
 			LOG(CameraSensor, Debug)
-				<< "Optional V4L2 control " << utils::hex(ctrl)
+				<< "Optional V4L2 control " << ctrl.name
 				<< " not supported";
 	}
 
@@ -188,14 +195,14 @@ int CameraSensor::validateSensorDriver()
 	 * Recommended controls are similar to optional controls, but will
 	 * become mandatory in the near future. Be loud if they're missing.
 	 */
-	static constexpr uint32_t recommendedControls[] = {
-		V4L2_CID_CAMERA_ORIENTATION,
+	static constexpr v4l2_cid recommendedControls[] = {
+		V4L2_CID(V4L2_CID_CAMERA_ORIENTATION),
 	};
 
-	for (uint32_t ctrl : recommendedControls) {
-		if (!controls.count(ctrl)) {
+	for (auto &ctrl : recommendedControls) {
+		if (!controls.count(ctrl.id)) {
 			LOG(CameraSensor, Warning)
-				<< "Recommended V4L2 control " << utils::hex(ctrl)
+				<< "Recommended V4L2 control " << ctrl.name
 				<< " not supported";
 			err = -EINVAL;
 		}
@@ -259,18 +266,20 @@ int CameraSensor::validateSensorDriver()
 	 * For raw sensors, make sure the sensor driver supports the controls
 	 * required by the CameraSensor class.
 	 */
-	static constexpr uint32_t mandatoryControls[] = {
-		V4L2_CID_EXPOSURE,
-		V4L2_CID_HBLANK,
-		V4L2_CID_PIXEL_RATE,
-		V4L2_CID_VBLANK,
+	static constexpr v4l2_cid mandatoryControls[] = {
+		V4L2_CID(V4L2_CID_EXPOSURE),
+		V4L2_CID(V4L2_CID_HBLANK),
+		V4L2_CID(V4L2_CID_PIXEL_RATE),
+		V4L2_CID(V4L2_CID_VBLANK),
 	};
 
+	#undef V4L2_CID
+
 	err = 0;
-	for (uint32_t ctrl : mandatoryControls) {
-		if (!controls.count(ctrl)) {
+	for (auto &ctrl : mandatoryControls) {
+		if (!controls.count(ctrl.id)) {
 			LOG(CameraSensor, Error)
-				<< "Mandatory V4L2 control " << utils::hex(ctrl)
+				<< "Mandatory V4L2 control " << ctrl.name
 				<< " not available";
 			err = -EINVAL;
 		}
-- 
2.30.2