From f7545c6d8d59b95f3edb9119c72bb1133f986a6e Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 17 Jun 2025 11:18:33 -0400 Subject: [PATCH 01/63] Update issue templates --- .gitlab/issue_templates/Bug.md | 24 ++++++++++++---------- .gitlab/issue_templates/Feature request.md | 20 +++++++++--------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index 86300792..0dbbc281 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -2,31 +2,33 @@ ## Summary -(Summarize the bug encountered concisely) + ## Configuration + -I'm using: - -**Camera:** (E.g. Raspberry Pi camera v2) - -**Motor controller:** (E.g. Sangaboard, or Arduino Nano) +* **Sever version**: v2 or v3 +* **Release/Branch**: e.g. v3.0.0-alpha1 +* **Hardware Configuration:** + * Raspberry Pi Camera v2 + * Sangabouard v0.5 ## Steps to reproduce -(How one can reproduce the issue - this is very important) + ## Relevant logs and/or screenshots -To access your microscope log, either: + + ## Additional details -(Anything else you think might be relevant to mention) + -/label ~bug +/label ~bug diff --git a/.gitlab/issue_templates/Feature request.md b/.gitlab/issue_templates/Feature request.md index b3230205..5b5f999f 100644 --- a/.gitlab/issue_templates/Feature request.md +++ b/.gitlab/issue_templates/Feature request.md @@ -2,18 +2,18 @@ ## Summary -(Summarize the bug encountered concisely) + -## Configuration - -I'm using: - -**Camera:** (E.g. Raspberry Pi camera v2) - -**Motor controller:** (E.g. Sangaboard, or Arduino Nano) ## Additional details -(Anything else you think might be relevant to mention) + + + +/label ~feature From dd24a3863d85baf405c49c6c38c5b5afdf080e23 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 17 Jun 2025 15:58:45 +0000 Subject: [PATCH 02/63] Apply suggestions from code review of branch update-issue-templates Co-authored-by: Joe Knapper --- .gitlab/issue_templates/Bug.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index 0dbbc281..b7e40f0d 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -21,6 +21,8 @@ From c659ba8f5e83152a611696821103e13daced34bf Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 23 Jun 2025 13:07:26 +0100 Subject: [PATCH 40/63] Minor tidying to Picamera Thing --- .../things/camera/picamera.py | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 222ab108..4372dabc 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -97,7 +97,7 @@ class StreamingPiCamera2(BaseCamera): def __init__(self, camera_num: int = 0): self.camera_num = camera_num self.camera_configs: dict[str, dict] = {} - # NB persistent controls will be updated with settings, in __enter__. + # Note persistent controls will be updated with settings, in __enter__. self.persistent_controls = { "AeEnable": False, "AnalogueGain": 1.0, @@ -129,25 +129,34 @@ class StreamingPiCamera2(BaseCamera): """ with self.picamera() as cam: for i in range(discard_frames): - # Discard frames, so we know our data is fresh + # Discard frames, so data is fresh cam.capture_metadata() - for k, v in cam.capture_metadata().items(): - if k in self.persistent_controls: - if k in self.persistent_control_tolerances: - if ( - np.abs(self.persistent_controls[k] - v) - < self.persistent_control_tolerances[k] - ): - logging.debug( - f"Ignoring a small change in persistent control {k}" - f"from {self.persistent_controls[k]} to {v}" - "while updating persistent controls." - ) - continue # Ignore small changes, to avoid drift - self.persistent_controls[k] = v - self.thing_settings.update( - self.persistent_controls - ) # TODO: Is this saving to the wrong place? + for key, value in cam.capture_metadata().items(): + if key not in self.persistent_controls: + # Skip keys that are not persistent controls + continue + if self._control_value_within_tolerance(key, value): + logging.debug( + "Ignoring a small change in persistent control %s from %s to " + "%s while updating persistent controls.", + key, + self.persistent_controls[key], + self.persistent_controls[key], + ) + else: + self.persistent_controls[key] = value + self.thing_settings.update(self.persistent_controls) + + def _control_value_within_tolerance(self, key: str, value: float) -> bool: + """Return True if the updated value for a persistent control is within + tolerance of current value. This allows ignoring small changes + """ + if key not in self.persistent_control_tolerances: + # Not tolerance range set, so it is not within tollerance + return False + + difference = np.abs(self.persistent_controls[key] - value) + return difference < self.persistent_control_tolerances[key] def settings_to_persistent_controls(self): """Update the persistent controls dict from the settings dict @@ -246,7 +255,6 @@ class StreamingPiCamera2(BaseCamera): so will fail if it's run before those are populated in `__enter__`. """ if "tuning" in self.thing_settings: - # TODO: should this be a separate file? self.tuning = self.thing_settings["tuning"].dict else: logging.info("Did not find tuning in settings, reading from camera...") @@ -288,6 +296,7 @@ class StreamingPiCamera2(BaseCamera): self.populate_default_tuning() self.initialise_tuning() self.initialise_picamera() + # populate sensor modes by reading the property self.sensor_modes self.settings_to_persistent_controls() self.settings_to_properties() @@ -420,10 +429,8 @@ class StreamingPiCamera2(BaseCamera): self.lores_mjpeg_stream.stop() logging.info("Stopped MJPEG stream.") - # Increase the resolution for taking an image - time.sleep( - 0.2 - ) # Sprinkled a sleep to prevent camera getting confused by rapid commands + # Adding a sleep to prevent camera getting confused by rapid commands + time.sleep(0.2) @thing_action def capture_image( From 62a07622fad6390e1e6cd1ff11c121f4df26ae87 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Mon, 23 Jun 2025 14:58:57 +0100 Subject: [PATCH 41/63] If not provided, find resize from image size when stitching --- .../things/smart_scan.py | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 5ec78a4d..618fec2f 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -1080,7 +1080,7 @@ class SmartScanThing(Thing): self, logger: InvocationLogger, scan_name: str, - stitch_resize: float, + stitch_resize: Optional[float] = None, overlap: float = 0.0, ) -> None: """Generate a stitched image based on stage position metadata @@ -1100,7 +1100,6 @@ class SmartScanThing(Thing): try: with open(json_fpath, "r", encoding="utf-8") as data_file: data_loaded = json.load(data_file) - logger.info(data_loaded) overlap = data_loaded["overlap"] except (json.decoder.JSONDecodeError, FileNotFoundError, TypeError): # As there is no schema or pydantic model this should handle @@ -1117,6 +1116,29 @@ class SmartScanThing(Thing): "Attempting stitch with overlap value of 0.1" ) overlap = 0.1 + + if stitch_resize is None: + try: + with open(json_fpath, "r", encoding="utf-8") as data_file: + data_loaded = json.load(data_file) + capture_resolution = data_loaded["capture resolution"] + stitch_resize = STITCHING_RESOLUTION[0] / capture_resolution[0] + except (json.decoder.JSONDecodeError, FileNotFoundError, TypeError): + # As there is no schema or pydantic model this should handle + # the file not being there, it not being json in the file, + # or the imported data not being indexable + logger.warning( + f"Couldn't read scan data, is {SCAN_DATA_FILENAME} missing or corrupt? " + "Attempting stitch with resize value of 0.5" + ) + stitch_resize = 0.5 + except KeyError: + logger.warning( + "Value for capture resolution not found in scan data. " + "Attempting stitch with resize value of 0.5" + ) + stitch_resize = 0.5 + self.run_subprocess( logger, [ From c6682f539df31473a1b890f2c7b411b6520ebda2 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Mon, 23 Jun 2025 15:37:16 +0000 Subject: [PATCH 42/63] Correct spelling of Sangaboard --- .gitlab/issue_templates/Bug.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index b7e40f0d..3d928164 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -11,7 +11,7 @@ * **Release/Branch**: e.g. v3.0.0-alpha1 * **Hardware Configuration:** * Raspberry Pi Camera v2 - * Sangabouard v0.5 + * Sangaboard v0.5 ## Steps to reproduce From afea1e36581ec2d5fcdd70f6f36409fbcf8dbd83 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 23 Jun 2025 17:22:28 +0100 Subject: [PATCH 43/63] Create function for flattening and rounding array and converting to a list --- .../things/camera/picamera_recalibrate_utils.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 790cf04f..6b4e0fc0 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -436,12 +436,12 @@ def set_static_lst( alsc["n_iter"] = 0 # disable the adaptive part alsc["luminance_strength"] = 1.0 alsc["calibrations_Cr"] = [ - {"ct": 4500, "table": np.reshape(cr, (-1)).round(3).tolist()} + {"ct": 4500, "table": as_flat_rounded_list(cr, round_to=3)} ] alsc["calibrations_Cb"] = [ - {"ct": 4500, "table": np.reshape(cb, (-1)).round(3).tolist()} + {"ct": 4500, "table": as_flat_rounded_list(cb, round_to=3)} ] - alsc["luminance_lut"] = np.reshape(luminance, (-1)).round(3).tolist() + alsc["luminance_lut"] = as_flat_rounded_list(luminance, round_to=3) def set_static_ccm(tuning: dict, c: list) -> None: @@ -544,3 +544,8 @@ def recreate_camera_manager(): del Picamera2._cm gc.collect() Picamera2._cm = picamera2.picamera2.CameraManager() + + +def as_flat_rounded_list(array: np.ndarray, round_to: int = 3) -> list[float]: + """Flatten array, round, and then convert to list""" + return np.reshape(array, -1).round(round_to).tolist() From cdae90013a17c7c4d44e6de2c96f99f91c9a1684 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 23 Jun 2025 16:24:40 +0000 Subject: [PATCH 44/63] Modify comment for clarity in the function that copies the lens shading algorithm --- .../things/camera/picamera_recalibrate_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 6b4e0fc0..812f8379 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -501,10 +501,10 @@ def copy_alsc_section(from_tuning: dict, to_tuning: dict): This is done in-place, i.e. modifying to_tuning. """ + # Using Picamera2 function to find the relevant sub-dict for each tuning file from_i = index_of_algorithm(from_tuning["algorithms"], "rpi.alsc") to_i = index_of_algorithm(to_tuning["algorithms"], "rpi.alsc") - # Please excuse the clumsy update-and-delete - this lets us use - # the nice Picamera2 function to find the relevant sub-dict. + # Updating the dictionary in place. to_tuning["algorithms"][to_i] = from_tuning["algorithms"][from_i] From febb0d2690e5b6298263787087320c53d0354e08 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 23 Jun 2025 17:47:18 +0100 Subject: [PATCH 45/63] Clarify white balance code. --- .../camera/picamera_recalibrate_utils.py | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 812f8379..4f4d5224 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -255,6 +255,8 @@ def adjust_white_balance_from_raw( camera.configure(config) camera.start() channels = channels_from_bayer_array(camera.capture_array("raw")) + # TODO: read black level from camera rather than hard-coding 64 + blacklevel = 64 if luminance is not None and Cr is not None and Cb is not None: # Reconstruct a low-resolution image from the lens shading tables # and use it to normalise the raw image, to compensate for @@ -269,17 +271,23 @@ def adjust_white_balance_from_raw( channels = channels * channel_gains logging.info(f"After gains, channel maxima are {np.max(channels, axis=(1, 2))}") if method == "centre": - _, h, w = channels.shape - blue, g1, g2, red = ( - np.mean( - channels[:, 9 * h // 20 : 11 * h // 20, 9 * w // 20 : 11 * w // 20], - axis=(1, 2), - ) - - 64 + _, height, width = channels.shape + # Cut out the central 10% from 9/20 to 11/20... + low_y_range = 9 * height // 20 + hi_y_range = 11 * height // 20 + low_x_range = 9 * width // 20 + hi_x_range = 11 * width // 20 + # ... and then take the mean of each bayer channel. + centre_means = np.mean( + channels[:, low_y_range:hi_y_range, low_x_range:hi_x_range], + axis=(1, 2), ) + # Subtract blacklevel before splitting into channels + blue, g1, g2, red = centre_means - blacklevel else: - # TODO: read black level from camera rather than hard-coding 64 - blue, g1, g2, red = np.percentile(channels, percentile, axis=(1, 2)) - 64 + blue, g1, g2, red = ( + np.percentile(channels, percentile, axis=(1, 2)) - blacklevel + ) green = (g1 + g2) / 2.0 new_awb_gains = (green / red, green / blue) if Cr is not None and Cb is not None: From 7c3a9ad01fdd615b9be0d67e4e2ca088c3cb7edb Mon Sep 17 00:00:00 2001 From: jaknapper Date: Mon, 23 Jun 2025 18:08:27 +0100 Subject: [PATCH 46/63] Copy sharpest image from stack, not the most average sharpness! --- src/openflexure_microscope_server/things/autofocus.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index ab6a4c0a..64013551 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -353,8 +353,7 @@ class AutofocusThing(Thing): image to images dir.""" image_list = glob.glob(os.path.join(stack_dir, "*")) image_list = sorted(image_list, key=os.path.getsize) - central_index = (len(image_list) - 1) // 2 - central_image = image_list[central_index] + central_image = image_list[-1] xy_location = os.path.basename(stack_dir) logger.info(central_image) From 11c8f47e20d0b14a25ff52643f584969c16f064a Mon Sep 17 00:00:00 2001 From: jaknapper Date: Mon, 23 Jun 2025 18:13:46 +0100 Subject: [PATCH 47/63] Rename central image to sharpest --- src/openflexure_microscope_server/things/autofocus.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 64013551..7e0d652c 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -353,8 +353,8 @@ class AutofocusThing(Thing): image to images dir.""" image_list = glob.glob(os.path.join(stack_dir, "*")) image_list = sorted(image_list, key=os.path.getsize) - central_image = image_list[-1] + sharpest_image = image_list[-1] xy_location = os.path.basename(stack_dir) - logger.info(central_image) - shutil.copy(central_image, os.path.join(images_dir, f"{xy_location}.jpeg")) + logger.info(sharpest_image) + shutil.copy(sharpest_image, os.path.join(images_dir, f"{xy_location}.jpeg")) From be07966631ec097a3c8acd81c5b471a96f394dee Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 24 Jun 2025 10:18:30 +0100 Subject: [PATCH 48/63] Remove the overwriting of internal globals, requires removing future annoations --- .../things/autofocus.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index dd4a388a..e4ed09b9 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -6,7 +6,6 @@ camera together to perform an autofocus routine. See repository root for licensing information. """ -from __future__ import annotations from contextlib import contextmanager import logging import time @@ -37,9 +36,14 @@ CaptureDep = direct_thing_client_dependency(CaptureThing, "/capture/") SETTLING_TIME = 0.3 -class JPEGSharpnessMonitor: - __globals__ = globals() # Required for FastAPI dependency +class SharpnessDataArrays(BaseModel): + jpeg_times: NDArray + jpeg_sizes: NDArray + stage_times: NDArray + stage_positions: list[dict[str, int]] + +class JPEGSharpnessMonitor: def __init__(self, stage: Stage, camera: Camera, portal: BlockingPortal): self.camera = camera self.stage = stage @@ -140,13 +144,6 @@ class JPEGSharpnessMonitor: SharpnessMonitorDep = Annotated[JPEGSharpnessMonitor, Depends()] -class SharpnessDataArrays(BaseModel): - jpeg_times: NDArray - jpeg_sizes: NDArray - stage_times: NDArray - stage_positions: list[dict[str, int]] - - class AutofocusThing(Thing): """The Thing concerned with combinations of z axis movements and the camera. From a43a82213d7f2431e5768feb7b5bbe71bc6a2cc8 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 24 Jun 2025 14:07:58 +0100 Subject: [PATCH 49/63] Improve docs for hardware specific tests --- .../picamera2/test_acquisition.py | 15 +++++-- .../picamera2/test_exposure_time_drift.py | 8 +++- .../picamera2/test_sensor_mode.py | 6 ++- .../picamera2/test_tuning.py | 43 ++++++++++++++++--- 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_acquisition.py b/hardware-specific-tests/picamera2/test_acquisition.py index 2929d83d..6d66cc5e 100644 --- a/hardware-specific-tests/picamera2/test_acquisition.py +++ b/hardware-specific-tests/picamera2/test_acquisition.py @@ -1,11 +1,13 @@ -from labthings_picamera2 import StreamingPiCamera2 -from labthings_fastapi.server import ThingServer -from labthings_fastapi.client import ThingClient from fastapi.testclient import TestClient from PIL import Image import numpy as np from pytest import fixture +from labthings_fastapi.server import ThingServer +from labthings_fastapi.client import ThingClient + +from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2 + @fixture(scope="module") def client(): @@ -17,10 +19,17 @@ def client(): def test_calibration(client): + """ + Check that full auto calibrate completes without an exception + """ client.full_auto_calibrate() def test_jpeg_and_array(client): + """ + Check that grabbing a jpeg from the stream results in the same size + image as a array capture or a jpeg capture. + """ blob = client.grab_jpeg() mjpeg_frame = Image.open(blob.open()) assert mjpeg_frame diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index ee75d179..d7a24f00 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -5,12 +5,18 @@ from fastapi.testclient import TestClient from labthings_fastapi.server import ThingServer from labthings_fastapi.client import ThingClient -from labthings_picamera2.thing import StreamingPiCamera2 + +from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2 logging.basicConfig(level=logging.DEBUG) def test_exposure_time_drift(): + """ + Capture 10 full resolution images and check that the exposure time remains constant + + This confirms that automatic exposure time adjustment is fully turned off + """ cam = StreamingPiCamera2() server = ThingServer() server.add_thing(cam, "/camera/") diff --git a/hardware-specific-tests/picamera2/test_sensor_mode.py b/hardware-specific-tests/picamera2/test_sensor_mode.py index 91694a89..84f6ce7c 100644 --- a/hardware-specific-tests/picamera2/test_sensor_mode.py +++ b/hardware-specific-tests/picamera2/test_sensor_mode.py @@ -5,12 +5,16 @@ import numpy as np from labthings_fastapi.server import ThingServer from labthings_fastapi.client import ThingClient -from labthings_picamera2.thing import StreamingPiCamera2 + +from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2 logging.basicConfig(level=logging.DEBUG) def test_sensor_mode(): + """ + Test capturing raw arrays in two different sensor modes + """ cam = StreamingPiCamera2() server = ThingServer() server.add_thing(cam, "/camera/") diff --git a/hardware-specific-tests/picamera2/test_tuning.py b/hardware-specific-tests/picamera2/test_tuning.py index 65ab87f8..96691cc8 100644 --- a/hardware-specific-tests/picamera2/test_tuning.py +++ b/hardware-specific-tests/picamera2/test_tuning.py @@ -1,23 +1,30 @@ +""" +Tests that check that a tuning file can be reloaded +""" + import os -from picamera2 import Picamera2 -from labthings_picamera2 import recalibrate_utils import pytest +from picamera2 import Picamera2 + +from openflexure_microscope_server.things.camera import recalibrate_utils MODEL = Picamera2.global_camera_info()[0]["Model"] -def check_camera_available(): - assert len(Picamera2.global_camera_info()) >= 1 - - def load_default_tuning(): + """ + Return the default tuning file for the connected camera. + """ fname = f"{MODEL}.json" return Picamera2.load_tuning_file(fname) def generate_bad_tuning(): + """ + Return a tuning file with an invalid version number to force an error when loaded. + """ default_tuning = load_default_tuning() bad_tuning = default_tuning.copy() bad_tuning["version"] = 999 @@ -25,6 +32,14 @@ def generate_bad_tuning(): def print_tuning(read_file=False): + """ + Print the path of the default tuning file from the the environment variable. + + :param read_file: Boolean, set true to also print the file contents. + + This is useful for debuging. As PyTest suppresses the printing by default the + -s option is needed when running pylint to see this. + """ key = "LIBCAMERA_RPI_TUNING_FILE" if key in os.environ: print(f"Tuning file environment variable: {os.environ[key]}") @@ -36,6 +51,16 @@ def print_tuning(read_file=False): def _test_bad_tuning_after_good_tuning(configure): + """ + Load the default tuning file into the camera, re-load with a broken tuning file, + check it errors. Finally check the default tuning file will load again afterwards. + + :param configure: Boolean, set true to configure the camera on initial loading + + This test checks that: + recalibrate_utils.recreate_camera_manager() is working as expected. As the default + PiCamera2 behaviour does not expect the tuning file to be reloaded. + """ bad_tuning = generate_bad_tuning() default_tuning = load_default_tuning() print_tuning() @@ -61,6 +86,12 @@ def _test_bad_tuning_after_good_tuning(configure): del cam +# Note: The tests below are marked with +# @pytest.mark.filterwarnings("ignore: Exception ignored") +# as the picamera will throw an exception ignored warning when deleting +# the picamera object. This is expected + + @pytest.mark.filterwarnings("ignore: Exception ignored") def test_bad_tuning_after_good_tuning_noconfigure(): _test_bad_tuning_after_good_tuning(False) From 83832bf375a1cf582b37f035a5fb8f9ffe92e598 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Tue, 24 Jun 2025 15:53:57 +0100 Subject: [PATCH 50/63] Update import name of recalibrate_utils --- hardware-specific-tests/picamera2/test_tuning.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hardware-specific-tests/picamera2/test_tuning.py b/hardware-specific-tests/picamera2/test_tuning.py index 96691cc8..572562b1 100644 --- a/hardware-specific-tests/picamera2/test_tuning.py +++ b/hardware-specific-tests/picamera2/test_tuning.py @@ -7,7 +7,9 @@ import os import pytest from picamera2 import Picamera2 -from openflexure_microscope_server.things.camera import recalibrate_utils +from openflexure_microscope_server.things.camera import ( + picamera_recalibrate_utils as recalibrate_utils, +) MODEL = Picamera2.global_camera_info()[0]["Model"] From 9966cb0fbb86d4a4cbc2ef734fecbe3a82f517ad Mon Sep 17 00:00:00 2001 From: jaknapper Date: Tue, 24 Jun 2025 15:55:19 +0100 Subject: [PATCH 51/63] Longer wait in exposure time test with fewer loops and lower exposure time --- .../picamera2/test_exposure_time_drift.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index d7a24f00..cf8e1976 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -23,11 +23,12 @@ def test_exposure_time_drift(): with TestClient(server.app) as test_client: client = ThingClient.from_url("/camera/", client=test_client) - client.exposure_time = 50000 - time.sleep(0.1) + # 5444 is an exposure time supported by PiCamera2 + client.exposure_time = 5444 + time.sleep(0.5) initial_et = client.exposure_time print(f"Before capture, et is {client.exposure_time}") - for i in range(10): + for i in range(5): client.capture_jpeg(resolution="full") print(f"After capture, et is {client.exposure_time}") final_et = client.exposure_time From 4e78f3d104b111cef2fb86d04c50a4b79162a978 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 24 Jun 2025 17:01:16 +0100 Subject: [PATCH 52/63] Fix exposure time adjusting when setting the same value and add test Closes #433 --- .../picamera2/test_exposure_time_drift.py | 49 +++++++++++++++---- .../things/camera/picamera.py | 7 +++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index cf8e1976..1fe65e53 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -1,3 +1,10 @@ +""" +Check exposure times do not drift. + +This can get very tedious. Recommende running pytest with -s option +to monitor progress. +""" + import logging import time @@ -11,7 +18,7 @@ from openflexure_microscope_server.things.camera.picamera import StreamingPiCame logging.basicConfig(level=logging.DEBUG) -def test_exposure_time_drift(): +def _test_exposure_time_drift(desired_time): """ Capture 10 full resolution images and check that the exposure time remains constant @@ -23,16 +30,40 @@ def test_exposure_time_drift(): with TestClient(server.app) as test_client: client = ThingClient.from_url("/camera/", client=test_client) - # 5444 is an exposure time supported by PiCamera2 - client.exposure_time = 5444 + exposure_tol = cam.persistent_control_tolerances["ExposureTime"] + client.exposure_time = desired_time + print(f"Setting desired time of {desired_time}") time.sleep(0.5) - initial_et = client.exposure_time - print(f"Before capture, et is {client.exposure_time}") - for i in range(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 + for i in range(10): client.capture_jpeg(resolution="full") - print(f"After capture, et is {client.exposure_time}") - final_et = client.exposure_time - assert initial_et == final_et + 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 + frame_et = client.exposure_time + print(f"Frame {i} captured with exposure time {frame_et}") + # Check no further drift in value + assert first_et == frame_et + + # Set the exposure time to the value it already is. To check it doesn't shift + print(f"Setting exposure time to {frame_et} to check it doesn't change") + client.exposure_time = frame_et + time.sleep(0.5) + # Check before and after capture + assert client.exposure_time == frame_et + client.capture_jpeg(resolution="full") + assert client.exposure_time == frame_et + print("Exposure time didn't change!!") + print(f"End of test for exposure target {desired_time}") + + +def test_exposure_time_drift(): + for desired_time in [100, 1000, 10000]: + _test_exposure_time_drift(desired_time) if __name__ == "__main__": diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 4372dabc..ecba1694 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -201,6 +201,13 @@ class StreamingPiCamera2(BaseCamera): "ExposureTime", int, description="The exposure time in microseconds" ) + @exposure_time.setter + def exposure_time(self, value: int): + with self.picamera() as cam: + # Note: This set a value 1 higher than requested as picamera2 always sets + # a lower value than requested, even if the requested is allowed + cam.set_controls({"ExposureTime": value + 1}) + _sensor_modes = None @thing_property From 8a4ad49c03f2406046770c23b73397c60aae5c72 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 09:17:32 +0000 Subject: [PATCH 53/63] Apply suggestions from code review of branch jpeg-capture-in-stacking Co-authored-by: Richard Bowman --- src/openflexure_microscope_server/things/camera/__init__.py | 2 -- src/openflexure_microscope_server/things/camera/picamera.py | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 7d771af6..543db45d 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -146,8 +146,6 @@ class BaseCamera(Thing): ) -> None: """Capture an image and save it to disk - This will set the event `acquired` once the image has been acquired, so - that the stage may be moved while it's saved. save_resolution can be set to resize the image before saving. By default this is None meaning that the image is saved at original resoltion. diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index ecba1694..bef59148 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -141,7 +141,7 @@ class StreamingPiCamera2(BaseCamera): "%s while updating persistent controls.", key, self.persistent_controls[key], - self.persistent_controls[key], + value, ) else: self.persistent_controls[key] = value @@ -442,14 +442,14 @@ class StreamingPiCamera2(BaseCamera): @thing_action def capture_image( self, - stream_name: Literal["main", "lores", "raw", "full"] = "main", + stream_name: Literal["main", "lores", "raw"] = "main", wait: Optional[float] = 0.9, ): """Acquire one image from the camera. Return it as a PIL Image - stream_name: (Optional) The PiCamera2 stream to use, should be one of ["main", "lores", "raw", "full"]. Default = "main" + stream_name: (Optional) The PiCamera2 stream to use, should be one of ["main", "lores", "raw"]. Default = "main" wait: (Optional, float) Set a timeout in seconds. A TimeoutError is raised if this time is exceeded during capture. Default = 0.9s, lower than the 1s timeout default in picamera yaml settings From 59d15fb186ee2a5a655e5133558bfa0af6b7c66d Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Tue, 24 Jun 2025 17:41:05 +0100 Subject: [PATCH 54/63] Add further comment to picamera tests --- hardware-specific-tests/picamera2/test_tuning.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hardware-specific-tests/picamera2/test_tuning.py b/hardware-specific-tests/picamera2/test_tuning.py index 572562b1..6ce4562b 100644 --- a/hardware-specific-tests/picamera2/test_tuning.py +++ b/hardware-specific-tests/picamera2/test_tuning.py @@ -93,6 +93,11 @@ def _test_bad_tuning_after_good_tuning(configure): # as the picamera will throw an exception ignored warning when deleting # the picamera object. This is expected +# The 2 pairs of repeated identical tests exist because pytest only +# starts once, and so by running the test more than one time checks +# that the camera can be initialised multiple times without restarting +# python. + @pytest.mark.filterwarnings("ignore: Exception ignored") def test_bad_tuning_after_good_tuning_noconfigure(): From 2d49bf20613b5b99f277caf117724d49824e0323 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 10:21:54 +0100 Subject: [PATCH 55/63] Remove duplicate exposure property from picamera thing --- .../things/camera/picamera.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index bef59148..fcbffc4f 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -555,15 +555,6 @@ class StreamingPiCamera2(BaseCamera): piexif.insert(piexif.dump(exif_dict), path) return JPEGBlob.from_temporary_directory(folder, fname) - @thing_property - def exposure(self) -> float: - """An alias for `exposure_time` to fit the micromanager API""" - return self.exposure_time - - @exposure.setter # type: ignore - def exposure(self, value): - self.exposure_time = value - @thing_property def capture_metadata(self) -> dict: """Return the metadata from the camera""" From 0933ece63aeae990d92984f8cec1aec4de16340b Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 14:22:01 +0000 Subject: [PATCH 56/63] Apply suggestions from code review of branch jpeg-capture-in-stacking Co-authored-by: Beth Probert --- .../picamera2/test_acquisition.py | 12 ++++++++++++ .../picamera2/test_exposure_time_drift.py | 15 ++++++++++----- .../picamera2/test_sensor_mode.py | 3 +++ hardware-specific-tests/picamera2/test_tuning.py | 12 ++++++------ pyproject.toml | 2 +- .../things/autofocus.py | 3 ++- .../things/camera/__init__.py | 2 ++ .../things/smart_scan.py | 9 +++++++-- 8 files changed, 43 insertions(+), 15 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_acquisition.py b/hardware-specific-tests/picamera2/test_acquisition.py index 6d66cc5e..3d8ef5c0 100644 --- a/hardware-specific-tests/picamera2/test_acquisition.py +++ b/hardware-specific-tests/picamera2/test_acquisition.py @@ -11,6 +11,11 @@ from openflexure_microscope_server.things.camera.picamera import StreamingPiCame @fixture(scope="module") def client(): + """ + A pytest fixture that initialises a test client for the StreamingPiCamera2 Thing. + This fixture sets up a ThingServer, registers a StreamingPiCamera2 instance at the + "/camera/" endpoint, and provides a ThingClient for interacting with it during tests. + """ server = ThingServer() server.add_thing(StreamingPiCamera2(), "/camera/") with TestClient(server.app) as test_client: @@ -30,13 +35,20 @@ def test_jpeg_and_array(client): Check that grabbing a jpeg from the stream results in the same size image as a array capture or a jpeg capture. """ + # Grab a jpeg from the stream blob = client.grab_jpeg() mjpeg_frame = Image.open(blob.open()) assert mjpeg_frame + + # Capture a jpeg blob = client.capture_jpeg(resolution="main") jpeg_capture = Image.open(blob.open()) assert jpeg_capture + + # Capture an array arrlist = client.capture_array(stream_name="main") array_main = np.array(arrlist) + + # Verify image sizes are the same assert mjpeg_frame.size == jpeg_capture.size assert array_main.shape[1::-1] == jpeg_capture.size diff --git a/hardware-specific-tests/picamera2/test_exposure_time_drift.py b/hardware-specific-tests/picamera2/test_exposure_time_drift.py index 1fe65e53..d7ea142b 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -1,7 +1,7 @@ """ Check exposure times do not drift. -This can get very tedious. Recommende running pytest with -s option +This can get very tedious. Recommend running pytest with -s option to monitor progress. """ @@ -38,16 +38,18 @@ def _test_exposure_time_drift(desired_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 + 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 - frame_et = client.exposure_time - print(f"Frame {i} captured with exposure time {frame_et}") - # Check no further drift in value - assert first_et == frame_et + else: + frame_et = client.exposure_time + print(f"Frame {i} captured with exposure time {frame_et}") + # Check no further drift in value + assert first_et == frame_et # Set the exposure time to the value it already is. To check it doesn't shift print(f"Setting exposure time to {frame_et} to check it doesn't change") @@ -62,6 +64,9 @@ def _test_exposure_time_drift(desired_time): 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) diff --git a/hardware-specific-tests/picamera2/test_sensor_mode.py b/hardware-specific-tests/picamera2/test_sensor_mode.py index 84f6ce7c..57da8463 100644 --- a/hardware-specific-tests/picamera2/test_sensor_mode.py +++ b/hardware-specific-tests/picamera2/test_sensor_mode.py @@ -24,6 +24,9 @@ def test_sensor_mode(): for size in [(3280, 2464), (1640, 1232)]: client.sensor_mode = {"output_size": size, "bit_depth": 10} arr = np.array(client.capture_array(stream_name="raw")) + # Check that the array dimensions match the requested image size. + # Note: Numpy array shape is (y,x), but the sensor is set with (x,y) + # hence the need to compare index 0 with index 1. assert arr.shape[0] == size[1] diff --git a/hardware-specific-tests/picamera2/test_tuning.py b/hardware-specific-tests/picamera2/test_tuning.py index 6ce4562b..a6fd9b76 100644 --- a/hardware-specific-tests/picamera2/test_tuning.py +++ b/hardware-specific-tests/picamera2/test_tuning.py @@ -33,14 +33,14 @@ def generate_bad_tuning(): return bad_tuning -def print_tuning(read_file=False): +def print_tuning(read_file: bool = False): """ Print the path of the default tuning file from the the environment variable. :param read_file: Boolean, set true to also print the file contents. - This is useful for debuging. As PyTest suppresses the printing by default the - -s option is needed when running pylint to see this. + This is useful for debugging. As pytest suppresses the printing by default the + -s option is needed when running pytest to see this. """ key = "LIBCAMERA_RPI_TUNING_FILE" if key in os.environ: @@ -52,7 +52,7 @@ def print_tuning(read_file=False): print("Tuning file environment variable not set") -def _test_bad_tuning_after_good_tuning(configure): +def _test_bad_tuning_after_good_tuning(configure: bool = False): """ Load the default tuning file into the camera, re-load with a broken tuning file, check it errors. Finally check the default tuning file will load again afterwards. @@ -66,14 +66,14 @@ def _test_bad_tuning_after_good_tuning(configure): bad_tuning = generate_bad_tuning() default_tuning = load_default_tuning() print_tuning() - print("opening camera with explicitly specified tuning") + print("opening camera with default tuning") with Picamera2(tuning=default_tuning) as cam: print_tuning() if configure: cam.configure(cam.create_preview_configuration()) del cam recalibrate_utils.recreate_camera_manager() - print(f"Opening camera with tuning['version'] = {bad_tuning['version']}") + print(f"Opening camera with bad tuning - ['version'] = {bad_tuning['version']}") with pytest.raises(IndexError): # The bad version should cause a problem cam = Picamera2(tuning=bad_tuning) diff --git a/pyproject.toml b/pyproject.toml index 53f992e5..6d038360 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,7 +40,7 @@ dev = [ "matplotlib~=3.10" ] pi = [ - "picamera2~=0.3.12", + "picamera2~=0.3.27", ] [project.scripts] diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 7e0d652c..e17d792d 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -294,6 +294,7 @@ class AutofocusThing(Thing): stack_z_range = stack_dz * (images_to_capture - 1) if stack_z_range > 0: + # Perform backlash corrected move. See issue #420 stage.move_relative(z=-(STACK_OVERSHOOT + stack_z_range / 2)) stage.move_relative(z=STACK_OVERSHOOT) time.sleep(0.3) @@ -348,7 +349,7 @@ class AutofocusThing(Thing): images_dir: str, stack_dir: str, logger: InvocationLogger, - ): + ) -> None: """Gets a list of images in a folder (stack_dir), sorts them by filesize, and copies the sharpest image to images dir.""" image_list = glob.glob(os.path.join(stack_dir, "*")) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 543db45d..b16ff8cc 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -31,6 +31,8 @@ class JPEGBlob(Blob): class PNGBlob(Blob): + """A class representing a PNG image as a LabThings FastAPI Blob""" + media_type: str = "image/png" diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 618fec2f..0f7ee409 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -377,10 +377,13 @@ class SmartScanThing(Thing): return (next_point[0], next_point[1], z_estimate) @_scan_running - def _calc_displacement_from_test_image(self, overlap): + def _calc_displacement_from_test_image(self, overlap: int) -> tuple[int, int]: """ Take a test image and use camera stage mapping to calculate x and y displacement + :param overlap: The desired overlap as a fraction of the image. i.e. 0.5 means + that each image should overlap its nearest neighbour by 50%. + Return (dx, dy) - the x and y displacments in steps """ test_jpg = self._cam.grab_jpeg() @@ -422,7 +425,9 @@ class SmartScanThing(Thing): dx, dy = self._calc_displacement_from_test_image(overlap) stitch_resize = STITCHING_RESOLUTION[0] / self.capture_resolution[0] - self._scan_logger.debug(f"Resizing images when stitching by {stitch_resize}") + self._scan_logger.debug( + f"Resizing images when stitching by a factor of {stitch_resize}" + ) self._scan_logger.info( f"Based on an overlap of {overlap}, we will make steps of {dx}, {dy}" From ce5b5b781cff46cfde82900e8c49c0a7c963d9d2 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 15:55:48 +0100 Subject: [PATCH 57/63] Add typehints and docstrings as suggested in review --- .../things/camera/__init__.py | 15 ++++++++++++--- .../things/camera/picamera.py | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index b16ff8cc..6c5652bd 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -76,7 +76,9 @@ class BaseCamera(Thing): self._memory_metadata = None @thing_action - def start_streaming(self, main_resolution, buffer_count) -> None: + def start_streaming( + self, main_resolution: tuple[int, int], buffer_count: int + ) -> None: """Start (or stop and restart) the camera with the given resolution for the main stream, and buffer_count number of images in the buffer""" raise NotImplementedError( @@ -132,7 +134,11 @@ class BaseCamera(Thing): return JPEGBlob.from_bytes(frame) @thing_action - def capture_image(self, stream_name, wait): + def capture_image( + self, + stream_name: Literal["main", "lores", "raw"], + wait: Optional[float], + ) -> None: """Capture a PIL image from stream stream_name with timeout wait""" raise NotImplementedError( "CameraThings must define their own capture_image method" @@ -211,7 +217,10 @@ class BaseCamera(Thing): ) -> Image: """Capture an image in memory and return it with metadata CaptureError raised if the capture fails for any reason - returns tuple with PIL Image, and dict of metadata + returns tuple with PIL Image, and dict of metadata. + + This robust capturing method attempts to capture the image five times + each time with a 5 second timeout set. """ for capture_attempts in range(5): try: diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index fcbffc4f..991f9381 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -444,7 +444,7 @@ class StreamingPiCamera2(BaseCamera): self, stream_name: Literal["main", "lores", "raw"] = "main", wait: Optional[float] = 0.9, - ): + ) -> None: """Acquire one image from the camera. Return it as a PIL Image From 0751457ed3a5cf04c21ab1d524d9f77174d68695 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 16:01:59 +0100 Subject: [PATCH 58/63] Remove rednundant if __main__ from hardware tests --- hardware-specific-tests/picamera2/test_exposure_time_drift.py | 4 ---- hardware-specific-tests/picamera2/test_sensor_mode.py | 4 ---- 2 files changed, 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 d7ea142b..4e712e5e 100644 --- a/hardware-specific-tests/picamera2/test_exposure_time_drift.py +++ b/hardware-specific-tests/picamera2/test_exposure_time_drift.py @@ -69,7 +69,3 @@ def test_exposure_time_drift(): """ for desired_time in [100, 1000, 10000]: _test_exposure_time_drift(desired_time) - - -if __name__ == "__main__": - test_exposure_time_drift() diff --git a/hardware-specific-tests/picamera2/test_sensor_mode.py b/hardware-specific-tests/picamera2/test_sensor_mode.py index 57da8463..fbc575d6 100644 --- a/hardware-specific-tests/picamera2/test_sensor_mode.py +++ b/hardware-specific-tests/picamera2/test_sensor_mode.py @@ -28,7 +28,3 @@ def test_sensor_mode(): # Note: Numpy array shape is (y,x), but the sensor is set with (x,y) # hence the need to compare index 0 with index 1. assert arr.shape[0] == size[1] - - -if __name__ == "__main__": - test_sensor_mode() From 83e0e08ccce35c6188fae644e72131b0addf8980 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 15:12:28 +0000 Subject: [PATCH 59/63] Apply suggestions from code review of branch jpeg-capture-in-stacking Co-authored-by: Beth Probert --- .../things/camera/__init__.py | 1 + .../things/camera/picamera.py | 20 +++++++++---------- .../camera/picamera_recalibrate_utils.py | 12 +++++------ 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 6c5652bd..31cfe0e3 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -250,6 +250,7 @@ class BaseCamera(Thing): try: image.save(jpeg_path, quality=95, subsampling=0) try: + # Load EXIF metadata from image so it can be added to. exif_dict = piexif.load(jpeg_path) exif_dict["Exif"][piexif.ExifIFD.UserComment] = json.dumps( metadata diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 991f9381..24fa2f94 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -152,7 +152,7 @@ class StreamingPiCamera2(BaseCamera): tolerance of current value. This allows ignoring small changes """ if key not in self.persistent_control_tolerances: - # Not tolerance range set, so it is not within tollerance + # Not tolerance range set, so it is not within tolerance return False difference = np.abs(self.persistent_controls[key] - value) @@ -419,7 +419,7 @@ class StreamingPiCamera2(BaseCamera): ) @thing_action - def stop_streaming(self, stop_web_stream=True) -> None: + def stop_streaming(self, stop_web_stream: bool = True) -> None: """ Stop the MJPEG stream """ @@ -617,7 +617,7 @@ class StreamingPiCamera2(BaseCamera): self.update_persistent_controls() @thing_action - def calibrate_lens_shading(self): + def calibrate_lens_shading(self) -> None: """Take an image and use it for flat-field correction. This method requires an empty (i.e. bright) field of view. It will take @@ -643,7 +643,7 @@ class StreamingPiCamera2(BaseCamera): ) @colour_correction_matrix.setter # type: ignore - def colour_correction_matrix(self, value): + def colour_correction_matrix(self, value) -> None: self.thing_settings["colour_correction_matrix"] = value self.calibrate_colour_correction(value) @@ -664,14 +664,14 @@ class StreamingPiCamera2(BaseCamera): self.colour_correction_matrix = c @thing_action - def calibrate_colour_correction(self, c: tuple): + def calibrate_colour_correction(self, c: tuple) -> None: """Overwrite the colour correction matrix in camera tuning""" with self.picamera(pause_stream=True): recalibrate_utils.set_static_ccm(self.tuning, c) self.initialise_picamera() @thing_action - def set_static_green_equalisation(self, offset: int = 65535): + def set_static_green_equalisation(self, offset: int = 65535) -> None: """Set the green equalisation to a static value. Green equalisation avoids the debayering algorithm becoming confused @@ -686,7 +686,7 @@ class StreamingPiCamera2(BaseCamera): self.initialise_picamera() @thing_action - def full_auto_calibrate(self): + def full_auto_calibrate(self) -> None: """Perform a full auto-calibration This function will call the other calibration actions in sequence: @@ -704,7 +704,7 @@ class StreamingPiCamera2(BaseCamera): self.calibrate_white_balance() @thing_action - def flat_lens_shading(self): + def flat_lens_shading(self) -> None: """Disable flat-field correction This method will set a completely flat lens shading table. It is not the @@ -786,7 +786,7 @@ class StreamingPiCamera2(BaseCamera): ) @thing_action - def flat_lens_shading_chrominance(self): + def flat_lens_shading_chrominance(self) -> None: """Disable flat-field correction This method will set the chrominance of the lens shading table to be @@ -801,7 +801,7 @@ class StreamingPiCamera2(BaseCamera): self.initialise_picamera() @thing_action - def reset_lens_shading(self): + def reset_lens_shading(self) -> None: """Revert to default lens shading settings This method will restore the default "adaptive" lens shading method used diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index 4f4d5224..d3073f75 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -69,13 +69,13 @@ def load_default_tuning(cam: Picamera2) -> dict: tuning_dir = "/usr/share/libcamera/ipa/raspberrypi" # from picamera2 v0.3.9 # The directory above has been removed from the search path seems - # find odd - as that's where the files currently are on a default + # odd - as that's where the files currently are on a default # Raspbian image. This may need updating if the files have moved # in future updates to the system libcamera package return cam.load_tuning_file(fname, dir=tuning_dir) -def set_minimum_exposure(camera: Picamera2): +def set_minimum_exposure(camera: Picamera2) -> None: """Enable manual exposure, with low gain and shutter speed We set exposure mode to manual, analog and digital gain @@ -135,7 +135,7 @@ def test_exposure_settings(camera: Picamera2, percentile: float) -> ExposureTest return result -def check_convergence(test: ExposureTest, target: int, tolerance: float): +def check_convergence(test: ExposureTest, target: int, tolerance: float) -> bool: """Check whether the brightness is within the specified target range""" return abs(test.level - target) < target * tolerance @@ -222,7 +222,7 @@ def adjust_shutter_and_gain_from_raw( # Check the gain is still changing - if not, we have probably hit the maximum if camera.capture_metadata()["AnalogueGain"] == test.analog_gain: - logging.info(f"Gain has maxed out. at {test.analog_gain}") + logging.info(f"Gain has maxed out at {test.analog_gain}") break if check_convergence(test, target_white_level, tolerance): @@ -504,7 +504,7 @@ def index_of_algorithm(algorithms: list[dict], algorithm: str) -> int: raise ValueError(f"Algorithm {algorithm} is not available.") -def copy_alsc_section(from_tuning: dict, to_tuning: dict): +def copy_alsc_section(from_tuning: dict, to_tuning: dict) -> None: """Copy the `rpi.alsc` algorithm from one tuning to another. This is done in-place, i.e. modifying to_tuning. @@ -544,7 +544,7 @@ def raw_channels_from_camera(camera: Picamera2) -> LensShadingTables: return channels_from_bayer_array(raw_image) -def recreate_camera_manager(): +def recreate_camera_manager() -> None: """Delete and recreate the camera manager. This is necessary to ensure the tuning file is re-read. From ee8c58c99fa60b10fa12d72929798d38e72839ff Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 17:18:55 +0100 Subject: [PATCH 60/63] Docstrings, typehints, and clarifications in PiCamera and its utils --- .../things/camera/picamera.py | 126 +++++++++++++++--- .../camera/picamera_recalibrate_utils.py | 16 ++- 2 files changed, 119 insertions(+), 23 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 24fa2f94..fe60aa14 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -1,3 +1,20 @@ +""" +This SubModule interacts with a Raspberry Pi camera using the Picamera2 library. + +The Picamera2 library uses LibCamera as the underlying camera stack. This gives us +some control of the GPU pipeline for the image. + +The API documentation for PiCamera2 is unfortunatly not in a standard auto-generated +website. For documentation of the PiCamera2 API there is a PDF called +"The Picamera2 Library" available at: +https://datasheets.raspberrypi.com/camera/picamera2-manual.pdf + +For information on the algorithms used to tune/calibrate the Raspberry Pi Camera see +the guide called "Raspberry Pi Camera Algorithm and Tuning Guide" +Available at: +https://datasheets.raspberrypi.com/camera/raspberry-pi-camera-guide.pdf +""" + from __future__ import annotations from datetime import datetime import json @@ -71,6 +88,11 @@ class PicameraStreamOutput(Output): class SensorMode(BaseModel): + """ + A Pydantic model holding all the information about a specific + sensor mode as reported by the PiCamera. + """ + unpacked: str bit_depth: int size: tuple[int, int] @@ -81,11 +103,28 @@ class SensorMode(BaseModel): class SensorModeSelector(BaseModel): + """ + A Pydantic model holding the two values needed to select a PiCamera + Sensor mode. The output size and the bit depth. + + This is a Pydantic modell so that it can be saved to the disk. + """ + output_size: tuple[int, int] bit_depth: int class LensShading(BaseModel): + """ + A Pydantic model holding the lens shading tables. + + PiCamera needs three numpy arrays for lens shading correction. Each array is + (12, 16) in size. The arrays are luminance, red-difference chroma (Cr), and + blue-difference chroma (Cb). + + This is a Pydantic modell so that it can be saved to the disk. + """ + luminance: list[list[float]] Cr: list[list[float]] Cb: list[list[float]] @@ -203,6 +242,14 @@ class StreamingPiCamera2(BaseCamera): @exposure_time.setter def exposure_time(self, value: int): + """ + Custom setter for the above exposure_time PicameraControl. + + This is overriding the standard setter for a PicameraControl, as + the value needs to be adjusted before setting to behave as expected. + + See comment within the function for more detail. + """ with self.picamera() as cam: # Note: This set a value 1 higher than requested as picamera2 always sets # a lower value than requested, even if the requested is allowed @@ -375,8 +422,18 @@ class StreamingPiCamera2(BaseCamera): main_resolution: the resolution for the main configuration. Defaults to (820, 616), 1/4 sensor size. buffer_count: the number of frames to hold in the buffer. Higher uses more memory, - lower may cause dropped frames. Defaults to 6. + lower may cause dropped frames. Value must be between 1 and 8, Defaults to 6. """ + + # Buffer count can't be negative, zero, or too high. + if buffer_count < 1 or buffer_count < 8: + # 8 is slightly arbitrary. 6 is the PiCamera default for video + # and the documentation only says that setting values higher gives + # diminishing returns, and that the true maximum is hardware dependent + raise ValueError( + f"Can't set a buffer count of {buffer_count}. " + "Buffer count must be an integer from 1-8" + ) with self.picamera() as picam: try: if picam.started: @@ -388,7 +445,6 @@ class StreamingPiCamera2(BaseCamera): sensor=self.thing_settings.get("sensor_mode", None), controls=self.persistent_controls, ) - # Set buffer count - can't be negative stream_config["buffer_count"] = buffer_count picam.configure(stream_config) logging.info("Starting picamera MJPEG stream...") @@ -626,8 +682,10 @@ class StreamingPiCamera2(BaseCamera): the processed images. It should not affect raw images. """ with self.picamera(pause_stream=True) as cam: - # Suppress lint warning that L, Cr, and Cb are not lowercase, as this is the - # Standard format for these mathematical vars. + # Suppress lint warning that L, Cr, and Cb are not lowercase, as these are + # the standard mathematical terms for: + # luminance (L), red-difference chroma (Cr), and blue-difference chroma + # (Cb). L, Cr, Cb = recalibrate_utils.lst_from_camera(cam) # noqa: N806 recalibrate_utils.set_static_lst(self.tuning, L, Cr, Cb) self.initialise_picamera() @@ -649,8 +707,14 @@ class StreamingPiCamera2(BaseCamera): @thing_action def reset_ccm(self): - """Overwrite the colour correction matrix in camera tuning with default values from the documentation""" - c = [ + """ + Overwrite the colour correction matrix in camera tuning with default values. + + These values are from the Raspberry Pi Camera Algorithm and Tuning Guide, page + 45. + """ + # This is flattened 3x3 matrix. See `calibrate_colour_correction` + col_corr_matrix = [ 1.80439, -0.73699, -0.06739, @@ -661,13 +725,25 @@ class StreamingPiCamera2(BaseCamera): -0.56403, 1.64781, ] - self.colour_correction_matrix = c + self.colour_correction_matrix = col_corr_matrix @thing_action - def calibrate_colour_correction(self, c: tuple) -> None: - """Overwrite the colour correction matrix in camera tuning""" + def calibrate_colour_correction( + self, + col_corr_matrix: tuple[ + float, float, float, float, float, float, float, float, float + ], + ) -> None: + """Overwrite the colour correction matrix in camera tuning + + col_corr_matrix: This is a 9 value tuple used to specify the 3x3 + matrix that the GPU pipeline uses to convert from the camera R,G,B vector + to the standard R,G,B. + + See page Raspberry Pi Camera Algorithm and Tuning Guide, page 45. + """ with self.picamera(pause_stream=True): - recalibrate_utils.set_static_ccm(self.tuning, c) + recalibrate_utils.set_static_ccm(self.tuning, col_corr_matrix) self.initialise_picamera() @thing_action @@ -710,29 +786,43 @@ class StreamingPiCamera2(BaseCamera): This method will set a completely flat lens shading table. It is not the same as the default behaviour, which is to use an adaptive lens shading table. + + This flat table is used to take an image wth no lens shading so that the + correct lens shading table can be calibrated. """ with self.picamera(pause_stream=True): - f = np.ones((12, 16)) - recalibrate_utils.set_static_lst(self.tuning, f, f, f) + # Generate and array of ones of the correct size for each channel + flat_array = np.ones((12, 16)) + recalibrate_utils.set_static_lst( + self.tuning, flat_array, flat_array, flat_array + ) self.initialise_picamera() @thing_property def lens_shading_tables(self) -> Optional[LensShading]: """The current lens shading (i.e. flat-field correction) - This returns the current lens shading correction, as three 2D lists - each with dimensions 16x12. This assumes that we are using a static - lens shading table - if adaptive control is enabled, or if there - are multiple LSTs in use for different colour temperatures, - we return a null value to avoid confusion. + Return the current lens shading correction, as three 2D lists each with + dimensions 16x12, if a static lens shading table is in use. + + Return None if: + - adaptive control is enabled + - multiple LSTs in use (for different colour temperatures), """ if not self.lens_shading_is_static: return None + + # Note "alsc" is the Picamera2 term for "Automatic Lens Shading Correction" alsc = Picamera2.find_tuning_algo(self.tuning, "rpi.alsc") - if any(len(alsc[f"calibrations_C{c}"]) != 1 for c in ("r", "b")): + + # Check there is exactly 1 correction table for red-difference chroma (Cr) + # and blue-difference chroma (Cb) + if len(alsc["calibrations_Cr"]) != 1 or len(alsc["calibrations_Cb"]) != 1: + # If there is not exactly one table, then lens shading isn't static. return None def reshape_lst(lin: list[float]) -> list[list[float]]: + """Reshape the 192 element list into a 2D 16x12 list""" w, h = 16, 12 return [lin[w * i : w * (i + 1)] for i in range(h)] diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index d3073f75..cf51efe8 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py @@ -326,10 +326,11 @@ def channels_from_bayer_array(bayer_array: np.ndarray) -> np.ndarray: return channels -def get_16x12_grid(chan: np.ndarray, dx: int, dy: int): +def get_16x12_grid(chan: np.ndarray, dx: int, dy: int) -> np.ndarray: """Compresses channel down to a 16x12 grid - from libcamera - This is taken from https://git.linuxtv.org/libcamera.git/tree/utils/raspberrypi/ctt/ctt_alsc.py + This is taken from + https://git.linuxtv.org/libcamera.git/tree/utils/raspberrypi/ctt/ctt_alsc.py for consistency. """ grid = [] @@ -347,7 +348,7 @@ def get_16x12_grid(chan: np.ndarray, dx: int, dy: int): return np.reshape(np.array(grid), (12, 16)) -def upsample_channels(grids: np.ndarray, shape: tuple[int]): +def upsample_channels(grids: np.ndarray, shape: tuple[int]) -> np.ndarray: """Zoom an image in the last two dimensions This is effectively the inverse operation of `get_16x12_grid` @@ -452,14 +453,19 @@ def set_static_lst( alsc["luminance_lut"] = as_flat_rounded_list(luminance, round_to=3) -def set_static_ccm(tuning: dict, c: list) -> None: +def set_static_ccm( + tuning: dict, + col_corr_matrix: tuple[ + float, float, float, float, float, float, float, float, float + ], +) -> None: """Update the `rpi.alsc` section of a camera tuning dict to use a static correcton. `tuning` will be updated in-place to set its shading to static, and disable any adaptive tweaking by the algorithm. """ ccm = Picamera2.find_tuning_algo(tuning, "rpi.ccm") - ccm["ccms"] = [{"ct": 2860, "ccm": c}] + ccm["ccms"] = [{"ct": 2860, "ccm": col_corr_matrix}] def get_static_ccm(tuning: dict) -> None: From 0389ab760ae7bd7f36bc2302ae2e0d111bb8ea63 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 16:52:23 +0000 Subject: [PATCH 61/63] Apply suggestions from code review of branch jpeg-capture-in-stacking Co-authored-by: Joe Knapper --- src/openflexure_microscope_server/things/camera/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 31cfe0e3..14376e0f 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -248,6 +248,10 @@ class BaseCamera(Thing): if save_resolution is not None and image.size != save_resolution: image = image.resize(save_resolution, Image.BOX) try: + # Per PIL documentation, (https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#jpeg) + # there are two factors when saving a JPEG. subsampling affects the colour, quality the pixels + # subsampling = 0 disables subsampling of colour + # quality = 95 is the maximum recommended - above this, JPEG compression is disabled, file size increases and quality is barely or not affected image.save(jpeg_path, quality=95, subsampling=0) try: # Load EXIF metadata from image so it can be added to. From 761957d7fb3e32ac11f4e143bc69711a38466939 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Jun 2025 17:53:43 +0100 Subject: [PATCH 62/63] Clarify exposure variables in docstring --- .../things/camera/picamera.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index fe60aa14..fce23500 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -623,11 +623,17 @@ class StreamingPiCamera2(BaseCamera): target_white_level: int = 700, percentile: float = 99.9, ): - """Adjust exposure to hit the target white level + """Adjust exposure until a the target white level is reached - Starting from the minimum exposure, we gradually increase exposure until - we hit the specified white level. We use a percentile rather than the - maximum, in order to be robust to a small number of noisy/bright pixels. + Starting from the minimum exposure, gradually increase exposure until + the image reaches the specified white level. + + :param target_white_level: The target 10bit white level. 10-bit data has a + theoretical maximum of 1023, but with black level correction the true maxiumum + is about 950. Default is 700 as this is approximately 70% saturated. + :param percentile: The percentile to use instead of maximum. Default 99.9. When + calculating the brightest pixel, a percentile is used rather than the + maximum in order to be robust to a small number of noisy/bright pixels. """ with self.picamera(pause_stream=True) as cam: recalibrate_utils.adjust_shutter_and_gain_from_raw( From b49aecb8027d9fffd1e95a315863ff0d1cd5cd12 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 25 Jun 2025 17:32:05 +0000 Subject: [PATCH 63/63] Apply suggestions from code review of branch jpeg-capture-in-stacking --- src/openflexure_microscope_server/things/camera/picamera.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index fce23500..56f8ff84 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -4,7 +4,7 @@ This SubModule interacts with a Raspberry Pi camera using the Picamera2 library. The Picamera2 library uses LibCamera as the underlying camera stack. This gives us some control of the GPU pipeline for the image. -The API documentation for PiCamera2 is unfortunatly not in a standard auto-generated +The API documentation for PiCamera2 is unfortunately not in a standard auto-generated website. For documentation of the PiCamera2 API there is a PDF called "The Picamera2 Library" available at: https://datasheets.raspberrypi.com/camera/picamera2-manual.pdf @@ -426,7 +426,7 @@ class StreamingPiCamera2(BaseCamera): """ # Buffer count can't be negative, zero, or too high. - if buffer_count < 1 or buffer_count < 8: + if buffer_count < 1 or buffer_count > 8: # 8 is slightly arbitrary. 6 is the PiCamera default for video # and the documentation only says that setting values higher gives # diminishing returns, and that the true maximum is hardware dependent