Apply suggestions from code review of branch opencv-camera-switcher

Co-authored-by: Joe Knapper <joe.knapper@glasgow.ac.uk>

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,

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: