Merge branch 'Camera-tweaks' into 'v3'

Stop exposure walking on camera reload, reset CCM on full calibration

See merge request openflexure/openflexure-microscope-server!334

hardware-specific-tests/picamera2/test_exposure_time_drift.py

@@ -4,8 +4,13 @@ This can get very tedious. Recommend running pytest with -s option
 to monitor progress.
 """
 
+from typing import Optional, Any
 import logging
 import time
+import tempfile
+import os
+import json
+from contextlib import contextmanager
 
 from fastapi.testclient import TestClient
 
@@ -16,33 +21,53 @@ from openflexure_microscope_server.things.camera.picamera import StreamingPiCame
 
 logging.basicConfig(level=logging.DEBUG)
 
+# The tolerance used to be a setting of the camera, and any changes within tolerance
+# were ignored. Now this isn't needed we always set a value 1 larger than the last
+# accepted value as the PiCamera always rounds down.
+# The tolerance is needed in this test as when setting an arbitrary value it is rounded
+# down to a hardware compatible one.
+EXPOSURE_TOL = 30
 
-def _test_exposure_time_drift(desired_time):
-    """Capture 10 full resolution images and check that the exposure time remains constant.
 
-    This confirms that automatic exposure time adjustment is fully turned off
+@contextmanager
+def camera_test_client(settings_folder: Optional[str] = None):
+    """Yield a camera ThingClient on a camera server.
+
+    This is a context manager not a pytest fixture as it needs to be created
+    multiple times in some tests.
     """
     cam = StreamingPiCamera2()
-    server = ThingServer()
+    server = ThingServer(settings_folder=settings_folder)
     server.add_thing(cam, "/camera/")
 
     with TestClient(server.app) as test_client:
         client = ThingClient.from_url("/camera/", client=test_client)
-        exposure_tol = cam.persistent_control_tolerances["ExposureTime"]
+        yield client
+    del server
+    del cam
+
+
+def _test_exposure_time_drift(desired_time: int) -> None:
+    """Capture 10 full res images and check that the exposure time remains constant.
+
+    This confirms that automatic exposure time adjustment is fully turned off during
+    capture.
+    """
+    with camera_test_client() as client:
         client.exposure_time = desired_time
         print(f"Setting desired time of {desired_time}")
         time.sleep(0.5)
         pre_capture_et = client.exposure_time
         print(f"Pre-capture the time is set to {pre_capture_et}")
         # Check exp is set correctly within known tolerance
-        assert abs(pre_capture_et - desired_time) < exposure_tol
+        assert abs(pre_capture_et - desired_time) < EXPOSURE_TOL
 
         for i in range(10):
             client.capture_jpeg(resolution="full")
             if i == 0:
                 # Exposure can update on first capture, due to frame rate restrictions
                 first_et = client.exposure_time
-                assert abs(first_et - pre_capture_et) < exposure_tol
+                assert abs(first_et - desired_time) < EXPOSURE_TOL
             else:
                 frame_et = client.exposure_time
                 print(f"Frame {i} captured with exposure time {frame_et}")
@@ -65,3 +90,111 @@ def test_exposure_time_drift():
     """Performs the exposure time test for a range of exposure time values."""
     for desired_time in [100, 1000, 10000]:
         _test_exposure_time_drift(desired_time)
+
+
+def test_exposure_time_on_start_and_stop_stream():
+    """Start and stop stream in the same way a scan does and check exposure doesn't drift.
+
+    Take images using capture_to_memory() just as a scan would.
+    """
+    desired_time = 1000
+    with camera_test_client() as client:
+        # Set a desired exposure time
+        client.exposure_time = desired_time
+        print(f"Setting desired time of {desired_time}")
+        time.sleep(0.5)
+        # Take a couple of images to make sure that the exposure is adjusted to
+        # a hardware compatible value.
+        for i in range(2):
+            client.capture_jpeg(resolution="full")
+        # Save this time.
+        set_time = client.exposure_time
+        assert abs(set_time - desired_time) < EXPOSURE_TOL
+
+        # Mimic doing 10 smart scans. Change to full res, take some images. Return
+        # to standard preview resolution.
+        for i in range(10):
+            print(f"Starting simulation scan {i}")
+            # This will need updating if we start supporting other Picamera models
+            # It is currently used here to mimic the behaviour in in scanning.
+            client.start_streaming(main_resolution=(3280, 2464))
+            time.sleep(0.5)
+            for _j in range(5):
+                client.capture_to_memory(buffer_max=1)
+            # Reset to main resolution
+            time.sleep(0.5)
+            client.start_streaming()
+        # Check after all of this the exposure time is the same.
+        assert client.exposure_time == set_time
+
+
+def _load_camera_and_return_exposure(tmpdir: str) -> int:
+    """Load a camera (using any settings in tempdir) take images and check exposure."""
+    with camera_test_client(settings_folder=tmpdir) as client:
+        # Set a desired exposure time
+        time.sleep(0.5)
+        # Take a couple of images to make sure that the exposure is adjusted to
+        # a hardware compatible value.
+        for i in range(2):
+            client.capture_jpeg(resolution="full")
+        # Save this time.
+        return client.exposure_time
+
+
+def _load_setting(setting_file: str) -> dict[str, Any]:
+    """Load settings json from disk to dictionary."""
+    with open(setting_file, "r", encoding="utf-8") as file_obj:
+        return json.load(file_obj)
+
+
+def _save_setting(settings: dict[str, Any], setting_file: str):
+    """Save settings dictionary to disk."""
+    with open(setting_file, "w", encoding="utf-8") as file_obj:
+        json.dump(settings, file_obj)
+
+
+def test_exposure_time_saves_and_loads():
+    """Check that exposure time saves to disk and loads correctly."""
+    with tempfile.TemporaryDirectory() as tmpdir:
+        setting_file = os.path.join(tmpdir, "camera", "settings.json")
+
+        # Create a server, take some images, and get the exposure time
+        initial_exposure = _load_camera_and_return_exposure(tmpdir)
+        settings = _load_setting(setting_file)
+        assert settings["exposure_time"] == initial_exposure
+
+        # Adjust exposure to 1000 and save
+        settings["exposure_time"] = 1000
+        _save_setting(settings, setting_file)
+
+        # Create a server, take some images, and get the exposure time
+        recorded_exposure = _load_camera_and_return_exposure(tmpdir)
+        settings = _load_setting(setting_file)
+        assert settings["exposure_time"] == recorded_exposure
+        # Check it was set correctly within tolerance
+        assert abs(recorded_exposure - 1000) < EXPOSURE_TOL
+
+        # Load a second time without changing the file. Exposure should not change
+        recorded_exposure_second_load = _load_camera_and_return_exposure(tmpdir)
+        settings = _load_setting(setting_file)
+        assert settings["exposure_time"] == recorded_exposure_second_load
+        # Check it was set to exactly the value previously saved to disk
+        assert recorded_exposure == recorded_exposure_second_load
+
+        # Repeat with 2000
+        settings["exposure_time"] = 2000
+        _save_setting(settings, setting_file)
+
+        # Create a server, take some images, and get the exposure time
+        recorded_exposure = _load_camera_and_return_exposure(tmpdir)
+        settings = _load_setting(setting_file)
+        assert settings["exposure_time"] == recorded_exposure
+        # Check it was set correctly within tolerance
+        assert abs(recorded_exposure - 2000) < EXPOSURE_TOL
+
+        # Load a second time without changing the file. Exposure should not change
+        recorded_exposure_second_load = _load_camera_and_return_exposure(tmpdir)
+        settings = _load_setting(setting_file)
+        assert settings["exposure_time"] == recorded_exposure_second_load
+        # Check it was set to exactly the value previously saved to disk
+        assert recorded_exposure == recorded_exposure_second_load

src/openflexure_microscope_server/things/camera/__init__.py

@@ -342,7 +342,7 @@ class BaseCamera(lt.Thing):
         logger: lt.deps.InvocationLogger,
         metadata_getter: lt.deps.GetThingStates,
         buffer_max: int = 1,
-    ) -> None:
+    ) -> int:
         """Capture an image to memory. This can be saved later with ``save_from_memory``.
 
         Note that only one image is held in memory so this will overwrite any image

src/openflexure_microscope_server/things/camera/picamera.py

@@ -233,7 +233,7 @@ class StreamingPiCamera2(BaseCamera):
 
     @exposure_time.setter
     def exposure_time(self, value: int):
-        _exposure_time = value
+        self._exposure_time = value
         if self.streaming:
             with self._streaming_picamera() as cam:
                 # Note: This set a value 1 higher than requested as picamera2 always
@@ -250,7 +250,8 @@ class StreamingPiCamera2(BaseCamera):
             "Brightness": 0,
             "ColourGains": self.colour_gains,
             "Contrast": 1,
-            "ExposureTime": self.exposure_time,
+            # Must also set plus 1 or the exposure drifts with start and stop stream.
+            "ExposureTime": self.exposure_time + 1,
             "Saturation": 1,
             "Sharpness": 1,
         }
@@ -754,6 +755,7 @@ class StreamingPiCamera2(BaseCamera):
         self.set_static_green_equalisation()
         self.calibrate_lens_shading()
         self.calibrate_white_balance()
+        self.reset_ccm()
         self.set_background(portal)
 
     @lt.thing_action