From fe1b84a92266bca48aaeff47c3f11d3d99404747 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 18 Jul 2025 19:49:36 +0100 Subject: [PATCH] Complete first pass (untested on hardware) of bg detect refactor. --- ofm_config_full.json | 3 +- ofm_config_simulation.json | 3 +- .../background_detect.py | 63 ++++++++++++------- .../things/camera/__init__.py | 29 +++++++-- .../things/camera/picamera.py | 2 +- .../things/camera/simulation.py | 2 +- .../things/smart_scan.py | 16 ++--- ...ck_background_detect.py => mock_camera.py} | 17 ++--- tests/test_smart_scan.py | 8 +-- .../paneBackgroundDetect.vue | 35 ++--------- 10 files changed, 90 insertions(+), 88 deletions(-) rename tests/mock_things/{mock_background_detect.py => mock_camera.py} (57%) diff --git a/ofm_config_full.json b/ofm_config_full.json index 1cfedb26..98a1cbc7 100644 --- a/ofm_config_full.json +++ b/ofm_config_full.json @@ -11,8 +11,7 @@ "kwargs": { "scans_folder": "/var/openflexure/scans/" } - }, - "/background_detect/": "openflexure_microscope_server.things.background_detect:BackgroundDetectThing" + } }, "settings_folder": "/var/openflexure/settings/", "log_folder": "/var/openflexure/logs/" diff --git a/ofm_config_simulation.json b/ofm_config_simulation.json index a254fa85..fd9dcae2 100644 --- a/ofm_config_simulation.json +++ b/ofm_config_simulation.json @@ -11,8 +11,7 @@ "kwargs": { "scans_folder": "./openflexure/scans/" } - }, - "/background_detect/": "openflexure_microscope_server.things.background_detect:BackgroundDetectThing" + } }, "settings_folder": "./openflexure/settings/", "log_folder": "./openflexure/logs/" diff --git a/src/openflexure_microscope_server/background_detect.py b/src/openflexure_microscope_server/background_detect.py index 71a1d8a9..e2286b9f 100644 --- a/src/openflexure_microscope_server/background_detect.py +++ b/src/openflexure_microscope_server/background_detect.py @@ -5,12 +5,32 @@ for analysis. Information from this images is used to detect whether an image fr current camera field of view contains sample. """ -from typing import Optional +from typing import Optional, Any import cv2 import numpy as np from pydantic import BaseModel from pydantic.errors import PydanticUserError from scipy.stats import norm +from labthings_fastapi.thing_description import type_to_dataschema + + +class BackgroundDetectorStatus(BaseModel): + """The status information about a background detector instance needed for the GUI. + + Each BackgroundDetectAlgorithm must be able to return one of these models when + ``status`` is called. + """ + + ready: bool + """True if ready to be used, if False this detector isn't initialised for use. + + This could be called ``has_background_data`` or similar, but the more generic + ``ready`` is used in case more complex methods are added in the future, which + need different initialisation. + """ + settings: BaseModel + """The settings for this this background detect Algorithm""" + settings_schema: dict[str, Any] class BackgroundDetectAlgorithm: @@ -24,12 +44,21 @@ class BackgroundDetectAlgorithm: def __init__(self): """Initialise the algorithm settings.""" try: - _settings: BaseModel = self.settings_data_model() + self._settings: BaseModel = self.settings_data_model() except PydanticUserError as e: raise NotImplementedError( "BackgroundDetectAlgorithms must set their own settings data model." ) from e + @property + def status(self) -> BackgroundDetectorStatus: + """The status information needed for the GUI. Read only.""" + return BackgroundDetectorStatus( + ready=self.background_data is not None, + settings=self.settings, + settings_schema=type_to_dataschema(self.settings_data_model), + ) + # Requires a getter and a setter to support being a BaseModel but being # saved to file as a dict _background_data: Optional[BaseModel] = None @@ -40,21 +69,16 @@ class BackgroundDetectAlgorithm: bd = self._background_data if bd is None: return None - try: - return self.background_data_model(**bd) - except PydanticUserError as e: - raise NotImplementedError( - "BackgroundDetectAlgorithms must set their own background data model." - ) from e + return bd @background_data.setter def background_data(self, value: Optional[BaseModel | dict]) -> None: if value is None: self._background_data = None elif isinstance(value, self.background_data_model): - self._background_data = value.model_dump() - elif isinstance(value, dict): self._background_data = value + elif isinstance(value, dict): + self._background_data = self.background_data_model(**value) else: raise TypeError( f"Cannot set background_data with an object of type {type(value)}" @@ -63,23 +87,18 @@ class BackgroundDetectAlgorithm: @property def settings(self) -> BaseModel: """The statistics of the background image.""" - bd = self._background_data - if bd is None: - return None - return self.settings_data_model(**bd) + return self._settings @settings.setter - def settings(self, value: Optional[BaseModel | dict]) -> None: - if value is None: - self._settings = None - elif isinstance(value, self.settings_data_model): - self._settings = value.model_dump() - elif isinstance(value, dict): + def settings(self, value: BaseModel | dict) -> None: + if isinstance(value, self.settings_data_model): self._settings = value + elif isinstance(value, dict): + self._settings = self.settings_data_model(**value) else: raise TypeError(f"Cannot set settings with an object of type {type(value)}") - def image_is_sample(self, image: np.ndarrayl) -> tuple[bool, str]: + def image_is_sample(self, image: np.ndarray) -> tuple[bool, str]: """Label the current image as either background or sample. :returns: A tuple of the result (boolean), and explanation string. The @@ -172,7 +191,7 @@ class ColourChannelDetectLUV(BackgroundDetectAlgorithm): mask = self.background_mask(image_luv) return (1 - np.count_nonzero(mask) / np.prod(mask.shape)) * 100 - def image_is_sample(self, image: np.ndarrayl) -> tuple[bool, str]: + def image_is_sample(self, image: np.ndarray) -> tuple[bool, str]: """Label the current image as either background or sample. :returns: A tuple of the result (boolean), and explanation string. The diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 7a26335c..3beeb7b5 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -20,7 +20,11 @@ import piexif import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray -from openflexure_microscope_server.background_detect import BackgroundDetectLUV +from openflexure_microscope_server.background_detect import ( + ColourChannelDetectLUV, + BackgroundDetectAlgorithm, + BackgroundDetectorStatus, +) LOGGER = logging.getLogger(__name__) @@ -166,7 +170,7 @@ class BaseCamera(lt.Thing): This must be run by all child camera classes. """ super().__init__() - self.background_detectors = {"Colour Channels (LUV)": BackgroundDetectLUV()} + self.background_detectors = {"Colour Channels (LUV)": ColourChannelDetectLUV()} self._detector_name = "Colour Channels (LUV)" def __enter__(self) -> None: @@ -484,23 +488,36 @@ class BaseCamera(lt.Thing): self._detector_name = name @property - def active_detector(self): + def active_detector(self) -> BackgroundDetectAlgorithm: """The active background detector instance.""" return self.background_detectors[self.detector_name] + @lt.thing_property + def background_detector_status(self) -> BackgroundDetectorStatus: + """The status of the active detector for the UI.""" + return self.active_detector.status + @lt.thing_setting def background_detector_data(self) -> str: - """The name of the active background selector.""" + """The data for each background detector, used to save to disk.""" data = {} for name, obj in self.background_detectors.items(): + bg_data = ( + None + if obj.background_data is None + else obj.background_data.model_dump() + ) data[name] = { "settings": obj.settings.model_dump(), - "background_data": obj.background_data.model_dump(), + "background_data": bg_data, } @detector_name.setter def detector_name(self, data: str) -> None: - """Validate and set detector_name.""" + """Set the data for each detector. This should only be called on init. + + Do not call over HTTP. Would be good to have a way to make this private. + """ for name, instance_data in data.items(): if name in self.background_detectors: obj = self.background_detectors[name] diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index fea7b149..fba605e0 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -118,7 +118,7 @@ class StreamingPiCamera2(BaseCamera): :param camera_num: The number of the camera. This should generally be left as 0 as most Raspberry Pi boards only support 1 camera. """ - super().__init() + super().__init__() self._setting_save_in_progress = False self.camera_num = camera_num self.camera_configs: dict[str, dict] = {} diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index 225e474a..76d11eb9 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/src/openflexure_microscope_server/things/camera/simulation.py @@ -58,7 +58,7 @@ class SimulatedCamera(BaseCamera): :param frame_interval: Nominally the time between frames on the MJPEG stream, however the rate may be slower due to calculation time for focus. """ - super().__init() + super().__init__() self.shape = shape self.glyph_shape = glyph_shape self.canvas_shape = canvas_shape diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 3db02813..c830c93c 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -1,7 +1,7 @@ """The core sample scanning functionality for the OpenFlexure Microscope. SmartScan provides sample scanning functionality including automatic background -detection (via the `BackgroundDetectThing`) and automatic path planning via +detection (via the `CameraThing`) and automatic path planning via `scan_planners`. It manages the directories of past scans via `scan_directories`. It also controls external processes for live stitching composite images, and the creation of the final stitched images. @@ -29,7 +29,6 @@ from openflexure_microscope_server import scan_planners # Things from .autofocus import AutofocusThing from .camera_stage_mapping import CameraStageMapper -from .background_detect import BackgroundDetectThing from .camera import CameraDependency as CamDep from .stage import StageDependency as StageDep @@ -37,9 +36,6 @@ CSMDep = lt.deps.direct_thing_client_dependency( CameraStageMapper, "/camera_stage_mapping/" ) AutofocusDep = lt.deps.direct_thing_client_dependency(AutofocusThing, "/autofocus/") -BackgroundDep = lt.deps.direct_thing_client_dependency( - BackgroundDetectThing, "/background_detect/" -) JPEGBlob = lt.blob.blob_type("image/jpeg") ZipBlob = lt.blob.blob_type("application/zip") @@ -110,7 +106,6 @@ class SmartScanThing(lt.Thing): self._cam: Optional[CamDep] = None self._metadata_getter: Optional[lt.deps.GetThingStates] = None self._csm: Optional[CSMDep] = None - self._background_detect: Optional[BackgroundDep] = None self._ongoing_scan: Optional[scan_directories.ScanDirectory] = None self._starting_position: Optional[Mapping[str, int]] = None self._capture_thread: Optional[ErrorCapturingThread] = None @@ -128,14 +123,13 @@ class SmartScanThing(lt.Thing): cam: CamDep, metadata_getter: lt.deps.GetThingStates, csm: CSMDep, - background_detect: BackgroundDep, scan_name: str = "", ): """Move the stage to cover an area, taking images that can be tiled together. The stage will move in a pattern that grows outwards from the starting point, stopping once it is surrounded by "background" (as detected by the - background_detect Thing) or reaches the "max_range" measured in steps. + camera Thing) or reaches the "max_range" measured in steps. """ got_lock = self._scan_lock.acquire(timeout=0.1) if not got_lock: @@ -149,7 +143,6 @@ class SmartScanThing(lt.Thing): self._cam = cam self._metadata_getter = metadata_getter self._csm = csm - self._background_detect = background_detect self._capture_thread = None self._scan_images_taken = 0 @@ -183,7 +176,6 @@ class SmartScanThing(lt.Thing): self._cam = None self._metadata_getter = None self._csm = None - self._background_detect = None self._capture_thread = None self._ongoing_scan = None self._scan_images_taken = None @@ -207,7 +199,7 @@ class SmartScanThing(lt.Thing): ) if self.skip_background: - if not self._background_detect.background_distributions: + if not self._cam.background_detector_status.ready: raise RuntimeError( "Background is not set: you need to calibrate background detection." ) @@ -511,7 +503,7 @@ class SmartScanThing(lt.Thing): capture_image = True # If skipping background, take an image to check if current field of view is background if self._scan_data["skip_background"]: - capture_image, bg_message = self._background_detect.image_is_sample() + capture_image, bg_message = self._cam.image_is_sample() if not capture_image: route_planner.mark_location_visited( diff --git a/tests/mock_things/mock_background_detect.py b/tests/mock_things/mock_camera.py similarity index 57% rename from tests/mock_things/mock_background_detect.py rename to tests/mock_things/mock_camera.py index 80419030..1c2ecdbf 100644 --- a/tests/mock_things/mock_background_detect.py +++ b/tests/mock_things/mock_camera.py @@ -1,4 +1,4 @@ -"""Testing submodule with mock Background Detect Things. +"""Testing submodule with mock Camera Things. These mocks are designed to be inserted as dependencies to provide specific functionality and return values. @@ -7,10 +7,13 @@ The mocks do not subclass Things. Instead, they return predefined answers to functions. """ -from openflexure_microscope_server.things.background_detect import ChannelDistributions +from openflexure_microscope_server.background_detect import ( + BackgroundDetectorStatus, + ColourChannelDetectSettings, +) -class MockBackgroundDetectThing: +class MockCameraThing: """A mock background detect Thing that imports no code from BackgroundDetectThing. The class needs functionality added to it over time as more complex @@ -18,8 +21,8 @@ class MockBackgroundDetectThing: is not artificially inflated. """ - background_distributions = ChannelDistributions( - means=[128.0, 128.0, 128.0], - standard_deviations=[3.0, 3.0, 3.0], - colorspace="LUV", + background_detector_status = BackgroundDetectorStatus( + ready=True, + settings=ColourChannelDetectSettings(), + settings_schema={"fake": "schema"}, ) diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index 0ec1ae3f..1a0be750 100644 --- a/tests/test_smart_scan.py +++ b/tests/test_smart_scan.py @@ -30,7 +30,7 @@ from openflexure_microscope_server.things.smart_scan import ( from .mock_things.mock_csm import MockCSMThing from .mock_things.mock_autofocus import MockAutoFocusThing from .mock_things.mock_stage import MockStageThing -from .mock_things.mock_background_detect import MockBackgroundDetectThing +from .mock_things.mock_camera import MockCameraThing # A global logger to pass in as an Invocation Logger LOGGER = logging.getLogger("mock-invocation_logger") @@ -169,10 +169,9 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): cancel_mock = 1 # not called af_mock = MockAutoFocusThing() stage_mock = MockStageThing() - cam_mock = 4 # not called + cam_mock = MockCameraThing() meta_mock = 5 # not called csm_mock = MockCSMThing() - bkgrnd_det_mock = MockBackgroundDetectThing() class MockedSmartScanThing(SmartScanThing): """Mocked version of SmartScanThing with a patched _run_scan method.""" @@ -192,7 +191,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): assert self._cam is cam_mock assert self._metadata_getter is meta_mock assert self._csm is csm_mock - assert self._background_detect is bkgrnd_det_mock assert self._capture_thread is None assert self._scan_images_taken == 0 @@ -212,7 +210,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): cam=cam_mock, metadata_getter=meta_mock, csm=csm_mock, - background_detect=bkgrnd_det_mock, scan_name="FooBar", ) except Exception as e: @@ -227,7 +224,6 @@ def _run_only_outer_scan(adjust_initial_state: Optional[Callable] = None): assert mock_ss_thing._cam is None assert mock_ss_thing._metadata_getter is None assert mock_ss_thing._csm is None - assert mock_ss_thing._background_detect is None assert mock_ss_thing._capture_thread is None assert mock_ss_thing._scan_images_taken is None diff --git a/webapp/src/components/tabContentComponents/backgroundDetectComponents/paneBackgroundDetect.vue b/webapp/src/components/tabContentComponents/backgroundDetectComponents/paneBackgroundDetect.vue index bef06118..be57f9ac 100644 --- a/webapp/src/components/tabContentComponents/backgroundDetectComponents/paneBackgroundDetect.vue +++ b/webapp/src/components/tabContentComponents/backgroundDetectComponents/paneBackgroundDetect.vue @@ -8,37 +8,13 @@
  • Settings
    -
    - -
    -
    - -
    -
    - -
    +

    TODO!

    • +