fix(api): Fix path planning after failed tip drop

api/src/opentrons/hardware_control/api.py

@@ -1241,10 +1241,10 @@ async def pick_up_tip(
         if prep_after:
             await self.prepare_for_aspirate(mount)
 
-    async def drop_tip(self, mount: top_types.Mount, home_after: bool = True) -> None:
-        """Drop tip at the current location."""
-
-        spec, _remove = self.plan_check_drop_tip(mount, home_after)
+    async def tip_drop_moves(
+        self, mount: top_types.Mount, home_after: bool = True
+    ) -> None:
+        spec, _ = self.plan_check_drop_tip(mount, home_after)
 
         for move in spec.drop_moves:
             self._backend.set_active_current(move.current)
@@ -1272,7 +1272,20 @@ async def drop_tip(self, mount: top_types.Mount, home_after: bool = True) -> Non
             await self.move_rel(mount, shake[0], speed=shake[1])
 
         self._backend.set_active_current(spec.ending_current)
-        _remove()
+
+    async def drop_tip(self, mount: top_types.Mount, home_after: bool = True) -> None:
+        """Drop tip at the current location."""
+        await self.tip_drop_moves(mount, home_after)
+
+        # todo(mm, 2024-10-17): Ideally, callers would be able to replicate the behavior
+        # of this method via self.drop_tip_moves() plus other public methods. This
+        # currently prevents that: there is no public equivalent for
+        # instrument.set_current_volume().
+        instrument = self.get_pipette(mount)
+        instrument.set_current_volume(0)
+
+        self.set_current_tiprack_diameter(mount, 0.0)
+        await self.remove_tip(mount)
 
     async def create_simulating_module(
         self,

api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py

@@ -929,6 +929,8 @@ def plan_check_drop_tip(
     ) -> Tuple[DropTipSpec, Callable[[], None]]:
         ...
 
+    # todo(mm, 2024-10-17): The returned _remove_tips() callable is not used by anything
+    # anymore. Delete it.
     def plan_check_drop_tip(  # type: ignore[no-untyped-def]
         self,
         mount,

api/src/opentrons/hardware_control/ot3api.py

@@ -2289,17 +2289,10 @@ def set_working_volume(
         )
         instrument.working_volume = tip_volume
 
-    async def drop_tip(
+    async def tip_drop_moves(
         self, mount: Union[top_types.Mount, OT3Mount], home_after: bool = False
     ) -> None:
-        """Drop tip at the current location."""
         realmount = OT3Mount.from_mount(mount)
-        instrument = self._pipette_handler.get_pipette(realmount)
-
-        def _remove_tips() -> None:
-            instrument.set_current_volume(0)
-            instrument.current_tiprack_diameter = 0.0
-            instrument.remove_tip()
 
         await self._move_to_plunger_bottom(realmount, rate=1.0, check_current_vol=False)
 
@@ -2326,11 +2319,27 @@ def _remove_tips() -> None:
         if home_after:
             await self._home([Axis.by_mount(mount)])
 
-        _remove_tips()
-        # call this in case we're simulating
+        # call this in case we're simulating:
         if isinstance(self._backend, OT3Simulator):
             self._backend._update_tip_state(realmount, False)
 
+    async def drop_tip(
+        self, mount: Union[top_types.Mount, OT3Mount], home_after: bool = False
+    ) -> None:
+        """Drop tip at the current location."""
+        await self.tip_drop_moves(mount=mount, home_after=home_after)
+
+        # todo(mm, 2024-10-17): Ideally, callers would be able to replicate the behavior
+        # of this method via self.drop_tip_moves() plus other public methods. This
+        # currently prevents that: there is no public equivalent for
+        # instrument.set_current_volume().
+        realmount = OT3Mount.from_mount(mount)
+        instrument = self._pipette_handler.get_pipette(realmount)
+        instrument.set_current_volume(0)
+
+        self.set_current_tiprack_diameter(mount, 0.0)
+        await self.remove_tip(mount)
+
     async def clean_up(self) -> None:
         """Get the API ready to stop cleanly."""
         await self._backend.clean_up()

api/src/opentrons/hardware_control/protocols/__init__.py

@@ -58,6 +58,8 @@ class HardwareControlInterface(
     def get_robot_type(self) -> Type[OT2RobotType]:
         return OT2RobotType
 
+    # todo(mm, 2024-10-17): This probably belongs in InstrumentConfigurer, alongside
+    # add_tip() and remove_tip().
     def cache_tip(self, mount: MountArgType, tip_length: float) -> None:
         ...
 
@@ -87,6 +89,8 @@ class FlexHardwareControlInterface(
     def get_robot_type(self) -> Type[FlexRobotType]:
         return FlexRobotType
 
+    # todo(mm, 2024-10-17): This probably belongs in InstrumentConfigurer, alongside
+    # add_tip() and remove_tip().
     def cache_tip(self, mount: MountArgType, tip_length: float) -> None:
         ...
 

api/src/opentrons/hardware_control/protocols/instrument_configurer.py

@@ -142,6 +142,9 @@ def get_instrument_max_height(
         """
         ...
 
+    # todo(mm, 2024-10-17): Can this be made non-async?
+    # todo(mm, 2024-10-17): Consider deleting this in favor of cache_tip(), which is
+    # the same except for `assert`s, if we can do so without breaking anything.
     async def add_tip(self, mount: MountArgType, tip_length: float) -> None:
         """Inform the hardware that a tip is now attached to a pipette.
 
@@ -150,6 +153,7 @@ async def add_tip(self, mount: MountArgType, tip_length: float) -> None:
         """
         ...
 
+    # todo(mm, 2024-10-17): Can this be made non-async?
     async def remove_tip(self, mount: MountArgType) -> None:
         """Inform the hardware that a tip is no longer attached to a pipette.
 

api/src/opentrons/hardware_control/protocols/liquid_handler.py

@@ -164,6 +164,11 @@ async def pick_up_tip(
         """
         ...
 
+    async def tip_drop_moves(
+        self, mount: MountArgType, home_after: bool = True
+    ) -> None:
+        ...
+
     async def drop_tip(
         self,
         mount: MountArgType,

api/src/opentrons/protocol_engine/execution/tip_handler.py

@@ -238,6 +238,7 @@ async def pick_up_tip(
         await self._hardware_api.tip_pickup_moves(
             mount=hw_mount, presses=None, increment=None
         )
+        # Allow TipNotAttachedError to propagate.
         await self.verify_tip_presence(pipette_id, TipPresenceStatus.PRESENT)
 
         self._hardware_api.cache_tip(hw_mount, actual_tip_length)
@@ -270,9 +271,14 @@ async def drop_tip(self, pipette_id: str, home_after: Optional[bool]) -> None:
         else:
             kwargs = {}
 
-        await self._hardware_api.drop_tip(mount=hw_mount, **kwargs)
+        await self._hardware_api.tip_drop_moves(mount=hw_mount, **kwargs)
+
+        # Allow TipNotAttachedError to propagate.
         await self.verify_tip_presence(pipette_id, TipPresenceStatus.ABSENT)
 
+        await self._hardware_api.remove_tip(hw_mount)
+        self._hardware_api.set_current_tiprack_diameter(hw_mount, 0)
+
     async def add_tip(self, pipette_id: str, tip: TipGeometry) -> None:
         """See documentation on abstract base class."""
         hw_mount = self._state_view.pipettes.get_mount(pipette_id).to_hw_mount()

api/tests/opentrons/protocol_engine/execution/test_tip_handler.py

@@ -1,7 +1,6 @@
 """Pipetting execution handler."""
 import pytest
-from decoy import Decoy
-from mock import AsyncMock, patch
+from decoy import Decoy, matchers
 
 from typing import Dict, ContextManager, Optional, OrderedDict
 from contextlib import nullcontext as does_not_raise
@@ -91,30 +90,30 @@ async def test_create_tip_handler(
     )
 
 
-@pytest.mark.ot3_only
 @pytest.mark.parametrize("tip_state", [TipStateType.PRESENT, TipStateType.ABSENT])
 async def test_flex_pick_up_tip_state(
     decoy: Decoy,
     mock_state_view: StateView,
     mock_labware_data_provider: LabwareDataProvider,
     tip_rack_definition: LabwareDefinition,
     tip_state: TipStateType,
+    mock_hardware_api: HardwareAPI,
 ) -> None:
     """Test the protocol engine's pick_up_tip logic."""
-    from opentrons.hardware_control.ot3api import OT3API
-
-    ot3_hardware_api = decoy.mock(cls=OT3API)
-    decoy.when(ot3_hardware_api.get_robot_type()).then_return(FlexRobotType)
-
     subject = HardwareTipHandler(
         state_view=mock_state_view,
-        hardware_api=ot3_hardware_api,
+        hardware_api=mock_hardware_api,
         labware_data_provider=mock_labware_data_provider,
     )
-    decoy.when(subject._state_view.config.robot_type).then_return("OT-3 Standard")
     decoy.when(mock_state_view.pipettes.get_mount("pipette-id")).then_return(
         MountType.LEFT
     )
+    decoy.when(mock_state_view.pipettes.get_serial_number("pipette-id")).then_return(
+        "pipette-serial"
+    )
+    decoy.when(mock_state_view.labware.get_definition("labware-id")).then_return(
+        tip_rack_definition
+    )
     decoy.when(mock_state_view.pipettes.state.nozzle_configuration_by_id).then_return(
         {"pipette-id": MOCK_MAP}
     )
@@ -134,30 +133,34 @@ async def test_flex_pick_up_tip_state(
         )
     ).then_return(42)
 
-    with patch.object(
-        ot3_hardware_api, "cache_tip", AsyncMock(spec=ot3_hardware_api.cache_tip)
-    ) as mock_add_tip:
-        if tip_state == TipStateType.PRESENT:
+    if tip_state == TipStateType.PRESENT:
+        await subject.pick_up_tip(
+            pipette_id="pipette-id",
+            labware_id="labware-id",
+            well_name="B2",
+        )
+        decoy.verify(mock_hardware_api.cache_tip(Mount.LEFT, 42), times=1)
+    else:
+        decoy.when(
+            await subject.verify_tip_presence(
+                pipette_id="pipette-id", expected=TipPresenceStatus.PRESENT
+            )
+        ).then_raise(TipNotAttachedError())
+        # if a TipNotAttchedError is caught, we should not add any tip information
+        with pytest.raises(TipNotAttachedError):
             await subject.pick_up_tip(
                 pipette_id="pipette-id",
                 labware_id="labware-id",
                 well_name="B2",
             )
-            mock_add_tip.assert_called_once()
-        else:
-            decoy.when(
-                await subject.verify_tip_presence(
-                    pipette_id="pipette-id", expected=TipPresenceStatus.PRESENT
-                )
-            ).then_raise(TipNotAttachedError())
-            # if a TipNotAttchedError is caught, we should not add any tip information
-            with pytest.raises(TipNotAttachedError):
-                await subject.pick_up_tip(
-                    pipette_id="pipette-id",
-                    labware_id="labware-id",
-                    well_name="B2",
-                )
-            mock_add_tip.assert_not_called()
+        decoy.verify(
+            mock_hardware_api.cache_tip(
+                mount=matchers.Anything(),
+                tip_length=matchers.Anything(),
+            ),
+            ignore_extra_args=True,
+            times=0,
+        )
 
 
 async def test_pick_up_tip(
@@ -228,6 +231,8 @@ async def test_pick_up_tip(
     )
 
 
+# todo(mm, 2024-10-17): Test that when verify_tip_presence raises,
+# the hardware API state is NOT updated.
 async def test_drop_tip(
     decoy: Decoy,
     mock_state_view: StateView,
@@ -251,8 +256,13 @@ async def test_drop_tip(
     await subject.drop_tip(pipette_id="pipette-id", home_after=True)
 
     decoy.verify(
-        await mock_hardware_api.drop_tip(mount=Mount.RIGHT, home_after=True),
-        times=1,
+        await mock_hardware_api.tip_drop_moves(mount=Mount.RIGHT, home_after=True)
+    )
+    decoy.verify(await mock_hardware_api.remove_tip(mount=Mount.RIGHT))
+    decoy.verify(
+        mock_hardware_api.set_current_tiprack_diameter(
+            mount=Mount.RIGHT, tiprack_diameter=0
+        )
     )