fix(api): api location cache fix

api/src/opentrons/protocols/api_support/definitions.py

@@ -1,6 +1,6 @@
 from .types import APIVersion
 
-MAX_SUPPORTED_VERSION = APIVersion(2, 9)
+MAX_SUPPORTED_VERSION = APIVersion(2, 10)
 #: The maximum supported protocol API version in this release
 
 V2_MODULE_DEF_VERSION = APIVersion(2, 3)

api/src/opentrons/protocols/context/engine/protocol_context.py

@@ -132,8 +132,15 @@ def get_rail_lights_on(self) -> bool:
     def door_closed(self) -> bool:
         raise NotImplementedError()
 
-    def get_last_location(self) -> Optional[types.Location]:
+    def get_last_location(
+        self,
+        mount: Optional[types.Mount] = None,
+    ) -> Optional[types.Location]:
         raise NotImplementedError()
 
-    def set_last_location(self, location: Optional[types.Location]) -> None:
+    def set_last_location(
+        self,
+        location: Optional[types.Location],
+        mount: Optional[types.Mount] = None,
+    ) -> None:
         raise NotImplementedError()

api/src/opentrons/protocols/context/protocol.py

@@ -160,9 +160,16 @@ def door_closed(self) -> bool:
         ...
 
     @abstractmethod
-    def get_last_location(self) -> Optional[types.Location]:
+    def get_last_location(
+        self,
+        mount: Optional[types.Mount] = None,
+    ) -> Optional[types.Location]:
         ...
 
     @abstractmethod
-    def set_last_location(self, location: Optional[types.Location]) -> None:
+    def set_last_location(
+        self,
+        location: Optional[types.Location],
+        mount: Optional[types.Mount] = None,
+    ) -> None:
         ...

api/src/opentrons/protocols/context/protocol_api/instrument_context.py

@@ -154,7 +154,17 @@ def move_to(self,
                 minimum_z_height: Optional[float],
                 speed: Optional[float]) -> None:
         """Move the instrument."""
-        last_location = self._protocol_interface.get_last_location()
+        # prevent direct movement bugs in PAPI version >= 2.10
+        location_cache_mount = (
+            self._mount
+            if self._api_version >= APIVersion(2, 10) else
+            None
+        )
+
+        last_location = self._protocol_interface.get_last_location(
+            mount=location_cache_mount
+        )
+
         if last_location:
             from_lw = last_location.labware
         else:
@@ -190,7 +200,10 @@ def move_to(self,
             self._protocol_interface.set_last_location(None)
             raise
         else:
-            self._protocol_interface.set_last_location(location)
+            self._protocol_interface.set_last_location(
+                location=location,
+                mount=location_cache_mount
+            )
 
     def get_mount(self) -> types.Mount:
         """Get the mount this pipette is attached to."""

api/src/opentrons/protocols/context/protocol_api/protocol_context.py

@@ -86,6 +86,7 @@ def __init__(self,
         self._bundled_data: Dict[str, bytes] = bundled_data or {}
         self._default_max_speeds = AxisMaxSpeeds()
         self._last_location: Optional[types.Location] = None
+        self._last_mount: Optional[types.Mount] = None
         self._loaded_modules: Set['AbstractModule'] = set()
 
     @classmethod
@@ -195,8 +196,8 @@ def load_module(
         hc_mod_instance = None
         for mod in available_modules:
             compatible = module_geometry.models_compatible(
-                    module_geometry.module_model_from_string(mod.model()),
-                    resolved_model)
+                module_geometry.module_model_from_string(mod.model()),
+                resolved_model)
             if compatible and mod not in self._loaded_modules:
                 self._loaded_modules.add(mod)
                 hc_mod_instance = SynchronousAdapter(mod)
@@ -295,10 +296,21 @@ def door_closed(self) -> bool:
         """Check if door is closed."""
         return DoorState.CLOSED == self._hw_manager.hardware.door_state
 
-    def get_last_location(self) -> Optional[types.Location]:
+    def get_last_location(
+        self,
+        mount: Optional[types.Mount] = None,
+    ) -> Optional[types.Location]:
         """Get the most recent moved to location."""
-        return self._last_location
+        if mount is None or mount == self._last_mount:
+            return self._last_location
 
-    def set_last_location(self, location: Optional[types.Location]) -> None:
+        return None
+
+    def set_last_location(
+        self,
+        location: Optional[types.Location],
+        mount: Optional[types.Mount] = None,
+    ) -> None:
         """Set the most recent moved to location."""
         self._last_location = location
+        self._last_mount = mount

api/tests/opentrons/protocol_api/test_context.py

@@ -147,6 +147,53 @@ def fake_plan_move(from_loc, to_loc, deck,
     assert test_args[0].labware.as_well() == lw.wells()[0]
 
 
+async def test_location_cache_two_pipettes(ctx, get_labware_def, hardware):
+    """It should be invalidated when next movement is a different pipette
+    than the cached location."""
+    ctx.home()
+    left = ctx.load_instrument('p10_single', Mount.LEFT)
+    right = ctx.load_instrument('p10_single', Mount.RIGHT)
+
+    left_loc = Location(point=Point(1, 2, 3), labware="1")
+    right_loc = Location(point=Point(3, 4, 5), labware="2")
+
+    with mock.patch.object(papi_geometry.planning, "plan_moves") as m:
+        # The first moves. The location cache is empty.
+        left.move_to(left_loc)
+        assert m.call_args[0][0].labware.is_empty
+        assert m.call_args[0][1] == left_loc
+        # The second move the location cache is not used because we're moving
+        # a different pipette.
+        right.move_to(right_loc)
+        assert m.call_args[0][0].labware.is_empty
+        assert m.call_args[0][1] == right_loc
+
+
+async def test_location_cache_two_pipettes_fails_pre_2_10(
+        ctx,
+        get_labware_def, hardware
+):
+    """It should reuse location cache even if cached location was set by
+    move of a different pipette."""
+    ctx.home()
+    left = ctx.load_instrument('p10_single', Mount.LEFT)
+    right = ctx.load_instrument('p10_single', Mount.RIGHT)
+    left._implementation._api_version = APIVersion(2, 9)
+    right._implementation._api_version = APIVersion(2, 9)
+
+    left_loc = Location(point=Point(1, 2, 3), labware="1")
+    right_loc = Location(point=Point(3, 4, 5), labware="2")
+
+    with mock.patch.object(papi_geometry.planning, "plan_moves") as m:
+        # The first moves. The location cache is empty.
+        left.move_to(left_loc)
+        assert m.call_args[0][0].labware.is_empty
+        assert m.call_args[0][1] == left_loc
+        right.move_to(right_loc)
+        assert m.call_args[0][0].labware == left_loc.labware
+        assert m.call_args[0][1] == right_loc
+
+
 async def test_move_uses_arc(ctx, monkeypatch, get_labware_def, hardware):
     ctx.connect(hardware)
     ctx.home()