| Message ID | 20200728234225.3505868-5-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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. Thanks j > } > } > > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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@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@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
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 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 CameraManager that would create two cameras with the exact same name. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/camera_manager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)