From 5e6aeb9bbf92e2f34d3bc6b77d3e4d434ce7d9f6 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 2 Jul 2025 16:03:12 +0100 Subject: [PATCH 1/4] Move SangaboardThing into repo --- .../things/camera/simulation.py | 4 +- .../things/stage/__init__.py | 83 ++------- .../things/stage/dummy.py | 6 +- .../things/stage/sangaboard.py | 161 ++++++++++++++++++ 4 files changed, 179 insertions(+), 75 deletions(-) create mode 100644 src/openflexure_microscope_server/things/stage/sangaboard.py diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index c27ebc20..5f6dfe49 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/src/openflexure_microscope_server/things/camera/simulation.py @@ -25,7 +25,7 @@ from labthings_fastapi.dependencies.metadata import GetThingStates from labthings_fastapi.server import ThingServer from . import BaseCamera, JPEGBlob, ArrayModel -from ..stage import StageProtocol as Stage +from ..stage import BaseStage # The ratio between "motor" steps and pixels # higher related to a faster movement @@ -35,7 +35,7 @@ RATIO = 0.2 class SimulatedCamera(BaseCamera): """A Thing representing an OpenCV camera""" - _stage: Optional[Stage] = None + _stage: Optional[BaseStage] = None _server: Optional[ThingServer] = None def __init__( diff --git a/src/openflexure_microscope_server/things/stage/__init__.py b/src/openflexure_microscope_server/things/stage/__init__.py index 81e30ce6..9ae9423a 100644 --- a/src/openflexure_microscope_server/things/stage/__init__.py +++ b/src/openflexure_microscope_server/things/stage/__init__.py @@ -1,65 +1,12 @@ from __future__ import annotations -from typing import Any, Protocol, TypeAlias, runtime_checkable +from typing import TypeAlias +from collections.abc import Sequence, Mapping + from labthings_fastapi.descriptors.property import PropertyDescriptor from labthings_fastapi.thing import Thing from labthings_fastapi.decorators import thing_action, thing_property from labthings_fastapi.dependencies.invocation import CancelHook from labthings_fastapi.dependencies.thing import direct_thing_client_dependency -from collections.abc import Sequence, Mapping - - -@runtime_checkable -class StageProtocol(Protocol): - """A protocol for the OpenFlexure translation stage""" - - _axis_names: Sequence[str] - - @property - def axis_names(self) -> Sequence[str]: - """The names of the stage's axes, in order.""" - ... - - @property - def position(self) -> Mapping[str, int]: - """Current position of the stage""" - ... - - @property - def moving() -> bool: - "Whether the stage is in motion" - ... - - @property - def thing_state(self) -> Mapping[str, Any]: - """Summary metadata describing the current state of the stage""" - ... - - def move_relative( - self, - cancel: CancelHook, - block_cancellation: bool = False, - **kwargs: Mapping[str, int], - ): - """Make a relative move. Keyword arguments should be axis names.""" - ... - - def move_absolute( - self, - cancel: CancelHook, - block_cancellation: bool = False, - **kwargs: Mapping[str, int], - ): - """Make an absolute move. Keyword arguments should be axis names.""" - ... - - def set_zero_position(self): - """Make the current position zero in all axes - - This action does not move the stage, but resets the position to zero. - It is intended for use after manually or automatically recentring the - stage. - """ - ... class BaseStage(Thing): @@ -99,16 +46,6 @@ class BaseStage(Thing): """Summary metadata describing the current state of the stage""" return {"position": self.position} - -class StageStub(BaseStage): - """A Stub Thing for the OpenFlexure translation stage - - 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. - """ - @thing_action def move_relative( self, @@ -117,7 +54,9 @@ class StageStub(BaseStage): **kwargs: Mapping[str, int], ): """Make a relative move. Keyword arguments should be axis names.""" - raise NotImplementedError("StageStub should not be used directly") + raise NotImplementedError( + "StageThings must define their own move_relative method" + ) @thing_action def move_absolute( @@ -127,7 +66,9 @@ class StageStub(BaseStage): **kwargs: Mapping[str, int], ): """Make an absolute move. Keyword arguments should be axis names.""" - raise NotImplementedError("StageStub should not be used directly") + raise NotImplementedError( + "StageThings must define their own move_absolute method" + ) @thing_action def set_zero_position(self): @@ -137,7 +78,9 @@ class StageStub(BaseStage): It is intended for use after manually or automatically recentring the stage. """ - raise NotImplementedError("StageStub should not be used directly") + raise NotImplementedError( + "StageThings must define their own set_zero_position method" + ) -StageDependency: TypeAlias = direct_thing_client_dependency(StageStub, "/stage/") +StageDependency: TypeAlias = direct_thing_client_dependency(BaseStage, "/stage/") diff --git a/src/openflexure_microscope_server/things/stage/dummy.py b/src/openflexure_microscope_server/things/stage/dummy.py index 044a6625..c6df8d25 100644 --- a/src/openflexure_microscope_server/things/stage/dummy.py +++ b/src/openflexure_microscope_server/things/stage/dummy.py @@ -75,9 +75,9 @@ class DummyStage(BaseStage): ): """Make an absolute move. Keyword arguments should be axis names.""" displacement = { - k: int(v) - self.position[k] - for k, v in kwargs.items() - if k in self.axis_names + axis: int(pos) - self.position[axis] + for axis, pos in kwargs.items() + if axis in self.axis_names } self.move_relative( cancel, block_cancellation=block_cancellation, **displacement diff --git a/src/openflexure_microscope_server/things/stage/sangaboard.py b/src/openflexure_microscope_server/things/stage/sangaboard.py new file mode 100644 index 00000000..75613152 --- /dev/null +++ b/src/openflexure_microscope_server/things/stage/sangaboard.py @@ -0,0 +1,161 @@ +from __future__ import annotations +import logging +import threading +import time +from typing import Iterator, Literal +from contextlib import contextmanager +from collections.abc import Mapping + +from fastapi import HTTPException + +import sangaboard +from labthings_fastapi.decorators import thing_action +from labthings_fastapi.dependencies.invocation import ( + CancelHook, + InvocationCancelledError, +) + +from . import BaseStage + + +class SangaboardThing(BaseStage): + def __init__(self, port: str = None, **kwargs): + """A Thing to manage a Sangaboard motor controller + + Internally, this uses the `pysangaboard` library. + """ + self.sangaboard_kwargs = kwargs + self.sangaboard_kwargs["port"] = port + + def __enter__(self): + self._sangaboard = sangaboard.Sangaboard(**self.sangaboard_kwargs) + self._sangaboard_lock = threading.RLock() + with self.sangaboard() as sb: + if sb.version_tuple[0] != 1: + raise RuntimeError("labthings-sangaboard requires firmware v1") + sb.query("blocking_moves false") + self.update_position() + + def __exit__(self, _exc_type, _exc_value, _traceback): + with self.sangaboard() as sb: + sb.close() + + @contextmanager + def sangaboard(self) -> Iterator[sangaboard.Sangaboard]: + """Return the wrapped `sangaboard.Sangaboard` instance. + + This is protected by a `threading.RLock`, which may change in future. + """ + with self._sangaboard_lock: + yield self._sangaboard + + def update_position(self): + """Read position from the stage and set the corresponding property.""" + with self.sangaboard() as sb: + self.position = dict(zip(self.axis_names, sb.position)) + + @thing_action + def move_relative( + self, + cancel: CancelHook, + block_cancellation: bool = False, + **kwargs: Mapping[str, int], + ): + """Make a relative move. Keyword arguments should be axis names.""" + displacement = [kwargs.get(k, 0) for k in self.axis_names] + with self.sangaboard() as sb: + self.moving = True + try: + sb.move_rel(displacement) + if block_cancellation: + sb.query("notify_on_stop") + else: + while sb.query("moving?") == "true": + cancel.sleep(0.1) + except 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, + # and to mark the invocation as "cancelled" rather than stopped. + sb.query("stop") + raise e + finally: + self.moving = False + self.update_position() + + @thing_action + def move_absolute( + self, + cancel: CancelHook, + block_cancellation: bool = False, + **kwargs: Mapping[str, int], + ): + """Make an absolute move. Keyword arguments should be axis names.""" + with self.sangaboard(): + self.update_position() + displacement = { + axis: int(pos) - self.position[axis] + for axis, pos in kwargs.items() + if axis in self.axis_names + } + self.move_relative( + cancel, block_cancellation=block_cancellation, **displacement + ) + + @thing_action + def abort_move(self): + """Abort a current move""" + if self.moving: + # Skip the lock - because we need to write **before** the current query + # finishes. This merits further careful thought for thread safety. + # TODO: more robust aborts + logging.warning("Aborting move: this is an experimental feature!") + tc = self._sangaboard.termination_character + self._sangaboard._ser.write(("stop" + tc).encode()) + else: + raise HTTPException(status_code=409, detail="Stage is not moving.") + + @thing_action + def set_zero_position(self): + """Make the current position zero in all axes + + This action does not move the stage, but resets the position to zero. + It is intended for use after manually or automatically recentring the + stage. + """ + with self.sangaboard() as sb: + sb.zero_position() + self.update_position() + + @thing_action + def flash_led( + self, + number_of_flashes: int = 10, + dt: float = 0.5, + led_channel: Literal["cc"] = "cc", + ) -> None: + """Flash the LED to identify the board + + This is intended to be useful in situations where there are multiple + Sangaboards in use, and it is necessary to identify which one is + being addressed. + """ + led_command = f"led_{led_channel}" + with self.sangaboard() as sb: + return_value = sb.query(f"{led_command}?") + if not return_value.startswith("CC LED:"): + raise IOError("The sangaboard does not support LED control") + + # Reading and setting LED brightness suffers from repeated reads and writes + # decreasing the value. Rather than use the value the code warns that the value + # cannot be used. + intended_brightness = float(return_value[7:]) + on_brightness = 0.32 + logging.warning( + "Brightness control is not yet implemented. Desired brightness: " + f"{intended_brightness}. Set brightness: {on_brightness}" + ) + for i in range(number_of_flashes): + sb.query(f"{led_command} 0") + time.sleep(dt) + sb.query(f"{led_command} {on_brightness}") + time.sleep(dt) From 4e20067c6811234513bacd06da556a25e871ff85 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 2 Jul 2025 17:27:09 +0100 Subject: [PATCH 2/4] Update json configuration and dependencies for SangaboardThing being in the repository --- ofm_config_full.json | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ofm_config_full.json b/ofm_config_full.json index 4e5beeee..38a76685 100644 --- a/ofm_config_full.json +++ b/ofm_config_full.json @@ -1,7 +1,7 @@ { "things": { "/camera/": "openflexure_microscope_server.things.camera.picamera:StreamingPiCamera2", - "/stage/": "labthings_sangaboard:SangaboardThing", + "/stage/": "openflexure_microscope_server.things.stage.sangaboard:SangaboardThing", "/auto_recentre_stage/": "openflexure_microscope_server.things.auto_recentre_stage:RecentringThing", "/autofocus/": "openflexure_microscope_server.things.autofocus:AutofocusThing", "/camera_stage_mapping/": "openflexure_microscope_server.things.camera_stage_mapping:CameraStageMapper", diff --git a/pyproject.toml b/pyproject.toml index 4822ec60..96f885b9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ classifiers = [ ] dependencies = [ "labthings-fastapi[server] == 0.0.9", - "labthings-sangaboard", + "sangaboard", "camera-stage-mapping ~= 0.1.10", "opencv-python ~= 4.11.0", "openflexure-stitching[libvips]==0.1.0", From 222a0c973dca58143aa286ce325bdddd30f81044 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 3 Jul 2025 13:16:25 +0100 Subject: [PATCH 3/4] Remove old sangaboard abort_move action as moves are not cancelled with a cancel hook --- .../things/stage/sangaboard.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/openflexure_microscope_server/things/stage/sangaboard.py b/src/openflexure_microscope_server/things/stage/sangaboard.py index 75613152..a1187d89 100644 --- a/src/openflexure_microscope_server/things/stage/sangaboard.py +++ b/src/openflexure_microscope_server/things/stage/sangaboard.py @@ -6,8 +6,6 @@ from typing import Iterator, Literal from contextlib import contextmanager from collections.abc import Mapping -from fastapi import HTTPException - import sangaboard from labthings_fastapi.decorators import thing_action from labthings_fastapi.dependencies.invocation import ( @@ -101,19 +99,6 @@ class SangaboardThing(BaseStage): cancel, block_cancellation=block_cancellation, **displacement ) - @thing_action - def abort_move(self): - """Abort a current move""" - if self.moving: - # Skip the lock - because we need to write **before** the current query - # finishes. This merits further careful thought for thread safety. - # TODO: more robust aborts - logging.warning("Aborting move: this is an experimental feature!") - tc = self._sangaboard.termination_character - self._sangaboard._ser.write(("stop" + tc).encode()) - else: - raise HTTPException(status_code=409, detail="Stage is not moving.") - @thing_action def set_zero_position(self): """Make the current position zero in all axes From 544ede0bf9ae85e26a0da95056c80d63e521184c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 3 Jul 2025 12:26:14 +0000 Subject: [PATCH 4/4] Apply suggestions from code review of branch include-sangaboard Co-authored-by: Beth Probert --- .../things/stage/sangaboard.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/openflexure_microscope_server/things/stage/sangaboard.py b/src/openflexure_microscope_server/things/stage/sangaboard.py index a1187d89..7e18c035 100644 --- a/src/openflexure_microscope_server/things/stage/sangaboard.py +++ b/src/openflexure_microscope_server/things/stage/sangaboard.py @@ -20,7 +20,10 @@ class SangaboardThing(BaseStage): def __init__(self, port: str = None, **kwargs): """A Thing to manage a Sangaboard motor controller - Internally, this uses the `pysangaboard` library. + Internally, this uses the `pysangaboard` package from PyPi. This imports + as `sangaboard`. As `pysangaboard` does not support some features added + to the Sangaboard firmware v1 (LED flashing, aborting moves, etc) this + functionality is accessed by directly querying the serial interface. """ self.sangaboard_kwargs = kwargs self.sangaboard_kwargs["port"] = port @@ -30,7 +33,9 @@ class SangaboardThing(BaseStage): self._sangaboard_lock = threading.RLock() with self.sangaboard() as sb: if sb.version_tuple[0] != 1: - raise RuntimeError("labthings-sangaboard requires firmware v1") + raise RuntimeError( + "Please update your Sangaboard Firmware. v1 is required." + ) sb.query("blocking_moves false") self.update_position() @@ -47,7 +52,7 @@ class SangaboardThing(BaseStage): with self._sangaboard_lock: yield self._sangaboard - def update_position(self): + def update_position(self) -> None: """Read position from the stage and set the corresponding property.""" with self.sangaboard() as sb: self.position = dict(zip(self.axis_names, sb.position)) @@ -58,9 +63,9 @@ class SangaboardThing(BaseStage): cancel: CancelHook, block_cancellation: bool = False, **kwargs: Mapping[str, int], - ): + ) -> None: """Make a relative move. Keyword arguments should be axis names.""" - displacement = [kwargs.get(k, 0) for k in self.axis_names] + displacement = [kwargs.get(axis, 0) for axis in self.axis_names] with self.sangaboard() as sb: self.moving = True try: @@ -86,7 +91,7 @@ class SangaboardThing(BaseStage): cancel: CancelHook, block_cancellation: bool = False, **kwargs: Mapping[str, int], - ): + ) -> None: """Make an absolute move. Keyword arguments should be axis names.""" with self.sangaboard(): self.update_position() @@ -100,7 +105,7 @@ class SangaboardThing(BaseStage): ) @thing_action - def set_zero_position(self): + def set_zero_position(self) -> None: """Make the current position zero in all axes This action does not move the stage, but resets the position to zero.