Consolidate functionality into BaseCamera from Capture Thing and StreamingPicamera2

hardware-specific-tests/picamera2/test_acquisition.py

@@ -6,6 +6,7 @@ from PIL import Image
 import numpy as np
 from pytest import fixture
 
+
 @fixture(scope="module")
 def client():
     server = ThingServer()
@@ -14,9 +15,11 @@ def client():
         client = ThingClient.from_url("/camera/", client=test_client)
         yield client
 
+
 def test_calibration(client):
     client.full_auto_calibrate()
 
+
 def test_jpeg_and_array(client):
     blob = client.grab_jpeg()
     mjpeg_frame = Image.open(blob.open())
@@ -28,5 +31,3 @@ def test_jpeg_and_array(client):
     array_main = np.array(arrlist)
     assert mjpeg_frame.size == jpeg_capture.size
     assert array_main.shape[1::-1] == jpeg_capture.size
-
-

hardware-specific-tests/picamera2/test_tuning.py

@@ -5,7 +5,7 @@ from labthings_picamera2 import recalibrate_utils
 import pytest
 
 
-MODEL = Picamera2.global_camera_info()[0]['Model']
+MODEL = Picamera2.global_camera_info()[0]["Model"]
 
 
 def check_camera_available():

ofm_config_full.json

@@ -13,8 +13,7 @@
                 "scans_folder": "/var/openflexure/scans/"
             }
         },
-        "/background_detect/": "openflexure_microscope_server.things.background_detect:BackgroundDetectThing",
+        "/background_detect/": "openflexure_microscope_server.things.background_detect:BackgroundDetectThing"
-        "/capture/": "openflexure_microscope_server.things.capture:CaptureThing"
     },
     "settings_folder": "/var/openflexure/settings/",
     "log_folder": "/var/openflexure/logs/"

src/openflexure_microscope_server/things/autofocus.py

@@ -18,7 +18,6 @@ import glob
 from fastapi import Depends
 import numpy as np
 from pydantic import BaseModel
-from PIL import Image
 
 from labthings_fastapi.thing import Thing
 from labthings_fastapi.dependencies.blocking_portal import BlockingPortal
@@ -31,11 +30,8 @@ from labthings_fastapi.dependencies.invocation import InvocationLogger
 from .camera import RawCameraDependency as Camera
 from .camera import CameraDependency as WrappedCamera
 from .stage import StageDependency as Stage
-from .capture import CaptureThing
 
 
-CaptureDep = direct_thing_client_dependency(CaptureThing, "/capture/")
-
 STACK_OVERSHOOT = 200
 SETTLING_TIME = 0.3
 
@@ -288,7 +284,6 @@ class AutofocusThing(Thing):
         stage: Stage,
         logger: InvocationLogger,
         metadata_getter: GetThingStates,
-        capture: CaptureDep,
         images_dir: str,
         stack_dir: str,
         capture_resolution: tuple[int, int],
@@ -307,29 +302,19 @@ class AutofocusThing(Thing):
         for capture_count in range(images_to_capture):
             time.sleep(SETTLING_TIME)
 
-            jpeg_path = os.path.join(
+            jpeg_path = os.path.join(stack_dir, f"{capture_count}.jpeg")
-                stack_dir,
-                f"{capture_count}.jpeg",
-            )
             start_time = time.time()
-            image, metadata = capture._capture_image(
+            cam.capture_to_memory(logger=logger, metadata_getter=logger)
-                cam=cam,
-                metadata_getter=metadata_getter,
-                logger=logger,
-            )
             captured_time = time.time()
 
             if capture_count + 1 < images_to_capture:
                 stage.move_relative(z=stack_dz)
             moved_time = time.time()
-            if image.size != capture_resolution:
+
-                image = image.resize(capture_resolution, Image.BOX)
+            cam.save_from_memory(
-            downsampled_time = time.time()
-            capture._save_capture(
                 jpeg_path=jpeg_path,
-                image=image,
-                metadata=metadata,
                 logger=logger,
+                save_resolution=capture_resolution,
             )
             saved_time = time.time()
             time_remaining = SETTLING_TIME - (saved_time - start_time)
@@ -337,10 +322,9 @@ class AutofocusThing(Thing):
                 time.sleep(time_remaining)
                 logger.info(f"Settled for an extra {round(time_remaining, 3)} seconds")
             logger.debug(f"Capturing took {round(captured_time - start_time, 2)} s")
-            logger.debug(f"Resizing took {round(downsampled_time - moved_time, 2)} s")
+            logger.debug(f"Saving took {round(saved_time - moved_time, 2)} s")
-            logger.debug(f"Saving took {round(saved_time - downsampled_time, 2)} s")
             logger.debug(
-                f"Effective settling time was {round(saved_time - moved_time, 2)} s"
+                f"Effective settling time was {round(time.time() - moved_time, 2)} s"
             )
 
         self.copy_sharpest_image_from_stack(images_dir, stack_dir, logger)

src/openflexure_microscope_server/things/camera/__init__.py

@@ -8,9 +8,10 @@ See repository root for licensing information.
 
 from __future__ import annotations
 import logging
-from typing import Literal, Protocol, runtime_checkable, Optional
+from typing import Literal, Optional, Tuple
 
 from pydantic import RootModel
+from PIL import Image
 
 from labthings_fastapi.thing import Thing
 from labthings_fastapi.decorators import thing_action, thing_property
@@ -18,6 +19,7 @@ from labthings_fastapi.dependencies.metadata import GetThingStates
 from labthings_fastapi.dependencies.blocking_portal import BlockingPortal
 from labthings_fastapi.dependencies.thing import direct_thing_client_dependency
 from labthings_fastapi.dependencies.raw_thing import raw_thing_dependency
+from labthings_fastapi.dependencies.invocation import InvocationLogger
 from labthings_fastapi.outputs.mjpeg_stream import MJPEGStreamDescriptor
 from labthings_fastapi.outputs.blob import Blob
 from labthings_fastapi.types.numpy import NDArray
@@ -26,25 +28,30 @@ from labthings_fastapi.types.numpy import NDArray
 class JPEGBlob(Blob):
     media_type: str = "image/jpeg"
 
+
 class PNGBlob(Blob):
     media_type: str = "image/png"
 
+
 class ArrayModel(RootModel):
     """A model for an array"""
+
     root: NDArray
 
+
+class CaptureError(RuntimeError):
+    """An error trying to capture from a CameraThing"""
+
+
+class NoImageInMemoryError(RuntimeError):
+    """An error called if no image in in memory when an method is called to use that image"""
+
+
 class BaseCamera(Thing):
-    """A stub for a camera, to allow dependencies
+    """The base class for all cameras. All cameras must directly inherit from this class"""
-
-
-    As of LabThings-FastAPI 0.0.7, we can't make a thing client dependency
-    based on a protocol, because the protocol is not a Thing, and its
-    methods/properties are not decorated as Affordances. This stub class
-    is a workaround for that limitation, and should not be used directly.
-
-    This stub class should be used for dependencies on the CameraProtocol.
-    """
 
+    _memory_image: Optional[Image] = None
+    _memory_metadata: Optional[dict] = None
     mjpeg_stream = MJPEGStreamDescriptor()
     lores_mjpeg_stream = MJPEGStreamDescriptor()
 
@@ -52,12 +59,33 @@ class BaseCamera(Thing):
         raise NotImplementedError("CameraThings must define their own __enter__ method")
 
     def __exit__(self, _exc_type, _exc_value, _traceback) -> None:
-        raise NotImplementedError("Cameras must not inherit from CameraStub")
+        raise NotImplementedError("CameraThings must define their own __exit__ method")
+
+    @thing_property
+    def image_in_memory(self) -> bool:
+        """True if an image is in memory ready to be saved"""
+        return self._memory_image is not None and self._memory_metadata is not None
+
+    @thing_action
+    def clear_image_memory(self) -> None:
+        """Clear any image in memory"""
+        self._memory_image = None
+        self._memory_metadata = None
+
+    @thing_action
+    def start_streaming(self, main_resolution, buffer_count) -> 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(
+            "CameraThings must define their own start_streaming method"
+        )
 
     @thing_property
     def stream_active(self) -> bool:
         "Whether the MJPEG stream is active"
-        raise NotImplementedError("Cameras must not inherit from CameraStub")
+        raise NotImplementedError(
+            "CameraThings must define their own stream_active method"
+        )
 
     @thing_action
     def capture_array(
@@ -65,7 +93,9 @@ class BaseCamera(Thing):
         stream_name: Literal["main", "lores", "raw", "full"] = "main",
         wait: Optional[float] = 5,
     ) -> NDArray:
-        raise NotImplementedError("Cameras must not inherit from CameraStub")
+        raise NotImplementedError(
+            "CameraThings must define their own capture_array method"
+        )
 
     @thing_action
     def capture_jpeg(
@@ -75,18 +105,152 @@ class BaseCamera(Thing):
         wait: Optional[float] = 5,
     ) -> JPEGBlob:
         """Acquire one image from the camera and return as a JPEG blob"""
-        raise NotImplementedError("Cameras must not inherit from CameraStub")
+        raise NotImplementedError(
+            "CameraThings must define their own capture_jpeg method"
+        )
 
     @thing_action
-    def start_streaming(self, main_resolution, buffer_count) -> None:
+    def grab_jpeg(
-        """Start (or stop and restart) the camera with the given resolution
+        self,
-        for the main stream, and buffer_count number of images in the buffer"""
+        portal: BlockingPortal,
-        raise NotImplementedError("Cameras must not inherit from CameraStub")
+        stream_name: Literal["main", "lores"] = "main",
+    ) -> JPEGBlob:
+        """Acquire one image from the preview stream and return as an array
+
+        This differs from `capture_jpeg` 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.
+        """
+        stream = (
+            self.lores_mjpeg_stream if stream_name == "lores" else self.mjpeg_stream
+        )
+        frame = portal.call(stream.grab_frame)
+        return JPEGBlob.from_bytes(frame)
 
     @thing_action
     def capture_image(self, stream_name, wait):
         """Capture a PIL image from stream stream_name with timeout wait"""
-        raise NotImplementedError("Cameras must not inherit from CameraStub")
+        raise NotImplementedError(
+            "CameraThings must define their own capture_image method"
+        )
+
+    @thing_action
+    def capture_and_save(
+        self,
+        jpeg_path: str,
+        logger: InvocationLogger,
+        metadata_getter: GetThingStates,
+        save_resolution: Optional[Tuple[int, int]] = None,
+    ) -> 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.
+        """
+        image, metadata = self._robust_image_capture(
+            metadata_getter,
+            logger=logger,
+        )
+
+        self._save_capture(
+            jpeg_path,
+            image,
+            metadata,
+            logger,
+            save_resolution,
+        )
+
+    @thing_action
+    def capture_to_memory(
+        self,
+        logger: InvocationLogger,
+        metadata_getter: GetThingStates,
+    ) -> None:
+        """
+        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
+        in memory.
+        """
+        self._memory_image, self._memory_metadata = self._robust_image_capture(
+            metadata_getter,
+            logger=logger,
+        )
+
+    @thing_action
+    def save_from_memory(
+        self,
+        jpeg_path: str,
+        logger: InvocationLogger,
+        save_resolution: Optional[Tuple[int, int]] = None,
+    ) -> None:
+        """
+        Save an image that has been captured to memory.
+        """
+        if not self.image_in_memory:
+            raise NoImageInMemoryError("No image in memory to save.")
+
+        self._save_capture(
+            jpeg_path=jpeg_path,
+            image=self._memory_image,
+            metadata=self._memory_metadata,
+            logger=logger,
+            save_resolution=save_resolution,
+        )
+        self.clear_image_memory()
+
+    def _robust_image_capture(
+        self,
+        metadata_getter: GetThingStates,
+        logger: InvocationLogger,
+    ) -> 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
+        """
+        for capture_attempts in range(5):
+            try:
+                metadata = metadata_getter()
+                image = self.capture_image(stream_name="main", wait=5)
+                return image, metadata
+            except TimeoutError:
+                logger.warning(
+                    f"Attempt {capture_attempts + 1} to capture image timed out. Do you have enough RAM?"
+                )
+        raise CaptureError("An error occurred while capturing after 5 attempts")
+
+    def _save_capture(
+        self,
+        jpeg_path: str,
+        image: Image,
+        metadata: dict,
+        logger: InvocationLogger,
+        save_resolution: Optional[Tuple[int, int]] = None,
+    ) -> None:
+        """Saving the captured image and metadata to disk
+        logger warning (via InvocationLogger) is raised if metadata is failed to be added
+        IOError is raised if the file cannot be saved
+        nothing is returned on success"""
+        if save_resolution is not None and image.size != save_resolution:
+            image = image.resize(save_resolution, Image.BOX)
+        try:
+            image.save(jpeg_path, quality=95, subsampling=0)
+            try:
+                exif_dict = piexif.load(jpeg_path)
+                exif_dict["Exif"][piexif.ExifIFD.UserComment] = json.dumps(
+                    metadata
+                ).encode("utf-8")
+                piexif.insert(piexif.dump(exif_dict), jpeg_path)
+            except:  # noqa: E722
+                # We need to capture any exception as there are many reasons metadata
+                # might not be added. We warn rather than log the error.
+                logger.warning(f"Failed to add metadata to {jpeg_path}")
+        except Exception as e:
+            raise IOError(f"An error occurred while saving {jpeg_path}") from e
 
 
 CameraDependency = direct_thing_client_dependency(BaseCamera, "/camera/")

src/openflexure_microscope_server/things/camera/picamera.py

@@ -556,43 +556,6 @@ class StreamingPiCamera2(BaseCamera):
         piexif.insert(piexif.dump(exif_dict), path)
         return JPEGBlob.from_temporary_directory(folder, fname)
 
-    @thing_action
-    def grab_jpeg(
-        self,
-        portal: BlockingPortal,
-        stream_name: Literal["main", "lores"] = "main",
-    ) -> JPEGBlob:
-        """Acquire one image from the preview stream and return as an array
-
-        This differs from `capture_jpeg` 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.
-        """
-        logging.debug(
-            f"StreamingPiCamera2.grab_jpeg(stream_name={stream_name}) starting"
-        )
-        stream = (
-            self.lores_mjpeg_stream if stream_name == "lores" else self.mjpeg_stream
-        )
-        frame = portal.call(stream.grab_frame)
-        logging.debug(
-            f"StreamingPiCamera2.grab_jpeg(stream_name={stream_name}) got frame"
-        )
-        return JPEGBlob.from_bytes(frame)
-
-    @thing_action
-    def grab_jpeg_size(
-        self,
-        portal: BlockingPortal,
-        stream_name: Literal["main", "lores"] = "main",
-    ) -> int:
-        """Acquire one image from the preview stream and return its size"""
-        stream = (
-            self.lores_mjpeg_stream if stream_name == "lores" else self.mjpeg_stream
-        )
-        return portal.call(stream.next_frame_size)
-
     @thing_property
     def exposure(self) -> float:
         """An alias for `exposure_time` to fit the micromanager API"""

src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py

@@ -219,7 +219,7 @@ def adjust_shutter_and_gain_from_raw(
             break
 
     if check_convergence(test, target_white_level, tolerance):
-        logging.info(f"Brightness has converged to within {tolerance * 100 :.0f}%.")
+        logging.info(f"Brightness has converged to within {tolerance * 100:.0f}%.")
     else:
         logging.warning(
             f"Failed to reach target brightness of {target_white_level}."
@@ -257,9 +257,11 @@ def adjust_white_balance_from_raw(
         channel_gains = 1 / grids
         if channel_gains.shape[1:] != channels.shape[1:]:
             channel_gains = upsample_channels(channel_gains, channels.shape[1:])
-        logging.info(f"Before gains, channel maxima are {np.max(channels, axis=(1,2))}")
+        logging.info(
+            f"Before gains, channel maxima are {np.max(channels, axis=(1, 2))}"
+        )
         channels = channels * channel_gains
-        logging.info(f"After gains, channel maxima are {np.max(channels, axis=(1,2))}")
+        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 = (
@@ -532,7 +534,7 @@ def raw_channels_from_camera(camera: Picamera2) -> LensShadingTables:
 
 def recreate_camera_manager():
     """Delete and recreate the camera manager.
-    
+
     This is necessary to ensure the tuning file is re-read.
     """
     del Picamera2._cm

src/openflexure_microscope_server/things/camera/simulation.py

@@ -34,7 +34,6 @@ from ..stage import StageProtocol as Stage
 RATIO = 0.2
 
 
-
 class SimulatedCamera(BaseCamera):
     """A Thing representing an OpenCV camera"""
 

src/openflexure_microscope_server/things/capture.py

@@ -1,102 +0,0 @@
-import time
-import piexif
-import json
-
-from labthings_fastapi.dependencies.metadata import GetThingStates
-from labthings_fastapi.thing import Thing
-from labthings_fastapi.decorators import thing_action
-from .camera import CameraDependency as CamDep
-from labthings_fastapi.dependencies.invocation import (
-    InvocationLogger,
-)
-
-
-class CaptureError(RuntimeError):
-    """An error trying to capture from Picamera"""
-
-
-class CaptureThing(Thing):
-    """A temporary Thing to handle capturing to disk with associated metadata
-    Will be moved to the camera Thing or dependency in due course"""
-
-    @thing_action
-    def _capture_and_save(
-        self,
-        jpeg_path: str,
-        cam: CamDep,
-        logger: InvocationLogger,
-        metadata_getter: GetThingStates,
-    ) -> 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.
-        """
-        capture_start = time.time()
-        image, metadata = self._capture_image(
-            cam,
-            metadata_getter,
-            logger=logger,
-        )
-        acquisition_time = time.time()
-        self._save_capture(
-            jpeg_path,
-            image,
-            metadata,
-            logger,
-        )
-        save_time = time.time()
-        acquisition_duration = round(acquisition_time - capture_start, 1)
-        saving_duration = round(save_time - acquisition_time, 1)
-        logger.info(
-            f"Acquired {jpeg_path} in {acquisition_duration}s then {saving_duration}s saving to disk"
-        )
-
-    @thing_action
-    def _capture_image(
-        self,
-        cam: CamDep,
-        metadata_getter: GetThingStates,
-        logger: InvocationLogger,
-    ):
-        """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
-        """
-        for capture_attempts in range(5):
-            try:
-                metadata = metadata_getter()
-                image = cam.capture_image(stream_name="main", wait=5)
-                return image, metadata
-            except TimeoutError:
-                logger.warning(
-                    f"Attempt {capture_attempts + 1} to capture image timed out. Do you have enough RAM?"
-                )
-        raise CaptureError("An error occurred while capturing after 5 attempts")
-
-    @thing_action
-    def _save_capture(
-        self,
-        jpeg_path: str,
-        image,
-        metadata: dict,
-        logger: InvocationLogger,
-    ) -> None:
-        """Saving the captured image and metadata to disk
-        logger warning (via InvocationLogger) is raised if metadata is failed to be added
-        IOError is raised if the file cannot be saved
-        nothing is returned on success"""
-        try:
-            image.save(jpeg_path, quality=95, subsampling=0)
-            try:
-                exif_dict = piexif.load(jpeg_path)
-                exif_dict["Exif"][piexif.ExifIFD.UserComment] = json.dumps(
-                    metadata
-                ).encode("utf-8")
-                piexif.insert(piexif.dump(exif_dict), jpeg_path)
-            except:  # noqa: E722
-                # We need to capture any exception as there are many reasons metadata
-                # might not be added. We warn rather than log the error.
-                logger.warning(f"Failed to add metadata to {jpeg_path}")
-        except Exception as e:
-            raise IOError(f"An error occurred while saving {jpeg_path}") from e