| Message ID | 20190228200151.2948-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Thu, Feb 28, 2019 at 09:01:48PM +0100, Jacopo Mondi wrote: > Provide a getter method to access the device node path. For video > devices it is usually the most informative description. This looks fine, but do you use it anywhere ? > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/include/v4l2_device.h | 1 + > src/libcamera/v4l2_device.cpp | 6 ++++++ > 2 files changed, 7 insertions(+) > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > index 733bd69959f3..e1f79e730ec5 100644 > --- a/src/libcamera/include/v4l2_device.h > +++ b/src/libcamera/include/v4l2_device.h > @@ -99,6 +99,7 @@ public: > const char *driverName() const { return caps_.driver(); } > const char *deviceName() const { return caps_.card(); } > const char *busName() const { return caps_.bus_info(); } > + const std::string deviceNode() const { return deviceNode_; } Did you mean const std::string &devicenode() const ? > > int getFormat(V4L2DeviceFormat *format); > int setFormat(V4L2DeviceFormat *format); > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 9bfa10e8a151..b498d4e0bf92 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -364,6 +364,12 @@ void V4L2Device::close() > * \return The string containing the device location > */ > > +/** > + * \fn const std::string V4L2Device::deviceNode() You don't have to set the return type. > + * \brief Retrieve the video device node path > + * \return The video device deviceNode path > + */ > + > std::string V4L2Device::logPrefix() const > { > return deviceNode_;
Hi Laurent, On Fri, Mar 01, 2019 at 12:18:56AM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Thu, Feb 28, 2019 at 09:01:48PM +0100, Jacopo Mondi wrote: > > Provide a getter method to access the device node path. For video > > devices it is usually the most informative description. > > This looks fine, but do you use it anywhere ? In IPU3 patches, I find it more informative for video devices to print the devnode path instead of the driver name (while for subdevices I think the entity name is the most informative one, see discussion on v1 with Niklas and Kieran) > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/include/v4l2_device.h | 1 + > > src/libcamera/v4l2_device.cpp | 6 ++++++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > > index 733bd69959f3..e1f79e730ec5 100644 > > --- a/src/libcamera/include/v4l2_device.h > > +++ b/src/libcamera/include/v4l2_device.h > > @@ -99,6 +99,7 @@ public: > > const char *driverName() const { return caps_.driver(); } > > const char *deviceName() const { return caps_.card(); } > > const char *busName() const { return caps_.bus_info(); } > > + const std::string deviceNode() const { return deviceNode_; } > > Did you mean const std::string &devicenode() const ? > Argh, this was trivial, I'm sorry > > > > int getFormat(V4L2DeviceFormat *format); > > int setFormat(V4L2DeviceFormat *format); > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 9bfa10e8a151..b498d4e0bf92 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -364,6 +364,12 @@ void V4L2Device::close() > > * \return The string containing the device location > > */ > > > > +/** > > + * \fn const std::string V4L2Device::deviceNode() > > You don't have to set the return type. Indeed > > > + * \brief Retrieve the video device node path > > + * \return The video device deviceNode path deviceNode here should probably be "device node" Thanks j > > + */ > > + > > std::string V4L2Device::logPrefix() const > > { > > return deviceNode_; > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index 733bd69959f3..e1f79e730ec5 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -99,6 +99,7 @@ public: const char *driverName() const { return caps_.driver(); } const char *deviceName() const { return caps_.card(); } const char *busName() const { return caps_.bus_info(); } + const std::string deviceNode() const { return deviceNode_; } int getFormat(V4L2DeviceFormat *format); int setFormat(V4L2DeviceFormat *format); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 9bfa10e8a151..b498d4e0bf92 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -364,6 +364,12 @@ void V4L2Device::close() * \return The string containing the device location */ +/** + * \fn const std::string V4L2Device::deviceNode() + * \brief Retrieve the video device node path + * \return The video device deviceNode path + */ + std::string V4L2Device::logPrefix() const { return deviceNode_;
Provide a getter method to access the device node path. For video devices it is usually the most informative description. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/include/v4l2_device.h | 1 + src/libcamera/v4l2_device.cpp | 6 ++++++ 2 files changed, 7 insertions(+)