fix(api): limit calibration jogs when going upward (#6565)

Changelog:
- better optional bounds checking for _move

Refactor the bounds checking done in _move to a new function in utils to
cut down on the size of the function a bit and add independent testability.

- allow bounds checking in move_rel

Use the new functionality to optionally check bounds in move_rel (by
passing along the checker spec to _move).

- check high bounds in jog, swallow errors

When we jog during labware calibration, it's possible to trigger hard
limits, most commonly in +z. We should catch and prevent these, since
hard limits mess up the positioning state. But we have no way to
actually display errors during jog actions, so we just swallow the error
at the jog level. So, if you jog too far, the jog just doesn't do
anything.

Closes #6562

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,

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:

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'}
 

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

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

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)

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

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)

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)