From 29518e9cb9c155fe1fe46672e701d7f6ad9591b1 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 3 Jun 2026 16:56:41 +0100 Subject: [PATCH] Update hardware tests and fix PNG saving with picamera --- .../things/camera/__init__.py | 14 ++++-- .../picamera2/test_acquisition.py | 50 ++++++++++++++++--- .../picamera2/test_exposure_time_drift.py | 8 +-- .../picamera2/test_streaming_mode.py | 2 +- tests/unit_tests/test_base_camera.py | 14 ++++-- 5 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 4ca25c0d..f494f01b 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -42,6 +42,10 @@ class ImageFormatInfo(BaseModel): supported_extensions: tuple[str, ...] """All supported extension (lowercase).""" + def path_matches(self, path: str) -> bool: + """Return True if path matches one of the supported extensions.""" + return path.lower().endswith(self.supported_extensions) + BASE_IMAGE_FORMATS: dict[str, ImageFormatInfo] = { "jpeg": ImageFormatInfo( @@ -462,7 +466,7 @@ class BaseCamera(OFMThing, ABC): complete, this error will not be raised until the data is accessed. Consider using ``grab_jpeg_as_array`` instead. - This differs from ``capture_jpeg`` in that it does not pause the MJPEG + This differs from ``capture`` in that it does not pause the MJPEG preview stream. Instead, we simply return the next frame from that stream (either "main" for the preview stream, or "lores" for the low resolution preview). No metadata is returned. @@ -679,8 +683,7 @@ class BaseCamera(OFMThing, ABC): image = image.resize(save_resolution, Image.Resampling.BOX) try: save_kwargs: dict[str, Any] = {} - jpeg_exts = BASE_IMAGE_FORMATS["jpeg"].supported_extensions - if resolved_path.lower().endswith(jpeg_exts): + if BASE_IMAGE_FORMATS["jpeg"].path_matches(resolved_path): # 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, @@ -689,6 +692,11 @@ class BaseCamera(OFMThing, ABC): # quality = 95 is the maximum recommended - above this, JPEG compression is # disabled, file size increases and quality is barely or not affected save_kwargs = {"quality": 95, "subsampling": 0} + if ( + BASE_IMAGE_FORMATS["png"].path_matches(resolved_path) + and image.mode == "RGBX" + ): + image = image.convert("RGB") image.save(resolved_path, **save_kwargs) try: self._add_metadata_to_capture(resolved_path, dict(metadata)) diff --git a/tests/hardware_specific_tests/picamera2/test_acquisition.py b/tests/hardware_specific_tests/picamera2/test_acquisition.py index b33f2154..0df00962 100644 --- a/tests/hardware_specific_tests/picamera2/test_acquisition.py +++ b/tests/hardware_specific_tests/picamera2/test_acquisition.py @@ -7,11 +7,8 @@ import numpy as np from PIL import Image -def test_jpeg_and_array(picamera_client): - """Check that a jpeg grabbed from the stream is the same size as other captures. - - Compare it to an array capture and a jpeg capture. - """ +def test_quick_capture_size(picamera_client): + """Check that a jpeg grabbed from the stream is the same size as a quick capture.""" # Grab a jpeg from the stream blob = picamera_client.grab_jpeg() mjpeg_frame = Image.open(blob.open()) @@ -20,13 +17,17 @@ def test_jpeg_and_array(picamera_client): assert mjpeg_frame.format == "JPEG" # Capture a jpeg - blob = picamera_client.capture_jpeg(stream_name="main") + blob = picamera_client.capture( + capture_mode="quick", + image_format="jpeg", + retain_image=True, + ) jpeg_capture = Image.open(blob.open()) jpeg_capture.verify() assert jpeg_capture.format == "JPEG" # Capture an array - arrlist = picamera_client.capture_as_array(stream_name="main") + arrlist = picamera_client.capture_as_array(capture_mode="quick") array_main = np.array(arrlist) # Verify image sizes are the same @@ -34,6 +35,41 @@ def test_jpeg_and_array(picamera_client): assert array_main.shape[1::-1] == jpeg_capture.size +def test_format(picamera_client): + """Check capture format is as requested.""" + # Capture a jpeg + blob = picamera_client.capture( + capture_mode="quick", + image_format="jpeg", + retain_image=True, + ) + jpeg_capture = Image.open(blob.open()) + jpeg_capture.verify() + assert jpeg_capture.format == "JPEG" + + blob = picamera_client.capture( + capture_mode="quick", + image_format="png", + retain_image=True, + ) + jpeg_capture = Image.open(blob.open()) + jpeg_capture.verify() + assert jpeg_capture.format == "PNG" + + +def test_standard_capture_size(picamera_client): + """Check standard capture mode captures at expected size.""" + # Capture a jpeg + blob = picamera_client.capture( + capture_mode="standard", + image_format="jpeg", + retain_image=True, + ) + jpeg_capture = Image.open(blob.open()) + jpeg_capture.verify() + assert jpeg_capture.size == (1640, 1232) + + def test_record_framerate(picamera_client): """Check that framerate monitoring creates a valid JSON log with good data.""" log_file = Path(picamera_client.record_framerate(duration=1.0)) diff --git a/tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py b/tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py index 9dff64e4..1710309e 100644 --- a/tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py +++ b/tests/hardware_specific_tests/picamera2/test_exposure_time_drift.py @@ -39,7 +39,7 @@ def _test_exposure_time_drift(desired_time: int) -> None: assert abs(pre_capture_et - desired_time) < EXPOSURE_TOL for i in range(10): - client.capture_jpeg(stream_name="full") + client.capture(capture_mode="full") if i == 0: # Exposure can update on first capture, due to frame rate restrictions first_et = client.exposure_time @@ -56,7 +56,7 @@ def _test_exposure_time_drift(desired_time: int) -> None: time.sleep(0.5) # Check before and after capture assert client.exposure_time == frame_et - client.capture_jpeg(stream_name="full") + client.capture(capture_mode="full") assert client.exposure_time == frame_et print("Exposure time didn't change!!") print(f"End of test for exposure target {desired_time}") @@ -82,7 +82,7 @@ def test_exposure_time_on_start_and_stop_stream(): # 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(stream_name="full") + client.capture(capture_mode="full") # Save this time. set_time = client.exposure_time assert abs(set_time - desired_time) < EXPOSURE_TOL @@ -112,7 +112,7 @@ def _load_camera_and_return_exposure(tmpdir: str) -> int: # 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(stream_name="full") + client.capture(capture_mode="full") # Save this time. return client.exposure_time diff --git a/tests/hardware_specific_tests/picamera2/test_streaming_mode.py b/tests/hardware_specific_tests/picamera2/test_streaming_mode.py index 4add4fb4..a327a271 100644 --- a/tests/hardware_specific_tests/picamera2/test_streaming_mode.py +++ b/tests/hardware_specific_tests/picamera2/test_streaming_mode.py @@ -14,7 +14,7 @@ def test_streaming_mode(): with camera_test_client() as client: for mode, res in ["default", (820, 616)], ["full_resolution", (3280, 2464)]: client.change_streaming_mode(mode=mode) - arr = np.array(client.capture_as_array(stream_name="main")) + arr = np.array(client.capture_as_array(capture_mode="quick")) # 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. diff --git a/tests/unit_tests/test_base_camera.py b/tests/unit_tests/test_base_camera.py index 1182d05d..4722568f 100644 --- a/tests/unit_tests/test_base_camera.py +++ b/tests/unit_tests/test_base_camera.py @@ -79,6 +79,7 @@ class MemorySaveTestCase: filename: str = "foobar.jpeg" save_resolution: Optional[tuple[int, int]] = None resize_needed: bool = False + convert_needed: bool = False save_kwargs: dict[str, int] = field( default_factory=lambda: {"quality": 95, "subsampling": 0} ) @@ -91,9 +92,9 @@ SAVE_TEST_CASES = [ MemorySaveTestCase("foobar.JPEG"), MemorySaveTestCase("foobar.JPG"), MemorySaveTestCase("foobar.png.jpeg"), - MemorySaveTestCase("foobar.png", save_kwargs={}), - MemorySaveTestCase("foobar.PNG", save_kwargs={}), - MemorySaveTestCase("foobar.jpeg.png", save_kwargs={}), + MemorySaveTestCase("foobar.png", save_kwargs={}, convert_needed=True), + MemorySaveTestCase("foobar.PNG", save_kwargs={}, convert_needed=True), + MemorySaveTestCase("foobar.jpeg.png", save_kwargs={}, convert_needed=True), MemorySaveTestCase(save_resolution=None, resize_needed=False), MemorySaveTestCase(save_resolution=(1000, 1200), resize_needed=False), MemorySaveTestCase(save_resolution=(2000, 2400), resize_needed=True), @@ -112,10 +113,12 @@ def test_save_from_memory(test_case, test_env, mocker): mocker.patch.object(type(camera), "capture_modes", capture_modes_mock) mock_image = mocker.Mock() - # Make resize return itself so we can track further calls of the Image object after - # a resize + # Make resize and convert return itself so we can track further calls of the Image + # object after a resize mock_image.resize.return_value = mock_image + mock_image.convert.return_value = mock_image mock_image.size = (1000, 1200) + mock_image.mode = "RGBX" camera._memory_buffer.get_image.return_value = ( mock_image, @@ -131,5 +134,6 @@ def test_save_from_memory(test_case, test_env, mocker): assert camera._memory_buffer.get_image.call_args.args == (33,) assert camera._add_metadata_to_capture.call_count == 1 assert mock_image.resize.call_count == (1 if test_case.resize_needed else 0) + assert mock_image.convert.call_count == (1 if test_case.convert_needed else 0) assert mock_image.save.call_count == 1 assert mock_image.save.call_args.kwargs == test_case.save_kwargs