From aefbfc8378fc360eed5f344b0893b92e1536bdcb Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 11 Oct 2024 15:17:42 -0400 Subject: [PATCH 01/10] Nozzle map values are never None. --- api/src/opentrons/protocol_engine/state/pipettes.py | 2 +- api/tests/opentrons/protocol_engine/state/test_pipette_view.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 26563b08ced2..74523bb4ecb7 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -112,7 +112,7 @@ class PipetteState: movement_speed_by_id: Dict[str, Optional[float]] static_config_by_id: Dict[str, StaticPipetteConfig] flow_rates_by_id: Dict[str, FlowRates] - nozzle_configuration_by_id: Dict[str, Optional[NozzleMap]] + nozzle_configuration_by_id: Dict[str, NozzleMap] liquid_presence_detection_by_id: Dict[str, bool] diff --git a/api/tests/opentrons/protocol_engine/state/test_pipette_view.py b/api/tests/opentrons/protocol_engine/state/test_pipette_view.py index 27bee5f1d154..3b4d04bd967b 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_view.py @@ -65,7 +65,7 @@ def get_pipette_view( movement_speed_by_id: Optional[Dict[str, Optional[float]]] = None, static_config_by_id: Optional[Dict[str, StaticPipetteConfig]] = None, flow_rates_by_id: Optional[Dict[str, FlowRates]] = None, - nozzle_layout_by_id: Optional[Dict[str, Optional[NozzleMap]]] = None, + nozzle_layout_by_id: Optional[Dict[str, NozzleMap]] = None, liquid_presence_detection_by_id: Optional[Dict[str, bool]] = None, ) -> PipetteView: """Get a pipette view test subject with the specified state.""" From a260a40cece2d0c44071f441fe25bb1b0441fd8b Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 14:53:47 -0400 Subject: [PATCH 02/10] Downstream simplifications from non-None nozzle map. --- .../protocol_engine/state/pipettes.py | 20 ++++++------------- .../core/engine/test_instrument_core.py | 7 ++----- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 74523bb4ecb7..cfce5c94487d 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -632,31 +632,23 @@ def get_plunger_axis(self, pipette_id: str) -> MotorAxis: def get_nozzle_layout_type(self, pipette_id: str) -> NozzleConfigurationType: """Get the current set nozzle layout configuration.""" - nozzle_map_for_pipette = self._state.nozzle_configuration_by_id.get(pipette_id) - if nozzle_map_for_pipette: - return nozzle_map_for_pipette.configuration - else: - return NozzleConfigurationType.FULL + nozzle_map_for_pipette = self._state.nozzle_configuration_by_id[pipette_id] + return nozzle_map_for_pipette.configuration def get_is_partially_configured(self, pipette_id: str) -> bool: """Determine if the provided pipette is partially configured.""" return self.get_nozzle_layout_type(pipette_id) != NozzleConfigurationType.FULL - def get_primary_nozzle(self, pipette_id: str) -> Optional[str]: + def get_primary_nozzle(self, pipette_id: str) -> str: """Get the primary nozzle, if any, related to the given pipette's nozzle configuration.""" - nozzle_map = self._state.nozzle_configuration_by_id.get(pipette_id) - return nozzle_map.starting_nozzle if nozzle_map else None + nozzle_map = self._state.nozzle_configuration_by_id[pipette_id] + return nozzle_map.starting_nozzle def _get_critical_point_offset_without_tip( self, pipette_id: str, critical_point: Optional[CriticalPoint] ) -> Point: """Get the offset of the specified critical point from pipette's mount position.""" - nozzle_map = self._state.nozzle_configuration_by_id.get(pipette_id) - # Nozzle map is unavailable only when there's no pipette loaded - # so this is merely for satisfying the type checker - assert ( - nozzle_map is not None - ), "Error getting critical point offset. Nozzle map not found." + nozzle_map = self._state.nozzle_configuration_by_id[pipette_id] match critical_point: case CriticalPoint.INSTRUMENT_XY_CENTER: return nozzle_map.instrument_xy_center_offset diff --git a/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py b/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py index bd3cebe94d77..cb68b77a96ed 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py @@ -1,5 +1,5 @@ """Test for the ProtocolEngine-based instrument API core.""" -from typing import cast, Optional, Union +from typing import cast, Optional from opentrons_shared_data.errors.exceptions import PipetteLiquidNotFoundError import pytest @@ -1227,17 +1227,14 @@ def test_configure_nozzle_layout( argnames=["pipette_channels", "nozzle_layout", "primary_nozzle", "expected_result"], argvalues=[ (96, NozzleConfigurationType.FULL, "A1", True), - (96, NozzleConfigurationType.FULL, None, True), (96, NozzleConfigurationType.ROW, "A1", True), (96, NozzleConfigurationType.COLUMN, "A1", True), (96, NozzleConfigurationType.COLUMN, "A12", True), (96, NozzleConfigurationType.SINGLE, "H12", True), (96, NozzleConfigurationType.SINGLE, "A1", True), (8, NozzleConfigurationType.FULL, "A1", True), - (8, NozzleConfigurationType.FULL, None, True), (8, NozzleConfigurationType.SINGLE, "H1", True), (8, NozzleConfigurationType.SINGLE, "A1", True), - (1, NozzleConfigurationType.FULL, None, True), ], ) def test_is_tip_tracking_available( @@ -1246,7 +1243,7 @@ def test_is_tip_tracking_available( subject: InstrumentCore, pipette_channels: int, nozzle_layout: NozzleConfigurationType, - primary_nozzle: Union[str, None], + primary_nozzle: str, expected_result: bool, ) -> None: """It should return whether tip tracking is available based on nozzle configuration.""" From 3cb634b0d44b4b8db716e11d525692a7ce32924f Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 11 Oct 2024 15:30:01 -0400 Subject: [PATCH 03/10] Scary todo comment. --- api/src/opentrons/protocol_engine/state/pipettes.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index cfce5c94487d..086671f88e53 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -167,6 +167,10 @@ def _set_load_pipette(self, state_update: update_types.StateUpdate) -> None: self._state.aspirated_volume_by_id[pipette_id] = None self._state.movement_speed_by_id[pipette_id] = None self._state.attached_tip_by_id[pipette_id] = None + # TODO(mm, 2024-10-11): This seems wrong--because of the order in which + # we process the individual attributes of StateUpdate, when we process a + # loadPipette command, won't we always reach here before static_config is + # populated? static_config = self._state.static_config_by_id.get(pipette_id) if static_config: self._state.nozzle_configuration_by_id[ From 1749383d36ac56ca7fdf9b45f9a013e505db99c7 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 13:25:30 -0400 Subject: [PATCH 04/10] Update PipetteStore tests to reflect actual LoadPipetteImplementation. --- .../state/test_pipette_store.py | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) 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 81ddf13f5d86..caab429e26b4 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -191,25 +191,52 @@ def test_location_state_update(subject: PipetteStore) -> None: ) -def test_handles_load_pipette(subject: PipetteStore) -> None: +def test_handles_load_pipette( + subject: PipetteStore, + supported_tip_fixture: pipette_definition.SupportedTipsDefinition, +) -> None: """It should add the pipette data to the state.""" - command = create_load_pipette_command( + dummy_command = create_succeeded_command() + + load_pipette_update = update_types.LoadPipetteUpdate( pipette_id="pipette-id", pipette_name=PipetteNameType.P300_SINGLE, mount=MountType.LEFT, + liquid_presence_detection=None, + ) + + config = LoadedStaticPipetteData( + model="pipette-model", + display_name="pipette name", + min_volume=1.23, + max_volume=4.56, + channels=7, + flow_rates=FlowRates( + default_aspirate={"a": 1}, + default_dispense={"b": 2}, + default_blow_out={"c": 3}, + ), + tip_configuration_lookup_table={4: supported_tip_fixture}, + nominal_tip_overlap={"default": 5}, + home_position=8.9, + nozzle_offset_z=10.11, + nozzle_map=get_default_nozzle_map(PipetteNameType.P300_SINGLE), + back_left_corner_offset=Point(x=1, y=2, z=3), + front_right_corner_offset=Point(x=4, y=5, z=6), + pipette_lld_settings={}, + ) + config_update = update_types.PipetteConfigUpdate( + pipette_id="pipette-id", + config=config, + serial_number="pipette-serial", ) subject.handle_action( SucceedCommandAction( private_result=None, - command=command, + command=dummy_command, state_update=update_types.StateUpdate( - loaded_pipette=update_types.LoadPipetteUpdate( - pipette_id="pipette-id", - pipette_name=PipetteNameType.P300_SINGLE, - mount=MountType.LEFT, - liquid_presence_detection=None, - ) + loaded_pipette=load_pipette_update, pipette_config=config_update ), ) ) From c1bf0f4ba6fb17f6a84bf5d758a89ccd45b1cf91 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 11 Oct 2024 16:08:48 -0400 Subject: [PATCH 05/10] Fix unused variable warning in Pyright. --- .../opentrons/protocol_engine/state/test_tip_state.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/state/test_tip_state.py b/api/tests/opentrons/protocol_engine/state/test_tip_state.py index dc603ac4ca8a..b95d94266e2d 100644 --- a/api/tests/opentrons/protocol_engine/state/test_tip_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_tip_state.py @@ -1367,24 +1367,24 @@ def _reconfigure_nozzle_layout(start: str, back_l: str, front_r: str) -> NozzleM return configure_nozzle_private_result.nozzle_map map = _reconfigure_nozzle_layout("A1", "A1", "A1") - for x in range(96): + for _ in range(96): _get_next_and_pickup(map) assert _get_next_and_pickup(map) is None subject.handle_action(actions.ResetTipsAction(labware_id="cool-labware")) map = _reconfigure_nozzle_layout("A12", "A12", "A12") - for x in range(96): + for _ in range(96): _get_next_and_pickup(map) assert _get_next_and_pickup(map) is None subject.handle_action(actions.ResetTipsAction(labware_id="cool-labware")) map = _reconfigure_nozzle_layout("H1", "H1", "H1") - for x in range(96): + for _ in range(96): _get_next_and_pickup(map) assert _get_next_and_pickup(map) is None subject.handle_action(actions.ResetTipsAction(labware_id="cool-labware")) map = _reconfigure_nozzle_layout("H12", "H12", "H12") - for x in range(96): + for _ in range(96): _get_next_and_pickup(map) assert _get_next_and_pickup(map) is None From 142f194253e9ea2345ff7c5153c0fb5db92d9e07 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 14:37:22 -0400 Subject: [PATCH 06/10] Todo comment for PipetteState. --- api/src/opentrons/protocol_engine/state/pipettes.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 086671f88e53..d685e8926fd8 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -104,6 +104,9 @@ class StaticPipetteConfig: class PipetteState: """Basic pipette data state and getter methods.""" + # todo(mm, 2024-10-14): It's getting difficult to ensure that all of these + # attributes are populated at the appropriate times. Refactor to a + # single dict-of-many-things instead of many dicts-of-single-things. pipettes_by_id: Dict[str, LoadedPipette] aspirated_volume_by_id: Dict[str, Optional[float]] current_location: Optional[CurrentPipetteLocation] From d4f782ea99cc828e0f915db3bf41d3a4d3e27d2c Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 14:37:16 -0400 Subject: [PATCH 07/10] Combine _set_load_pipette() and _update_pipette_config() --- .../protocol_engine/state/pipettes.py | 107 +++++++++--------- 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index d685e8926fd8..fb290fd4818a 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -143,9 +143,8 @@ def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" state_update = get_state_update(action) if state_update is not None: - self._set_load_pipette(state_update) + self._load_pipette_or_update_config(state_update) self._update_current_location(state_update) - self._update_pipette_config(state_update) self._update_pipette_nozzle_map(state_update) self._update_tip_state(state_update) @@ -155,7 +154,52 @@ def handle_action(self, action: Action) -> None: elif isinstance(action, SetPipetteMovementSpeedAction): self._state.movement_speed_by_id[action.pipette_id] = action.speed - def _set_load_pipette(self, state_update: update_types.StateUpdate) -> None: + def _load_pipette_or_update_config( + self, state_update: update_types.StateUpdate + ) -> None: + if state_update.pipette_config != update_types.NO_CHANGE: + config = state_update.pipette_config.config + self._state.static_config_by_id[ + state_update.pipette_config.pipette_id + ] = StaticPipetteConfig( + serial_number=state_update.pipette_config.serial_number, + model=config.model, + display_name=config.display_name, + min_volume=config.min_volume, + max_volume=config.max_volume, + channels=config.channels, + tip_configuration_lookup_table=config.tip_configuration_lookup_table, + nominal_tip_overlap=config.nominal_tip_overlap, + home_position=config.home_position, + nozzle_offset_z=config.nozzle_offset_z, + pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( + back_left_corner=config.back_left_corner_offset, + front_right_corner=config.front_right_corner_offset, + back_right_corner=Point( + config.front_right_corner_offset.x, + config.back_left_corner_offset.y, + config.back_left_corner_offset.z, + ), + front_left_corner=Point( + config.back_left_corner_offset.x, + config.front_right_corner_offset.y, + config.back_left_corner_offset.z, + ), + ), + bounding_nozzle_offsets=BoundingNozzlesOffsets( + back_left_offset=config.nozzle_map.back_left_nozzle_offset, + front_right_offset=config.nozzle_map.front_right_nozzle_offset, + ), + default_nozzle_map=config.nozzle_map, + lld_settings=config.pipette_lld_settings, + ) + self._state.flow_rates_by_id[ + state_update.pipette_config.pipette_id + ] = config.flow_rates + self._state.nozzle_configuration_by_id[ + state_update.pipette_config.pipette_id + ] = config.nozzle_map + if state_update.loaded_pipette != update_types.NO_CHANGE: pipette_id = state_update.loaded_pipette.pipette_id @@ -170,10 +214,15 @@ def _set_load_pipette(self, state_update: update_types.StateUpdate) -> None: self._state.aspirated_volume_by_id[pipette_id] = None self._state.movement_speed_by_id[pipette_id] = None self._state.attached_tip_by_id[pipette_id] = None - # TODO(mm, 2024-10-11): This seems wrong--because of the order in which - # we process the individual attributes of StateUpdate, when we process a - # loadPipette command, won't we always reach here before static_config is - # populated? + + # This should always be truthy because LoadPipetteImplementation always + # supplies state_update.pipette_config, which we process above, + # populating self._state.static_config_by_id[pipette_id]. + # + # todo(mm, 2024-10-14): Formalize this expectation by having + # state_update.loaded_pipette incorporate all the stuff in + # state_update.pipette_config, so LoadPipetteImplementation only has to + # supply state_update.loaded_pipette. static_config = self._state.static_config_by_id.get(pipette_id) if static_config: self._state.nozzle_configuration_by_id[ @@ -266,50 +315,6 @@ def _update_current_location(self, state_update: update_types.StateUpdate) -> No mount=loaded_pipette.mount, deck_point=new_deck_point ) - def _update_pipette_config(self, state_update: update_types.StateUpdate) -> None: - if state_update.pipette_config != update_types.NO_CHANGE: - config = state_update.pipette_config.config - self._state.static_config_by_id[ - state_update.pipette_config.pipette_id - ] = StaticPipetteConfig( - serial_number=state_update.pipette_config.serial_number, - model=config.model, - display_name=config.display_name, - min_volume=config.min_volume, - max_volume=config.max_volume, - channels=config.channels, - tip_configuration_lookup_table=config.tip_configuration_lookup_table, - nominal_tip_overlap=config.nominal_tip_overlap, - home_position=config.home_position, - nozzle_offset_z=config.nozzle_offset_z, - pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( - back_left_corner=config.back_left_corner_offset, - front_right_corner=config.front_right_corner_offset, - back_right_corner=Point( - config.front_right_corner_offset.x, - config.back_left_corner_offset.y, - config.back_left_corner_offset.z, - ), - front_left_corner=Point( - config.back_left_corner_offset.x, - config.front_right_corner_offset.y, - config.back_left_corner_offset.z, - ), - ), - bounding_nozzle_offsets=BoundingNozzlesOffsets( - back_left_offset=config.nozzle_map.back_left_nozzle_offset, - front_right_offset=config.nozzle_map.front_right_nozzle_offset, - ), - default_nozzle_map=config.nozzle_map, - lld_settings=config.pipette_lld_settings, - ) - self._state.flow_rates_by_id[ - state_update.pipette_config.pipette_id - ] = config.flow_rates - self._state.nozzle_configuration_by_id[ - state_update.pipette_config.pipette_id - ] = config.nozzle_map - def _update_pipette_nozzle_map( self, state_update: update_types.StateUpdate ) -> None: From 07153b80f59ab11083dee5292f10ad29c29588de Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 14:38:13 -0400 Subject: [PATCH 08/10] Revert "Combine _set_load_pipette() and _update_pipette_config()" This reverts commit cb1f6396bdd851d4e61fbaadf93fef12a78e9cb0. --- .../protocol_engine/state/pipettes.py | 107 +++++++++--------- 1 file changed, 51 insertions(+), 56 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index fb290fd4818a..d685e8926fd8 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -143,8 +143,9 @@ def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" state_update = get_state_update(action) if state_update is not None: - self._load_pipette_or_update_config(state_update) + self._set_load_pipette(state_update) self._update_current_location(state_update) + self._update_pipette_config(state_update) self._update_pipette_nozzle_map(state_update) self._update_tip_state(state_update) @@ -154,52 +155,7 @@ def handle_action(self, action: Action) -> None: elif isinstance(action, SetPipetteMovementSpeedAction): self._state.movement_speed_by_id[action.pipette_id] = action.speed - def _load_pipette_or_update_config( - self, state_update: update_types.StateUpdate - ) -> None: - if state_update.pipette_config != update_types.NO_CHANGE: - config = state_update.pipette_config.config - self._state.static_config_by_id[ - state_update.pipette_config.pipette_id - ] = StaticPipetteConfig( - serial_number=state_update.pipette_config.serial_number, - model=config.model, - display_name=config.display_name, - min_volume=config.min_volume, - max_volume=config.max_volume, - channels=config.channels, - tip_configuration_lookup_table=config.tip_configuration_lookup_table, - nominal_tip_overlap=config.nominal_tip_overlap, - home_position=config.home_position, - nozzle_offset_z=config.nozzle_offset_z, - pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( - back_left_corner=config.back_left_corner_offset, - front_right_corner=config.front_right_corner_offset, - back_right_corner=Point( - config.front_right_corner_offset.x, - config.back_left_corner_offset.y, - config.back_left_corner_offset.z, - ), - front_left_corner=Point( - config.back_left_corner_offset.x, - config.front_right_corner_offset.y, - config.back_left_corner_offset.z, - ), - ), - bounding_nozzle_offsets=BoundingNozzlesOffsets( - back_left_offset=config.nozzle_map.back_left_nozzle_offset, - front_right_offset=config.nozzle_map.front_right_nozzle_offset, - ), - default_nozzle_map=config.nozzle_map, - lld_settings=config.pipette_lld_settings, - ) - self._state.flow_rates_by_id[ - state_update.pipette_config.pipette_id - ] = config.flow_rates - self._state.nozzle_configuration_by_id[ - state_update.pipette_config.pipette_id - ] = config.nozzle_map - + def _set_load_pipette(self, state_update: update_types.StateUpdate) -> None: if state_update.loaded_pipette != update_types.NO_CHANGE: pipette_id = state_update.loaded_pipette.pipette_id @@ -214,15 +170,10 @@ def _load_pipette_or_update_config( self._state.aspirated_volume_by_id[pipette_id] = None self._state.movement_speed_by_id[pipette_id] = None self._state.attached_tip_by_id[pipette_id] = None - - # This should always be truthy because LoadPipetteImplementation always - # supplies state_update.pipette_config, which we process above, - # populating self._state.static_config_by_id[pipette_id]. - # - # todo(mm, 2024-10-14): Formalize this expectation by having - # state_update.loaded_pipette incorporate all the stuff in - # state_update.pipette_config, so LoadPipetteImplementation only has to - # supply state_update.loaded_pipette. + # TODO(mm, 2024-10-11): This seems wrong--because of the order in which + # we process the individual attributes of StateUpdate, when we process a + # loadPipette command, won't we always reach here before static_config is + # populated? static_config = self._state.static_config_by_id.get(pipette_id) if static_config: self._state.nozzle_configuration_by_id[ @@ -315,6 +266,50 @@ def _update_current_location(self, state_update: update_types.StateUpdate) -> No mount=loaded_pipette.mount, deck_point=new_deck_point ) + def _update_pipette_config(self, state_update: update_types.StateUpdate) -> None: + if state_update.pipette_config != update_types.NO_CHANGE: + config = state_update.pipette_config.config + self._state.static_config_by_id[ + state_update.pipette_config.pipette_id + ] = StaticPipetteConfig( + serial_number=state_update.pipette_config.serial_number, + model=config.model, + display_name=config.display_name, + min_volume=config.min_volume, + max_volume=config.max_volume, + channels=config.channels, + tip_configuration_lookup_table=config.tip_configuration_lookup_table, + nominal_tip_overlap=config.nominal_tip_overlap, + home_position=config.home_position, + nozzle_offset_z=config.nozzle_offset_z, + pipette_bounding_box_offsets=PipetteBoundingBoxOffsets( + back_left_corner=config.back_left_corner_offset, + front_right_corner=config.front_right_corner_offset, + back_right_corner=Point( + config.front_right_corner_offset.x, + config.back_left_corner_offset.y, + config.back_left_corner_offset.z, + ), + front_left_corner=Point( + config.back_left_corner_offset.x, + config.front_right_corner_offset.y, + config.back_left_corner_offset.z, + ), + ), + bounding_nozzle_offsets=BoundingNozzlesOffsets( + back_left_offset=config.nozzle_map.back_left_nozzle_offset, + front_right_offset=config.nozzle_map.front_right_nozzle_offset, + ), + default_nozzle_map=config.nozzle_map, + lld_settings=config.pipette_lld_settings, + ) + self._state.flow_rates_by_id[ + state_update.pipette_config.pipette_id + ] = config.flow_rates + self._state.nozzle_configuration_by_id[ + state_update.pipette_config.pipette_id + ] = config.nozzle_map + def _update_pipette_nozzle_map( self, state_update: update_types.StateUpdate ) -> None: From 61aaec7b1065c16bbf79197dd2a7db946a660395 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 14:42:51 -0400 Subject: [PATCH 09/10] Delete apparently dead code. --- api/src/opentrons/protocol_engine/state/pipettes.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index d685e8926fd8..ced8b6076f7d 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -96,7 +96,7 @@ class StaticPipetteConfig: nozzle_offset_z: float pipette_bounding_box_offsets: PipetteBoundingBoxOffsets bounding_nozzle_offsets: BoundingNozzlesOffsets - default_nozzle_map: NozzleMap + default_nozzle_map: NozzleMap # todo(mm, 2024-10-14): unused, remove? lld_settings: Optional[Dict[str, Dict[str, float]]] @@ -170,15 +170,6 @@ def _set_load_pipette(self, state_update: update_types.StateUpdate) -> None: self._state.aspirated_volume_by_id[pipette_id] = None self._state.movement_speed_by_id[pipette_id] = None self._state.attached_tip_by_id[pipette_id] = None - # TODO(mm, 2024-10-11): This seems wrong--because of the order in which - # we process the individual attributes of StateUpdate, when we process a - # loadPipette command, won't we always reach here before static_config is - # populated? - static_config = self._state.static_config_by_id.get(pipette_id) - if static_config: - self._state.nozzle_configuration_by_id[ - pipette_id - ] = static_config.default_nozzle_map def _update_tip_state(self, state_update: update_types.StateUpdate) -> None: if state_update.pipette_tip_state != update_types.NO_CHANGE: From c4941052f342091d390e9ba6639caa8011afb30d Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 14 Oct 2024 15:33:15 -0400 Subject: [PATCH 10/10] StateUpdate docstrings. --- .../protocol_engine/state/update_types.py | 50 +++++++++++++------ 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/update_types.py b/api/src/opentrons/protocol_engine/state/update_types.py index 0bf00cfdd86a..5d941d339335 100644 --- a/api/src/opentrons/protocol_engine/state/update_types.py +++ b/api/src/opentrons/protocol_engine/state/update_types.py @@ -66,9 +66,10 @@ class AddressableArea: @dataclasses.dataclass class PipetteLocationUpdate: - """Represents an update to perform on a pipette's location.""" + """An update to a pipette's location.""" pipette_id: str + """The ID of the already-loaded pipette.""" new_location: Well | AddressableArea | None | NoChangeType """The pipette's new logical location. @@ -82,19 +83,30 @@ class PipetteLocationUpdate: @dataclasses.dataclass class LabwareLocationUpdate: - """Represents an update to perform on a labware's location.""" + """An update to a labware's location.""" labware_id: str + """The ID of the already-loaded labware.""" new_location: LabwareLocation - """The labware's new logical location.""" + """The labware's new location.""" offset_id: typing.Optional[str] + """The ID of the labware's new offset, for its new location.""" @dataclasses.dataclass -class LoadedLabwareUpdate(LabwareLocationUpdate): - """Update loaded labware.""" +class LoadedLabwareUpdate: + """An update that loads a new labware.""" + + labware_id: str + """The unique ID of the new labware.""" + + new_location: LabwareLocation + """The labware's initial location.""" + + offset_id: typing.Optional[str] + """The ID of the labware's offset.""" display_name: typing.Optional[str] @@ -103,9 +115,15 @@ class LoadedLabwareUpdate(LabwareLocationUpdate): @dataclasses.dataclass class LoadPipetteUpdate: - """Update loaded pipette.""" + """An update that loads a new pipette. + + NOTE: Currently, if this is provided, a PipetteConfigUpdate must always be + provided alongside it to fully initialize everything. + """ pipette_id: str + """The unique ID of the new pipette.""" + pipette_name: PipetteNameType mount: MountType liquid_presence_detection: typing.Optional[bool] @@ -113,10 +131,14 @@ class LoadPipetteUpdate: @dataclasses.dataclass class PipetteConfigUpdate: - """Update pipette config.""" + """An update to a pipette's config.""" pipette_id: str + """The ID of the already-loaded pipette.""" + + # todo(mm, 2024-10-14): Does serial_number belong in LoadPipetteUpdate? serial_number: str + config: pipette_data_provider.LoadedStaticPipetteData @@ -237,7 +259,7 @@ def set_labware_location( new_location: LabwareLocation, new_offset_id: str | None, ) -> None: - """Set labware location.""" + """Set a labware's location. See `LabwareLocationUpdate`.""" self.labware_location = LabwareLocationUpdate( labware_id=labware_id, new_location=new_location, @@ -252,7 +274,7 @@ def set_loaded_labware( display_name: typing.Optional[str], location: LabwareLocation, ) -> None: - """Add loaded labware to state.""" + """Add a new labware to state. See `LoadedLabwareUpdate`.""" self.loaded_labware = LoadedLabwareUpdate( definition=definition, labware_id=labware_id, @@ -268,7 +290,7 @@ def set_load_pipette( mount: MountType, liquid_presence_detection: typing.Optional[bool], ) -> None: - """Add loaded pipette to state.""" + """Add a new pipette to state. See `LoadPipetteUpdate`.""" self.loaded_pipette = LoadPipetteUpdate( pipette_id=pipette_id, pipette_name=pipette_name, @@ -282,13 +304,13 @@ def update_pipette_config( config: pipette_data_provider.LoadedStaticPipetteData, serial_number: str, ) -> None: - """Update pipette config.""" + """Update a pipette's config. See `PipetteConfigUpdate`.""" self.pipette_config = PipetteConfigUpdate( pipette_id=pipette_id, config=config, serial_number=serial_number ) def update_pipette_nozzle(self, pipette_id: str, nozzle_map: NozzleMap) -> None: - """Update pipette nozzle map.""" + """Update a pipette's nozzle map. See `PipetteNozzleMapUpdate`.""" self.pipette_nozzle_map = PipetteNozzleMapUpdate( pipette_id=pipette_id, nozzle_map=nozzle_map ) @@ -296,7 +318,7 @@ def update_pipette_nozzle(self, pipette_id: str, nozzle_map: NozzleMap) -> None: def update_pipette_tip_state( self, pipette_id: str, tip_geometry: typing.Optional[TipGeometry] ) -> None: - """Update tip state.""" + """Update a pipette's tip state. See `PipetteTipStateUpdate`.""" self.pipette_tip_state = PipetteTipStateUpdate( pipette_id=pipette_id, tip_geometry=tip_geometry ) @@ -304,7 +326,7 @@ def update_pipette_tip_state( def mark_tips_as_used( self, pipette_id: str, labware_id: str, well_name: str ) -> None: - """Mark tips in a tip rack as used. See `MarkTipsUsedState`.""" + """Mark tips in a tip rack as used. See `TipsUsedUpdate`.""" self.tips_used = TipsUsedUpdate( pipette_id=pipette_id, labware_id=labware_id, well_name=well_name )