diff --git a/src/openflexure_microscope_server/things/stage/__init__.py b/src/openflexure_microscope_server/things/stage/__init__.py index fd495186..2c06eaf3 100644 --- a/src/openflexure_microscope_server/things/stage/__init__.py +++ b/src/openflexure_microscope_server/things/stage/__init__.py @@ -11,8 +11,10 @@ As the object will be used as a context manager create the hardware connection i from __future__ import annotations +import queue +import threading from collections.abc import Mapping, Sequence -from typing import Any, Literal, overload +from typing import Any, Literal, Optional, overload import labthings_fastapi as lt @@ -30,6 +32,45 @@ class RedefinedBaseMovementError(RuntimeError): """ +class JogCommand: + """A base class for jog operations.""" + + def __init__(self, displacement: Optional[Sequence[int]]) -> None: + """Initialise a JogCommand. + + :param displacement: The distances as a sequence of moves for each axis. + None for stop motion. + """ + super().__init__() + self.displacement = None if displacement is None else tuple(displacement) + + def __repr__(self) -> str: + """Represent the command as a string.""" + class_name = type(self).__name__ + if self.displacement is None: + return f"<{class_name}>STOP" + return f"<{class_name}>{self.displacement}" + + +class JogQueue(queue.Queue[JogCommand]): + """A class queue for JogCommands. This always returns the most recent command.""" + + def __init__(self) -> None: + """Set up a queue with a max size of 1.""" + super().__init__(maxsize=1) + + def put( + self, item: JogCommand, block: bool = False, timeout: Optional[float] = None + ) -> None: + """Put the next command into the queue, bumping anything already there.""" + try: + # First remove existing item if present + self.get_nowait() + except queue.Empty: + pass + super().put(item, block=block, timeout=timeout) + + class BaseStage(lt.Thing): """A base stage class for OpenFlexure translation stages. @@ -55,7 +96,11 @@ class BaseStage(lt.Thing): that all code in the child class uses the hardware reference frame. """ super().__init__(thing_server_interface) - self._hardware_position = dict.fromkeys(self._axis_names, 0) + self._hardware_lock = threading.RLock() + self._jog_lock = threading.Lock() + self._jog_queue = JogQueue() + self._jog_thread: Optional[threading.Thread] = None + self._hardware_position: Mapping[str, int] = dict.fromkeys(self._axis_names, 0) # This must be the last thing the function does in case it is caught in a try. if ( @@ -87,6 +132,12 @@ class BaseStage(lt.Thing): ) """Used to convert coordinates between the program frame and the hardware frame.""" + def update_position(self) -> None: + """Read position from the stage and set the corresponding property.""" + raise NotImplementedError( + "StageThings must define their own update_position method" + ) + @overload def _apply_axis_direction(self, position: list[int] | tuple[int]) -> list[int]: ... @@ -177,9 +228,129 @@ class BaseStage(lt.Thing): Make sure to use and update ``self._hardware_position`` not ``self.position``. """ raise NotImplementedError( - "StageThings must define their own move_absolute method" + "StageThings must define their own _hardware_move_absolute method" ) + def _hardware_start_move_relative(self, displacement: Sequence[int]) -> None: + """Start a relative move.""" + raise NotImplementedError( + "StageThings must define their own _hardware_start_move_relative method" + ) + + def _hardware_stop(self) -> None: + raise NotImplementedError( + "StageThings must define their own _hardware_stop method" + ) + + def _poll_moving(self) -> bool: + """Determine if the stage is still moving.""" + raise NotImplementedError( + "StageThings must define their own _poll_moving method" + ) + + def _estimate_move_duration(self, displacement: Sequence[int]) -> float: + """Calculate the expected duration of a move with the given displacement.""" + raise NotImplementedError( + "StageThings must define their own _estimate_move_duration method" + ) + + @lt.action + def jog(self, stop: bool = False, **kwargs: int) -> None: + """Make a relative move that may be interrupted by a future ``jog``. + + This action makes a relative move. If another ``jog`` action is called while + a ``jog`` is already in progress, the first will be stopped and the second + will start immediately. This allows for responsive manual control of the + stage, for example with a joystick. + + :param stop: if this is set to ``True`` the jog will be terminated. + :param kwargs: Keyword arguments should be axis names. + """ + if stop: + self._send_jog_command(JogCommand(None)) + return + + hardware_moves = self._apply_axis_direction(kwargs) + move = [hardware_moves.get(axis, 0) for axis in self.axis_names] + if all(ax == 0 for ax in move): + self.logger.warning( + "Requested jog movement is is empty. Sending STOP instead." + ) + self._send_jog_command(JogCommand(None)) + else: + self._send_jog_command(JogCommand(move)) + + def _send_jog_command(self, command: JogCommand) -> None: + """Send a jog command to the background jog thread. + + This function will start the background thread if it is not running. + This function acquires ``_jog_lock`` and uses the ``_jog_send`` event to signal + the thread to read the next command. As commands interrupt each other, this + function should never block for a long time. + + :param command: the jog command to send. + """ + if not self._jog_lock.acquire(timeout=0.1): + self.logger.warning( + "Could not send a jog message, this indicates a lock error." + ) + return + try: + # Make sure the queue exists. + # Check the background thread is running, and restart it if not. + if self._jog_thread is None or not self._jog_thread.is_alive(): + self.logger.debug("Starting background thread for jog commands") + self._jog_queue = JogQueue() + self._jog_thread = threading.Thread( + target=self._jog_loop, args=(command,) + ) + self._jog_thread.start() + else: + self._jog_queue.put(command) + finally: + self._jog_lock.release() + + def _jog_loop(self, first_command: JogCommand) -> None: + """Execute jog commands in a background thread. + + This function is intended to be run in a background thread. It will look at + ``self._jog_command`` when the ``self._jog_send`` event is set. + """ + # Timeout for checking queue + timeout = 0.1 + command: Optional[JogCommand] = first_command + + # prevent others using the stage while jogging. + with self._hardware_lock: + while command is not None: + if command.displacement is not None: + self._hardware_start_move_relative(command.displacement) + timeout = self._estimate_move_duration(command.displacement) + else: + self._hardware_stop() + # Next iteration, we will probably time out. + timeout = 0.1 + self.update_position() + command = self._get_from_jog_queue(timeout) + + def _get_from_jog_queue(self, timeout: float) -> Optional[JogCommand]: + """Get the next JogCommand from the jog queue. + + :param timeout: The estimtated time the move will take for the queue timeout. + :return: The jog command or None if the stage stops before a command is + received. + """ + while True: + try: + return self._jog_queue.get(timeout=timeout) + except queue.Empty: + if not self._poll_moving(): + # The stage is no longer moving, return None + return None + # If we reached here then the stage is still moving. Shorten timeout and + # check again. + timeout = 0.1 + @lt.action def set_zero_position(self) -> None: """Make the current position zero in all axes. diff --git a/src/openflexure_microscope_server/things/stage/dummy.py b/src/openflexure_microscope_server/things/stage/dummy.py index 7130fcb7..3b1aaf0a 100644 --- a/src/openflexure_microscope_server/things/stage/dummy.py +++ b/src/openflexure_microscope_server/things/stage/dummy.py @@ -2,15 +2,30 @@ from __future__ import annotations +import queue +import threading import time +from collections.abc import Sequence from types import TracebackType -from typing import Any, Optional, Self +from typing import Any, Mapping, Optional, Self import labthings_fastapi as lt from . import BaseStage +class DummyStageMovement: + """A class used internally to send movements to the dummy stages movement thread.""" + + def __init__(self, displacement: Optional[Sequence[int]]) -> None: + """Initialise with a displacement. + + :param displacement: A sequence of integers or None to stop the motion. + """ + self.started = threading.Event() + self.displacement = displacement + + class DummyStage(BaseStage): """A dummy stage for testing purposes. @@ -32,12 +47,22 @@ class DummyStage(BaseStage): also doing computationally heavy tasks like simulated image blurring. """ super().__init__(thing_server_interface, **kwargs) + self._movement_enabled = False + self._move_thread: Optional[threading.Thread] = None + self._move_queue = queue.Queue[DummyStageMovement]() + self._movement_ongoing = False + self.step_time = step_time - self.instantaneous_position = self._hardware_position + self.instantaneous_position: Mapping[str, int] = self._hardware_position + self._inst_pos_lock = threading.Lock() + self._abort_move = threading.Event() def __enter__(self) -> Self: - """Register the stage position when the Thing context manager is opened.""" + """Register the stage position and start move thread running.""" self.instantaneous_position = self._hardware_position + self._movement_enabled = True + self._move_thread = threading.Thread(target=self._move_loop) + self._move_thread.start() return self def __exit__( @@ -46,49 +71,143 @@ class DummyStage(BaseStage): _exc_value: Optional[BaseException], _traceback: Optional[TracebackType], ) -> None: - """Nothing to do when the Thing context manager is closed.""" + """Stop the move thread.""" + if self._move_thread is not None and self._move_thread.is_alive(): + self._movement_enabled = False + self._move_thread.join() axis_inverted: dict[str, bool] = lt.setting( default={"x": True, "y": False, "z": False}, readonly=True ) """Used to convert coordinates between the program frame and the hardware frame.""" + def update_position(self) -> None: + """Read position from the stage and set the corresponding property.""" + pass + + def _set_pos_during_move( + self, displacement: Sequence[int], fraction_complete: float + ) -> None: + """Set the instantaneous position based on the completed fraction of an ongoing move.""" + self.instantaneous_position = { + ax: self._hardware_position[ax] + int(fraction_complete * disp) + for ax, disp in zip(self.axis_names, displacement, strict=True) + } + + def _move_loop(self) -> None: + """Run the move loop. This should be run in a thread on enter. + + This controls all movement of the dummy stage. + """ + # Start with no move request + movement_request: Optional[DummyStageMovement] = None + # Loop until the server ends. + while self._movement_enabled: + # First check for a new movement request. These will interrupt ongoing moves + movement_request = self._check_for_new_move_request() + + # If there is a new movement. + if movement_request is not None: + # Set the hardware position from instantaneous before continuing. + self._hardware_position = self.instantaneous_position + + if movement_request.displacement is None: + # If it is a stop command, stop moving + self._movement_ongoing = False + movement_request.started.set() + else: + # Record the displacement for future iterations + displacement = movement_request.displacement + # If it is a move command, set up the variables for the move + self._movement_ongoing = True + movement_request.started.set() + fraction_complete = 0.0 + dt = self.step_time + max_displacement = max(abs(v) for v in displacement) + # Always wait at least dt + total_time = max((dt * max_displacement), dt) + start_time = time.time() + time.sleep(dt) + + # If a movement is ongoing + if self._movement_ongoing: + fraction_complete = (time.time() - start_time) / total_time + if fraction_complete < 1: + self._set_pos_during_move(displacement, fraction_complete) + else: + # move is complete + fraction_complete = 1.0 + self._set_pos_during_move(displacement, fraction_complete) + self._hardware_position = self.instantaneous_position + self._movement_ongoing = False + + def _check_for_new_move_request(self) -> Optional[DummyStageMovement]: + """Check for new move request to the move_loop.""" + try: + # Timeout sets the motor speed time + timeout = self.step_time * 10 if self._movement_ongoing else 0.1 + return self._move_queue.get(timeout=timeout) + except queue.Empty: + return None + + def _hardware_start_move_relative(self, displacement: Sequence[int]) -> None: + """Start a relative move. + + This starts the stage moving, but does not wait for the move to complete. It + sets ``self.moving`` to ``True``: resetting it is the responsibility of the + calling code. + """ + with self._hardware_lock: + self.moving = True + self._move_queue.put(DummyStageMovement(displacement)) + + def _hardware_stop(self) -> None: + with self._hardware_lock: + self._move_queue.put(DummyStageMovement(None)) + self.moving = False + + def _poll_moving(self) -> bool: + """Determine if the stage is still moving.""" + moving = self._movement_ongoing + if self.moving != moving: + self.moving = moving + return moving + + def _estimate_move_duration(self, displacement: Sequence[int]) -> float: + """Calculate the expected duration of a move with the given displacement.""" + max_displacement = max(abs(d) for d in displacement) + # This does not yet check the board's speed. + return max_displacement * self.step_time + def _hardware_move_relative( self, block_cancellation: bool = False, **kwargs: int, ) -> None: """Make a relative move. Keyword arguments should be axis names.""" - displacement = [kwargs.get(k, 0) for k in self.axis_names] - self.moving = True - try: - fraction_complete = 0.0 - dt = self.step_time - max_displacement = max(abs(v) for v in displacement) - start_time = time.time() - while time.time() - start_time < dt * max_displacement: - if block_cancellation: - time.sleep(dt) - else: - 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) - for ax, disp in zip(self.axis_names, displacement, strict=True) - } - fraction_complete = 1.0 - 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, - # and to mark the invocation as "cancelled" rather than stopped. - raise e - finally: - self.moving = False - self._hardware_position = { - ax: self._hardware_position[ax] + int(fraction_complete * disp) - for ax, disp in zip(self.axis_names, displacement, strict=True) - } - self.instantaneous_position = self._hardware_position + with self._hardware_lock: + displacement = [kwargs.get(k, 0) for k in self.axis_names] + self.moving = True + move_request = DummyStageMovement(displacement) + self._move_queue.put(move_request) + + move_request.started.wait(timeout=0.1) + while self._movement_ongoing: + try: + if block_cancellation: + time.sleep(self.step_time * 10) + else: + lt.cancellable_sleep(self.step_time * 10) + + 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, and to mark the invocation as "cancelled" + # rather than stopped. + self._move_queue.put(DummyStageMovement(None)) + raise e + finally: + self.moving = False def _hardware_move_absolute( self, @@ -113,5 +232,6 @@ class DummyStage(BaseStage): It is intended for use after manually or automatically recentring the stage. """ - self._hardware_position = dict.fromkeys(self.axis_names, 0) - self.instantaneous_position = self._hardware_position + with self._hardware_lock: + self._hardware_position = dict.fromkeys(self.axis_names, 0) + self.instantaneous_position = self._hardware_position diff --git a/src/openflexure_microscope_server/things/stage/sangaboard.py b/src/openflexure_microscope_server/things/stage/sangaboard.py index ba09f8a1..128586f3 100644 --- a/src/openflexure_microscope_server/things/stage/sangaboard.py +++ b/src/openflexure_microscope_server/things/stage/sangaboard.py @@ -2,11 +2,10 @@ from __future__ import annotations -import threading -from contextlib import contextmanager +from collections.abc import Sequence from copy import copy from types import TracebackType -from typing import Any, Iterator, Literal, Optional, Self +from typing import Any, Literal, Optional, Self import semver @@ -49,16 +48,21 @@ class SangaboardThing(BaseStage): """ self.sangaboard_kwargs = copy(kwargs) self.sangaboard_kwargs["port"] = port - self._sangaboard_lock = threading.RLock() + # Set a default step time, before reading on enter. + self._step_time = 0.001 + super().__init__(thing_server_interface, **kwargs) def __enter__(self) -> Self: """Connect to the sangaboard when the Thing context manager is opened.""" self._sangaboard = sangaboard.Sangaboard(**self.sangaboard_kwargs) - with self.sangaboard() as sb: - sb.query("blocking_moves false") + with self._hardware_lock: + self._sangaboard.query("blocking_moves false") + # Read step time only on enter as there is no provision for changing speed. + self._step_time = self._sangaboard.step_time * 1e-6 self.check_firmware() self.update_position() + return self def __exit__( @@ -68,17 +72,8 @@ class SangaboardThing(BaseStage): _traceback: Optional[TracebackType], ) -> None: """Close the sangaboard connection when the Thing context manager is closed.""" - 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 + with self._hardware_lock: + self._sangaboard.close() axis_inverted: dict[str, bool] = lt.setting( default={"x": True, "y": False, "z": True}, readonly=True @@ -87,9 +82,9 @@ class SangaboardThing(BaseStage): def update_position(self) -> None: """Read position from the stage and set the corresponding property.""" - with self.sangaboard() as sb: + with self._hardware_lock: self._hardware_position = dict( - zip(self.axis_names, sb.position, strict=True) + zip(self.axis_names, self._sangaboard.position, strict=True) ) def check_firmware(self) -> None: @@ -99,11 +94,11 @@ class SangaboardThing(BaseStage): Log a warning if the version is below RECOMMENDED_VERSION """ - with self.sangaboard() as sb: + with self._hardware_lock: # This will raise a ValueError is if the firmware version cannot be parsed # this error will stop the microscope booting as we cannot ensure valid # firmware. - version = semver.Version.parse(sb.firmware_version) + version = semver.Version.parse(self._sangaboard.firmware_version) # Raise an error if version is below required if version < REQUIRED_VERSION: @@ -119,6 +114,41 @@ class SangaboardThing(BaseStage): f"{RECOMMENDED_VERSION}. Visit {WIKI_URL} for instructions on updating." ) + def _hardware_start_move_relative(self, displacement: Sequence[int]) -> None: + """Start a relative move. + + This starts the stage moving, but does not wait for the move to complete. It + sets ``self.moving`` to ``True``: resetting it is the responsibility of the calling code. + """ + with self._hardware_lock: + self.moving = True + self._sangaboard.move_rel(displacement) + + def _hardware_stop(self) -> None: + """Stop any motion of the stage as soon as possible.""" + with self._hardware_lock: + self._sangaboard.query("stop") + self.moving = False + + def _poll_moving(self) -> bool: + """Determine if the stage is still moving. + + This also sets ``moving`` if the status has changed. + + :return: whether the stage is still moving. + """ + with self._hardware_lock: + moving = self._sangaboard.query("moving?") == "true" + if self.moving != moving: + self.moving = moving + return moving + + def _estimate_move_duration(self, displacement: Sequence[int]) -> float: + """Calculate the expected duration of a move with the given displacement.""" + max_displacement = max(abs(d) for d in displacement) + # This does not yet check the board's speed. + return max_displacement * self._step_time + def _hardware_move_relative( self, block_cancellation: bool = False, @@ -126,20 +156,24 @@ class SangaboardThing(BaseStage): ) -> None: """Make a relative move in the coordinate system used by the sangaboard.""" displacement = [kwargs.get(axis, 0) for axis in self.axis_names] - with self.sangaboard() as sb: - self.moving = True + duration = self._estimate_move_duration(displacement) + with self._hardware_lock: try: - sb.move_rel(displacement) + self._sangaboard.move_rel(displacement) if block_cancellation: - sb.query("notify_on_stop") + self._sangaboard.query("notify_on_stop") else: - while sb.query("moving?") == "true": + if duration > 0.2: + lt.cancellable_sleep( + duration - 0.1 + ) # Avoid unnecessary polling + while self._sangaboard.query("moving?") == "true": 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, # and to mark the invocation as "cancelled" rather than stopped. - sb.query("stop") + self._hardware_stop() raise e finally: self.moving = False @@ -151,7 +185,7 @@ class SangaboardThing(BaseStage): **kwargs: int, ) -> None: """Make a absolute move in the coordinate system used by the sangaboard.""" - with self.sangaboard(): + with self._hardware_lock: self.update_position() displacement = { axis: int(pos) - self._hardware_position[axis] @@ -170,8 +204,8 @@ class SangaboardThing(BaseStage): It is intended for use after manually or automatically recentring the stage. """ - with self.sangaboard() as sb: - sb.zero_position() + with self._hardware_lock: + self._sangaboard.zero_position() self.update_position() def set_led( @@ -186,8 +220,8 @@ class SangaboardThing(BaseStage): being addressed. """ led_command = f"led_{led_channel}" - with self.sangaboard() as sb: - return_value = sb.query(f"{led_command}?") + with self._hardware_lock: + return_value = self._sangaboard.query(f"{led_command}?") if not return_value.startswith("CC LED:"): raise IOError("The sangaboard does not support LED control") @@ -196,6 +230,6 @@ class SangaboardThing(BaseStage): # cannot be used. if led_on: on_brightness = 0.32 - sb.query(f"{led_command} {on_brightness}") + self._sangaboard.query(f"{led_command} {on_brightness}") else: - sb.query(f"{led_command} 0") + self._sangaboard.query(f"{led_command} 0") diff --git a/tests/shared_utils/__init__.py b/tests/shared_utils/__init__.py index f0a5ab00..dda3b725 100644 --- a/tests/shared_utils/__init__.py +++ b/tests/shared_utils/__init__.py @@ -1 +1,10 @@ """Shared utilities for all test suites.""" + +import functools + + +def get_method_name(method): + """Reliably get a methods name even if wrapped.""" + while isinstance(method, functools.partial): + method = method.func + return getattr(method, "__name__", type(method).__name__) diff --git a/tests/unit_tests/test_stage.py b/tests/unit_tests/test_stage.py index 25c19fa1..727f4c4d 100644 --- a/tests/unit_tests/test_stage.py +++ b/tests/unit_tests/test_stage.py @@ -1,6 +1,9 @@ """Test the stage without creating a full HTTP server and socket connection.""" import itertools +import logging +import threading +import time import pytest from hypothesis import HealthCheck, given, settings @@ -12,10 +15,13 @@ from labthings_fastapi.testing import create_thing_without_server from openflexure_microscope_server.things.camera.simulation import SimulatedCamera from openflexure_microscope_server.things.stage import ( BaseStage, + JogCommand, + JogQueue, RedefinedBaseMovementError, ) from openflexure_microscope_server.things.stage.dummy import DummyStage +from ..shared_utils import get_method_name from ..shared_utils.lt_test_utils import LabThingsTestEnv # Keep the size and number of moves fairly small or the tests can take forever @@ -31,7 +37,27 @@ path3d = st.lists(point3d, min_size=5, max_size=10) @pytest.fixture def dummy_stage(): """Return a dummy stage with a very low step time.""" - return create_thing_without_server(DummyStage, step_time=0.000001) + return create_thing_without_server(DummyStage, step_time=0.0001) + + +def test_not_implemented_methods(): + """Check all methods a child class should implement raise NotImplemented.""" + stage = create_thing_without_server(BaseStage) + methods_and_args = [ + (stage.update_position, ()), + (stage._hardware_move_relative, ()), + (stage._hardware_move_absolute, ()), + (stage._hardware_start_move_relative, ((1, 2, 3),)), + (stage._hardware_stop, ()), + (stage._poll_moving, ()), + (stage._estimate_move_duration, ((1, 2, 3),)), + (stage.set_zero_position, ()), + ] + for method, args in methods_and_args: + method_name = get_method_name(method) + msg = f"StageThings must define their own {method_name} method" + with pytest.raises(NotImplementedError, match=msg): + method(*args) def test_override_base_movement(): @@ -180,15 +206,20 @@ def _test_move_relative(dummy_stage, axis_inverted, path): y_dir = -1 if axis_inverted["y"] else 1 z_dir = -1 if axis_inverted["z"] else 1 - position = list(dummy_stage.position.values()) - for movement in path: - dummy_stage.move_relative(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 - assert position[0] == stage_pos[0] == hw_pos["x"] * x_dir - assert position[1] == stage_pos[1] == hw_pos["y"] * y_dir - assert position[2] == stage_pos[2] == hw_pos["z"] * z_dir + # Enter the stage to start the movement thread. + with dummy_stage: + assert dummy_stage._move_thread.is_alive() + position = list(dummy_stage.position.values()) + for movement in path: + dummy_stage.move_relative(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 + assert position[0] == stage_pos[0] == hw_pos["x"] * x_dir + assert position[1] == stage_pos[1] == hw_pos["y"] * y_dir + assert position[2] == stage_pos[2] == hw_pos["z"] * z_dir @given(path=path3d) @@ -234,14 +265,18 @@ def _test_move_absolute(dummy_stage, axis_inverted, path): y_dir = -1 if axis_inverted["y"] else 1 z_dir = -1 if axis_inverted["z"] else 1 position = list(dummy_stage.position.values()) - for move_to in path: - dummy_stage.move_absolute(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 - assert position[0] == stage_pos[0] == hw_pos["x"] * x_dir - assert position[1] == stage_pos[1] == hw_pos["y"] * y_dir - assert position[2] == stage_pos[2] == hw_pos["z"] * z_dir + + # Enter the stage to start the movement thread. + with dummy_stage: + assert dummy_stage._move_thread.is_alive() + for move_to in path: + dummy_stage.move_absolute(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 + assert position[0] == stage_pos[0] == hw_pos["x"] * x_dir + assert position[1] == stage_pos[1] == hw_pos["y"] * y_dir + assert position[2] == stage_pos[2] == hw_pos["z"] * z_dir @given(path=path3d) @@ -300,3 +335,226 @@ def test_thing_description_equivalence(dummy_stage, mocker): assert sanga_actions == dummy_actions == base_actions assert sanga_properties == dummy_properties == base_properties + + +def test_jog_command_repr(): + """Test when printing a jog command the result is as expected. + + This tests the ``__repr__`` method of ``JogCommand``. + """ + # Jog should represent itself the same whether displacement is set with a list or a + # tuple. + assert str(JogCommand([1, 2, 3])) == "(1, 2, 3)" + assert str(JogCommand((1, 2, 3))) == "(1, 2, 3)" + # Stop should be very clear. + assert str(JogCommand(None)) == "STOP" + + +def test_empty_jog_sends_stop(dummy_stage, mocker, caplog): + """Check that calling ``jog`` with no args, warns then sends STOP.""" + mock_send = mocker.patch.object(dummy_stage, "_send_jog_command") + with caplog.at_level(logging.WARNING): + dummy_stage.jog() + # This should warn as stop should be sent explicitly + assert len(caplog.records) == 1 + assert mock_send.call_count == 1 + # Check it is a stop command (displacement is None) + assert mock_send.call_args.args[0].displacement is None + + +def test_jog_commands_are_sent(dummy_stage, mocker, caplog): + """Check that ``jog`` forwards commands to ``_send_jog_command``.""" + mock_send = mocker.patch.object(dummy_stage, "_send_jog_command") + with caplog.at_level(logging.INFO): + dummy_stage.jog(x=1, y=0, z=0) + dummy_stage.jog(x=0, y=2, z=0) + dummy_stage.jog(x=0, y=0, z=3) + dummy_stage.jog(stop=True) + # Normal jogging operation shouldn't be filling up the logs. + assert len(caplog.records) == 0 + # All 4 commands sent + assert mock_send.call_count == 4 + # Check commands are as expected + command_0 = mock_send.call_args_list[0].args[0] + # -1 as axis inversion is applied + assert command_0.displacement == (-1, 0, 0) + command_1 = mock_send.call_args_list[1].args[0] + assert command_1.displacement == (0, 2, 0) + command_2 = mock_send.call_args_list[2].args[0] + assert command_2.displacement == (0, 0, 3) + command_3 = mock_send.call_args_list[3].args[0] + assert command_3.displacement is None + + +def test_send_jog_commands(dummy_stage, mocker, caplog): + """Check that the jog command acts as expected.""" + + # Create a way to make mock threads. + def mock_thread_factory(*_args, **_kwargs): + """Return a mock thread instance that claims to be alive.""" + mock_instance = mocker.Mock() + mock_instance.is_alive.return_value = True + return mock_instance + + # Mock both the Queue class and threading.Thread to incercept calls. + mock_queue = mocker.patch( + "openflexure_microscope_server.things.stage.JogQueue", side_effect=mocker.Mock + ) + mock_thread = mocker.patch( + "openflexure_microscope_server.things.stage.threading.Thread", + side_effect=mock_thread_factory, + ) + + commands = [ + JogCommand([1, 1, 1]), + JogCommand([2, 2, 2]), + JogCommand([3, 3, 3]), + JogCommand([4, 4, 4]), + ] + # First call, will create a new thread and a new queue + dummy_stage._send_jog_command(commands[0]) + + # Both a new queue and a new thread are created + assert mock_queue.call_count == 1 + assert mock_thread.call_count == 1 + # The thread target is the jog loop + assert mock_thread.call_args.kwargs["target"] == dummy_stage._jog_loop + # Args are just the first command + thread_args = mock_thread.call_args.kwargs["args"] + assert len(thread_args) == 1 + assert thread_args[0] is commands[0] + # Nothing yet put in the thread + assert dummy_stage._jog_queue.put.call_count == 0 + + # Send second command: + dummy_stage._send_jog_command(commands[1]) + + # No new queue or thread created + assert mock_queue.call_count == 1 + assert mock_thread.call_count == 1 + # Put is used instead + assert dummy_stage._jog_queue.put.call_count == 1 + # Called with the second command + assert dummy_stage._jog_queue.put.call_args.args[0] is commands[1] + + # Make it so the thread has finished + dummy_stage._jog_thread.is_alive.return_value = False + assert not dummy_stage._jog_thread.is_alive() + + # call with the 3rd command, will create a new thread and a new queue + dummy_stage._send_jog_command(commands[2]) + + # Thread is alive again + assert dummy_stage._jog_thread.is_alive() + # Both a new queue and a new thread are created + assert mock_queue.call_count == 2 + assert mock_thread.call_count == 2 + # The thread target is the jog loop + assert mock_thread.call_args.kwargs["target"] == dummy_stage._jog_loop + # Args are just the first command + thread_args = mock_thread.call_args.kwargs["args"] + assert len(thread_args) == 1 + assert thread_args[0] is commands[2] + # New queue is never used + assert dummy_stage._jog_queue.put.call_count == 0 + + # Finally acquire the jog lock + with dummy_stage._jog_lock: + # and check a warning is thrown + with caplog.at_level(logging.WARNING): + dummy_stage._send_jog_command(commands[3]) + assert len(caplog.records) == 1 + # No new thread or queue created + assert mock_queue.call_count == 2 + assert mock_thread.call_count == 2 + # And still nothing added to the queue + assert dummy_stage._jog_queue.put.call_count == 0 + + +def test_get_jog_from_queue_most_recent(dummy_stage): + """Test that the jog queue gives the most recent Jog Command.""" + # Try to stack 4 moves in the queue, only 1 should be queued. + for i in range(4): + dummy_stage._jog_queue.put(JogCommand([i, i, i])) + + command = dummy_stage._get_from_jog_queue(0.001) + assert isinstance(command, JogCommand) + # Should be the last one queued + assert command.displacement == (3, 3, 3) + # Nothing else is queued + assert dummy_stage._get_from_jog_queue(0.001) is None + + +def _setup_jog_loop(command, dummy_stage, mocker): + """Set up a jog loop in a thread and return. + + :return: The thread, and the mocks for ``_hardware_start_move_relative``, + ``_hardware_stop``, and ``_poll_moving``. + """ + mocker.patch.object(dummy_stage, "_estimate_move_duration", return_value=0.01) + mock_poll = mocker.patch.object(dummy_stage, "_poll_moving", return_value=True) + + def stop_mock_move(*_args, **_kwargs): + """Set the return value of poll to false on stop.""" + mock_poll.return_value = False + + mock_move = mocker.patch.object(dummy_stage, "_hardware_start_move_relative") + mock_stop = mocker.patch.object( + dummy_stage, "_hardware_stop", side_effect=stop_mock_move + ) + + dummy_stage._jog_queue = JogQueue() + thread = threading.Thread(target=dummy_stage._jog_loop, args=(command,)) + thread.start() + return thread, mock_move, mock_stop, mock_poll + + +def test_jog_loop_jog_once_only(dummy_stage, mocker): + """Check if jogging once the jog loops breaks when the move ends. + + This checks there is no need for an explicit stop command. + """ + command = JogCommand([1, 1, 0]) + thread, mock_move, mock_stop, mock_poll = _setup_jog_loop( + command, dummy_stage, mocker + ) + time.sleep(0.05) + assert thread.is_alive() + # Move was called + assert mock_move.call_count == 1 + # Claim the motors have stopped + mock_poll.return_value = False + # Wait longer than 0.1s + time.sleep(0.15) + assert not thread.is_alive() + # Move never called again + assert mock_move.call_count == 1 + # Stop never called. + assert mock_stop.call_count == 0 + + +def test_jog_loop_jog_twice_then_stop(dummy_stage, mocker): + """Check if jogging twice then calling stop.""" + command = JogCommand([1, 0, 0]) + command2 = JogCommand([2, 0, 0]) + command3 = JogCommand(None) + thread, mock_move, mock_stop, mock_poll = _setup_jog_loop( + command, dummy_stage, mocker + ) + time.sleep(0.05) + assert thread.is_alive() + # Move was called + assert mock_move.call_count == 1 + assert mock_stop.call_count == 0 + + dummy_stage._jog_queue.put(command2) + time.sleep(0.05) + assert thread.is_alive() + assert mock_move.call_count == 2 + assert mock_stop.call_count == 0 + + dummy_stage._jog_queue.put(command3) + time.sleep(0.15) + assert not thread.is_alive() + assert mock_move.call_count == 2 + assert mock_stop.call_count == 1 diff --git a/webapp/src/App.vue b/webapp/src/App.vue index 673d4708..67fe4ebc 100644 --- a/webapp/src/App.vue +++ b/webapp/src/App.vue @@ -29,6 +29,8 @@ import loadingContent from "./components/loadingContent.vue"; import Mousetrap from "mousetrap"; import { eventBus } from "./eventBus.js"; +const move_keys = ["up", "down", "left", "right", "pageup", "pagedown"]; + Mousetrap.prototype.stopCallback = function (e, element) { // if the element has the class "mousetrap" then no need to stop if ((" " + element.className + " ").indexOf(" Mousetrap ") > -1) { @@ -65,6 +67,10 @@ export default { keyboardManual: [], systemDark: undefined, themeObserver: undefined, + keysDown: new Set(), + lastJogTime: 0, + jogDistance: 600, + jogTime: 300, }; }, @@ -130,34 +136,31 @@ export default { this.toggleModalElement(this.$refs["keyboardManualModal"]); // Calls the mixin }); - // Arrow keys Mousetrap.bind( - ["up", "down", "left", "right"], - (event) => { - this.arrowKeysDown[event.keyCode] = true; //Add key to array - this.navigateKeyHandler(); + move_keys, + (event, key) => { + event.preventDefault(); + this.keysDown.add(key); + this.updateJogFromKeys(); }, "keydown", ); + Mousetrap.bind( - ["up", "down", "left", "right"], - (event) => { - delete this.arrowKeysDown[event.keyCode]; //Remove key from array + move_keys, + (event, key) => { + event.preventDefault(); + this.keysDown.delete(key); + this.updateJogFromKeys(); }, "keyup", ); + this.keyboardManual.push({ shortcut: "←↑→↓", description: "Move the microscope stage", }); - // Focus keys - Mousetrap.bind("pageup", () => { - eventBus.emit("globalMoveStepEvent", { x: 0, y: 0, z: 1 }); - }); - Mousetrap.bind("pagedown", () => { - eventBus.emit("globalMoveStepEvent", { x: 0, y: 0, z: -1 }); - }); this.keyboardManual.push({ shortcut: "pgup / pgdn", description: "Move the microscope focus", @@ -240,47 +243,85 @@ export default { eventBus.emit("globalTogglePreview", false); }, - // Handle global mouse wheel events to be associated with navigation + /** + * Handle global mouse wheel events to be associated with navigation + */ wheelMonitor: function (event) { // Only capture scroll if the event target's parent contains the "scrollTarget" class if ( event.target.parentNode.classList.contains("scrollTarget") || event.target.classList.contains("scrollTarget") ) { - const z_steps = event.deltaY / 100; + const z_rel = event.deltaY / 100; // Emit a signal to move, acted on by panelControl.vue - eventBus.emit("globalMoveStepEvent", { - x_steps: 0, - y_steps: 0, - z_steps: z_steps, - absolute: false, - }); + const navigationStepSize = this.$store.state.navigationStepSize; + const z = z_rel * navigationStepSize.z; + // Don't use `jog() due to variable size of jogs here and the rate limiting in + // `jog()`. No need to invert on z, as navigationInvert.z isn't exposed. + this.invokeAction("stage", "jog", { x: 0, y: 0, z: z }); + eventBus.emit("globalUpdatePositionEvent"); } }, - navigateKeyHandler: function () { - // Calculate movement array - var x_rel = 0; - var y_rel = 0; - // 37 corresponds to the left key - if (37 in this.arrowKeysDown) { - x_rel = x_rel - 1; + /** + * Jog for key-presses. + * + * This is a similar to the function in stageControlButtons.vue however it uses + * uses the key repeat to fire in case a key up is missed. It debounces any + * request to jog that is too recent after the last jog. + */ + jog(x, y, z) { + // Manually debounce extra requests from keyboard repeat rate. + // This is used rather than an interval in case of missing a repeat. + const now = Date.now(); + const navigationInvert = this.$store.state.navigationInvert; + if (now - this.lastJogTime < this.jogTime) { + return; } - // 39 corresponds to the right key - if (39 in this.arrowKeysDown) { - x_rel = x_rel + 1; + this.lastJogTime = now; + + this.invokeAction("stage", "jog", { + x: x * this.jogDistance * (navigationInvert.x ? -1 : 1), + y: y * this.jogDistance * (navigationInvert.y ? -1 : 1), + z: z * this.jogDistance, + }); + eventBus.emit("globalUpdatePositionEvent"); + }, + + /** + * Stop jogging on key-up + * + * This is also similar to the function in stageControlButtons.vue. It handles + * stopping jogging and resetting the `lastJogTime` so there is no delay when + * starting a new jog after an old jog finished. + */ + jogStop() { + this.invokeAction("stage", "jog", { stop: true }); + this.lastJogTime = 0; + setTimeout(() => { + eventBus.emit("globalUpdatePositionEvent"); + }, 100); + }, + + /** + * Track which keys are still down on keypress (or key repeat). + */ + updateJogFromKeys() { + let x = 0, + y = 0, + z = 0; + if (this.keysDown.has("left")) x -= 1; + if (this.keysDown.has("right")) x += 1; + if (this.keysDown.has("up")) y += 1; + if (this.keysDown.has("down")) y -= 1; + if (this.keysDown.has("pageup")) z += 1; + if (this.keysDown.has("pagedown")) z -= 1; + + if (x || y || z) { + this.jog(x, y, z); + } else { + this.jogStop(); } - // 38 corresponds to the up key - if (38 in this.arrowKeysDown) { - y_rel = y_rel + 1; - } - // 40 corresponds to the down key - if (40 in this.arrowKeysDown) { - y_rel = y_rel - 1; - } - // Make a position request - // Emit a signal to move, acted on by panelControl.vue - eventBus.emit("globalMoveStepEvent", { x: x_rel, y: y_rel, z: 0 }); }, }, }; diff --git a/webapp/src/components/modalComponents/calibrationWizardComponents/csmSteps/focusStep.vue b/webapp/src/components/modalComponents/calibrationWizardComponents/csmSteps/focusStep.vue index e6a89c52..847a4a2b 100644 --- a/webapp/src/components/modalComponents/calibrationWizardComponents/csmSteps/focusStep.vue +++ b/webapp/src/components/modalComponents/calibrationWizardComponents/csmSteps/focusStep.vue @@ -3,58 +3,21 @@

Use the buttons below to bring the sample into focus.

You may also adjust the z position with page up and page down.

- + diff --git a/webapp/src/components/tabContentComponents/controlComponents/positionControl.vue b/webapp/src/components/tabContentComponents/controlComponents/positionControl.vue index 7c14416e..f6db90db 100644 --- a/webapp/src/components/tabContentComponents/controlComponents/positionControl.vue +++ b/webapp/src/components/tabContentComponents/controlComponents/positionControl.vue @@ -76,6 +76,7 @@ export default { return { setPosition: null, moveLock: false, + jogging: false, }; }, @@ -92,8 +93,7 @@ export default { eventBus.on("globalUpdatePositionEvent", this.updatePosition); // A global signal listener to perform a move action in pixels eventBus.on("globalMoveInImageCoordinatesEvent", this.onMoveImage); - // A global signal listener to perform a move in multiples of a step size - eventBus.on("globalMoveStepEvent", this.onMoveStep); + // Update the current position in text boxes await this.updatePosition(); }, @@ -103,7 +103,6 @@ export default { eventBus.off("globalMoveEvent", this.move); eventBus.off("globalUpdatePositionEvent", this.updatePosition); eventBus.off("globalMoveInImageCoordinatesEvent", this.onMoveImage); - eventBus.off("globalMoveStepEvent", this.onMoveStep); }, methods: { @@ -115,22 +114,12 @@ export default { this.moveInImageCoordinatesRequest(payload.x, payload.y, payload.absolute); }, - onMoveStep(payload) { - const { x: x_steps, y: y_steps, z: z_steps } = payload; - const navigationStepSize = this.$store.state.navigationStepSize; - const navigationInvert = this.$store.state.navigationInvert; - const x = x_steps * navigationStepSize.x * (navigationInvert.x ? -1 : 1); - const y = y_steps * navigationStepSize.y * (navigationInvert.y ? -1 : 1); - const z = z_steps * navigationStepSize.z * (navigationInvert.z ? -1 : 1); - const movePayload = { x, y, z, absolute: false }; - eventBus.emit("globalMoveEvent", movePayload); - }, - async move(payload) { const { x, y, z, absolute } = payload; // Move the stage, by updating the controls and starting a move task // This is equivalent to clicking the "move" button. if (this.moveLock) return; // Discard move requests if we're already moving + if (this.jogging) return; // Discard move requests if a jog is in progress // NB moveLock is just boolean flag - it's not as safe as a "proper" lock. this.moveLock = true; // This will also be set by the task submitter, but // setting it here avoids multiple moves being requested simultaneously. diff --git a/webapp/src/components/tabContentComponents/controlComponents/stageControlButtons.vue b/webapp/src/components/tabContentComponents/controlComponents/stageControlButtons.vue index 775ce234..f75a08a4 100644 --- a/webapp/src/components/tabContentComponents/controlComponents/stageControlButtons.vue +++ b/webapp/src/components/tabContentComponents/controlComponents/stageControlButtons.vue @@ -1,34 +1,75 @@