diff --git a/api/src/opentrons/api/calibration.py b/api/src/opentrons/api/calibration.py index 5394018c091..77b94b8b227 100755 --- a/api/src/opentrons/api/calibration.py +++ b/api/src/opentrons/api/calibration.py @@ -9,6 +9,7 @@ from opentrons.types import Point, Mount, Location from opentrons.protocol_api import labware from opentrons.hardware_control import CriticalPoint, ThreadedAsyncLock +from opentrons.hardware_control.types import OutOfBoundsMove, MotionChecks from .models import Container from .util import robot_is_busy, RobotBusy @@ -236,8 +237,12 @@ def jog(self, instrument, distance, axis): instrument.name, distance, axis)) self._set_state('moving') if instrument._context: - self._hardware.move_rel( - Mount[inst.mount.upper()], Point(**{axis: distance})) + try: + self._hardware.move_rel( + Mount[inst.mount.upper()], Point(**{axis: distance}), + check_bounds=MotionChecks.HIGH) + except OutOfBoundsMove: + log.exception('Out of bounds jog') else: calibration_functions.jog_instrument( instrument=inst, diff --git a/api/src/opentrons/hardware_control/api.py b/api/src/opentrons/hardware_control/api.py index 2a8d7465ea5..21b6e590505 100644 --- a/api/src/opentrons/hardware_control/api.py +++ b/api/src/opentrons/hardware_control/api.py @@ -13,7 +13,8 @@ from opentrons.config import ( robot_configs, feature_flags as ff) -from .util import use_or_initialize_loop, DeckTransformState +from .util import ( + use_or_initialize_loop, DeckTransformState, check_motion_bounds) from .pipette import ( Pipette, generate_hardware_configs, load_from_config_and_check_skip) from .controller import Controller @@ -25,7 +26,8 @@ from .types import (Axis, HardwareAPILike, CriticalPoint, MustHomeError, NoTipAttachedError, DoorState, DoorStateNotification, PipettePair, TipAttachedError, - HardwareAction, PairedPipetteConfigValueError) + HardwareAction, PairedPipetteConfigValueError, + MotionChecks) from . import modules, robot_calibration as rb_cal if TYPE_CHECKING: @@ -913,7 +915,8 @@ async def move_to( async def move_rel(self, mount: Union[top_types.Mount, PipettePair], delta: top_types.Point, speed: float = None, - max_speeds: Dict[Axis, float] = None): + max_speeds: Dict[Axis, float] = None, + check_bounds: MotionChecks = MotionChecks.NONE): """ Move the critical point of the specified mount by a specified displacement in a specified direction, at the specified speed. 'speed' sets the speed of all axes to the given value. So, if multiple @@ -952,7 +955,8 @@ async def move_rel(self, mount: Union[top_types.Mount, PipettePair], await self._cache_and_maybe_retract_mount(primary_mount) await self._move( target_position, speed=speed, - max_speeds=max_speeds, secondary_z=secondary_z) + max_speeds=max_speeds, secondary_z=secondary_z, + check_bounds=check_bounds) async def _cache_and_maybe_retract_mount(self, mount: top_types.Mount): """ Retract the 'other' mount if necessary @@ -1036,7 +1040,8 @@ async def _move(self, target_position: 'OrderedDict[Axis, float]', speed: float = None, home_flagged_axes: bool = True, max_speeds: Dict[Axis, float] = None, acquire_lock: bool = True, - secondary_z: Axis = None): + secondary_z: Axis = None, + check_bounds: MotionChecks = MotionChecks.NONE): """ Worker function to apply robot motion. Robot motion means the kind of motions that are relevant to the robot, @@ -1070,7 +1075,6 @@ async def _move(self, target_position: 'OrderedDict[Axis, float]', target_position)) raise ValueError("Moves must specify either exactly an " "x, y, and (z or a) or none of them") - primary_transformed, secondary_transformed =\ self._get_transformed(to_transform_primary, to_transform_secondary) transformed = (*primary_transformed, secondary_transformed[2]) @@ -1080,25 +1084,14 @@ async def _move(self, target_position: 'OrderedDict[Axis, float]', # fuse the specified axes and the transformed values back together. # While we do this iteration, we’ll also check axis bounds. bounds = self._backend.axis_bounds - for idx, ax in enumerate(target_position.keys()): - if ax in Axis.gantry_axes(): - smoothie_pos[ax.name] = transformed[idx] - if smoothie_pos[ax.name] < bounds[ax.name][0]\ - or smoothie_pos[ax.name] > bounds[ax.name][1]: - deck_mins = self._deck_from_smoothie({ax: bound[0] - for ax, bound - in bounds.items()}) - deck_max = self._deck_from_smoothie({ax: bound[1] - for ax, bound - in bounds.items()}) - self._log.warning( - "Out of bounds move: {}={} (transformed: {}) not in" - "limits ({}, {}) (transformed: ({}, {})" - .format(ax.name, - target_position[ax], - smoothie_pos[ax.name], - deck_mins[ax], deck_max[ax], - bounds[ax.name][0], bounds[ax.name][1])) + to_check = {ax: transformed[idx] + for idx, ax in enumerate(target_position.keys()) + if ax in Axis.gantry_axes()} + check_motion_bounds( + to_check, + target_position, + bounds, check_bounds) + smoothie_pos.update({ax.name: pos for ax, pos in to_check.items()}) checked_maxes = max_speeds or {} str_maxes = {ax.name: val for ax, val in checked_maxes.items()} async with contextlib.AsyncExitStack() as stack: diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 599a5277856..e413f536203 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -249,9 +249,9 @@ async def connect(self, port: str = None): await self.update_fw_version() @property - def axis_bounds(self) -> Dict[str, Tuple[float, float]]: + def axis_bounds(self) -> Dict[Axis, Tuple[float, float]]: """ The (minimum, maximum) bounds for each axis. """ - return {ax: (0, pos + .05) for ax, pos + return {Axis[ax]: (0, pos) for ax, pos in self._smoothie_driver.homed_position.items() if ax not in 'BC'} diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 5a3f3b93d33..f2a4239bdc2 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -259,9 +259,9 @@ async def build_module( sim_model=sim_model) @property - def axis_bounds(self) -> Dict[str, Tuple[float, float]]: + def axis_bounds(self) -> Dict[Axis, Tuple[float, float]]: """ The (minimum, maximum) bounds for each axis. """ - return {ax: (0, pos + 0.5) for ax, pos in _HOME_POSITION.items() + return {Axis[ax]: (0, pos) for ax, pos in _HOME_POSITION.items() if ax not in 'BC'} @property diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index 48418ddd069..a7524adb964 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -12,6 +12,25 @@ MODULE_LOG = logging.getLogger(__name__) +class OutOfBoundsMove(RuntimeError): + def __init__(self, message: str): + self.message = message + super().__init__() + + def __str__(self) -> str: + return f'OutOfBoundsMove: {self.message}' + + def __repr__(self) -> str: + return f'<{str(self.__class__)}: {self.message}>' + + +class MotionChecks(enum.Enum): + NONE = 0 + LOW = 1 + HIGH = 2 + BOTH = 3 + + class Axis(enum.Enum): X = 0 Y = 1 diff --git a/api/src/opentrons/hardware_control/util.py b/api/src/opentrons/hardware_control/util.py index 9555c345692..2480b068f97 100644 --- a/api/src/opentrons/hardware_control/util.py +++ b/api/src/opentrons/hardware_control/util.py @@ -2,9 +2,9 @@ import asyncio import logging from enum import Enum -from typing import Dict, Any, Optional, List, Tuple +from typing import Dict, Any, Optional, List, Mapping, Tuple -from .types import CriticalPoint +from .types import CriticalPoint, MotionChecks, OutOfBoundsMove, Axis from opentrons.types import Point mod_log = logging.getLogger(__name__) @@ -48,3 +48,46 @@ class DeckTransformState(Enum): def __str__(self): return self.name + + +def check_motion_bounds( + target_smoothie: Mapping[Axis, float], + target_deck: Mapping[Axis, float], + bounds: Mapping[Axis, Tuple[float, float]], + checks: MotionChecks): + """ + Log or raise an error (depending on checks) if a specified + target position is outside the acceptable bounds of motion + + Which axes are checked is defined by the elements of + target_smoothie. + + The target_deck and mapping does not need to be + entirely filled; it is used only for logging + """ + bounds_message_format = ( + 'Out of bounds move: {axis}=({tsp} motor controller, {tdp} deck) too ' + '{dir} for limit {limsp}') + for ax in target_smoothie.keys(): + if target_smoothie[ax] < bounds[ax][0]: + bounds_message = bounds_message_format.format( + axis=ax, + tsp=target_smoothie[ax], + tdp=target_deck.get(ax, 'unknown'), + dir='low', + limsp=bounds.get(ax, ('unknown',))[0], + ) + mod_log.warning(bounds_message) + if checks.value & MotionChecks.LOW.value: + raise OutOfBoundsMove(bounds_message) + elif target_smoothie[ax] > bounds[ax][1]: + bounds_message = bounds_message_format.format( + axis=ax, + tsp=target_smoothie[ax], + tdp=target_deck.get(ax, 'unknown'), + dir='high', + limsp=bounds.get(ax, (None, 'unknown'))[1], + ) + mod_log.warning(bounds_message) + if checks.value & MotionChecks.HIGH.value: + raise OutOfBoundsMove(bounds_message) diff --git a/api/tests/opentrons/api/test_calibration.py b/api/tests/opentrons/api/test_calibration.py index c0bd455412e..fe0559cbd25 100755 --- a/api/tests/opentrons/api/test_calibration.py +++ b/api/tests/opentrons/api/test_calibration.py @@ -7,6 +7,7 @@ from opentrons.api import models from opentrons.types import Point, Location, Mount from opentrons.hardware_control import CriticalPoint, API +from opentrons.hardware_control.types import MotionChecks state = partial(state, 'calibration') @@ -282,7 +283,7 @@ async def test_jog_api2(main_router, model): ) expected = [ - mock.call(Mount.RIGHT, point) + mock.call(Mount.RIGHT, point, check_bounds=MotionChecks.HIGH) for point in [Point(x=1), Point(y=2), Point(z=3)]] assert jog.mock_calls == expected diff --git a/api/tests/opentrons/hardware_control/test_moves.py b/api/tests/opentrons/hardware_control/test_moves.py index 60622815cd5..c17cf616bf4 100644 --- a/api/tests/opentrons/hardware_control/test_moves.py +++ b/api/tests/opentrons/hardware_control/test_moves.py @@ -3,7 +3,8 @@ from opentrons import types from opentrons import hardware_control as hc from opentrons.config import robot_configs -from opentrons.hardware_control.types import Axis, CriticalPoint +from opentrons.hardware_control.types import ( + Axis, CriticalPoint, OutOfBoundsMove, MotionChecks) from opentrons.hardware_control.robot_calibration import ( RobotCalibration, DeckCalibration) @@ -288,62 +289,6 @@ async def test_other_mount_retracted( == types.Point(54, 20, 218) -async def catch_oob_moves(hardware_api, is_robot, toggle_new_calibration): - await hardware_api.home() - # Check axis max checking for move and move rel - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.RIGHT, types.Point(1, 0, 0)) - assert await hardware_api.gantry_position(types.Mount.RIGHT)\ - == types.Point(418, 353, 218) - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.RIGHT, types.Point(0, 1, 0)) - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.RIGHT, types.Point(0, 0, 1)) - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.RIGHT, - types.Point(419, 353, 218)) - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.RIGHT, - types.Point(418, 354, 218)) - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.RIGHT, - types.Point(418, 353, 219)) - assert await hardware_api.gantry_position(types.Mount.RIGHT)\ - == types.Point(418, 353, 218) - # Axis min checking for move and move rel - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.RIGHT, - types.Point(-1, 353, 218)) - assert await hardware_api.gantry_position(types.Mount.RIGHT)\ - == types.Point(418, 353, 218) - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.RIGHT, - types.Point(418, -1, 218)) - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.RIGHT, - types.Point(418, 353, -1)) - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.RIGHT, types.Point(-419, 0, 0)) - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.RIGHT, types.Point(0, -354, 0)) - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.RIGHT, types.Point(0, 0, -219)) - assert await hardware_api.gantry_position(types.Mount.RIGHT)\ - == types.Point(418, 353, 218) - # Make sure we are checking after mount offset and critical points - # are applied - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.LEFT, types.Point(33, 0, 0)) - with pytest.raises(RuntimeError): - await hardware_api.move_to(types.Mount.LEFT, types.Point(385, 0, 0)) - await hardware_api.move_to(types.Mount.RIGHT, types.Point(50, 50, 100)) - await hardware_api.cache_instruments({types.Mount.LEFT: 'p10_single'}) - with pytest.raises(RuntimeError): - await hardware_api.move_rel(types.Mount.LEFT, types.Point(0, 0, 12)) - await hardware_api.pick_up_tip(types.Mount.LEFT) - await hardware_api.move_rel(types.Mount.LEFT, types.Point(0, 0, 0)) - - async def test_shake_during_pick_up( hardware_api, monkeypatch, toggle_new_calibration): await hardware_api.home() @@ -436,3 +381,11 @@ async def test_shake_during_drop( mock.call(types.Mount.RIGHT, types.Point(-1, 0, 0), speed=50), mock.call(types.Mount.RIGHT, types.Point(0, 0, 20))] move_rel.assert_has_calls(move_rel_calls) + + +async def test_move_rel_bounds( + hardware_api, toggle_new_calibration): + with pytest.raises(OutOfBoundsMove): + await hardware_api.move_rel( + types.Mount.RIGHT, types.Point(0, 0, 2000), + check_bounds=MotionChecks.HIGH) diff --git a/api/tests/opentrons/hardware_control/test_util.py b/api/tests/opentrons/hardware_control/test_util.py index d56b317dd40..fbf30d6366e 100644 --- a/api/tests/opentrons/hardware_control/test_util.py +++ b/api/tests/opentrons/hardware_control/test_util.py @@ -1,9 +1,11 @@ from typing import List -from opentrons.hardware_control.util import plan_arc -from opentrons.hardware_control.types import CriticalPoint +from opentrons.hardware_control.util import plan_arc, check_motion_bounds +from opentrons.hardware_control.types import ( + CriticalPoint, MotionChecks, OutOfBoundsMove, Axis) from opentrons.types import Point + import pytest @@ -68,3 +70,30 @@ def test_cp_blending(): assert arc2[0][1] == CriticalPoint.TIP assert arc2[1][1] is None assert arc2[2][1] is None + + +@pytest.mark.parametrize( + 'xformed,deck,bounds,check,catch,phrase', + [ + # acceptable moves, iterating through checks + ({Axis.X: 2}, {Axis.X: -15}, {Axis.X: (1, 4)}, MotionChecks.NONE, False, ''), # noqa(E501) + ({Axis.X: 2}, {Axis.X: -10}, {Axis.X: (1, 4)}, MotionChecks.LOW, False, ''), # noqa(E501) + ({Axis.X: 2}, {Axis.X: 0}, {Axis.X: (1, 4)}, MotionChecks.HIGH, False, ''), # noqa(E501) + ({Axis.X: 2}, {Axis.X: 10}, {Axis.X: (1, 4)}, MotionChecks.BOTH, False, ''), # noqa(E501) + # unacceptable low, with and without checking + ({Axis.Z: 1}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.NONE, False, ''), # noqa(E501) + ({Axis.Z: 1}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.LOW, True, 'low'), # noqa(E501) + ({Axis.Z: 1}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.HIGH, False, ''), # noqa(E501) + ({Axis.Z: 1}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.BOTH, True, 'low'), # noqa(E501) + # unacceptable high, with and without checking + ({Axis.Z: 5}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.NONE, False, ''), # noqa(E501) + ({Axis.Z: 5}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.LOW, False, ''), # noqa(E501) + ({Axis.Z: 5}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.HIGH, True, 'high'), # noqa(E501) + ({Axis.Z: 5}, {Axis.Z: 3}, {Axis.Z: (2, 4)}, MotionChecks.BOTH, True, 'high') # noqa(E501) + ]) +def test_check_motion_bounds(xformed, deck, bounds, check, catch, phrase): + if catch: + with pytest.raises(OutOfBoundsMove, match=phrase): + check_motion_bounds(xformed, deck, bounds, check) + else: + check_motion_bounds(xformed, deck, bounds, check)