diff --git a/api/src/opentrons/hardware_control/api.py b/api/src/opentrons/hardware_control/api.py index ea4c44265c3..636ad165983 100644 --- a/api/src/opentrons/hardware_control/api.py +++ b/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, diff --git a/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py b/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py index 99a7a49d41a..152c06f66ac 100644 --- a/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py +++ b/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, diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index cdd69fc2f90..54c776d39a3 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/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() diff --git a/api/src/opentrons/hardware_control/protocols/__init__.py b/api/src/opentrons/hardware_control/protocols/__init__.py index cff17ff1d9a..1f3442ded3a 100644 --- a/api/src/opentrons/hardware_control/protocols/__init__.py +++ b/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: ... diff --git a/api/src/opentrons/hardware_control/protocols/instrument_configurer.py b/api/src/opentrons/hardware_control/protocols/instrument_configurer.py index 11e718a9aff..a4ba63c8e56 100644 --- a/api/src/opentrons/hardware_control/protocols/instrument_configurer.py +++ b/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. diff --git a/api/src/opentrons/hardware_control/protocols/liquid_handler.py b/api/src/opentrons/hardware_control/protocols/liquid_handler.py index 8baa786dc9f..8707fc33024 100644 --- a/api/src/opentrons/hardware_control/protocols/liquid_handler.py +++ b/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, diff --git a/api/src/opentrons/protocol_engine/execution/tip_handler.py b/api/src/opentrons/protocol_engine/execution/tip_handler.py index 3fed8510a43..6e496246b8f 100644 --- a/api/src/opentrons/protocol_engine/execution/tip_handler.py +++ b/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() diff --git a/api/tests/opentrons/protocol_engine/execution/test_tip_handler.py b/api/tests/opentrons/protocol_engine/execution/test_tip_handler.py index 5abeb7308b6..b80e11241e6 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_tip_handler.py +++ b/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,7 +90,6 @@ 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, @@ -99,22 +97,23 @@ async def test_flex_pick_up_tip_state( 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 + ) )