[libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set devnode in MediaEntity

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Jan 16, 2019 at 02:59:47PM +0100, Jacopo Mondi wrote:
> The MediaEntity::setDeviceNode() function was designed to set the device
> node path associated with a MediaEntity. The function was there, but the
> devnode_ member field was never actually set. Fix this.
> 
> While at there add a getter method for the devnode_ member as it will
> soon be used.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/media_object.h |  1 +
>  src/libcamera/media_object.cpp       | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h
> index a10f7e1..fad55a0 100644
> --- a/src/libcamera/include/media_object.h
> +++ b/src/libcamera/include/media_object.h
> @@ -85,6 +85,7 @@ class MediaEntity : public MediaObject
>  public:
>  	const std::string &name() const { return name_; }
>  	unsigned int function() const { return function_; }
> +	const std::string &devnode() const { return devnode_; }

As the setter is called setDeviceNode(), should this be called
deviceNode() ? We usually try not to abbreviate when not necessary.

>  	unsigned int deviceMajor() const { return major_; }
>  	unsigned int deviceMinor() const { return minor_; }
>  
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index 76dd326..4e90443 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -263,6 +263,15 @@ void MediaPad::addLink(MediaLink *link)
>   * \return The entity's function
>   */
>  
> +/**
> + * \fn MediaEntity::devnode()
> + * \brief Retrieve the entity's device node path, if any
> + *
> + * \sa int MediaEntity::setDeviceNode(const std::string &devnode)

I think you can abbreviate that to \sa setDeviceNode()

With these addressed,

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

> + *
> + * \return The entity's device node path, or an empty string if it is not set
> + */
> +
>  /**
>   * \fn MediaEntity::deviceMajor()
>   * \brief Retrieve the major number of the interface associated with the entity
> @@ -330,6 +339,8 @@ int MediaEntity::setDeviceNode(const std::string &devnode)
>  		return ret;
>  	}
>  
> +	devnode_ = devnode;
> +
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart