[libcamera-devel] [PATCH] libcamera: camera_sensor: Print missing controls names

[Umang Jain]

Hi Jacopo,

On 11/22/22 2:00 PM, Jacopo Mondi via libcamera-devel wrote:
> Since the very beginning the camera sensor class has validated the
> controls available from the camera sensor driver, and complained
> accordingly when any of them was missing.
>
> The complaint message reported however only the numerical identifier of
> the V4L2 control, making debugging harder.

I had similar pain recently but  the printing control-id made things a 
bit better in my case :D
      ed591e705c (libcamera: v4l2_device: Log control id instead of 
errorIdx)
>
> Associate to each control a human readable identifier and use it in
> debug messages.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> ---
>   src/libcamera/camera_sensor.cpp | 38 ++++++++++++++++-----------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index ea373458a164..0780ce5a7007 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -213,31 +213,31 @@ int CameraSensor::validateSensorDriver()
>   	 * 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 const std::map<uint32_t, std::string> optionalControls = {
> +		{ V4L2_CID_CAMERA_SENSOR_ROTATION, "Rotation" },
>   	};
>   
>   	const ControlIdMap &controls = subdev_->controls().idmap();
> -	for (uint32_t ctrl : optionalControls) {
> +	for (const auto &[ctrl, name] : optionalControls) {
>   		if (!controls.count(ctrl))
>   			LOG(CameraSensor, Debug)
> -				<< "Optional V4L2 control " << utils::hex(ctrl)
> -				<< " not supported";
> +				<< "Optional V4L2 control '" << name
> +				<< "' not supported";
>   	}
>   
>   	/*
>   	 * 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 const std::map<uint32_t, std::string> recommendedControls = {
> +		{ V4L2_CID_CAMERA_ORIENTATION, "Orientation" },
>   	};
>   
> -	for (uint32_t ctrl : recommendedControls) {
> +	for (const auto &[ctrl, name] : recommendedControls) {
>   		if (!controls.count(ctrl)) {
>   			LOG(CameraSensor, Warning)
> -				<< "Recommended V4L2 control " << utils::hex(ctrl)
> -				<< " not supported";
> +				<< "Recommended V4L2 control '" << name
> +				<< "' not supported";
>   			err = -EINVAL;
>   		}
>   	}
> @@ -300,20 +300,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_ANALOGUE_GAIN,
> -		V4L2_CID_EXPOSURE,
> -		V4L2_CID_HBLANK,
> -		V4L2_CID_PIXEL_RATE,
> -		V4L2_CID_VBLANK,
> +	static const std::map<uint32_t, std::string> mandatoryControls = {
> +		{ V4L2_CID_ANALOGUE_GAIN, "Analogue gain" },
> +		{ V4L2_CID_EXPOSURE, "Exposure" },
> +		{ V4L2_CID_HBLANK, "Horizontal blanking" },
> +		{ V4L2_CID_PIXEL_RATE, "Pixel Rate" },
> +		{ V4L2_CID_VBLANK, "Vertical blanking" }
>   	};
>   
>   	err = 0;
> -	for (uint32_t ctrl : mandatoryControls) {
> +	for (const auto &[ctrl, name] : mandatoryControls) {
>   		if (!controls.count(ctrl)) {
>   			LOG(CameraSensor, Error)
> -				<< "Mandatory V4L2 control " << utils::hex(ctrl)
> -				<< " not available";
> +				<< "Mandatory V4L2 control '" << name
> +				<< "' not available";
>   			err = -EINVAL;
>   		}
>   	}