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

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-01-16 14:59:47 +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>

I would have split this in two patches at it clearly does two different 
things. For small patches like this it's not a huge issue but as patch 
size grows the cost of review increases, at least for me.

Also with smaller patches you can apply from the bottom once they 
received proper review and decrease the size of the next version of a 
series and release fixes earlier to master, if appropriate of course.

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  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_; }
>  	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)
> + *
> + * \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;
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund