From 376780ea28c2a18d04e5ab24ebc1ff75fbcf47cb Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 28 Jul 2025 14:42:05 +0100 Subject: [PATCH 1/6] Stop exposure walking on camera reload, reset CCM on full calibration --- src/openflexure_microscope_server/things/camera/picamera.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 7d44beaa..5993e627 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -226,7 +226,7 @@ class StreamingPiCamera2(BaseCamera): if not self._setting_save_in_progress and self.streaming: with self._streaming_picamera() as cam: cam_value = cam.capture_metadata()["ExposureTime"] - if cam_value != self._exposure_time: + if abs(cam_value - self._exposure_time) > 30: self._exposure_time = cam_value self.save_settings() return self._exposure_time @@ -754,6 +754,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 From b90b02cf7d20877566a8ed18ebd783eac32e6975 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 29 Jul 2025 12:11:45 +0100 Subject: [PATCH 2/6] Better fix for exposure drift with more complete testing. Also fix return value for capture_to_memory --- .../picamera2/test_exposure_time_drift.py | 59 +++++++++++++++++-- .../things/camera/__init__.py | 2 +- .../things/camera/picamera.py | 5 +- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index 8e566612..e0177f4d 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -16,11 +16,19 @@ 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. + """Capture 10 full res images and check that the exposure time remains constant. - This confirms that automatic exposure time adjustment is fully turned off + This confirms that automatic exposure time adjustment is fully turned off during + capture. """ cam = StreamingPiCamera2() server = ThingServer() @@ -28,21 +36,20 @@ def _test_exposure_time_drift(desired_time): with TestClient(server.app) as test_client: client = ThingClient.from_url("/camera/", client=test_client) - exposure_tol = cam.persistent_control_tolerances["ExposureTime"] 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 +72,45 @@ 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(): + """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. + """ + cam = StreamingPiCamera2() + server = ThingServer() + server.add_thing(cam, "/camera/") + desired_time = 1000 + # Create a test client + with TestClient(server.app) as test_client: + client = ThingClient.from_url("/camera/", client=test_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 + + # Mimick 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 mimick 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 abs(client.exposure_time - set_time) < EXPOSURE_TOL diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 661bdf33..4225b43d 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/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 diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 5993e627..fca1433e 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -226,7 +226,7 @@ class StreamingPiCamera2(BaseCamera): if not self._setting_save_in_progress and self.streaming: with self._streaming_picamera() as cam: cam_value = cam.capture_metadata()["ExposureTime"] - if abs(cam_value - self._exposure_time) > 30: + if cam_value != self._exposure_time: self._exposure_time = cam_value self.save_settings() return self._exposure_time @@ -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, } From 38f4a745c3b728fd7a251a9808c5bc04ba30c1fc Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 29 Jul 2025 13:59:29 +0100 Subject: [PATCH 3/6] Fix bug with setting exposure time on load, add associated test --- .../picamera2/test_exposure_time_drift.py | 93 ++++++++++++++++++- .../things/camera/picamera.py | 2 +- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index e0177f4d..dd02c066 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -6,6 +6,9 @@ to monitor progress. import logging import time +import tempfile +import os +import json from fastapi.testclient import TestClient @@ -75,10 +78,9 @@ def test_exposure_time_drift(): def test_exposure_time_on_start_and_stop_stream(): - """Capture 10 full res images and check that the exposure time remains constant. + """Start and stop stream in the same way a scan does and check exposere doesn't drift. - This confirms that automatic exposure time adjustment is fully turned off during - capture. + Take images using capture_to_memory() just as a scan would. """ cam = StreamingPiCamera2() server = ThingServer() @@ -99,12 +101,12 @@ def test_exposure_time_on_start_and_stop_stream(): set_time = client.exposure_time assert abs(set_time - desired_time) < EXPOSURE_TOL - # Mimick doing 10 smart scans. Change to full res, take some images. Return + # 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 mimick the behaviour in in scanning. + # 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): @@ -114,3 +116,84 @@ def test_exposure_time_on_start_and_stop_stream(): client.start_streaming() # Check after all of this the exposure time is the same. assert abs(client.exposure_time - set_time) < EXPOSURE_TOL + + +def _load_camera_and_return_exposure(tmpdir: str) -> int: + """Load a camera (using any settings in tempdir) take images and check exposure.""" + cam = StreamingPiCamera2() + server = ThingServer(settings_folder=tmpdir) + server.add_thing(cam, "/camera/") + # Create a test client + with TestClient(server.app) as test_client: + client = ThingClient.from_url("/camera/", client=test_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. + exposure_time = client.exposure_time + # close the server + del server + del cam + return exposure_time + + +def _load_setting(setting_file): + """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, setting_file): + """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 correctly within tolerance + 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 correctly within tolerance + assert recorded_exposure == recorded_exposure_second_load diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index fca1433e..6ee9e200 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/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 From bcfe8c1eb5fa403cdffbf79a471da0647d283500 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 29 Jul 2025 15:17:45 +0000 Subject: [PATCH 4/6] Apply suggestions from code review of branch Camera-tweaks Co-authored-by: Beth Probert --- hardware-specific-tests/picamera2/test_exposure_time_drift.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index dd02c066..89aeded2 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -27,7 +27,7 @@ logging.basicConfig(level=logging.DEBUG) EXPOSURE_TOL = 30 -def _test_exposure_time_drift(desired_time): +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 @@ -78,7 +78,7 @@ def test_exposure_time_drift(): def test_exposure_time_on_start_and_stop_stream(): - """Start and stop stream in the same way a scan does and check exposere doesn't drift. + """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. """ From f710612a51e329926206c6279c4214a88cee1bc8 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 29 Jul 2025 17:24:43 +0100 Subject: [PATCH 5/6] Refactor exposure tests to minimise repitition on set-up. Fix typehints and docstrings --- .../picamera2/test_exposure_time_drift.py | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index 89aeded2..e3c2cd1e 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -4,11 +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 @@ -27,18 +29,31 @@ logging.basicConfig(level=logging.DEBUG) EXPOSURE_TOL = 30 +@contextmanager +def camera_test_client(settings_folder: Optional[str] = None): + """Yield a camera ThingClinet 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(settings_folder=settings_folder) + server.add_thing(cam, "/camera/") + + with TestClient(server.app) as test_client: + client = ThingClient.from_url("/camera/", client=test_client) + 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. """ - cam = StreamingPiCamera2() - server = ThingServer() - server.add_thing(cam, "/camera/") - - with TestClient(server.app) as test_client: - client = ThingClient.from_url("/camera/", client=test_client) + with camera_test_client() as client: client.exposure_time = desired_time print(f"Setting desired time of {desired_time}") time.sleep(0.5) @@ -82,13 +97,8 @@ def test_exposure_time_on_start_and_stop_stream(): Take images using capture_to_memory() just as a scan would. """ - cam = StreamingPiCamera2() - server = ThingServer() - server.add_thing(cam, "/camera/") desired_time = 1000 - # Create a test client - with TestClient(server.app) as test_client: - client = ThingClient.from_url("/camera/", client=test_client) + with camera_test_client() as client: # Set a desired exposure time client.exposure_time = desired_time print(f"Setting desired time of {desired_time}") @@ -115,17 +125,12 @@ def test_exposure_time_on_start_and_stop_stream(): time.sleep(0.5) client.start_streaming() # Check after all of this the exposure time is the same. - assert abs(client.exposure_time - set_time) < EXPOSURE_TOL + 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.""" - cam = StreamingPiCamera2() - server = ThingServer(settings_folder=tmpdir) - server.add_thing(cam, "/camera/") - # Create a test client - with TestClient(server.app) as test_client: - client = ThingClient.from_url("/camera/", client=test_client) + 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 @@ -133,20 +138,16 @@ def _load_camera_and_return_exposure(tmpdir: str) -> int: for i in range(2): client.capture_jpeg(resolution="full") # Save this time. - exposure_time = client.exposure_time - # close the server - del server - del cam - return exposure_time + return client.exposure_time -def _load_setting(setting_file): +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, setting_file): +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) @@ -177,7 +178,7 @@ def test_exposure_time_saves_and_loads(): 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 correctly within tolerance + # Check it was set to exactly the value previously saved to disk assert recorded_exposure == recorded_exposure_second_load # Repeat with 2000 @@ -195,5 +196,5 @@ def test_exposure_time_saves_and_loads(): 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 correctly within tolerance + # Check it was set to exactly the value previously saved to disk assert recorded_exposure == recorded_exposure_second_load From f0c6ded1b6073b032a90a17c76c76de4e5ce1423 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 29 Jul 2025 16:34:38 +0000 Subject: [PATCH 6/6] Apply suggestions from code review of branch Camera-tweaks Co-authored-by: Beth Probert --- hardware-specific-tests/picamera2/test_exposure_time_drift.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index e3c2cd1e..1fd653f0 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -31,7 +31,7 @@ EXPOSURE_TOL = 30 @contextmanager def camera_test_client(settings_folder: Optional[str] = None): - """Yield a camera ThingClinet on a camera server. + """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.