Merge branch 'start-to-remove-deps' into 'v3'

Start to remove deps

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

src/openflexure_microscope_server/things/camera/__init__.py

@@ -10,7 +10,6 @@ from __future__ import annotations
 
 import io
 import json
-import logging
 import os
 import tempfile
 import time
@@ -34,8 +33,6 @@ from openflexure_microscope_server.background_detect import (
 )
 from openflexure_microscope_server.ui import ActionButton, PropertyControl
 
-LOGGER = logging.getLogger(__name__)
-
 
 class JPEGBlob(lt.blob.Blob):
     """A class representing a JPEG image as a LabThings FastAPI Blob."""
@@ -282,8 +279,6 @@ class BaseCamera(lt.Thing):
     @lt.action
     def capture_jpeg(
         self,
-        metadata_getter: lt.deps.GetThingStates,
-        logger: lt.deps.InvocationLogger,
         stream_name: str = "main",
         wait: Optional[float] = None,
     ) -> JPEGBlob:
@@ -292,9 +287,6 @@ class BaseCamera(lt.Thing):
         This will use the internal capture image functionally of capture_image of
         the specific camera being used.
 
-        :param metadata_getter: LabThings GetThingStates dependency, automatically
-            injected.
-        :param logger: LabThings InvocationLogger dependency, automatically injected.
         :param stream_name: A stream name supported by this camera.
         :param wait: (Optional, float) Set a timeout in seconds. If None it will
             use the default for the underlying camera.
@@ -305,13 +297,12 @@ class BaseCamera(lt.Thing):
 
         img = self.capture_image(stream_name, wait)
 
-        capture_metadata = self._capture_metadata(metadata_getter())
+        capture_metadata = self._capture_metadata()
 
         self._save_capture(
             jpeg_path=jpeg_path,
             image=img,
             metadata=capture_metadata,
-            logger=logger,
         )
 
         return JPEGBlob.from_temporary_directory(directory, fname)
@@ -390,70 +381,43 @@ class BaseCamera(lt.Thing):
     def capture_and_save(
         self,
         jpeg_path: str,
-        logger: lt.deps.InvocationLogger,
-        metadata_getter: lt.deps.GetThingStates,
         save_resolution: Optional[Tuple[int, int]] = None,
     ) -> None:
         """Capture an image and save it to disk.
 
         :param jpeg_path: The path to save the file to
-        :param logger: This should be injected automatically by Labthings FastAPI
-            when calling the action
-        :param metadata_getter: This should be injected automatically by Labthings
-            FastAPI when calling the action
         :param save_resolution: can be set to resize the image before saving. By
             default this is None meaning that the image is saved at original resolution.
         """
-        image, capture_metadata = self._robust_image_capture(
+        image, capture_metadata = self._robust_image_capture()
-            metadata_getter,
-            logger=logger,
-        )
 
-        self._save_capture(
+        self._save_capture(jpeg_path, image, capture_metadata, save_resolution)
-            jpeg_path,
-            image,
-            capture_metadata,
-            logger,
-            save_resolution,
-        )
 
     @lt.action
-    def capture_to_memory(
+    def capture_to_memory(self, buffer_max: int = 1) -> int:
-        self,
-        logger: lt.deps.InvocationLogger,
-        metadata_getter: lt.deps.GetThingStates,
-        buffer_max: int = 1,
-    ) -> int:
         """Capture an image to memory. This can be saved later with ``save_from_memory``.
 
         Note that only one image is held in memory so this will overwrite any image
         in memory.
 
-        :param logger: This should be injected automatically by Labthings FastAPI
-            when calling the action
-        :param metadata_getter: This should be injected automatically by Labthings
-            FastAPI when calling the action
         :param buffer_max: The maximum number of images that should be in the buffer
             once this images is added. Default is 1.
 
         :returns: the buffer id of the image captured
         """
-        image, metadata = self._robust_image_capture(metadata_getter, logger)
+        image, metadata = self._robust_image_capture()
         return self._memory_buffer.add_image(image, metadata, buffer_max=buffer_max)
 
     @lt.action
     def save_from_memory(
         self,
         jpeg_path: str,
-        logger: lt.deps.InvocationLogger,
         save_resolution: Optional[Tuple[int, int]] = None,
         buffer_id: Optional[int] = None,
     ) -> None:
         """Save an image that has been captured to memory.
 
         :param jpeg_path: The path to save the file to
-        :param logger: This should be injected automatically by Labthings FastAPI
-            when calling the action
         :param save_resolution: can be set to resize the image before saving. By
             default this is None meaning that the image is saved at original
             resolution.
@@ -466,7 +430,6 @@ class BaseCamera(lt.Thing):
             jpeg_path=jpeg_path,
             image=image,
             metadata=metadata,
-            logger=logger,
             save_resolution=save_resolution,
         )
 
@@ -475,11 +438,7 @@ class BaseCamera(lt.Thing):
         """Clear all images in memory."""
         self._memory_buffer.clear()
 
-    def _robust_image_capture(
+    def _robust_image_capture(self) -> Tuple[Image, Mapping[str, Any]]:
-        self,
-        metadata_getter: lt.deps.GetThingStates,
-        logger: lt.deps.InvocationLogger,
-    ) -> Tuple[Image, Mapping[str, Any]]:
         """Capture an image in memory and return it with metadata.
 
         This robust capturing method attempts to capture the image five times
@@ -491,22 +450,19 @@ class BaseCamera(lt.Thing):
         """
         for capture_attempts in range(5):
             try:
-                metadata = metadata_getter()
+                capture_metadata = self._capture_metadata()
-                capture_metadata = self._capture_metadata(metadata)
 
                 image = self.capture_image(stream_name="main", wait=5)
                 return image, capture_metadata
             except TimeoutError:
-                logger.warning(
+                self.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 _capture_metadata(
+    def _capture_metadata(self) -> dict:
-        self,
-        metadata: Mapping[str, Any],
-    ) -> dict:
         """Return the metadata for a capture, from the thing states, time and known names."""
+        metadata = self._thing_server_interface.get_thing_states()
         current_time = datetime.now()
         return {
             "capture_time": current_time.timestamp(),
@@ -566,12 +522,11 @@ class BaseCamera(lt.Thing):
         jpeg_path: str,
         image: Image,
         metadata: dict,
-        logger: lt.deps.InvocationLogger,
         save_resolution: Optional[Tuple[int, int]] = None,
     ) -> None:
         """Save the captured image and metadata to disk.
 
-        A warning (via InvocationLogger) is raised if metadata is failed to be added
+        A warning is logged if metadata cannot be added.
 
         :raises IOError: if the file cannot be saved
 
@@ -593,7 +548,7 @@ class BaseCamera(lt.Thing):
             except Exception:
                 # We need to capture any exception as there are many reasons metadata
                 # might not be added. We warn rather than log the error.
-                logger.exception(f"Failed to add metadata to {jpeg_path}")
+                self.logger.exception(f"Failed to add metadata to {jpeg_path}")
         except Exception as e:
             raise IOError(f"An error occurred while saving {jpeg_path}") from e
 
@@ -639,7 +594,7 @@ class BaseCamera(lt.Thing):
     def detector_name(self, name: str) -> None:
         """Validate and set detector_name."""
         if name not in self.background_detectors:
-            LOGGER.warning(f"{name} is not a valid background detector name.")
+            self.logger.warning(f"{name} is not a valid background detector name.")
         self._detector_name = name
 
     @property
@@ -697,7 +652,7 @@ class BaseCamera(lt.Thing):
                 obj.settings = instance_data["settings"]
                 obj.background_data = instance_data["background_data"]
             else:
-                LOGGER.warning(
+                self.logger.warning(
                     f"No background detector named {name}, settings will be discarded."
                 )
 

src/openflexure_microscope_server/things/stage/__init__.py

@@ -133,24 +133,15 @@ class BaseStage(lt.Thing):
         self.axis_inverted = direction
 
     @lt.action
-    def move_relative(
+    def move_relative(self, block_cancellation: bool = False, **kwargs: int) -> None:
-        self,
-        cancel: lt.deps.CancelHook,
-        block_cancellation: bool = False,
-        **kwargs: int,
-    ) -> None:
         """Make a relative move. Keyword arguments should be axis names."""
         self._hardware_move_relative(
-            cancel=cancel,
             block_cancellation=block_cancellation,
             **self._apply_axis_direction(kwargs),
         )
 
     def _hardware_move_relative(
-        self,
+        self, block_cancellation: bool = False, **kwargs: int
-        cancel: lt.deps.CancelHook,
-        block_cancellation: bool = False,
-        **kwargs: int,
     ) -> Never:
         """Make a relative move in the coordinate system used by the physical hardware.
 
@@ -161,22 +152,15 @@ class BaseStage(lt.Thing):
         )
 
     @lt.action
-    def move_absolute(
+    def move_absolute(self, block_cancellation: bool = False, **kwargs: int) -> None:
-        self,
-        cancel: lt.deps.CancelHook,
-        block_cancellation: bool = False,
-        **kwargs: int,
-    ) -> None:
         """Make an absolute move. Keyword arguments should be axis names."""
         self._hardware_move_absolute(
-            cancel=cancel,
             block_cancellation=block_cancellation,
             **self._apply_axis_direction(kwargs),
         )
 
     def _hardware_move_absolute(
         self,
-        cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
         **kwargs: int,
     ) -> Never:
@@ -212,20 +196,16 @@ class BaseStage(lt.Thing):
         return (position_dict["x"], position_dict["y"], position_dict["z"])
 
     @lt.action
-    def move_to_xyz_position(
+    def move_to_xyz_position(self, xyz_pos: tuple[int, int, int]) -> None:
-        self, cancel: lt.deps.CancelHook, xyz_pos: tuple[int, int, int]
-    ) -> None:
         """Move to the location specified by an (x, y, z) tuple.
 
-        :param cancel: A cancel hook for cancelling the move. This dependency should be
-            injected automatically by LabThings-FastAPI
         :param xyz_pos: The (x, y, z) position to move to.
 
         :raises KeyError: if this stage does not have axes named "x", "y", and "z".
 
         This method provides the interface expected by the camera_stage_mapping.
         """
-        self.move_absolute(cancel=cancel, x=xyz_pos[0], y=xyz_pos[1], z=xyz_pos[2])
+        self.move_absolute(x=xyz_pos[0], y=xyz_pos[1], z=xyz_pos[2])
 
 
 StageDependency = lt.deps.direct_thing_client_dependency(BaseStage, "stage")

src/openflexure_microscope_server/things/stage/dummy.py

@@ -55,7 +55,6 @@ class DummyStage(BaseStage):
 
     def _hardware_move_relative(
         self,
-        cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
         **kwargs: int,
     ) -> None:
@@ -71,7 +70,7 @@ class DummyStage(BaseStage):
                 if block_cancellation:
                     time.sleep(dt)
                 else:
-                    cancel.sleep(dt)
+                    lt.cancellable_sleep(dt)
                 fraction_complete = (time.time() - start_time) / (dt * max_displacement)
                 self.instantaneous_position = {
                     ax: self._hardware_position[ax] + int(fraction_complete * disp)
@@ -93,7 +92,6 @@ class DummyStage(BaseStage):
 
     def _hardware_move_absolute(
         self,
-        cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
         **kwargs: int,
     ) -> None:
@@ -104,7 +102,7 @@ class DummyStage(BaseStage):
             if axis in self.axis_names
         }
         self._hardware_move_relative(
-            cancel, block_cancellation=block_cancellation, **displacement
+            block_cancellation=block_cancellation, **displacement
         )
 
     @lt.action

src/openflexure_microscope_server/things/stage/sangaboard.py

@@ -2,7 +2,6 @@
 
 from __future__ import annotations
 
-import logging
 import threading
 import time
 from collections.abc import Mapping
@@ -18,8 +17,6 @@ import sangaboard
 
 from . import BaseStage
 
-LOGGER = logging.getLogger(__name__)
-
 REQUIRED_VERSION = semver.Version.parse("1.0.0")
 RECOMMENDED_VERSION = semver.Version.parse("1.0.4")
 
@@ -117,14 +114,13 @@ class SangaboardThing(BaseStage):
 
             # Warn if version is below recommended
             if version < RECOMMENDED_VERSION:
-                LOGGER.warning(
+                self.logger.warning(
                     f"Sangaboard firmware version {version} is below the recommended "
                     f"{RECOMMENDED_VERSION}."
                 )
 
     def _hardware_move_relative(
         self,
-        cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
         **kwargs: int,
     ) -> None:
@@ -138,7 +134,7 @@ class SangaboardThing(BaseStage):
                     sb.query("notify_on_stop")
                 else:
                     while sb.query("moving?") == "true":
-                        cancel.sleep(0.1)
+                        lt.cancellable_sleep(0.1)
             except lt.exceptions.InvocationCancelledError as e:
                 # If the move has been cancelled, stop it but don't handle the exception.
                 # We need the exception to propagate in order to stop any calling tasks,
@@ -151,7 +147,6 @@ class SangaboardThing(BaseStage):
 
     def _hardware_move_absolute(
         self,
-        cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
         **kwargs: int,
     ) -> None:
@@ -164,7 +159,7 @@ class SangaboardThing(BaseStage):
                 if axis in self.axis_names
             }
             self._hardware_move_relative(
-                cancel, block_cancellation=block_cancellation, **displacement
+                block_cancellation=block_cancellation, **displacement
             )
 
     @lt.action
@@ -203,7 +198,7 @@ class SangaboardThing(BaseStage):
             # cannot be used.
             intended_brightness = float(return_value[7:])
             on_brightness = 0.32
-            LOGGER.warning(
+            self.logger.warning(
                 "Brightness control is not yet implemented. Desired brightness: "
                 f"{intended_brightness}. Set brightness: {on_brightness}"
             )

src/openflexure_microscope_server/things/system.py

@@ -121,9 +121,9 @@ class OpenFlexureSystem(lt.Thing):
         return CommandOutput(output=out, error=err)
 
     @lt.action
-    def get_things_state(self, metadata_getter: lt.deps.GetThingStates) -> Mapping:
+    def get_things_state(self) -> Mapping:
         """Metadata summarising the current state of all Things in the server."""
-        return metadata_getter()
+        return self._thing_server_interface.get_thing_states()
 
     @property
     def thing_state(self) -> Mapping[str, Any]:

tests/mock_things/mock_cancel.py

@@ -1,19 +0,0 @@
-"""Create a fake CancelHook used instead of a dependency, it cannot be used to cancel!
-
-Certain actions require a CancelHook to be supplied if called directly. The only method
-we use from the LabThings CancelHook is ``sleep()``. The CanceHook ``sleep()`` works
-exactly like ``time.sleep()`` except with raise an ``InvocationCancelledError`` if the
-server receives a cancellation request.
-
-This is just an object with a sleep method.
-"""
-
-import time
-
-
-class MockCancel:
-    """A class to use when directly calling an action with CancelHook dependency."""
-
-    def sleep(self, time_in_seconds: float) -> None:
-        """Sleep for the input number of seconds."""
-        time.sleep(time_in_seconds)

tests/test_stage.py

@@ -18,8 +18,6 @@ from openflexure_microscope_server.things.stage import (
 )
 from openflexure_microscope_server.things.stage.dummy import DummyStage
 
-from .mock_things.mock_cancel import MockCancel
-
 # Keep the size and number of moves fairly small or the tests can take forever
 point3d = st.tuples(
     st.integers(min_value=-100, max_value=100),
@@ -60,12 +58,7 @@ def test_override_base_movement():
 
     class BadStage1(BaseStage):
         @lt.action
-        def move_relative(
+        def move_relative(self, block_cancellation: bool = False, **kwargs: int):
-            self,
-            cancel: lt.deps.CancelHook,
-            block_cancellation: bool = False,
-            **kwargs: int,
-        ):
             pass
 
     with pytest.raises(RedefinedBaseMovementError):
@@ -73,12 +66,7 @@ def test_override_base_movement():
 
     class BadStage2(BaseStage):
         @lt.action
-        def move_absolute(
+        def move_absolute(self, block_cancellation: bool = False, **kwargs: int):
-            self,
-            cancel: lt.deps.CancelHook,
-            block_cancellation: bool = False,
-            **kwargs: int,
-        ):
             pass
 
     with pytest.raises(RedefinedBaseMovementError):
@@ -200,7 +188,6 @@ def _test_move_relative(dummy_stage, axis_inverted, path):
     :param axis_inverted: Is used to set the inversion.
     :param path: The 3d path to move over, generated by hypothesis.
     """
-    cancel = MockCancel()
     dummy_stage.axis_inverted = axis_inverted
     # Explicitly do axes calculation here to check logic in main code.
     x_dir = -1 if axis_inverted["x"] else 1
@@ -209,9 +196,7 @@ def _test_move_relative(dummy_stage, axis_inverted, path):
 
     position = list(dummy_stage.position.values())
     for movement in path:
-        dummy_stage.move_relative(
+        dummy_stage.move_relative(x=movement[0], y=movement[1], z=movement[2])
-            cancel=cancel, x=movement[0], y=movement[1], z=movement[2]
-        )
         position = [pos + move for pos, move in zip(position, movement, strict=True)]
         stage_pos = dummy_stage.get_xyz_position()
         hw_pos = dummy_stage._hardware_position
@@ -257,7 +242,6 @@ def _test_move_absolute(dummy_stage, axis_inverted, path):
     :param axis_inverted: Is used to set the inversion.
     :param path: The 3d path to move over, generated by hypothesis.
     """
-    cancel = MockCancel()
     dummy_stage.axis_inverted = axis_inverted
     # Explicitly do axes calculation here to check logic in main code.
     x_dir = -1 if axis_inverted["x"] else 1
@@ -265,9 +249,7 @@ def _test_move_absolute(dummy_stage, axis_inverted, path):
     z_dir = -1 if axis_inverted["z"] else 1
     position = list(dummy_stage.position.values())
     for move_to in path:
-        dummy_stage.move_absolute(
+        dummy_stage.move_absolute(x=move_to[0], y=move_to[1], z=move_to[2])
-            cancel=cancel, x=move_to[0], y=move_to[1], z=move_to[2]
-        )
         position = move_to
         stage_pos = dummy_stage.get_xyz_position()
         hw_pos = dummy_stage._hardware_position