[libcamera-devel,v8,4/9] libcamera: camera_sensor: Generate a sensor ID

9211 diff mbox series

The ID is generated from information in the firmware description of the
sensor if available or from module and model information if the sensor
is virtual (for example VIMC).

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
* Changes sinve v7
- Use device path stripped of /sys/devices/ prefix instead of modalias
  for virtual sensors.

* Changes sinve v6
- Do not allow for subclassing
- Add support for virtual sensors
- Use new sysfs:: helper.

* Changes since v5
- Use new utils:: helper.
- Allow for subclassing

* Changes since v4
- Fix spelling.

* Changes since v3
- Update commit message.
- Add description of how ID are generated to comment.
---
 include/libcamera/internal/camera_sensor.h |  3 ++
 src/libcamera/camera_sensor.cpp            | 37 ++++++++++++++++++++++
 2 files changed, 40 insertions(+)

Hi Niklas,

Thank you for the patch.

On Wed, Aug 05, 2020 at 12:48:55PM +0200, Niklas Söderlund wrote:
> The ID is generated from information in the firmware description of the
> sensor if available or from module and model information if the sensor
> is virtual (for example VIMC).
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
> * Changes sinve v7
> - Use device path stripped of /sys/devices/ prefix instead of modalias
>   for virtual sensors.
> 
> * Changes sinve v6
> - Do not allow for subclassing
> - Add support for virtual sensors
> - Use new sysfs:: helper.
> 
> * Changes since v5
> - Use new utils:: helper.
> - Allow for subclassing
> 
> * Changes since v4
> - Fix spelling.
> 
> * Changes since v3
> - Update commit message.
> - Add description of how ID are generated to comment.
> ---
>  include/libcamera/internal/camera_sensor.h |  3 ++
>  src/libcamera/camera_sensor.cpp            | 37 ++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 06c8292ca30129de..0f65fca46877e89a 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -67,11 +67,14 @@ protected:
>  	std::string logPrefix() const override;
>  
>  private:
> +	int generateId();
> +
>  	const MediaEntity *entity_;
>  	std::unique_ptr<V4L2Subdevice> subdev_;
>  	unsigned int pad_;
>  
>  	std::string model_;
> +	std::string id_;
>  
>  	V4L2Subdevice::Formats formats_;
>  	Size resolution_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 350f49accad99c7b..695c4ad0028d5c4c 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -9,6 +9,7 @@
>  
>  #include <algorithm>
>  #include <float.h>
> +#include <fstream>

Do you still need this ?

>  #include <iomanip>
>  #include <limits.h>
>  #include <math.h>
> @@ -16,7 +17,9 @@
>  
>  #include <libcamera/property_ids.h>
>  
> +#include "libcamera/internal/file.h"
>  #include "libcamera/internal/formats.h"
> +#include "libcamera/internal/sysfs.h"
>  #include "libcamera/internal/utils.h"
>  
>  /**
> @@ -204,6 +207,11 @@ int CameraSensor::init()
>  	if (ret < 0)
>  		return ret;
>  
> +	/* Generate a unique ID for the sensor. */
> +	ret = generateId();
> +	if (ret)
> +		return ret;
> +
>  	/* Retrieve and store the camera sensor properties. */
>  	const ControlInfoMap &controls = subdev_->controls();
>  	int32_t propertyValue;
> @@ -541,4 +549,33 @@ std::string CameraSensor::logPrefix() const
>  	return "'" + entity_->name() + "'";
>  }
>  
> +int CameraSensor::generateId()
> +{
> +	const std::string devPath = subdev_->devicePath();
> +
> +	/* Try to get ID from firmware description. */
> +	id_ = sysfs::firmwareNodePath(devPath);
> +	if (!id_.empty())
> +		return 0;
> +
> +	/*
> +	 * Virtual sensors not described in firmware
> +	 *
> +	 * Verify it's a platform device and construct ID from the deive path
> +	 * and model of sensor.
> +	 */
> +	if (devPath.rfind("/sys/devices/platform/", 0) == 0) {

s/rfind/find/ as the match will occur at position 0 in most cases.

Shame https://en.cppreference.com/w/cpp/string/basic_string/starts_with
was only introduced in C++20. We could add it to utils:: if needed.

> +		id_ = devPath + " " + model();
> +
> +		static const std::string dropStr = "/sys/devices/";
> +		if (id_.find(dropStr) == 0)

Given that id_ is guaranteed to start with "/sys/devices/platform/", I
think you can drop this check :-)

> +			id_.erase(0, dropStr.length());

		id_ = devPath.substr(strlen("/sys/devices/")) + " " + model();

(you'll need to include string.h)

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

> +
> +		return 0;
> +	}
> +
> +	LOG(CameraSensor, Error) << "Can't generate sensor ID";
> +	return -EINVAL;
> +}
> +
>  } /* namespace libcamera */