[libcamera-devel] [PATCH] CameraSensor: Don't fail to add camera with faulty lens

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Sat, Sep 03, 2022 at 12:48:03PM +0100, Kieran Bingham via libcamera-devel wrote:
> If the CameraSensor fails to identify the VCM specified, it could still
> be possible to continue to operate the sensor. Autofocus support will be
> disabled, but this would be no different to operating a camera with a
> fixed focus. While of course the fixed focus position may not be
> suitable, it would provide a better user experience to be able to
> continue to operate the camera, while still reporting that the lens is
> disabled.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=146
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> The Surface devices now have a VCM registered, but they don't
> successfully identify the device. While that should also be fixed,
> failing to operate a camera because the VCM isn't operating seems to be
> a bad user experience.

I think that's reasonable, while there's clearly a problem on the kernel
side, this degraded mode is implemented without much overhead here, so
I'm fine with it.

>  src/libcamera/camera_sensor.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index d055c16a4885..dcee7b9a0210 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -467,8 +467,8 @@ int CameraSensor::discoverAncillaryDevices()
>  			ret = focusLens_->init();
>  			if (ret) {
>  				LOG(CameraSensor, Error)
> -					<< "CameraLens initialisation failed";
> -				return ret;
> +					<< "CameraLens initialisation failed: Lens disabled";

I'd reset focusLens_ here to avoid exposin a non-working CameraLens
through focusLens().

> +				break;

And you can drop the break, there's one just after. The message could
also be shortened slightly, up to you.

			ret = focusLens_->init();
			if (ret) {
				LOG(CameraSensor, Error)
					<< "Lens initialisation failed, lens disabled";
				focusLens_.reset();
			}
			break;

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  			}
>  			break;
>  

-- 
Regards,

Laurent Pinchart