fix(api): limit calibration jogs when going upward

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)