Repository: opentrons/opentrons
Issue: Bug: Pipette moves too low when calibrating labware

Currently, robot moves too low when calibrating labware, potentially crashing into labware

## Issue:
Expected: Pipette moves approximately to top of labware
Observed: Pipette crashes into deck

## Environment
Robot: Small Smoke (commit 321538)
Run App: v3.0.0-mac-2018-02-07_20-18-v3a-3210538
Protocol: multiple, including basic logo.py
[logo.txt](https://github.com/Opentrons/opentrons/files/1709577/logo.txt)


Changed files:
api/opentrons/api/calibration.py, api/opentrons/robot/robot.py, api/tests/opentrons/protocol/test_robot.py, api/opentrons/robot/mover.py, api/opentrons/util/calibration_functions.py

diff --git a/api/opentrons/api/calibration.py b/api/opentrons/api/calibration.py
index f20f5a5b28..96451d77dc 100755
--- a/api/opentrons/api/calibration.py
+++ b/api/opentrons/api/calibration.py
@@ -138,7 +138,6 @@ class CalibrationManager:
             instrument=inst,
             save=True
         )
-        inst.robot.max_deck_height.cache_clear()
 
     # TODO (artyom, 20171003): along with session, consider extracting this
     # into abstract base class or find any other way to keep notifications
diff --git a/api/opentrons/robot/mover.py b/api/opentrons/robot/mover.py
index d462e24c20..c8ac2d3edb 100755
--- a/api/opentrons/robot/mover.py
+++ b/api/opentrons/robot/mover.py
@@ -64,31 +64,15 @@ class Mover:
         return update(pose_tree, self, Point(*defaults(dst_x, dst_y, dst_z)))
 
     def home(self, pose_tree):
-        position = self._driver.home(axis=''.join(self._axis_mapping.values()))
-        # map from driver axis names to xyz and expand position
-        # into point object
-        point = Point(
-            x=position.get(self._axis_mapping.get('x', ''), 0.0),
-            y=position.get(self._axis_mapping.get('y', ''), 0.0),
-            z=position.get(self._axis_mapping.get('z', ''), 0.0)
-        )
-
-        return update(pose_tree, self, point)
+        self._driver.home(axis=''.join(self._axis_mapping.values()))
+        return self.update_pose_from_driver(pose_tree)
 
     def fast_home(self, pose_tree, safety_margin):
-        position = self._driver.fast_home(
+        self._driver.fast_home(
             axis=''.join(self._axis_mapping.values()),
             safety_margin=safety_margin
         )
-        # map from driver axis names to xyz and expand position
-        # into point object
-        point = Point(
-            x=position.get(self._axis_mapping.get('x', ''), 0.0),
-            y=position.get(self._axis_mapping.get('y', ''), 0.0),
-            z=position.get(self._axis_mapping.get('z', ''), 0.0)
-        )
-
-        return update(pose_tree, self, point)
+        return self.update_pose_from_driver(pose_tree)
 
     def set_speed(self, value):
         self._driver.set_speed(value)
@@ -115,16 +99,19 @@ class Mover:
         assert axis in self._axis_mapping, "mapping is not set for " + axis
 
         if axis in self._axis_mapping:
-            position = self._driver.probe_axis(
-                self._axis_mapping[axis],
-                movement)
-            point = Point(
-                x=position.get(self._axis_mapping.get('x', ''), 0.0),
-                y=position.get(self._axis_mapping.get('y', ''), 0.0),
-                z=position.get(self._axis_mapping.get('z', ''), 0.0)
-            )
-
-            return update(pose_tree, self, point)
+            self._driver.probe_axis(self._axis_mapping[axis], movement)
+            return self.update_pose_from_driver(pose_tree)
 
     def delay(self, seconds):
         self._driver.delay(seconds)
+
+    def update_pose_from_driver(self, pose_tree):
+        # map from driver axis names to xyz and expand position
+        # into point object
+        point = Point(
+            x=self._driver.position.get(self._axis_mapping.get('x', ''), 0.0),
+            y=self._driver.position.get(self._axis_mapping.get('y', ''), 0.0),
+            z=self._driver.position.get(self._axis_mapping.get('z', ''), 0.0)
+        )
+
+        return update(pose_tree, self, point)
diff --git a/api/opentrons/robot/robot.py b/api/opentrons/robot/robot.py
index 9977fb0804..ca1b58b9d8 100755
--- a/api/opentrons/robot/robot.py
+++ b/api/opentrons/robot/robot.py
@@ -15,10 +15,8 @@ from opentrons.util.log import get_logger
 
 log = get_logger(__name__)
 
-# TODO (andy) this is the height the tip will travel above the deck's tallest
-# container. This should not be a single value, but should be optimized given
-# the movements context (ie sterility vs speed)
-TIP_CLEARANCE = 5
+TIP_CLEARANCE_DECK = 20    # clearance when moving between different labware
+TIP_CLEARANCE_LABWARE = 5  # clearance when staying within a single labware
 
 
 class InstrumentMosfet(object):
@@ -192,8 +190,6 @@ class Robot(object):
 
         self.INSTRUMENT_DRIVERS_CACHE = {}
 
-        self.arc_height = TIP_CLEARANCE
-
         # TODO (artyom, 09182017): once protocol development experience
         # in the light of Session concept is fully fleshed out, we need
         # to properly communicate deprecation of commands. For now we'll
@@ -526,6 +522,16 @@ class Robot(object):
         self.poses = self._actuators['left']['plunger'].home(self.poses)
         self.poses = self._actuators['right']['plunger'].home(self.poses)
 
+        # next move should not use any previously used instrument or labware
+        # to prevent robot.move_to() from using risky path optimization
+        self._previous_instrument = None
+        self._prev_container = None
+
+        # explicitly update carriage Mover positions in pose tree
+        # because their Mover.home() commands aren't used here
+        for a in self._actuators.values():
+            self.poses = a['carriage'].update_pose_from_driver(self.poses)
+
     def move_head(self, *args, **kwargs):
         self.poses = self.gantry.move(self.poses, **kwargs)
 
@@ -628,8 +634,10 @@ class Robot(object):
                 # Z arc height will be used for the new pipette
                 self._prev_container = None
 
+        self._previous_instrument = instrument
+
         if strategy == 'arc':
-            arc_coords = self._create_arc(target, placeable)
+            arc_coords = self._create_arc(instrument, target, placeable)
             for coord in arc_coords:
                 self.poses = instrument._move(
                     self.poses,
@@ -644,7 +652,7 @@ class Robot(object):
             raise RuntimeError(
                 'Unknown move strategy: {}'.format(strategy))
 
-    def _create_arc(self, destination, placeable=None):
+    def _create_arc(self, inst, destination, placeable=None):
         """
         Returns a list of coordinates to arrive to the destination coordinate
         """
@@ -656,19 +664,23 @@ class Robot(object):
         elif isinstance(placeable, containers.Container):
             this_container = placeable
 
-        arc_top = self.max_deck_height()
+        arc_top = self.max_deck_height() + TIP_CLEARANCE_DECK
 
         # movements that stay within the same container do not need to avoid
         # other containers on the deck, so the travel height of arced movements
         # can be relative to just that one container's height
         if this_container and self._prev_container == this_container:
             arc_top = self.max_placeable_height_on_deck(this_container)
+            arc_top += TIP_CLEARANCE_LABWARE
 
         self._prev_container = this_container
 
+        # if instrument is currently taller than arc_top, no need to move down
+        _, _, pip_z = pose_tracker.absolute(self.poses, inst)
+
         # TODO (andy): there is no check here for if this height will hit
         # the limit switches, so if a tall labware is used, we risk collision
-        arc_top += self.arc_height
+        arc_top = max(arc_top, destination[2], pip_z)
 
         strategy = [
             {'z': arc_top},
@@ -805,6 +817,7 @@ class Robot(object):
         if self._is_available_slot(location, share, slot, name):
             location.add(container, label or name)
         self.add_container_to_pose_tracker(location, container)
+        self.max_deck_height.cache_clear()
         return container
 
     def add_module(self, module, slot, label=None):
@@ -981,6 +994,8 @@ class Robot(object):
             *delta, save
         )
 
+        self.max_deck_height.cache_clear()
+
     @lru_cache()
     def max_deck_height(self):
         return pose_tracker.max_z(self.poses, self._deck)
diff --git a/api/opentrons/util/calibration_functions.py b/api/opentrons/util/calibration_functions.py
index 231236c1f7..50a806ff1b 100644
--- a/api/opentrons/util/calibration_functions.py
+++ b/api/opentrons/util/calibration_functions.py
@@ -162,13 +162,7 @@ def update_instrument_config(instrument, measured_center) -> (Point, float):
 
 
 def move_instrument_for_probing_prep(instrument, robot):
-    # TODO(artyom, ben 20171026): calculate from robot dimensions
-    robot.poses = instrument._move(
-        robot.poses,
-        x=191.5,
-        y=75.0,
-        z=128
-    )
+    instrument.move_to(robot.deck['5'].top(150))
 
 
 def jog_instrument(instrument, axis, robot, distance):
diff --git a/api/tests/opentrons/protocol/test_robot.py b/api/tests/opentrons/protocol/test_robot.py
index b3f405e0cc..0550b4a81e 100644
--- a/api/tests/opentrons/protocol/test_robot.py
+++ b/api/tests/opentrons/protocol/test_robot.py
@@ -93,36 +93,49 @@ def test_comment(robot):
 
 
 def test_create_arc(robot):
-    from opentrons.robot.robot import TIP_CLEARANCE
+    from opentrons.robot.robot import TIP_CLEARANCE_DECK, TIP_CLEARANCE_LABWARE
 
     p200 = pipette.Pipette(
         robot, mount='left', name='my-fancy-pancy-pipette'
     )
     plate = containers_load(robot, '96-flat', '1')
     plate2 = containers_load(robot, '96-flat', '2')
-    robot.poses = p200._move(robot.poses, x=10, y=10, z=10)
+
+    new_labware_height = 10
+    robot.poses = p200._move(robot.poses, x=10, y=10, z=new_labware_height)
     robot.calibrate_container_with_instrument(plate, p200, save=False)
 
-    res = robot._create_arc((0, 0, 0), plate[0])
-    # TODO(artyom 20171030): confirm expected values are correct after merging
-    # calibration work
+    trash_height = robot.max_placeable_height_on_deck(robot.fixed_trash)
+    assert robot.max_deck_height() == trash_height
+
+    res = robot._create_arc(p200, (0, 0, 0), plate[0])
+    arc_top = robot.max_deck_height() + TIP_CLEARANCE_DECK
     expected = [
-        {'z': robot.max_deck_height() + TIP_CLEARANCE},
+        {'z': arc_top},
         {'x': 0, 'y': 0},
         {'z': 0}
     ]
     assert res == expected
 
-    robot.poses = p200._move(robot.poses, x=10, y=10, z=100)
-    robot.calibrate_container_with_instrument(
-        plate2, p200, save=False
-    )
-    res = robot._create_arc((0, 0, 0), plate2[0])
+    res = robot._create_arc(p200, (0, 0, 0), plate[1])
+    arc_top = robot.max_placeable_height_on_deck(plate) + TIP_CLEARANCE_LABWARE
+    expected = [
+        {'z': arc_top},
+        {'x': 0, 'y': 0},
+        {'z': 0}
+    ]
+    assert res == expected
+
+    new_labware_height = 200
+    robot.poses = p200._move(robot.poses, x=10, y=10, z=new_labware_height)
+    robot.calibrate_container_with_instrument(plate2, p200, save=False)
+
+    assert robot.max_deck_height() == new_labware_height
 
-    # TODO(artyom 20171030): confirm expected values are correct after merging
-    # calibration work
+    res = robot._create_arc(p200, (0, 0, 0), plate2[0])
+    arc_top = robot.max_deck_height() + TIP_CLEARANCE_DECK
     expected = [
-        {'z': robot.max_deck_height() + TIP_CLEARANCE},
+        {'z': arc_top},
         {'x': 0, 'y': 0},
         {'z': 0}
     ]