From f0913d7219ab41b55b3980a1262a884007223ad6 Mon Sep 17 00:00:00 2001 From: Sanniti Pimpley Date: Tue, 2 Aug 2022 17:39:14 -0400 Subject: [PATCH] feat(papiv2): retract pipettes before shaking or opening latch (#11268) Closes #11210, closes #11209 Co-authored-by: Mike Cousins --- .../opentrons/protocol_api/module_contexts.py | 46 ++++++++-- .../heater_shaker/open_labware_latch.py | 18 ++-- .../set_and_wait_for_shake_speed.py | 18 ++-- .../protocol_engine/state/pipettes.py | 33 ++++++- .../protocols/geometry/module_geometry.py | 63 +++++++++++++ .../protocol_api/test_module_context.py | 69 +++++++++++--- .../heater_shaker/test_open_labware_latch.py | 2 +- .../test_set_and_wait_for_shake_speed.py | 2 +- .../state/test_module_store.py | 4 +- .../state/test_pipette_store.py | 89 ++++++++++++++++++ .../geometry/test_module_geometry.py | 90 +++++++++++++++++-- 11 files changed, 397 insertions(+), 37 deletions(-) diff --git a/api/src/opentrons/protocol_api/module_contexts.py b/api/src/opentrons/protocol_api/module_contexts.py index 3db3253f5c3..b8374c5ff23 100644 --- a/api/src/opentrons/protocol_api/module_contexts.py +++ b/api/src/opentrons/protocol_api/module_contexts.py @@ -925,12 +925,16 @@ def set_and_wait_for_shake_speed(self, rpm: int) -> None: Set the heater shaker's target speed and wait until the specified speed has reached. Delays protocol execution until the target speed has been achieved. + + NOTE: Before shaking, this command will retract the pipettes up if they are + parked adjacent to the heater-shaker. """ if ( self._module.labware_latch_status == HeaterShakerLabwareLatchStatus.IDLE_CLOSED ): validated_speed = validate_heater_shaker_speed(rpm=rpm) + self._prepare_for_shake() self._module.set_speed(rpm=validated_speed) else: # TODO: Figure out whether to issue close latch behind the scenes instead @@ -943,11 +947,13 @@ def set_and_wait_for_shake_speed(self, rpm: int) -> None: def open_labware_latch(self) -> None: """Open the Heater-Shaker's labware latch. - Note that the labware latch needs to be closed before: - - * Shaking - * Pipetting to or from the labware on the Heater-Shaker - * Pipetting to or from labware to the left or right of the Heater-Shaker + NOTE: + 1. This command will retract the pipettes up if they are parked east or west + of the Heater-Shaker. + 2. The labware latch needs to be closed before: + * Shaking + * Pipetting to or from the labware on the Heater-Shaker + * Pipetting to or from labware to the left or right of the Heater-Shaker Raises an error when attempting to open the latch while the Heater-Shaker is shaking. """ @@ -956,6 +962,7 @@ def open_labware_latch(self) -> None: raise CannotPerformModuleAction( """Cannot open labware latch while module is shaking.""" ) + self._prepare_for_latch_open() self._module.open_labware_latch() # TODO: add API version requirement @@ -1019,3 +1026,32 @@ def flag_unsafe_move( is_plate_shaking=is_plate_shaking, is_labware_latch_closed=is_labware_latch_closed, ) + + def _prepare_for_shake(self) -> None: + """ + Before shaking, retracts pipettes if they're parked over a slot + adjacent to the heater-shaker. + """ + + if cast(HeaterShakerGeometry, self.geometry).is_pipette_blocking_shake_movement( + pipette_location=self._ctx.location_cache + ): + ctx_implementation = self._ctx._implementation + hardware = ctx_implementation.get_hardware() + for mount in types.Mount: + hardware.retract(mount=mount) + self._ctx.location_cache = None + + def _prepare_for_latch_open(self) -> None: + """ + Before opening latch, retracts pipettes if they're parked over a slot + east/ west of the heater-shaker. + """ + if cast(HeaterShakerGeometry, self.geometry).is_pipette_blocking_latch_movement( + pipette_location=self._ctx.location_cache + ): + ctx_implementation = self._ctx._implementation + hardware = ctx_implementation.get_hardware() + for mount in types.Mount: + hardware.retract(mount=mount) + self._ctx.location_cache = None diff --git a/api/src/opentrons/protocol_engine/commands/heater_shaker/open_labware_latch.py b/api/src/opentrons/protocol_engine/commands/heater_shaker/open_labware_latch.py index b79d03978d6..7272841fbc9 100644 --- a/api/src/opentrons/protocol_engine/commands/heater_shaker/open_labware_latch.py +++ b/api/src/opentrons/protocol_engine/commands/heater_shaker/open_labware_latch.py @@ -25,6 +25,11 @@ class OpenLabwareLatchParams(BaseModel): class OpenLabwareLatchResult(BaseModel): """Result data from opening a Heater-Shaker's labware latch.""" + pipetteRetracted: bool = Field( + ..., + description="Whether the pipette was retracted/ homed before starting shake.", + ) + class OpenLabwareLatchImpl( AbstractCommandImpl[OpenLabwareLatchParams, OpenLabwareLatchResult] @@ -51,10 +56,13 @@ async def execute(self, params: OpenLabwareLatchParams) -> OpenLabwareLatchResul hs_module_substate.raise_if_shaking() - # Move pipette away if it is close to the heater-shaker - if self._state_view.motion.check_pipette_blocking_hs_latch( - hs_module_substate.module_id - ): + pipette_should_retract = ( + self._state_view.motion.check_pipette_blocking_hs_latch( + hs_module_substate.module_id + ) + ) + if pipette_should_retract: + # Move pipette away if it is close to the heater-shaker # TODO(jbl 2022-07-28) replace home movement with a retract movement await self._movement.home( [ @@ -71,7 +79,7 @@ async def execute(self, params: OpenLabwareLatchParams) -> OpenLabwareLatchResul if hs_hardware_module is not None: await hs_hardware_module.open_labware_latch() - return OpenLabwareLatchResult() + return OpenLabwareLatchResult(pipetteRetracted=pipette_should_retract) class OpenLabwareLatch(BaseCommand[OpenLabwareLatchParams, OpenLabwareLatchResult]): diff --git a/api/src/opentrons/protocol_engine/commands/heater_shaker/set_and_wait_for_shake_speed.py b/api/src/opentrons/protocol_engine/commands/heater_shaker/set_and_wait_for_shake_speed.py index 181784b5bd3..05f46318c8b 100644 --- a/api/src/opentrons/protocol_engine/commands/heater_shaker/set_and_wait_for_shake_speed.py +++ b/api/src/opentrons/protocol_engine/commands/heater_shaker/set_and_wait_for_shake_speed.py @@ -28,6 +28,11 @@ class SetAndWaitForShakeSpeedParams(BaseModel): class SetAndWaitForShakeSpeedResult(BaseModel): """Result data from setting and waiting for a Heater-Shaker's shake speed.""" + pipetteRetracted: bool = Field( + ..., + description="Whether the pipette was retracted/ homed before starting shake.", + ) + class SetAndWaitForShakeSpeedImpl( AbstractCommandImpl[SetAndWaitForShakeSpeedParams, SetAndWaitForShakeSpeedResult] @@ -60,10 +65,13 @@ async def execute( # Verify speed from hs module view validated_speed = hs_module_substate.validate_target_speed(params.rpm) - # Move pipette away if it is close to the heater-shaker - if self._state_view.motion.check_pipette_blocking_hs_shaker( - hs_module_substate.module_id - ): + pipette_should_retract = ( + self._state_view.motion.check_pipette_blocking_hs_shaker( + hs_module_substate.module_id + ) + ) + if pipette_should_retract: + # Move pipette away if it is close to the heater-shaker # TODO(jbl 2022-07-28) replace home movement with a retract movement await self._movement.home( [ @@ -80,7 +88,7 @@ async def execute( if hs_hardware_module is not None: await hs_hardware_module.set_speed(rpm=validated_speed) - return SetAndWaitForShakeSpeedResult() + return SetAndWaitForShakeSpeedResult(pipetteRetracted=pipette_should_retract) class SetAndWaitForShakeSpeed( diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 46f3a4687ab..e99a96b96d3 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -1,7 +1,7 @@ """Basic pipette data state and store.""" from __future__ import annotations from dataclasses import dataclass -from typing import Dict, List, Mapping, Optional +from typing import Dict, List, Mapping, Optional, Union from opentrons.hardware_control.dev_types import PipetteDict from opentrons.types import MountType, Mount as HwMount @@ -22,6 +22,7 @@ BlowOutResult, TouchTipResult, thermocycler, + heater_shaker, ) from ..actions import Action, UpdateCommandAction from .abstract_store import HasState, HandlesActions @@ -100,12 +101,14 @@ def _handle_command(self, command: Command) -> None: MoveToCoordinatesResult, thermocycler.OpenLidResult, thermocycler.CloseLidResult, + heater_shaker.SetAndWaitForShakeSpeedResult, + heater_shaker.OpenLabwareLatchResult, ), ): # A command left the pipette in a place that we can't associate # with a logical well location. Set the current well to None # to reflect the fact that it's now unknown. - self._state.current_well = None + self._handle_current_well_clearing_commands(command_result=command.result) if isinstance(command.result, LoadPipetteResult): pipette_id = command.result.pipetteId @@ -146,6 +149,32 @@ def _handle_command(self, command: Command) -> None: pipette_id = command.params.pipetteId self._state.aspirated_volume_by_id[pipette_id] = 0 + def _handle_current_well_clearing_commands( + self, + command_result: Union[ + HomeResult, + MoveToCoordinatesResult, + thermocycler.OpenLidResult, + thermocycler.CloseLidResult, + heater_shaker.SetAndWaitForShakeSpeedResult, + heater_shaker.OpenLabwareLatchResult, + ], + ) -> None: + if ( + not isinstance( + command_result, + ( + heater_shaker.SetAndWaitForShakeSpeedResult, + heater_shaker.OpenLabwareLatchResult, + ), + ) + or command_result.pipetteRetracted + ): + # Clear current_well for all above commands except h/s commands. + # For h/s commands, clear current_well only if pipettes were moved before + # command execution for safety. + self._state.current_well = None + class PipetteView(HasState[PipetteState]): """Read-only view of computed pipettes state.""" diff --git a/api/src/opentrons/protocols/geometry/module_geometry.py b/api/src/opentrons/protocols/geometry/module_geometry.py index 4eeb3367033..2263a37ee4b 100644 --- a/api/src/opentrons/protocols/geometry/module_geometry.py +++ b/api/src/opentrons/protocols/geometry/module_geometry.py @@ -23,6 +23,7 @@ from opentrons.motion_planning.adjacent_slots_getters import ( get_north_south_slots, get_east_west_slots, + get_adjacent_slots, ) from opentrons.hardware_control.modules.types import ( @@ -436,6 +437,68 @@ def flag_unsafe_move( "Cannot move multi-channel pipette to non-tip rack labware north or south of Heater-Shaker" ) + def is_pipette_blocking_shake_movement( + self, pipette_location: Optional[Location] + ) -> bool: + """Check whether pipette is parked adjacent to heater-shaker. + + Returns True if pipette's last known location was on east/west/north/south of or + on the heater-shaker. Also returns True if last location is not known or is + not associated with a slot. + """ + if pipette_location is None: + # If we don't know the pipette's latest location then let's be extra + # cautious and call it blocking + return True + + pipette_location_slot = pipette_location.labware.first_parent() + if pipette_location_slot is None: + # If a location is not associated w/ a slot (e.g., if it has labware=None) + # then we don't know if it's close to the h/s, so, we will be cautious + # and call it blocking + return True + + heater_shaker_slot = self.parent + + assert isinstance( + heater_shaker_slot, str + ), "Could not determine module slot location" + + return heater_shaker_slot == pipette_location_slot or int( + pipette_location_slot + ) in get_adjacent_slots(int(heater_shaker_slot)) + + def is_pipette_blocking_latch_movement( + self, pipette_location: Optional[Location] + ) -> bool: + """Check whether pipette is parked east or west of heater-shaker. + + Returns True is pipette's last known location was on east/west of or on the + heater-shaker. Also returns True if last location is not known or is not + associated with a slot. + """ + if pipette_location is None: + # If we don't know the pipette's latest location then let's be extra + # cautious and call it blocking + return True + + pipette_location_slot = pipette_location.labware.first_parent() + if pipette_location_slot is None: + # If a location is not associated w/ a slot (e.g., if it has labware=None) + # then we don't know if it's close to the h/s, so, we will be cautious + # and call it blocking + return True + + heater_shaker_slot = self.parent + + assert isinstance( + heater_shaker_slot, str + ), "Could not determine module slot location" + + return heater_shaker_slot == pipette_location_slot or int( + pipette_location_slot + ) in get_east_west_slots(int(heater_shaker_slot)) + def _load_from_v1( definition: "ModuleDefinitionV1", parent: Location, api_level: APIVersion diff --git a/api/tests/opentrons/protocol_api/test_module_context.py b/api/tests/opentrons/protocol_api/test_module_context.py index 17ce450a531..9d21e9e52d9 100644 --- a/api/tests/opentrons/protocol_api/test_module_context.py +++ b/api/tests/opentrons/protocol_api/test_module_context.py @@ -7,7 +7,7 @@ import opentrons.protocol_api as papi import opentrons.protocols.geometry as papi_geometry -from opentrons.types import Point, Location +from opentrons.types import Point, Location, Mount from opentrons.drivers.types import HeaterShakerLabwareLatchStatus from opentrons.hardware_control.modules.magdeck import OFFSET_TO_LABWARE_BOTTOM from opentrons.hardware_control.modules.types import ( @@ -47,6 +47,12 @@ def mock_module_controller() -> mock.MagicMock: return mock.MagicMock() +@pytest.fixture +def mock_pipette_location() -> mock.MagicMock: + # mock_labware = papi.labware.Labware() + return mock.MagicMock(return_value=Location(point=Point(1, 2, 3), labware=None)) + + # Async because ProtocolContext.__init__() needs an event loop, # so this fixture needs to run in an event loop. @pytest.fixture @@ -120,6 +126,7 @@ def find_modules(resolved_model: ModuleModel, resolved_type: ModuleType): async def ctx_with_heater_shaker( mock_hardware: mock.AsyncMock, mock_module_controller: mock.MagicMock, + mock_pipette_location: mock.MagicMock, enable_heater_shaker_python_api: AsyncGenerator[None, None], ) -> ProtocolContext: """Context fixture with a mock heater-shaker.""" @@ -134,9 +141,11 @@ def find_modules(resolved_model: ModuleModel, resolved_type: ModuleType): return [] mock_hardware.find_modules.side_effect = find_modules - return ProtocolContext( + ctx = ProtocolContext( implementation=ProtocolContextImplementation(sync_hardware=mock_hardware), ) + ctx.location_cache = mock_pipette_location + return ctx # ______ load_module tests _______ @@ -677,8 +686,9 @@ def test_heater_shaker_temperature_properties( hs_mod = ctx_with_heater_shaker.load_module("heaterShakerModuleV1", 1) - assert hs_mod.current_temperature == 123.45 # type: ignore[union-attr] - assert hs_mod.target_temperature == 234.56 # type: ignore[union-attr] + assert isinstance(hs_mod, HeaterShakerContext) + assert hs_mod.current_temperature == 123.45 + assert hs_mod.target_temperature == 234.56 def test_heater_shaker_speed_properties( @@ -693,8 +703,9 @@ def test_heater_shaker_speed_properties( hs_mod = ctx_with_heater_shaker.load_module("heaterShakerModuleV1", 1) - assert hs_mod.current_speed == 12 # type: ignore[union-attr] - assert hs_mod.target_speed == 34 # type: ignore[union-attr] + assert isinstance(hs_mod, HeaterShakerContext) + assert hs_mod.current_speed == 12 + assert hs_mod.target_speed == 34 def test_heater_shaker_temp_and_speed_status( @@ -708,8 +719,9 @@ def test_heater_shaker_temp_and_speed_status( type(mock_module_controller).speed_status = mock_speed_status hs_mod = ctx_with_heater_shaker.load_module("heaterShakerModuleV1", 1) - assert hs_mod.temperature_status == "holding at target" # type: ignore[union-attr] - assert hs_mod.speed_status == "slowing down" # type: ignore[union-attr] + assert isinstance(hs_mod, HeaterShakerContext) + assert hs_mod.temperature_status == "holding at target" + assert hs_mod.speed_status == "slowing down" def test_heater_shaker_latch_status( @@ -728,8 +740,11 @@ def test_heater_shaker_latch_status( def test_heater_shaker_set_and_wait_for_shake_speed( ctx_with_heater_shaker: ProtocolContext, mock_module_controller: mock.MagicMock, + mock_pipette_location: mock.MagicMock, + mock_hardware: mock.AsyncMock, ) -> None: """It should issue a blocking set target shake speed.""" + # Mock setup mock_latch_status = mock.PropertyMock( return_value=HeaterShakerLabwareLatchStatus.IDLE_CLOSED ) @@ -740,9 +755,23 @@ def test_heater_shaker_set_and_wait_for_shake_speed( ) as mock_validator: mock_validator.return_value = 10 hs_mod = ctx_with_heater_shaker.load_module("heaterShakerModuleV1", 1) - hs_mod.set_and_wait_for_shake_speed(rpm=400) # type: ignore[union-attr] + assert isinstance(hs_mod, HeaterShakerContext) + hs_mod.geometry.is_pipette_blocking_shake_movement = mock.MagicMock( # type: ignore[attr-defined] + return_value=True + ) + + # Call subject method + hs_mod.set_and_wait_for_shake_speed(rpm=400) + + # Assert expected calls mock_validator.assert_called_once_with(rpm=400) + hs_mod.geometry.is_pipette_blocking_shake_movement.assert_called_with( # type: ignore[attr-defined] + pipette_location=mock_pipette_location + ) + calls = [mock.call(mount=Mount.LEFT), mock.call(mount=Mount.RIGHT)] + mock_hardware.retract.assert_has_calls(calls, any_order=True) mock_module_controller.set_speed.assert_called_once_with(rpm=10) + assert ctx_with_heater_shaker.location_cache is None @pytest.mark.parametrize( @@ -770,15 +799,33 @@ def test_heater_shaker_set_and_wait_for_shake_speed_raises( def test_heater_shaker_open_labware_latch( - ctx_with_heater_shaker: ProtocolContext, mock_module_controller: mock.MagicMock + ctx_with_heater_shaker: ProtocolContext, + mock_module_controller: mock.MagicMock, + mock_pipette_location: mock.MagicMock, + mock_hardware: mock.MagicMock, ) -> None: """It should issue a labware latch open command.""" + # Mocks mock_speed_status = mock.PropertyMock(return_value=SpeedStatus.IDLE) type(mock_module_controller).speed_status = mock_speed_status + # Get subject hs_mod = ctx_with_heater_shaker.load_module("heaterShakerModuleV1", 1) - hs_mod.open_labware_latch() # type: ignore[union-attr] + assert isinstance(hs_mod, HeaterShakerContext) + hs_mod.geometry.is_pipette_blocking_latch_movement = mock.MagicMock( # type: ignore[attr-defined] + return_value=True + ) + + # Call subject method + hs_mod.open_labware_latch() + # Assert calls + hs_mod.geometry.is_pipette_blocking_latch_movement.assert_called_with( # type: ignore[attr-defined] + pipette_location=mock_pipette_location + ) + calls = [mock.call(mount=Mount.LEFT), mock.call(mount=Mount.RIGHT)] + mock_hardware.retract.assert_has_calls(calls, any_order=True) mock_module_controller.open_labware_latch.assert_called_once() + assert ctx_with_heater_shaker.location_cache is None @pytest.mark.parametrize( diff --git a/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_open_labware_latch.py b/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_open_labware_latch.py index 0c9d024b9fd..5e44cfe626d 100644 --- a/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_open_labware_latch.py +++ b/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_open_labware_latch.py @@ -58,4 +58,4 @@ async def test_open_labware_latch( await movement.home([MotorAxis.RIGHT_Z, MotorAxis.LEFT_Z]), await hs_hardware.open_labware_latch(), ) - assert result == heater_shaker.OpenLabwareLatchResult() + assert result == heater_shaker.OpenLabwareLatchResult(pipetteRetracted=True) diff --git a/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_set_and_wait_for_shake_speed.py b/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_set_and_wait_for_shake_speed.py index 1b5301222b4..4c186e6682c 100644 --- a/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_set_and_wait_for_shake_speed.py +++ b/api/tests/opentrons/protocol_engine/commands/heater_shaker/test_set_and_wait_for_shake_speed.py @@ -64,4 +64,4 @@ async def test_set_and_wait_for_shake_speed( await movement.home([MotorAxis.RIGHT_Z, MotorAxis.LEFT_Z]), await hs_hardware.set_speed(rpm=1234), ) - assert result == heater_shaker.SetAndWaitForShakeSpeedResult() + assert result == heater_shaker.SetAndWaitForShakeSpeedResult(pipetteRetracted=True) diff --git a/api/tests/opentrons/protocol_engine/state/test_module_store.py b/api/tests/opentrons/protocol_engine/state/test_module_store.py index 483a046529a..daae7bec8ca 100644 --- a/api/tests/opentrons/protocol_engine/state/test_module_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_module_store.py @@ -269,7 +269,7 @@ def test_handle_hs_shake_commands(heater_shaker_v1_def: ModuleDefinition) -> Non ) set_shake_cmd = hs_commands.SetAndWaitForShakeSpeed.construct( # type: ignore[call-arg] params=hs_commands.SetAndWaitForShakeSpeedParams(moduleId="module-id", rpm=111), - result=hs_commands.SetAndWaitForShakeSpeedResult(), + result=hs_commands.SetAndWaitForShakeSpeedResult(pipetteRetracted=False), ) deactivate_cmd = hs_commands.DeactivateShaker.construct( # type: ignore[call-arg] params=hs_commands.DeactivateShakerParams(moduleId="module-id"), @@ -320,7 +320,7 @@ def test_handle_hs_labware_latch_commands( ) open_latch_cmd = hs_commands.OpenLabwareLatch.construct( # type: ignore[call-arg] params=hs_commands.OpenLabwareLatchParams(moduleId="module-id"), - result=hs_commands.OpenLabwareLatchResult(), + result=hs_commands.OpenLabwareLatchResult(pipetteRetracted=False), ) subject = ModuleStore() diff --git a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py index b15d1c91a07..e09fa1ca0db 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -261,6 +261,43 @@ def test_movement_commands_update_current_well( ), result=cmd.MoveToCoordinatesResult(), ), + cmd.thermocycler.OpenLid( + id="command-id-2", + key="command-key-2", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=cmd.thermocycler.OpenLidParams(moduleId="xyz"), + result=cmd.thermocycler.OpenLidResult(), + ), + cmd.thermocycler.CloseLid( + id="command-id-2", + key="command-key-2", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=cmd.thermocycler.CloseLidParams(moduleId="xyz"), + result=cmd.thermocycler.CloseLidResult(), + ), + cmd.heater_shaker.SetAndWaitForShakeSpeed( + id="command-id-2", + key="command-key-2", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=cmd.heater_shaker.SetAndWaitForShakeSpeedParams( + moduleId="xyz", + rpm=123, + ), + result=cmd.heater_shaker.SetAndWaitForShakeSpeedResult( + pipetteRetracted=True + ), + ), + cmd.heater_shaker.OpenLabwareLatch( + id="command-id-2", + key="command-key-2", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=cmd.heater_shaker.OpenLabwareLatchParams(moduleId="xyz"), + result=cmd.heater_shaker.OpenLabwareLatchResult(pipetteRetracted=True), + ), ], ) def test_movement_commands_without_well_clear_current_well( @@ -285,6 +322,58 @@ def test_movement_commands_without_well_clear_current_well( assert subject.state.current_well is None +@pytest.mark.parametrize( + "command", + [ + cmd.heater_shaker.SetAndWaitForShakeSpeed( + id="command-id-2", + key="command-key-2", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=cmd.heater_shaker.SetAndWaitForShakeSpeedParams( + moduleId="xyz", + rpm=123, + ), + result=cmd.heater_shaker.SetAndWaitForShakeSpeedResult( + pipetteRetracted=False + ), + ), + cmd.heater_shaker.OpenLabwareLatch( + id="command-id-2", + key="command-key-2", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + params=cmd.heater_shaker.OpenLabwareLatchParams(moduleId="xyz"), + result=cmd.heater_shaker.OpenLabwareLatchResult(pipetteRetracted=False), + ), + ], +) +def test_heater_shaker_command_without_movement( + subject: PipetteStore, command: cmd.Command +) -> None: + """Heater Shaker commands that dont't move pipettes shouldn't clear current_well.""" + load_pipette_command = create_load_pipette_command( + pipette_id="pipette-id", + pipette_name=PipetteName.P300_SINGLE, + mount=MountType.LEFT, + ) + move_command = create_move_to_well_command( + pipette_id="pipette-id", + labware_id="labware-id", + well_name="well-name", + ) + + subject.handle_action(UpdateCommandAction(command=load_pipette_command)) + subject.handle_action(UpdateCommandAction(command=move_command)) + subject.handle_action(UpdateCommandAction(command=command)) + + assert subject.state.current_well == CurrentWell( + pipette_id="pipette-id", + labware_id="labware-id", + well_name="well-name", + ) + + def test_tip_commands_update_has_tip(subject: PipetteStore) -> None: """It should update has_tip after a successful pickUpTip command.""" pipette_id = "pipette-id" diff --git a/api/tests/opentrons/protocols/geometry/test_module_geometry.py b/api/tests/opentrons/protocols/geometry/test_module_geometry.py index 7643ef48017..be3c7eacce7 100644 --- a/api/tests/opentrons/protocols/geometry/test_module_geometry.py +++ b/api/tests/opentrons/protocols/geometry/test_module_geometry.py @@ -1,6 +1,7 @@ import pytest +import mock -from typing import Union, ContextManager, Any +from typing import Union, ContextManager, Any, Optional from pytest_lazyfixture import lazy_fixture # type: ignore[import] from contextlib import nullcontext as does_not_raise from opentrons.types import Location, Point @@ -103,6 +104,12 @@ def heater_shaker_geometry() -> HeaterShakerGeometry: ) +@pytest.fixture +def mock_location() -> mock.MagicMock: + """Get a mocked out Location object.""" + return mock.MagicMock(return_value=Location(point=Point(1, 2, 3), labware=None)) + + @pytest.mark.parametrize( argnames=["api_version", "module_definition", "expected_geometry", "expected_repr"], argvalues=[ @@ -277,7 +284,7 @@ def test_hs_raises_when_moving_to_restricted_slots_while_shaking( [3, does_not_raise()], # non-adjacent ], ) -async def test_raises_when_moving_to_restricted_slots_while_latch_open( +def test_raises_when_moving_to_restricted_slots_while_latch_open( heater_shaker_geometry: HeaterShakerGeometry, destination_slot: int, expected_raise: ContextManager[Any], @@ -331,7 +338,7 @@ async def test_raises_when_moving_to_restricted_slots_while_latch_open( [7, False, does_not_raise()], # non-adjacent ], ) -async def test_raises_on_restricted_movement_with_multi_channel( +def test_raises_on_restricted_movement_with_multi_channel( heater_shaker_geometry: HeaterShakerGeometry, destination_slot: int, is_tiprack: bool, @@ -360,11 +367,14 @@ async def test_raises_on_restricted_movement_with_multi_channel( [9], # non-adjacent ], ) -async def test_does_not_raise_when_idle_and_latch_closed( +def test_does_not_raise_when_idle_and_latch_closed( heater_shaker_geometry: HeaterShakerGeometry, destination_slot: int, ) -> None: - """It should not raise if single channel pipette moves anywhere near heater-shaker when idle and latch closed.""" + """ + It should not raise if single channel pipette moves anywhere near heater-shaker + when idle and latch closed. + """ with does_not_raise(): heater_shaker_geometry.flag_unsafe_move( to_slot=destination_slot, @@ -373,3 +383,73 @@ async def test_does_not_raise_when_idle_and_latch_closed( is_labware_latch_closed=True, is_plate_shaking=False, ) + + +@pytest.mark.parametrize( + argnames=["pipette_slot", "expected_is_blocking"], + argvalues=[ + ("4", True), + ("6", True), + ("2", True), + ("5", True), + ("8", True), + (None, True), + ("1", False), + ], +) +def test_pipette_is_blocking_shake_movement( + heater_shaker_geometry: HeaterShakerGeometry, + mock_location: mock.MagicMock, + pipette_slot: Optional[str], + expected_is_blocking: bool, +) -> None: + """It should return True if pipette is blocking shake movement.""" + mock_location.labware.first_parent = mock.MagicMock(return_value=pipette_slot) + + assert ( + heater_shaker_geometry.is_pipette_blocking_shake_movement( + pipette_location=mock_location + ) + == expected_is_blocking + ) + + +@pytest.mark.parametrize( + argnames=["pipette_slot", "expected_is_blocking"], + argvalues=[("4", True), ("6", True), ("2", False), (None, True), ("1", False)], +) +def test_pipette_is_blocking_latch_movement( + heater_shaker_geometry: HeaterShakerGeometry, + mock_location: mock.MagicMock, + pipette_slot: Optional[str], + expected_is_blocking: bool, +) -> None: + """It should return True if pipette is blocking latch movement.""" + mock_location.labware.first_parent = mock.MagicMock(return_value=pipette_slot) + + assert ( + heater_shaker_geometry.is_pipette_blocking_latch_movement( + pipette_location=mock_location + ) + == expected_is_blocking + ) + + +def test_pipette_is_blocking_shake_and_latch_movements_with_no_pipette_slot( + heater_shaker_geometry: HeaterShakerGeometry, + mock_location: mock.MagicMock, +) -> None: + """It should return True if pipette's last location slot is not known.""" + + assert ( + heater_shaker_geometry.is_pipette_blocking_shake_movement( + pipette_location=Location(point=Point(3, 2, 1), labware=None) + ) + is True + ) + assert ( + heater_shaker_geometry.is_pipette_blocking_latch_movement( + pipette_location=Location(point=Point(3, 2, 1), labware=None) + ) + is True + )