[libcamera-devel] [PATCH 07/12] libcamera: deviceenumerator: add factory for DeviceEnumerators

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sunday, 23 December 2018 01:00:36 EET Niklas Söderlund wrote:
> Provide a factory for DeviceEnumerator objects. Depending on which
> libraries are available there will be different ways to enumerate
> information in the system. This factory hides this from the rest of the
> library.
> 
> Currently udev enumeration is the only supported implementation, a sysfs
> implementation is another method that surely will be added in the
> future.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/deviceenumerator.cpp       | 17 +++++++++++++++++
>  src/libcamera/include/deviceenumerator.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/src/libcamera/deviceenumerator.cpp
> b/src/libcamera/deviceenumerator.cpp index
> f4c40bf0376ab453..6d675fc78af8e586 100644
> --- a/src/libcamera/deviceenumerator.cpp
> +++ b/src/libcamera/deviceenumerator.cpp
> @@ -138,6 +138,23 @@ bool DeviceMatch::matchEntities(const
> std::vector<std::string> &entities) const * Enumerator Base
>   */
> 
> +DeviceEnumerator *DeviceEnumerator::create()
> +{
> +	DeviceEnumerator *enumerator;
> +
> +	/* TODO: add compile time checks to only try udev enumerator if libudev is
> available */
> +	enumerator = new DeviceEnumeratorUdev();
> +	if (!enumerator->init())
> +		return enumerator;
> +
> +	/* NOTE: Either udev is not available or initialization of it
> +	 * failed, use/fallback on sysfs enumerator */

Please have /* and */ on a line of their own.

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

> +
> +	/* TODO: add a sysfs based enumerator */
> +
> +	return NULL;
> +}
> +
>  DeviceEnumerator::~DeviceEnumerator()
>  {
>  	for (DeviceInfo *dev : devices_) {
> diff --git a/src/libcamera/include/deviceenumerator.h
> b/src/libcamera/include/deviceenumerator.h index
> 2c7ff3f336ba127d..6aa6e59d4a8a9729 100644
> --- a/src/libcamera/include/deviceenumerator.h
> +++ b/src/libcamera/include/deviceenumerator.h
> @@ -59,6 +59,8 @@ private:
>  class DeviceEnumerator
>  {
>  public:
> +	static DeviceEnumerator *create();
> +
>  	virtual ~DeviceEnumerator();
> 
>  	virtual int init() = 0;

-- 
Regards,

Laurent Pinchart