From 02a7bface3021cdd8c76597f22b9b0cdc5190c14 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 4 Dec 2024 10:17:24 -0500 Subject: [PATCH] refactor(api): Disallow direct access to `.state` through Protocol Engine state views (#17016) --- .../protocol_api/core/engine/protocol.py | 2 + .../protocol_engine/commands/load_module.py | 2 +- .../protocol_engine/execution/tip_handler.py | 4 +- .../protocol_engine/protocol_engine.py | 2 +- .../state/addressable_areas.py | 26 ++++---- .../protocol_engine/state/commands.py | 10 ++- .../opentrons/protocol_engine/state/files.py | 2 +- .../protocol_engine/state/labware.py | 6 +- .../protocol_engine/state/liquid_classes.py | 2 +- .../protocol_engine/state/liquids.py | 2 +- .../protocol_engine/state/modules.py | 8 +-- .../protocol_engine/state/pipettes.py | 2 +- .../opentrons/protocol_engine/state/tips.py | 2 +- .../opentrons/protocol_engine/state/wells.py | 2 +- .../core/engine/test_protocol_core.py | 62 +------------------ .../commands/test_load_module.py | 16 ++--- .../execution/test_tip_handler.py | 22 +++---- .../state/test_addressable_area_view.py | 37 ++++++----- .../state/test_command_view_old.py | 4 +- .../protocol_engine/test_protocol_engine.py | 8 +-- 20 files changed, 87 insertions(+), 134 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/protocol.py b/api/src/opentrons/protocol_api/core/engine/protocol.py index f6ab2c89214..d43fc9a2058 100644 --- a/api/src/opentrons/protocol_api/core/engine/protocol.py +++ b/api/src/opentrons/protocol_api/core/engine/protocol.py @@ -425,6 +425,8 @@ def load_module( raise InvalidModuleLocationError(deck_slot, model.name) robot_type = self._engine_client.state.config.robot_type + # todo(mm, 2024-12-03): This might be possible to remove: + # Protocol Engine will normalize the deck slot itself. normalized_deck_slot = deck_slot.to_equivalent_for_robot_type(robot_type) result = self._engine_client.execute_command_without_recovery( diff --git a/api/src/opentrons/protocol_engine/commands/load_module.py b/api/src/opentrons/protocol_engine/commands/load_module.py index a44212f9bf5..79e67182666 100644 --- a/api/src/opentrons/protocol_engine/commands/load_module.py +++ b/api/src/opentrons/protocol_engine/commands/load_module.py @@ -173,7 +173,7 @@ def _ensure_module_location( cutout_fixture_id = ModuleType.to_module_fixture_id(module_type) module_fixture = deck_configuration_provider.get_cutout_fixture( cutout_fixture_id, - self._state_view.addressable_areas.state.deck_definition, + self._state_view.labware.get_deck_definition(), ) cutout_id = ( self._state_view.addressable_areas.get_cutout_id_by_deck_slot_name(slot) diff --git a/api/src/opentrons/protocol_engine/execution/tip_handler.py b/api/src/opentrons/protocol_engine/execution/tip_handler.py index 2a6816dcfdd..8a58536c10b 100644 --- a/api/src/opentrons/protocol_engine/execution/tip_handler.py +++ b/api/src/opentrons/protocol_engine/execution/tip_handler.py @@ -324,8 +324,8 @@ async def verify_tip_presence( follow_singular_sensor: Optional[InstrumentProbeType] = None, ) -> None: """See documentation on abstract base class.""" - nozzle_configuration = ( - self._state_view.pipettes.state.nozzle_configuration_by_id[pipette_id] + nozzle_configuration = self._state_view.pipettes.get_nozzle_configuration( + pipette_id=pipette_id ) # Configuration metrics by which tip presence checking is ignored diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index ba5219691bf..3479e0a295b 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -427,7 +427,7 @@ async def finish( post_run_hardware_state: The state in which to leave the gantry and motors in after the run is over. """ - if self._state_store.commands.state.stopped_by_estop: + if self._state_store.commands.get_is_stopped_by_estop(): # This handles the case where the E-stop was pressed while we were *not* in the middle # of some hardware interaction that would raise it as an exception. For example, imagine # we were paused between two commands, or imagine we were executing a waitForDuration. diff --git a/api/src/opentrons/protocol_engine/state/addressable_areas.py b/api/src/opentrons/protocol_engine/state/addressable_areas.py index afd076380f7..bd7d8de0188 100644 --- a/api/src/opentrons/protocol_engine/state/addressable_areas.py +++ b/api/src/opentrons/protocol_engine/state/addressable_areas.py @@ -323,7 +323,7 @@ def _validate_addressable_area_for_simulation( return cutout_id -class AddressableAreaView(HasState[AddressableAreaState]): +class AddressableAreaView: """Read-only addressable area state view.""" _state: AddressableAreaState @@ -345,8 +345,8 @@ def deck_extents(self) -> Point: @cached_property def mount_offsets(self) -> Dict[str, Point]: """The left and right mount offsets of the robot.""" - left_offset = self.state.robot_definition["mountOffsets"]["left"] - right_offset = self.state.robot_definition["mountOffsets"]["right"] + left_offset = self._state.robot_definition["mountOffsets"]["left"] + right_offset = self._state.robot_definition["mountOffsets"]["right"] return { "left": Point(x=left_offset[0], y=left_offset[1], z=left_offset[2]), "right": Point(x=right_offset[0], y=right_offset[1], z=right_offset[2]), @@ -355,10 +355,10 @@ def mount_offsets(self) -> Dict[str, Point]: @cached_property def padding_offsets(self) -> Dict[str, float]: """The padding offsets to be applied to the deck extents of the robot.""" - rear_offset = self.state.robot_definition["paddingOffsets"]["rear"] - front_offset = self.state.robot_definition["paddingOffsets"]["front"] - left_side_offset = self.state.robot_definition["paddingOffsets"]["leftSide"] - right_side_offset = self.state.robot_definition["paddingOffsets"]["rightSide"] + rear_offset = self._state.robot_definition["paddingOffsets"]["rear"] + front_offset = self._state.robot_definition["paddingOffsets"]["front"] + left_side_offset = self._state.robot_definition["paddingOffsets"]["leftSide"] + right_side_offset = self._state.robot_definition["paddingOffsets"]["rightSide"] return { "rear": rear_offset, "front": front_offset, @@ -420,12 +420,12 @@ def _check_if_area_is_compatible_with_potential_fixtures( _get_conflicting_addressable_areas_error_string( self._state.potential_cutout_fixtures_by_cutout_id[cutout_id], self._state.loaded_addressable_areas_by_name, - self.state.deck_definition, + self._state.deck_definition, ) ) area_display_name = ( deck_configuration_provider.get_addressable_area_display_name( - area_name, self.state.deck_definition + area_name, self._state.deck_definition ) ) raise IncompatibleAddressableAreaError( @@ -504,7 +504,7 @@ def get_addressable_area_offsets_from_cutout( addressable_area_name: str, ) -> Point: """Get the offset form cutout fixture of an addressable area.""" - for addressable_area in self.state.deck_definition["locations"][ + for addressable_area in self._state.deck_definition["locations"][ "addressableAreas" ]: if addressable_area["id"] == addressable_area_name: @@ -568,7 +568,7 @@ def get_fixture_by_deck_slot_name( self, slot_name: DeckSlotName ) -> Optional[CutoutFixture]: """Get the Cutout Fixture currently loaded where a specific Deck Slot would be.""" - deck_config = self.state.deck_configuration + deck_config = self._state.deck_configuration if deck_config: slot_cutout_id = DECK_SLOT_TO_CUTOUT_MAP[slot_name] slot_cutout_fixture = None @@ -581,7 +581,7 @@ def get_fixture_by_deck_slot_name( if cutout_id == slot_cutout_id: slot_cutout_fixture = ( deck_configuration_provider.get_cutout_fixture( - cutout_fixture_id, self.state.deck_definition + cutout_fixture_id, self._state.deck_definition ) ) return slot_cutout_fixture @@ -605,7 +605,7 @@ def get_fixture_serial_from_deck_configuration_by_deck_slot( self, slot_name: DeckSlotName ) -> Optional[str]: """Get the serial number provided by the deck configuration for a Fixture at a given location.""" - deck_config = self.state.deck_configuration + deck_config = self._state.deck_configuration if deck_config: slot_cutout_id = DECK_SLOT_TO_CUTOUT_MAP[slot_name] # This will only ever be one under current assumptions diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index da99c14ef3e..8aa71c9c1f8 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -579,7 +579,7 @@ def _map_finish_exception_to_error_occurrence( ) -class CommandView(HasState[CommandState]): +class CommandView: """Read-only command state view.""" _state: CommandState @@ -916,7 +916,7 @@ def raise_fatal_command_error(self) -> None: fatal error of the overall run coming from anywhere in the Python script, including in between commands. """ - failed_command = self.state.failed_command + failed_command = self._state.failed_command if ( failed_command and failed_command.command.error @@ -932,12 +932,16 @@ def get_error_recovery_type(self, command_id: str) -> ErrorRecoveryType: The command ID is assumed to point to a failed command. """ - return self.state.command_error_recovery_types[command_id] + return self._state.command_error_recovery_types[command_id] def get_is_stopped(self) -> bool: """Get whether an engine stop has completed.""" return self._state.run_completed_at is not None + def get_is_stopped_by_estop(self) -> bool: + """Return whether the engine was stopped specifically by an E-stop.""" + return self._state.stopped_by_estop + def has_been_played(self) -> bool: """Get whether engine has started.""" return self._state.run_started_at is not None diff --git a/api/src/opentrons/protocol_engine/state/files.py b/api/src/opentrons/protocol_engine/state/files.py index fb3848c6d55..bd54d58a4f8 100644 --- a/api/src/opentrons/protocol_engine/state/files.py +++ b/api/src/opentrons/protocol_engine/state/files.py @@ -35,7 +35,7 @@ def _handle_state_update(self, state_update: update_types.StateUpdate) -> None: self._state.file_ids.extend(state_update.files_added.file_ids) -class FileView(HasState[FileState]): +class FileView: """Read-only engine created file state view.""" _state: FileState diff --git a/api/src/opentrons/protocol_engine/state/labware.py b/api/src/opentrons/protocol_engine/state/labware.py index 419e9974d5c..95b2baa1974 100644 --- a/api/src/opentrons/protocol_engine/state/labware.py +++ b/api/src/opentrons/protocol_engine/state/labware.py @@ -244,7 +244,7 @@ def _set_labware_location(self, state_update: update_types.StateUpdate) -> None: self._state.labware_by_id[labware_id].location = new_location -class LabwareView(HasState[LabwareState]): +class LabwareView: """Read-only labware state view.""" _state: LabwareState @@ -268,7 +268,7 @@ def get(self, labware_id: str) -> LoadedLabware: def get_id_by_module(self, module_id: str) -> str: """Return the ID of the labware loaded on the given module.""" - for labware_id, labware in self.state.labware_by_id.items(): + for labware_id, labware in self._state.labware_by_id.items(): if ( isinstance(labware.location, ModuleLocation) and labware.location.moduleId == module_id @@ -281,7 +281,7 @@ def get_id_by_module(self, module_id: str) -> str: def get_id_by_labware(self, labware_id: str) -> str: """Return the ID of the labware loaded on the given labware.""" - for labware in self.state.labware_by_id.values(): + for labware in self._state.labware_by_id.values(): if ( isinstance(labware.location, OnLabwareLocation) and labware.location.labwareId == labware_id diff --git a/api/src/opentrons/protocol_engine/state/liquid_classes.py b/api/src/opentrons/protocol_engine/state/liquid_classes.py index 7992735fecd..4010c7be821 100644 --- a/api/src/opentrons/protocol_engine/state/liquid_classes.py +++ b/api/src/opentrons/protocol_engine/state/liquid_classes.py @@ -54,7 +54,7 @@ def _handle_liquid_class_loaded_update( ] = state_update.liquid_class_id -class LiquidClassView(HasState[LiquidClassState]): +class LiquidClassView: """Read-only view of the LiquidClassState.""" _state: LiquidClassState diff --git a/api/src/opentrons/protocol_engine/state/liquids.py b/api/src/opentrons/protocol_engine/state/liquids.py index 775223c6a60..034e0c4030b 100644 --- a/api/src/opentrons/protocol_engine/state/liquids.py +++ b/api/src/opentrons/protocol_engine/state/liquids.py @@ -34,7 +34,7 @@ def _add_liquid(self, action: AddLiquidAction) -> None: self._state.liquids_by_id[action.liquid.id] = action.liquid -class LiquidView(HasState[LiquidState]): +class LiquidView: """Read-only liquid state view.""" _state: LiquidState diff --git a/api/src/opentrons/protocol_engine/state/modules.py b/api/src/opentrons/protocol_engine/state/modules.py index f2f9dc5e8e4..0292329b8ea 100644 --- a/api/src/opentrons/protocol_engine/state/modules.py +++ b/api/src/opentrons/protocol_engine/state/modules.py @@ -632,7 +632,7 @@ def _handle_absorbance_reader_commands( ) -class ModuleView(HasState[ModuleState]): +class ModuleView: """Read-only view of computed module state.""" _state: ModuleState @@ -860,8 +860,8 @@ def get_nominal_offset_to_child( Labware Position Check offset. """ if ( - self.state.deck_type == DeckType.OT2_STANDARD - or self.state.deck_type == DeckType.OT2_SHORT_TRASH + self._state.deck_type == DeckType.OT2_STANDARD + or self._state.deck_type == DeckType.OT2_SHORT_TRASH ): definition = self.get_definition(module_id) slot = self.get_location(module_id).slotName.id @@ -908,7 +908,7 @@ def get_nominal_offset_to_child( "Module location invalid for nominal module offset calculation." ) module_addressable_area = self.ensure_and_convert_module_fixture_location( - location, self.state.deck_type, module.model + location, self._state.deck_type, module.model ) module_addressable_area_position = ( addressable_areas.get_addressable_area_offsets_from_cutout( diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 6418f50ee90..9b7d289e890 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -350,7 +350,7 @@ def _fluid_stack_log_if_empty(self, pipette_id: str) -> fluid_stack.FluidStack: return stack -class PipetteView(HasState[PipetteState]): +class PipetteView: """Read-only view of computed pipettes state.""" _state: PipetteState diff --git a/api/src/opentrons/protocol_engine/state/tips.py b/api/src/opentrons/protocol_engine/state/tips.py index c5c097ec693..214e2a9bc07 100644 --- a/api/src/opentrons/protocol_engine/state/tips.py +++ b/api/src/opentrons/protocol_engine/state/tips.py @@ -119,7 +119,7 @@ def _set_used_tips(self, pipette_id: str, well_name: str, labware_id: str) -> No wells[well] = TipRackWellState.USED -class TipView(HasState[TipState]): +class TipView: """Read-only tip state view.""" _state: TipState diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 2791346de5c..fdcb8322094 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -135,7 +135,7 @@ def _handle_well_operated( ) -class WellView(HasState[WellState]): +class WellView: """Read-only well state view.""" _state: WellState diff --git a/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py b/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py index 9ccaac498f0..1cf6bc57049 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py @@ -7,7 +7,6 @@ from opentrons_shared_data.liquid_classes.liquid_class_definition import ( LiquidClassSchemaV1, ) -from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] from decoy import Decoy from opentrons_shared_data.deck import load as load_deck @@ -26,7 +25,7 @@ from opentrons.types import DeckSlotName, StagingSlotName, Mount, MountType, Point from opentrons.protocol_api import OFF_DECK from opentrons.hardware_control import SyncHardwareAPI, SynchronousAdapter -from opentrons.hardware_control.modules import AbstractModule, ModuleType +from opentrons.hardware_control.modules import AbstractModule from opentrons.hardware_control.modules.types import ( ModuleModel, TemperatureModuleModel, @@ -1200,7 +1199,6 @@ def test_add_labware_definition( "requested_model", "engine_model", "expected_core_cls", - "deck_def", "slot_name", "robot_type", ), @@ -1209,7 +1207,6 @@ def test_add_labware_definition( TemperatureModuleModel.TEMPERATURE_V1, EngineModuleModel.TEMPERATURE_MODULE_V1, TemperatureModuleCore, - lazy_fixture("ot2_standard_deck_def"), DeckSlotName.SLOT_1, "OT-2 Standard", ), @@ -1217,7 +1214,6 @@ def test_add_labware_definition( TemperatureModuleModel.TEMPERATURE_V2, EngineModuleModel.TEMPERATURE_MODULE_V2, TemperatureModuleCore, - lazy_fixture("ot3_standard_deck_def"), DeckSlotName.SLOT_D1, "OT-3 Standard", ), @@ -1225,7 +1221,6 @@ def test_add_labware_definition( MagneticModuleModel.MAGNETIC_V1, EngineModuleModel.MAGNETIC_MODULE_V1, MagneticModuleCore, - lazy_fixture("ot2_standard_deck_def"), DeckSlotName.SLOT_1, "OT-2 Standard", ), @@ -1233,7 +1228,6 @@ def test_add_labware_definition( ThermocyclerModuleModel.THERMOCYCLER_V1, EngineModuleModel.THERMOCYCLER_MODULE_V1, ThermocyclerModuleCore, - lazy_fixture("ot2_standard_deck_def"), DeckSlotName.SLOT_7, "OT-2 Standard", ), @@ -1241,7 +1235,6 @@ def test_add_labware_definition( ThermocyclerModuleModel.THERMOCYCLER_V2, EngineModuleModel.THERMOCYCLER_MODULE_V2, ThermocyclerModuleCore, - lazy_fixture("ot3_standard_deck_def"), DeckSlotName.SLOT_B1, "OT-3 Standard", ), @@ -1249,7 +1242,6 @@ def test_add_labware_definition( HeaterShakerModuleModel.HEATER_SHAKER_V1, EngineModuleModel.HEATER_SHAKER_MODULE_V1, HeaterShakerModuleCore, - lazy_fixture("ot3_standard_deck_def"), DeckSlotName.SLOT_A1, "OT-3 Standard", ), @@ -1265,7 +1257,6 @@ def test_load_module( engine_model: EngineModuleModel, expected_core_cls: Type[ModuleCore], subject: ProtocolCore, - deck_def: DeckDefinitionV5, slot_name: DeckSlotName, robot_type: RobotType, ) -> None: @@ -1281,23 +1272,6 @@ def test_load_module( [mock_hw_mod_1, mock_hw_mod_2] ) - if robot_type == "OT-2 Standard": - decoy.when(subject.get_slot_definition(slot_name)).then_return( - cast( - SlotDefV3, - {"compatibleModuleTypes": [ModuleType.from_model(requested_model)]}, - ) - ) - else: - decoy.when( - mock_engine_client.state.addressable_areas.state.deck_definition - ).then_return(deck_def) - decoy.when( - mock_engine_client.state.addressable_areas.get_cutout_id_by_deck_slot_name( - slot_name - ) - ).then_return("cutout" + slot_name.value) - decoy.when(mock_engine_client.state.config.robot_type).then_return(robot_type) decoy.when( @@ -1356,34 +1330,13 @@ def test_load_module( def test_load_mag_block( decoy: Decoy, mock_engine_client: EngineClient, - mock_sync_hardware_api: SyncHardwareAPI, subject: ProtocolCore, - ot3_standard_deck_def: DeckDefinitionV5, ) -> None: """It should issue a load module engine command.""" definition = ModuleDefinition.construct() # type: ignore[call-arg] decoy.when(mock_engine_client.state.config.robot_type).then_return("OT-3 Standard") - decoy.when(subject.get_slot_definition(DeckSlotName.SLOT_A2)).then_return( - cast( - SlotDefV3, - { - "compatibleModuleTypes": [ - ModuleType.from_model(MagneticBlockModel.MAGNETIC_BLOCK_V1) - ] - }, - ) - ) - decoy.when( - mock_engine_client.state.addressable_areas.state.deck_definition - ).then_return(ot3_standard_deck_def) - decoy.when( - mock_engine_client.state.addressable_areas.get_cutout_id_by_deck_slot_name( - DeckSlotName.SLOT_A2 - ) - ).then_return("cutout" + DeckSlotName.SLOT_A2.value) - decoy.when( mock_engine_client.execute_command_without_recovery( cmd.LoadModuleParams( @@ -1436,18 +1389,16 @@ def test_load_mag_block( @pytest.mark.parametrize( - ("requested_model", "engine_model", "deck_def", "expected_slot"), + ("requested_model", "engine_model", "expected_slot"), [ ( ThermocyclerModuleModel.THERMOCYCLER_V1, EngineModuleModel.THERMOCYCLER_MODULE_V1, - lazy_fixture("ot3_standard_deck_def"), DeckSlotName.SLOT_B1, ), ( ThermocyclerModuleModel.THERMOCYCLER_V2, EngineModuleModel.THERMOCYCLER_MODULE_V2, - lazy_fixture("ot3_standard_deck_def"), DeckSlotName.SLOT_B1, ), ], @@ -1459,7 +1410,6 @@ def test_load_module_thermocycler_with_no_location( requested_model: ModuleModel, engine_model: EngineModuleModel, subject: ProtocolCore, - deck_def: DeckDefinitionV5, expected_slot: DeckSlotName, ) -> None: """It should issue a load module engine command with location at 7.""" @@ -1469,14 +1419,6 @@ def test_load_module_thermocycler_with_no_location( decoy.when(mock_hw_mod.device_info).then_return({"serial": "xyz789"}) decoy.when(mock_sync_hardware_api.attached_modules).then_return([mock_hw_mod]) decoy.when(mock_engine_client.state.config.robot_type).then_return("OT-3 Standard") - decoy.when( - mock_engine_client.state.addressable_areas.state.deck_definition - ).then_return(deck_def) - decoy.when( - mock_engine_client.state.addressable_areas.get_cutout_id_by_deck_slot_name( - expected_slot - ) - ).then_return("cutout" + expected_slot.value) decoy.when( mock_engine_client.execute_command_without_recovery( diff --git a/api/tests/opentrons/protocol_engine/commands/test_load_module.py b/api/tests/opentrons/protocol_engine/commands/test_load_module.py index ce68f5c9f8a..e5098b5dc49 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_load_module.py +++ b/api/tests/opentrons/protocol_engine/commands/test_load_module.py @@ -57,7 +57,7 @@ async def test_load_module_implementation( deck_def = load_deck(STANDARD_OT3_DECK, 5) - decoy.when(state_view.addressable_areas.state.deck_definition).then_return(deck_def) + decoy.when(state_view.labware.get_deck_definition()).then_return(deck_def) decoy.when( state_view.addressable_areas.get_cutout_id_by_deck_slot_name( DeckSlotName.SLOT_D1 @@ -112,7 +112,7 @@ async def test_load_module_implementation_mag_block( deck_def = load_deck(STANDARD_OT3_DECK, 5) - decoy.when(state_view.addressable_areas.state.deck_definition).then_return(deck_def) + decoy.when(state_view.labware.get_deck_definition()).then_return(deck_def) decoy.when( state_view.addressable_areas.get_cutout_id_by_deck_slot_name( DeckSlotName.SLOT_D1 @@ -167,7 +167,7 @@ async def test_load_module_implementation_abs_reader( deck_def = load_deck(STANDARD_OT3_DECK, 5) - decoy.when(state_view.addressable_areas.state.deck_definition).then_return(deck_def) + decoy.when(state_view.labware.get_deck_definition()).then_return(deck_def) decoy.when( state_view.addressable_areas.get_cutout_id_by_deck_slot_name( DeckSlotName.SLOT_D3 @@ -221,7 +221,7 @@ async def test_load_module_raises_if_location_occupied( deck_def = load_deck(STANDARD_OT3_DECK, 5) - decoy.when(state_view.addressable_areas.state.deck_definition).then_return(deck_def) + decoy.when(state_view.labware.get_deck_definition()).then_return(deck_def) decoy.when( state_view.addressable_areas.get_cutout_id_by_deck_slot_name( DeckSlotName.SLOT_D1 @@ -303,9 +303,7 @@ async def test_load_module_raises_wrong_location( state_view.addressable_areas.get_slot_definition(slot_name.id) ).then_return(cast(SlotDefV3, {"compatibleModuleTypes": []})) else: - decoy.when(state_view.addressable_areas.state.deck_definition).then_return( - deck_def - ) + decoy.when(state_view.labware.get_deck_definition()).then_return(deck_def) decoy.when( state_view.addressable_areas.get_cutout_id_by_deck_slot_name(slot_name) ).then_return("cutout" + slot_name.value) @@ -361,9 +359,7 @@ async def test_load_module_raises_module_fixture_id_does_not_exist( state_view.addressable_areas.get_slot_definition(slot_name.id) ).then_return(cast(SlotDefV3, {"compatibleModuleTypes": []})) else: - decoy.when(state_view.addressable_areas.state.deck_definition).then_return( - deck_def - ) + decoy.when(state_view.labware.get_deck_definition()).then_return(deck_def) decoy.when( state_view.addressable_areas.get_cutout_id_by_deck_slot_name(slot_name) ).then_return("cutout" + slot_name.value) 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 c03a611966c..4e9a10fdfaa 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_tip_handler.py +++ b/api/tests/opentrons/protocol_engine/execution/test_tip_handler.py @@ -114,9 +114,9 @@ async def test_flex_pick_up_tip_state( 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} - ) + decoy.when( + mock_state_view.pipettes.get_nozzle_configuration("pipette-id") + ).then_return(MOCK_MAP) decoy.when( mock_state_view.geometry.get_nominal_tip_geometry( pipette_id="pipette-id", @@ -189,9 +189,9 @@ async def test_pick_up_tip( MountType.LEFT ) - decoy.when(mock_state_view.pipettes.state.nozzle_configuration_by_id).then_return( - {"pipette-id": MOCK_MAP} - ) + decoy.when( + mock_state_view.pipettes.get_nozzle_configuration(pipette_id="pipette-id") + ).then_return(MOCK_MAP) decoy.when( mock_state_view.geometry.get_nominal_tip_geometry( @@ -249,9 +249,9 @@ async def test_drop_tip( decoy.when(mock_state_view.pipettes.get_mount("pipette-id")).then_return( MountType.RIGHT ) - decoy.when(mock_state_view.pipettes.state.nozzle_configuration_by_id).then_return( - {"pipette-id": MOCK_MAP} - ) + decoy.when( + mock_state_view.pipettes.get_nozzle_configuration("pipette-id") + ).then_return(MOCK_MAP) await subject.drop_tip(pipette_id="pipette-id", home_after=True) @@ -558,8 +558,8 @@ async def test_verify_tip_presence_on_ot3( ) decoy.when( - mock_state_view.pipettes.state.nozzle_configuration_by_id - ).then_return({"pipette-id": MOCK_MAP}) + mock_state_view.pipettes.get_nozzle_configuration("pipette-id") + ).then_return(MOCK_MAP) await subject.verify_tip_presence("pipette-id", expected, None) diff --git a/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py b/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py index 30ca1b9e7c4..fc57cb73521 100644 --- a/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_addressable_area_view.py @@ -6,6 +6,7 @@ """ import inspect +from unittest.mock import sentinel import pytest from decoy import Decoy @@ -171,6 +172,7 @@ def test_get_addressable_area_for_simulation_not_loaded(decoy: Decoy) -> None: } }, use_simulated_deck_config=True, + deck_definition=sentinel.deck_definition, ) addressable_area = AddressableArea( @@ -185,7 +187,8 @@ def test_get_addressable_area_for_simulation_not_loaded(decoy: Decoy) -> None: decoy.when( deck_configuration_provider.get_potential_cutout_fixtures( - "abc", subject.state.deck_definition + "abc", + sentinel.deck_definition, ) ).then_return( ( @@ -202,7 +205,8 @@ def test_get_addressable_area_for_simulation_not_loaded(decoy: Decoy) -> None: decoy.when( deck_configuration_provider.get_cutout_position( - "cutoutA1", subject.state.deck_definition + "cutoutA1", + sentinel.deck_definition, ) ).then_return(DeckPoint(x=1, y=2, z=3)) @@ -211,7 +215,7 @@ def test_get_addressable_area_for_simulation_not_loaded(decoy: Decoy) -> None: "abc", DeckPoint(x=1, y=2, z=3), DeckSlotName.SLOT_A1, - subject.state.deck_definition, + sentinel.deck_definition, ) ).then_return(addressable_area) @@ -231,11 +235,12 @@ def test_get_addressable_area_for_simulation_raises(decoy: Decoy) -> None: } }, use_simulated_deck_config=True, + deck_definition=sentinel.deck_definition, ) decoy.when( deck_configuration_provider.get_potential_cutout_fixtures( - "abc", subject.state.deck_definition + "abc", sentinel.deck_definition ) ).then_return( ( @@ -252,7 +257,7 @@ def test_get_addressable_area_for_simulation_raises(decoy: Decoy) -> None: decoy.when( deck_configuration_provider.get_provided_addressable_area_names( - "bleh", "789", subject.state.deck_definition + "bleh", "789", sentinel.deck_definition ) ).then_return([]) @@ -322,10 +327,10 @@ def test_get_addressable_area_center() -> None: def test_get_fixture_height(decoy: Decoy) -> None: """It should return the height of the requested fixture.""" - subject = get_addressable_area_view() + subject = get_addressable_area_view(deck_definition=sentinel.deck_definition) decoy.when( deck_configuration_provider.get_cutout_fixture( - "someShortCutoutFixture", subject.state.deck_definition + "someShortCutoutFixture", sentinel.deck_definition ) ).then_return( { @@ -342,7 +347,7 @@ def test_get_fixture_height(decoy: Decoy) -> None: decoy.when( deck_configuration_provider.get_cutout_fixture( - "someTallCutoutFixture", subject.state.deck_definition + "someTallCutoutFixture", sentinel.deck_definition ) ).then_return( { @@ -394,11 +399,14 @@ def test_get_slot_definition() -> None: def test_get_slot_definition_raises_with_bad_slot_name(decoy: Decoy) -> None: """It should raise a SlotDoesNotExistError if a bad slot name is given.""" - subject = get_addressable_area_view() + deck_definition = cast(DeckDefinitionV5, {"otId": "fake"}) + subject = get_addressable_area_view( + deck_definition=deck_definition, + ) decoy.when( deck_configuration_provider.get_potential_cutout_fixtures( - "foo", subject.state.deck_definition + "foo", deck_definition ) ).then_raise(AddressableAreaDoesNotExistError()) @@ -438,11 +446,12 @@ def test_raise_if_area_not_in_deck_configuration_simulated_config(decoy: Decoy) ) }, }, + deck_definition=sentinel.deck_definition, ) decoy.when( deck_configuration_provider.get_potential_cutout_fixtures( - "mario", subject.state.deck_definition + "mario", sentinel.deck_definition ) ).then_return( ( @@ -461,7 +470,7 @@ def test_raise_if_area_not_in_deck_configuration_simulated_config(decoy: Decoy) decoy.when( deck_configuration_provider.get_potential_cutout_fixtures( - "luigi", subject.state.deck_definition + "luigi", sentinel.deck_definition ) ).then_return( ( @@ -478,13 +487,13 @@ def test_raise_if_area_not_in_deck_configuration_simulated_config(decoy: Decoy) decoy.when( deck_configuration_provider.get_provided_addressable_area_names( - "1up", "fire flower", subject.state.deck_definition + "1up", "fire flower", sentinel.deck_definition ) ).then_return([]) decoy.when( deck_configuration_provider.get_addressable_area_display_name( - "luigi", subject.state.deck_definition + "luigi", sentinel.deck_definition ) ).then_return("super luigi") diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index 0cbef9cf474..84b4a8606f6 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -892,7 +892,7 @@ def test_get_current() -> None: created_at=datetime(year=2022, month=2, day=2), ) subject = get_command_view(commands=[command_1, command_2]) - subject.state.command_history._set_most_recently_completed_command_id(command_1.id) + subject._state.command_history._set_most_recently_completed_command_id(command_1.id) assert subject.get_current() == CommandPointer( index=1, @@ -912,7 +912,7 @@ def test_get_current() -> None: created_at=datetime(year=2022, month=2, day=2), ) subject = get_command_view(commands=[command_1, command_2]) - subject.state.command_history._set_most_recently_completed_command_id(command_1.id) + subject._state.command_history._set_most_recently_completed_command_id(command_1.id) assert subject.get_current() == CommandPointer( index=1, diff --git a/api/tests/opentrons/protocol_engine/test_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_protocol_engine.py index d7e4b32e02a..34dac853ef8 100644 --- a/api/tests/opentrons/protocol_engine/test_protocol_engine.py +++ b/api/tests/opentrons/protocol_engine/test_protocol_engine.py @@ -684,7 +684,7 @@ async def test_finish( """It should be able to gracefully tell the engine it's done.""" completed_at = datetime(2021, 1, 1, 0, 0) - decoy.when(state_store.commands.state.stopped_by_estop).then_return(False) + decoy.when(state_store.commands.get_is_stopped_by_estop()).then_return(False) decoy.when(model_utils.get_timestamp()).then_return(completed_at) await subject.finish( @@ -719,7 +719,7 @@ async def test_finish_with_defaults( state_store: StateStore, ) -> None: """It should be able to gracefully tell the engine it's done.""" - decoy.when(state_store.commands.state.stopped_by_estop).then_return(False) + decoy.when(state_store.commands.get_is_stopped_by_estop()).then_return(False) await subject.finish() decoy.verify( @@ -761,7 +761,7 @@ async def test_finish_with_error( error=error, ) - decoy.when(state_store.commands.state.stopped_by_estop).then_return( + decoy.when(state_store.commands.get_is_stopped_by_estop()).then_return( stopped_by_estop ) decoy.when(model_utils.generate_id()).then_return("error-id") @@ -861,7 +861,7 @@ async def test_finish_stops_hardware_if_queue_worker_join_fails( await queue_worker.join(), ).then_raise(exception) - decoy.when(state_store.commands.state.stopped_by_estop).then_return(False) + decoy.when(state_store.commands.get_is_stopped_by_estop()).then_return(False) error_id = "error-id" completed_at = datetime(2021, 1, 1, 0, 0)