From 9e48f13d6fa8e830321e180c3f052f064d3a2cd7 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 2 Mar 2026 11:02:35 +0000 Subject: [PATCH] Apply suggestions from code review of branch opencv-camera-switcher Co-authored-by: Joe Knapper --- .../things/camera/opencv.py | 10 +++++++--- .../things/camera/opencv_utils.py | 8 ++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/opencv.py b/src/openflexure_microscope_server/things/camera/opencv.py index 99c8f408..efa7ed85 100644 --- a/src/openflexure_microscope_server/things/camera/opencv.py +++ b/src/openflexure_microscope_server/things/camera/opencv.py @@ -50,7 +50,7 @@ class OpenCVCamera(BaseCamera): self.cameras = opencv_utils.identify_cameras(all_camera_ids) if self.camera_name not in self.cameras: if self.camera_name: - self.logger.warning("{self.camera_name} not found.") + self.logger.warning(f"{self.camera_name} not found.") self._camera_name = next(iter(self.cameras)) self._start_stream() @@ -81,7 +81,7 @@ class OpenCVCamera(BaseCamera): """Set the name of the camera.""" if not self.cameras: # Don't try to validate if cameras dict is empty, just set the value. - # As this is the startup behaviour. On __enter__ we check if the + # As this is the startup behaviour, on __enter__ we check if the # initial camera is valid and that at least 1 camera exists. self._camera_name = value # Return so we don't try to start the stream @@ -171,7 +171,11 @@ class OpenCVCamera(BaseCamera): @lt.property def manual_camera_settings(self) -> list[PropertyControl]: - """The camera settings to expose as property controls in the settings panel.""" + """The camera settings to expose as property controls in the settings panel. + + The options for the camera selector are populated with camera names once the + server starts and available cameras are have been detected. + """ return [ property_control_for( self, diff --git a/src/openflexure_microscope_server/things/camera/opencv_utils.py b/src/openflexure_microscope_server/things/camera/opencv_utils.py index dc9b7651..e85ac7d7 100644 --- a/src/openflexure_microscope_server/things/camera/opencv_utils.py +++ b/src/openflexure_microscope_server/things/camera/opencv_utils.py @@ -25,7 +25,7 @@ elif sys.platform.startswith("linux"): elif sys.platform == "darwin": BACKEND = cv2.CAP_AVFOUNDATION else: - raise RuntimeError("Unsupported platform {sys.platform}") + raise RuntimeError(f"Unsupported platform {sys.platform}") MAX_CAMERAS = 12 @@ -44,7 +44,11 @@ def find_all_cameras() -> list[int]: def identify_cameras(camera_ids: list[int]) -> dict[str, int]: """For a list of camera IDs return a dictionary of name -> ID.""" - # Set default names mapping -d -> camera for easy replacement if names are found. + # When first creating the mapping the mapping with default names it goes from + # id -> camera name. This makes it easy to replace the name (based on a fixed camera + # id as the key) if the name is then found. + # Before returning this is swapped to be a mapping from camera name -> id. As this + # is what is needed to switch the cameras based on a name. name_dict = {n: f"Unknown Camera {n}" for n in camera_ids} # enumerate cameras works for all backends if it is installed: