| Message ID | 20200427222651.3987-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. 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@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`". > + * 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. > + */ > + 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 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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@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_; }
Hi Laurent, On Tue, Apr 28, 2020 at 01:26:51AM +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@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 ? > Yeah, no worries as you wish Speaking of this patch, I'll rebase CameraSensorInfo to use this method as there discussed with you and Niklas. Thanks j > 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" > + * Assume this is the standard naming scheme and parse the first part > + * of the entity name to obtain "devname". > + */ > + 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 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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" + * Assume this is the standard naming scheme and parse the first part + * of the entity name to obtain "devname". + */ + 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_; }
Add a new model() function to the CameraSensor class to report the camera sensor model. Signed-off-by: Laurent Pinchart <laurent.pinchart@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 ?