[libcamera-devel] [PATCH] libcamera: camera_sensor: Add model() function

[Laurent Pinchart]

Hi Niklas,

On Tue, Apr 28, 2020 at 01:06:25AM +0200, Niklas Söderlund wrote:
> On 2020-04-28 01:26:51 +0300, Laurent Pinchart wrote:
> > Add a new model() function to the CameraSensor class to report the
> > camera sensor model.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/camera_sensor.cpp       | 24 ++++++++++++++++++++++++
> >  src/libcamera/include/camera_sensor.h |  1 +
> >  2 files changed, 25 insertions(+)
> > 
> > Jacopo,
> > 
> > I'll need this in pipeline handlers to obtain the sensor model, in order
> > to construct a configuration file name, without having to generate the
> > whole CameraSensorInfo. Is this OK with you ? As I've taken the
> > implementation of the function from your code, should I add your
> > Signed-off-by ?
> > 
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index c893b90979d8..55a9fbd71e69 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -163,6 +163,30 @@ int CameraSensor::init()
> >  	return 0;
> >  }
> >  
> > +/**
> > + * \brief Retrieve the sensor model name
> > + *
> > + * The sensor model name is a free-formed string that uniquely identifies the
> > + * sensor model.
> > + *
> > + * \return The sensor model name
> > + */
> > +std::string CameraSensor::model() const
> > +{
> > +	/*
> > +	 * Extract the camera sensor model name from the media entity name.
> > +	 *
> > +	 * \todo There is no standardized naming scheme for sensor entities
> > +	 * in the Linux kernel at the moment. The most common naming scheme
> > +	 * is the one obtained by associating the sensor name and its I2C
> > +	 * device and bus addresses in the form of: "devname i2c-adapt:i2c-addr"
> 
> A quick test for me have this at "`devname` `i2c-adapt`-`i2c-addr`".

Correct.

> > +	 * Assume this is the standard naming scheme and parse the first part
> > +	 * of the entity name to obtain "devname".
> 
> Would it make sens to verify the full entity name to match our 
> assumption of the format and fail if it does not match? I'm thinking 
> filing is better then reporting the wrong name due to our assumption 
> about format is off.

We already have a problem here with vimc, as we'll report "Sensor"
instead of "Sensor B". I'll fix it with a proper heuristic. After
sleeping :-)

> > +	 */
> > +	std::string entityName = subdev_->entity()->name();
> > +	return entityName.substr(0, entityName.find(' '));
> > +}
> > +
> >  /**
> >   * \fn CameraSensor::entity()
> >   * \brief Retrieve the sensor media entity
> > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> > index 5277f7f7fe87..e68185370eb2 100644
> > --- a/src/libcamera/include/camera_sensor.h
> > +++ b/src/libcamera/include/camera_sensor.h
> > @@ -33,6 +33,7 @@ public:
> >  
> >  	int init();
> >  
> > +	std::string model() const;
> >  	const MediaEntity *entity() const { return entity_; }
> >  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
> >  	const std::vector<Size> &sizes() const { return sizes_; }

-- 
Regards,

Laurent Pinchart