[libcamera-devel] [PATCH] libcamera: device_enumerator: Don't stop if one device fails

[Kieran Bingham]

Hi Laurent,

On 14/02/2020 12:37, Laurent Pinchart wrote:
> If one device fails to enumerate, which isn't supposed to happen under
> normal conditions, both the sysfs and the udev enumerators stop
> enumeration of further devices. This potentially prevents working
> devices from being detected and handled. Fix it by skipping the faulty
> device.

Thank you,

This does indeed allow the system to continue and function with the
available cameras.

Interestingly, in the particular case that I was investigating - it
wasn't the newly added device that failed ... it was the existing camera
interface.

That can be investigated separately.

--
Kieran



> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/device_enumerator_sysfs.cpp | 15 +++++++--------
>  src/libcamera/device_enumerator_udev.cpp  |  8 +++++---
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
> index ad26affb38a3..bde736226f95 100644
> --- a/src/libcamera/device_enumerator_sysfs.cpp
> +++ b/src/libcamera/device_enumerator_sysfs.cpp
> @@ -33,7 +33,6 @@ int DeviceEnumeratorSysfs::enumerate()
>  {
>  	struct dirent *ent;
>  	DIR *dir;
> -	int ret = 0;
>  
>  	static const char * const sysfs_dirs[] = {
>  		"/sys/subsystem/media/devices",
> @@ -74,14 +73,14 @@ int DeviceEnumeratorSysfs::enumerate()
>  		}
>  
>  		std::shared_ptr<MediaDevice> media = createDevice(devnode);
> -		if (!media) {
> -			ret = -ENODEV;
> -			break;
> -		}
> +		if (!media)
> +			continue;
>  
>  		if (populateMediaDevice(media) < 0) {
> -			ret = -ENODEV;
> -			break;
> +			LOG(DeviceEnumerator, Warning)
> +				<< "Failed to populate media device /dev/media"
> +				<< idx << ", skipping";
> +			continue;
>  		}
>  
>  		addDevice(media);
> @@ -89,7 +88,7 @@ int DeviceEnumeratorSysfs::enumerate()
>  
>  	closedir(dir);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index b2c5fd221dcd..3837aaf97e6a 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -145,10 +145,12 @@ int DeviceEnumeratorUdev::enumerate()
>  			goto done;
>  		}
>  
> -		ret = addUdevDevice(dev);
> +		if (addUdevDevice(dev) < 0)
> +			LOG(DeviceEnumerator, Warning)
> +				<< "Failed to add device for '"
> +				<< syspath << "', skipping";
> +
>  		udev_device_unref(dev);
> -		if (ret < 0)
> -			break;
>  	}
>  
>  done:
> 

-- 
Regards
--
Kieran