refactor(api): Check moves for oob in hardware control

Hardware control now checks that incoming gantry moves are in acceptable bounds.
That means less than the home position and > 0, for XYZA. If the move is outside
bounds, hardware_control raises a RuntimeError.

This bounds checking is applied in deck coordinates after the mount offset and
critical points are applied, which means that top level move calls may succeed
or fail based on the state of the rest of the system. For instance, with no
pipette attached to the right mount, move_to(RIGHT, Point(50, 50, 0)) will
fail (because the mount on its own cannot descend to 0); but with a P300 single
with tip attached in the right mount, that move will move the end of the tip to
the deck.

api/src/opentrons/hardware_control/__init__.py

@@ -460,10 +460,28 @@ async def _move(self, target_position: 'OrderedDict[Axis, float]',
         # Since target_position is an OrderedDict with the axes ordered by
         # (x, y, z, a, b, c), and we’ll only have one of a or z (as checked
         # by the len(to_transform) check above) we can use an enumerate to
-        # fuse the specified axes and the transformed values back together
+        # 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()})
+                    raise RuntimeError(
+                        "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]))
         try:
             self._backend.move(smoothie_pos, speed=speed)
         except Exception:

api/src/opentrons/hardware_control/controller.py

@@ -171,3 +171,10 @@ def _set_temp_speed(self, speed):
                 yield
             finally:
                 self._smoothie_driver.pop_speed()
+
+    @property
+    def axis_bounds(self) -> Dict[str, Tuple[float, float]]:
+        """ The (minimum, maximum) bounds for each axis. """
+        return {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

@@ -125,3 +125,9 @@ async def update_module(
             loop: Optional[asyncio.AbstractEventLoop])\
             -> modules.AbstractModule:
         return module
+
+    @property
+    def axis_bounds(self) -> Dict[str, Tuple[float, float]]:
+        """ The (minimum, maximum) bounds for each axis. """
+        return {ax: (0, pos) for ax, pos in _HOME_POSITION.items()
+                if ax not in 'BC'}

api/tests/opentrons/hardware_control/test_moves.py

@@ -56,12 +56,12 @@ async def test_controller_musthome(hardware_api):
 
 async def test_home_specific_sim(hardware_api, monkeypatch):
     await hardware_api.home()
-    await hardware_api.move_to(types.Mount.RIGHT, types.Point(-10, 10, 20))
+    await hardware_api.move_to(types.Mount.RIGHT, types.Point(0, 10, 20))
     # Avoid the autoretract when moving two difference instruments
     hardware_api._last_moved_mount = None
     await hardware_api.move_rel(types.Mount.LEFT, types.Point(0, 0, -20))
     await hardware_api.home([Axis.Z, Axis.C])
-    assert hardware_api._current_position == {Axis.X: -10,
+    assert hardware_api._current_position == {Axis.X: 0,
                                               Axis.Y: 10,
                                               Axis.Z: 218,
                                               Axis.A: 20,
@@ -71,9 +71,9 @@ async def test_home_specific_sim(hardware_api, monkeypatch):
 
 async def test_retract(hardware_api):
     await hardware_api.home()
-    await hardware_api.move_to(types.Mount.RIGHT, types.Point(-10, 10, 20))
+    await hardware_api.move_to(types.Mount.RIGHT, types.Point(0, 10, 20))
     await hardware_api.retract(types.Mount.RIGHT, 10)
-    assert hardware_api._current_position == {Axis.X: -10,
+    assert hardware_api._current_position == {Axis.X: 0,
                                               Axis.Y: 10,
                                               Axis.Z: 218,
                                               Axis.A: 218,
@@ -97,11 +97,11 @@ async def test_move(hardware_api):
     # This assert implicitly checks that the mount offset is not applied to
     # relative moves; if you change this to move_to, the offset will be
     # applied again
-    rel_position = types.Point(30, 20, 10)
+    rel_position = types.Point(30, 20, -10)
     mount2 = types.Mount.LEFT
     target_position2 = {Axis.X: 60,
                         Axis.Y: 40,
-                        Axis.Z: 228,
+                        Axis.Z: 208,
                         Axis.A: 218,  # The other instrument is retracted
                         Axis.B: 19,
                         Axis.C: 19}
@@ -157,10 +157,15 @@ async def test_critical_point_applied(hardware_api, monkeypatch):
     target[Axis.A] = 0
     assert hardware_api.current_position(types.Mount.RIGHT) == target
     # And removing the tip should move us back to the original
+    await hardware_api.move_rel(types.Mount.RIGHT, types.Point(2.5, 0, 0))
     await hardware_api.drop_tip(types.Mount.RIGHT)
     target[Axis.A] = 33 + hc.DROP_TIP_RELEASE_DISTANCE
+    target_no_offset[Axis.X] = 2.5
+    target[Axis.X] = 2.5
     assert hardware_api.current_position(types.Mount.RIGHT) == target
     await hardware_api.move_to(types.Mount.RIGHT, types.Point(0, 0, 0))
+    target[Axis.X] = 0
+    target_no_offset[Axis.X] = 0
     target_no_offset[Axis.A] = 13
     target[Axis.A] = 0
     assert hardware_api._current_position == target_no_offset
@@ -203,3 +208,59 @@ async def test_other_mount_retracted(hardware_api):
     await hardware_api.move_to(types.Mount.LEFT, types.Point(20, 20, 0))
     assert hardware_api.gantry_position(types.Mount.RIGHT) \
         == types.Point(54, 20, 218)
+
+
+async def catch_oob_moves(hardware_api):
+    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 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 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 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 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))