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

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2020-02-17 13:03:24 +0200, 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.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
> Changes since v1:
> 
> - Add more warning messages to DeviceEnumeratorUdev
> - Use media->deviceNode() and print the driver name where appropriate
> ---
>  src/libcamera/device_enumerator_sysfs.cpp | 16 +++++++--------
>  src/libcamera/device_enumerator_udev.cpp  | 25 ++++++++++++++++-------
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
> index ad26affb38a3..197ca235879b 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,15 @@ 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 "
> +				<< media->deviceNode()
> +				<< " (" << media->driver() << "), skipping";
> +			continue;
>  		}
>  
>  		addDevice(media);
> @@ -89,7 +89,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..87638c59761c 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -82,8 +82,15 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  			return -ENODEV;
>  
>  		int ret = populateMediaDevice(media);
> -		if (ret == 0)
> -			addDevice(media);
> +		if (ret < 0) {
> +			LOG(DeviceEnumerator, Warning)
> +				<< "Failed to populate media device "
> +				<< media->deviceNode()
> +				<< " (" << media->driver() << "), skipping";
> +			return ret;
> +		}
> +
> +		addDevice(media);
>  		return 0;
>  	}
>  
> @@ -141,14 +148,18 @@ int DeviceEnumeratorUdev::enumerate()
>  		devnode = udev_device_get_devnode(dev);
>  		if (!devnode) {
>  			udev_device_unref(dev);
> -			ret = -ENODEV;
> -			goto done;
> +			LOG(DeviceEnumerator, Warning)
> +				<< "Failed to get device node for '"
> +				<< syspath << "', skipping";
> +			continue;
>  		}
>  
> -		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,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund