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

[Niklas Söderlund]

Hi Jacopo,

Thanks for your feedback.

On 2020-07-29 10:18:30 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Wed, Jul 29, 2020 at 01:42:25AM +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 refuse to add cameras to the
> 
> by refusing
> 
> > CameraManager that would create two cameras with the exact same name.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  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..7d83263f1fabf5da 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 registering camera with duplicated name '"
> >  				<< camera->name() << "'";
> > -			break;
> > +			return;
> 
> The error is not propagated up... not from here, not from the only
> caller PipelineHandler::registerCamera.
> 
> I guess pipelines then do not have a way to know if registration was
> successful or not. I don't think it's a big deal, they will notice
> while developing that something is wrong if a camera does not show up,
> an error code is not strictly needed.

That is true, and this should really not be happening. I originally had 
this error as fatal but changed my mind as if by some freak of nature 
some firmware ACPI would name their node path to "VIMC Sensor B" and we 
would fatal fail if VIMC driver was loaded and I thought it better to 
error and try to function with other cameras in the system.

I'm more then willing to make this error fatal. What do you guys think?

> 
> Thanks
>   j
> 
> >  		}
> >  	}
> >
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund