[libcamera-devel] [PATCH v5 5/5] libcamera: camera_manager: Enforce unique camera names

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Jul 29, 2020 at 01:50:55PM +0200, Niklas Söderlund wrote:
> The camera name have always been documented that it should be unique but
> it has never been enforced. Change this by refusing to add cameras to
> the CameraManager that would create two cameras with the exact same
> name.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> * Changes since v4
> - Update string in error message.
> 
> * Changes since v3
> - Update commit message.
> ---
>  src/libcamera/camera_manager.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index f60491d2c1a7500f..47e0f3129d346183 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -178,10 +178,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
>  
>  	for (std::shared_ptr<Camera> c : cameras_) {
>  		if (c->name() == camera->name()) {
> -			LOG(Camera, Warning)
> -				<< "Registering camera with duplicate name '"
> +			LOG(Camera, Error)
> +				<< "Skip registration of a camera with a duplicated name '"
>  				<< camera->name() << "'";
> -			break;
> +			return;

Shouldn't the error be propagated up to the pipelien handler ? Other
than that, this looks good to me, but I think we need to rename "name"
to "id".

>  		}
>  	}
>  

-- 
Regards,

Laurent Pinchart