From 2ab0dddab065a0c969f86215a86fcd746be9bb86 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 15:08:33 -0400 Subject: [PATCH 01/17] feat(api): build out well state for liquid height tracking Add a well state class that will be used for now just for liquid height tracking but in the future could be built out more. re EXEC-603 --- api/src/opentrons/cli/analyze.py | 1 + .../opentrons/protocol_engine/state/state.py | 14 +++ .../protocol_engine/state/state_summary.py | 1 + .../opentrons/protocol_engine/state/wells.py | 96 +++++++++++++++++++ api/src/opentrons/protocol_engine/types.py | 7 ++ 5 files changed, 119 insertions(+) create mode 100644 api/src/opentrons/protocol_engine/state/wells.py diff --git a/api/src/opentrons/cli/analyze.py b/api/src/opentrons/cli/analyze.py index 0daacd711b3..9c1a0b825ee 100644 --- a/api/src/opentrons/cli/analyze.py +++ b/api/src/opentrons/cli/analyze.py @@ -275,6 +275,7 @@ async def _do_analyze(protocol_source: ProtocolSource) -> RunResult: modules=[], labwareOffsets=[], liquids=[], + wells=[], ), parameters=[], ) diff --git a/api/src/opentrons/protocol_engine/state/state.py b/api/src/opentrons/protocol_engine/state/state.py index e0d04b3d050..46203e90a1a 100644 --- a/api/src/opentrons/protocol_engine/state/state.py +++ b/api/src/opentrons/protocol_engine/state/state.py @@ -25,6 +25,7 @@ from .modules import ModuleState, ModuleStore, ModuleView from .liquids import LiquidState, LiquidView, LiquidStore from .tips import TipState, TipView, TipStore +from .wells import WellState, WellView, WellStore from .geometry import GeometryView from .motion import MotionView from .config import Config @@ -47,6 +48,7 @@ class State: modules: ModuleState liquids: LiquidState tips: TipState + wells: WellState class StateView(HasState[State]): @@ -60,6 +62,7 @@ class StateView(HasState[State]): _modules: ModuleView _liquid: LiquidView _tips: TipView + _wells: WellView _geometry: GeometryView _motion: MotionView _config: Config @@ -99,6 +102,11 @@ def tips(self) -> TipView: """Get state view selectors for tip state.""" return self._tips + @property + def wells(self) -> WellView: + """Get state view selectors for well state.""" + return self._wells + @property def geometry(self) -> GeometryView: """Get state view selectors for derived geometry state.""" @@ -128,6 +136,7 @@ def get_summary(self) -> StateSummary: completedAt=self._state.commands.run_completed_at, startedAt=self._state.commands.run_started_at, liquids=self._liquid.get_all(), + wells=self._wells.get_all(), ) @@ -187,6 +196,7 @@ def __init__( ) self._liquid_store = LiquidStore() self._tip_store = TipStore() + self._well_store = WellStore() self._substores: List[HandlesActions] = [ self._command_store, @@ -196,6 +206,7 @@ def __init__( self._module_store, self._liquid_store, self._tip_store, + self._well_store, ] self._config = config self._change_notifier = change_notifier or ChangeNotifier() @@ -312,6 +323,7 @@ def _get_next_state(self) -> State: modules=self._module_store.state, liquids=self._liquid_store.state, tips=self._tip_store.state, + wells=self._well_store.state, ) def _initialize_state(self) -> None: @@ -327,6 +339,7 @@ def _initialize_state(self) -> None: self._modules = ModuleView(state.modules) self._liquid = LiquidView(state.liquids) self._tips = TipView(state.tips) + self._wells = WellView(state.wells) # Derived states self._geometry = GeometryView( @@ -356,6 +369,7 @@ def _update_state_views(self) -> None: self._modules._state = next_state.modules self._liquid._state = next_state.liquids self._tips._state = next_state.tips + self._wells._state = next_state.wells self._change_notifier.notify() if self._notify_robot_server is not None: self._notify_robot_server() diff --git a/api/src/opentrons/protocol_engine/state/state_summary.py b/api/src/opentrons/protocol_engine/state/state_summary.py index c7185cc2c0d..aa1c9ddae73 100644 --- a/api/src/opentrons/protocol_engine/state/state_summary.py +++ b/api/src/opentrons/protocol_engine/state/state_summary.py @@ -28,3 +28,4 @@ class StateSummary(BaseModel): startedAt: Optional[datetime] completedAt: Optional[datetime] liquids: List[Liquid] = Field(default_factory=list) + wells: List[float] diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py new file mode 100644 index 00000000000..91dab3ef7a8 --- /dev/null +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -0,0 +1,96 @@ +"""Basic well data state and store.""" +from dataclasses import dataclass +from typing import Dict, List, Optional +from opentrons.protocol_engine.actions.actions import ( + FailCommandAction, + SucceedCommandAction, +) +from opentrons.protocol_engine.commands.liquid_probe import LiquidProbe +from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError +from opentrons.protocol_engine.types import WellDetails + +from .abstract_store import HasState, HandlesActions +from ..actions import Action +from ..commands import Command + + +@dataclass +class WellState: + """State of all wells.""" + + current_liquid_heights: Dict[WellDetails, float] + + +class WellStore(HasState[WellState], HandlesActions): + """Well state container.""" + + _state: WellState + + def __init__(self) -> None: + """Initialize a well store and its state.""" + self._state = WellState(current_liquid_heights={}) + + def handle_action(self, action: Action) -> None: + """Modify state in reaction to an action.""" + if isinstance(action, SucceedCommandAction): + self._handle_succeded_command(action.command) + if isinstance(action, FailCommandAction): + self._handle_failed_command(action) + + def _handle_succeded_command(self, command: Command) -> None: + if isinstance(command, LiquidProbe): + well = WellDetails( + labware_id=command.params.labwareId, well_name=command.params.wellName + ) + if command.result is None: + self._set_liquid_height(well=well, height=None) + else: + self._set_liquid_height(well=well, height=command.result.z_position) + + def _handle_failed_command(self, action: FailCommandAction) -> None: + if isinstance(action.error, LiquidNotFoundError): + self._set_liquid_height(None) + + def _set_liquid_height(self, well: WellDetails, height: Optional[float]) -> None: + """Set the liquid height of the well.""" + if height is None: + del self._state.current_liquid_heights[well] + else: + self._state.current_liquid_heights[well] = height + + +class WellView(HasState[WellState]): + """Read-only well state view.""" + + _state: WellState + + def __init__(self, state: WellState) -> None: + """Initialize the computed view of well state. + + Arguments: + state: Well state dataclass used for all calculations. + """ + self._state = state + + def get_all(self) -> List[float]: + """Get all well liquid heights.""" + return list(self._state.current_liquid_heights.values()) + + def get_last_measured_liquid_height(self, well: WellDetails) -> Optional[float]: + """Returns the height of the liquid according to the most recent liquid level probe to this well. + + Returns None if no liquid probe has been done. + """ + try: + height = self._state.current_liquid_heights[well] + return height + except KeyError: + return None + + def has_measured_liquid_height(self, well: WellDetails) -> bool: + """Returns True if the well has been liquid level probed previously.""" + try: + height = self._state.current_liquid_heights[well] + return height is not None + except KeyError: + return False diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 9da73149043..190316001f5 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -310,6 +310,13 @@ class CurrentWell: well_name: str +class WellDetails(BaseModel): + """Identifying information needed for well state tracking.""" + + labware_id: str + well_name: str + + @dataclass(frozen=True) class CurrentAddressableArea: """The latest addressable area the robot has accessed.""" From 8edc9f213e142dba697555128324a4db9bc6c231 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 15:19:46 -0400 Subject: [PATCH 02/17] lint --- .../maintenance_runs/maintenance_run_data_manager.py | 1 + robot-server/robot_server/runs/run_data_manager.py | 1 + robot-server/tests/protocols/test_protocol_analyzer.py | 1 + robot-server/tests/runs/test_run_controller.py | 1 + robot-server/tests/runs/test_run_store.py | 2 ++ 5 files changed, 6 insertions(+) diff --git a/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py b/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py index 7b255a6f79d..18505bf203f 100644 --- a/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py +++ b/robot-server/robot_server/maintenance_runs/maintenance_run_data_manager.py @@ -32,6 +32,7 @@ def _build_run( pipettes=[], modules=[], liquids=[], + wells=[], ) return MaintenanceRun.construct( id=run_id, diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index 5ae7581d952..dce6815136d 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -65,6 +65,7 @@ def _build_run( pipettes=[], modules=[], liquids=[], + wells=[], ) errors.append(state_summary.dataError) else: diff --git a/robot-server/tests/protocols/test_protocol_analyzer.py b/robot-server/tests/protocols/test_protocol_analyzer.py index 8b55f05840b..6eab8bf5130 100644 --- a/robot-server/tests/protocols/test_protocol_analyzer.py +++ b/robot-server/tests/protocols/test_protocol_analyzer.py @@ -189,6 +189,7 @@ async def test_analyze( modules=[], labwareOffsets=[], liquids=[], + wells=[], ), parameters=[bool_parameter], ) diff --git a/robot-server/tests/runs/test_run_controller.py b/robot-server/tests/runs/test_run_controller.py index c6d58b229dc..90e0e439c32 100644 --- a/robot-server/tests/runs/test_run_controller.py +++ b/robot-server/tests/runs/test_run_controller.py @@ -65,6 +65,7 @@ def engine_state_summary() -> StateSummary: pipettes=[], modules=[], liquids=[], + wells=[], ) diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 94899c5c20e..e775a2a5a20 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -117,6 +117,7 @@ def state_summary() -> StateSummary: labwareOffsets=[], status=EngineStatus.IDLE, liquids=liquids, + wells=[], ) @@ -197,6 +198,7 @@ def invalid_state_summary() -> StateSummary: labwareOffsets=[], status=EngineStatus.IDLE, liquids=liquids, + wells=[], ) From 55d1954b80d91acbd4bd7fc259a75efac7994925 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:14:46 -0400 Subject: [PATCH 03/17] added tests --- api/src/opentrons/protocol_engine/types.py | 4 ++ .../protocol_engine/state/command_fixtures.py | 26 ++++++++++++ .../protocol_engine/state/test_well_store.py | 28 +++++++++++++ .../protocol_engine/state/test_well_view.py | 40 +++++++++++++++++++ .../tests/runs/test_run_data_manager.py | 6 +++ 5 files changed, 104 insertions(+) create mode 100644 api/tests/opentrons/protocol_engine/state/test_well_store.py create mode 100644 api/tests/opentrons/protocol_engine/state/test_well_view.py diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 190316001f5..861b6cd43d2 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -316,6 +316,10 @@ class WellDetails(BaseModel): labware_id: str well_name: str + def __hash__(self) -> int: + """Needed to make WellDetails a key in WellState dictionaries.""" + return hash((self.labware_id, self.well_name)) + @dataclass(frozen=True) class CurrentAddressableArea: diff --git a/api/tests/opentrons/protocol_engine/state/command_fixtures.py b/api/tests/opentrons/protocol_engine/state/command_fixtures.py index 66c6a34fe9f..577292e7eb6 100644 --- a/api/tests/opentrons/protocol_engine/state/command_fixtures.py +++ b/api/tests/opentrons/protocol_engine/state/command_fixtures.py @@ -295,6 +295,32 @@ def create_dispense_in_place_command( ) +def create_liquid_probe_command( + pipette_id: str = "pippete-id", + labware_id: str = "labware-id", + well_name: str = "well-name", + well_location: Optional[WellLocation] = None, + destination: DeckPoint = DeckPoint(x=0, y=0, z=0), +) -> cmd.LiquidProbe: + """Get a completed Liquid Probe command.""" + params = cmd.LiquidProbeParams( + pipetteId=pipette_id, + labwareId=labware_id, + wellName=well_name, + wellLocation=well_location or WellLocation(), + ) + result = cmd.LiquidProbeResult(position=destination, z_position=0.5) + + return cmd.LiquidProbe( + id="command-id", + key="command-key", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + params=params, + result=result, + ) + + def create_pick_up_tip_command( pipette_id: str, labware_id: str = "labware-id", diff --git a/api/tests/opentrons/protocol_engine/state/test_well_store.py b/api/tests/opentrons/protocol_engine/state/test_well_store.py new file mode 100644 index 00000000000..55d014333cb --- /dev/null +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -0,0 +1,28 @@ +"""Liquid state store tests.""" +import pytest +from opentrons.protocol_engine.state.wells import WellStore +from opentrons.protocol_engine.types import WellDetails +from opentrons.protocol_engine.actions.actions import SucceedCommandAction + +from .command_fixtures import create_liquid_probe_command + + +@pytest.fixture +def subject() -> WellStore: + """Liquid store test subject.""" + return WellStore() + + +def test_handles_liquid_probe_success(subject: WellStore) -> None: + """It should add the well to the state after a successful liquid probe.""" + well_details = WellDetails(labware_id="labware-id", well_name="well-name") + + liquid_probe = create_liquid_probe_command() + + subject.handle_action( + SucceedCommandAction(private_result=None, command=liquid_probe) + ) + + assert len(subject.state.current_liquid_heights) == 1 + + assert subject.state.current_liquid_heights[well_details] == 0.5 diff --git a/api/tests/opentrons/protocol_engine/state/test_well_view.py b/api/tests/opentrons/protocol_engine/state/test_well_view.py new file mode 100644 index 00000000000..102ac3ff4bb --- /dev/null +++ b/api/tests/opentrons/protocol_engine/state/test_well_view.py @@ -0,0 +1,40 @@ +"""Well view tests.""" +from opentrons.protocol_engine.types import WellDetails +import pytest +from opentrons.protocol_engine.state.wells import WellState, WellView + + +@pytest.fixture +def subject() -> WellView: + """Get a well view test subject.""" + well_details = WellDetails(labware_id="labware-id", well_name="well-name") + state = WellState(current_liquid_heights={well_details: 0.5}) + + return WellView(state) + + +def test_get_all(subject: WellView) -> None: + """Should return a list of well heights.""" + assert subject.get_all() == [0.5] + + +def test_get_last_measured_liquid_height(subject: WellView) -> None: + """Should return the height of a single well correctly for valid wells.""" + valid_details = WellDetails(labware_id="labware-id", well_name="well-name") + + invalid_details = WellDetails( + labware_id="wrong-labware-id", well_name="wrong-well-name" + ) + assert subject.get_last_measured_liquid_height(invalid_details) is None + assert subject.get_last_measured_liquid_height(valid_details) == 0.5 + + +def test_has_measured_liquid_height(subject: WellView) -> None: + """Should return True for measured wells and False for ones that have no measurements.""" + valid_details = WellDetails(labware_id="labware-id", well_name="well-name") + + invalid_details = WellDetails( + labware_id="wrong-labware-id", well_name="wrong-well-name" + ) + assert subject.has_measured_liquid_height(invalid_details) is False + assert subject.has_measured_liquid_height(valid_details) is True diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index ec7a8bef69e..11b43eb0c54 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -198,6 +198,7 @@ async def test_create( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, + wells=engine_state_summary.wells, ) @@ -269,6 +270,7 @@ async def test_create_with_options( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, + wells=engine_state_summary.wells, ) @@ -350,6 +352,7 @@ async def test_get_current_run( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, + wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) assert subject.current_run_id == run_id @@ -391,6 +394,7 @@ async def test_get_historical_run( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, + wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) @@ -633,6 +637,7 @@ async def test_update_current( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, + wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) @@ -688,6 +693,7 @@ async def test_update_current_noop( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, + wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) From f74cfa3f73fb1cf8ac49cf2af6a7ecf3fc51e936 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:15:54 -0400 Subject: [PATCH 04/17] docstring fix --- api/tests/opentrons/protocol_engine/state/test_well_store.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/state/test_well_store.py b/api/tests/opentrons/protocol_engine/state/test_well_store.py index 55d014333cb..58368104adc 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -1,4 +1,4 @@ -"""Liquid state store tests.""" +"""Well state store tests.""" import pytest from opentrons.protocol_engine.state.wells import WellStore from opentrons.protocol_engine.types import WellDetails @@ -9,7 +9,7 @@ @pytest.fixture def subject() -> WellStore: - """Liquid store test subject.""" + """Well store test subject.""" return WellStore() From 036094ed02a4738695db44111161152615744e8f Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:21:38 -0400 Subject: [PATCH 05/17] lint fix? --- robot-server/tests/runs/test_run_data_manager.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index 11b43eb0c54..ec7a8bef69e 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -198,7 +198,6 @@ async def test_create( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, - wells=engine_state_summary.wells, ) @@ -270,7 +269,6 @@ async def test_create_with_options( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, - wells=engine_state_summary.wells, ) @@ -352,7 +350,6 @@ async def test_get_current_run( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, - wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) assert subject.current_run_id == run_id @@ -394,7 +391,6 @@ async def test_get_historical_run( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, - wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) @@ -637,7 +633,6 @@ async def test_update_current( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, - wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) @@ -693,7 +688,6 @@ async def test_update_current_noop( pipettes=engine_state_summary.pipettes, modules=engine_state_summary.modules, liquids=engine_state_summary.liquids, - wells=engine_state_summary.wells, runTimeParameters=run_time_parameters, ) From 537007e15e6e4bc203578391acb56a8813358c81 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:28:57 -0400 Subject: [PATCH 06/17] test cov --- robot-server/tests/runs/test_run_data_manager.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index ec7a8bef69e..26efbc8c6c2 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -89,6 +89,7 @@ def engine_state_summary() -> StateSummary: pipettes=[LoadedPipette.construct(id="some-pipette-id")], # type: ignore[call-arg] modules=[LoadedModule.construct(id="some-module-id")], # type: ignore[call-arg] liquids=[Liquid(id="some-liquid-id", displayName="liquid", description="desc")], + wells=[], ) @@ -452,6 +453,7 @@ async def test_get_all_runs( pipettes=[LoadedPipette.construct(id="current-pipette-id")], # type: ignore[call-arg] modules=[LoadedModule.construct(id="current-module-id")], # type: ignore[call-arg] liquids=[Liquid(id="some-liquid-id", displayName="liquid", description="desc")], + wells=[], ) current_run_time_parameters: List[pe_types.RunTimeParameter] = [ pe_types.BooleanParameter( @@ -470,6 +472,7 @@ async def test_get_all_runs( pipettes=[LoadedPipette.construct(id="old-pipette-id")], # type: ignore[call-arg] modules=[LoadedModule.construct(id="old-module-id")], # type: ignore[call-arg] liquids=[], + wells=[] ) historical_run_time_parameters: List[pe_types.RunTimeParameter] = [ pe_types.BooleanParameter( From 7f2e7c4d1e023eb502e904d2d3349daefaf40a52 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:32:42 -0400 Subject: [PATCH 07/17] oops formatting --- robot-server/tests/runs/test_run_data_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index 26efbc8c6c2..6c7852cbf21 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -472,7 +472,7 @@ async def test_get_all_runs( pipettes=[LoadedPipette.construct(id="old-pipette-id")], # type: ignore[call-arg] modules=[LoadedModule.construct(id="old-module-id")], # type: ignore[call-arg] liquids=[], - wells=[] + wells=[], ) historical_run_time_parameters: List[pe_types.RunTimeParameter] = [ pe_types.BooleanParameter( From b74d83e62040b9d2179bb03df83771ae1606de45 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:48:53 -0400 Subject: [PATCH 08/17] last robot server cov fix --- robot-server/tests/maintenance_runs/test_run_data_manager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/robot-server/tests/maintenance_runs/test_run_data_manager.py b/robot-server/tests/maintenance_runs/test_run_data_manager.py index f1127f287fb..34f2c603513 100644 --- a/robot-server/tests/maintenance_runs/test_run_data_manager.py +++ b/robot-server/tests/maintenance_runs/test_run_data_manager.py @@ -68,6 +68,7 @@ def engine_state_summary() -> StateSummary: pipettes=[LoadedPipette.construct(id="some-pipette-id")], # type: ignore[call-arg] modules=[LoadedModule.construct(id="some-module-id")], # type: ignore[call-arg] liquids=[Liquid(id="some-liquid-id", displayName="liquid", description="desc")], + wells=[] ) From 6ab369096270693068e5d7d08c823db662804cb7 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 16 Jul 2024 16:52:18 -0400 Subject: [PATCH 09/17] formatting again --- robot-server/tests/maintenance_runs/test_run_data_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/tests/maintenance_runs/test_run_data_manager.py b/robot-server/tests/maintenance_runs/test_run_data_manager.py index 34f2c603513..f28beb985dd 100644 --- a/robot-server/tests/maintenance_runs/test_run_data_manager.py +++ b/robot-server/tests/maintenance_runs/test_run_data_manager.py @@ -68,7 +68,7 @@ def engine_state_summary() -> StateSummary: pipettes=[LoadedPipette.construct(id="some-pipette-id")], # type: ignore[call-arg] modules=[LoadedModule.construct(id="some-module-id")], # type: ignore[call-arg] liquids=[Liquid(id="some-liquid-id", displayName="liquid", description="desc")], - wells=[] + wells=[], ) From b2ee8f344893f4d15f2b03edb4e1bd2a191815c6 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Wed, 17 Jul 2024 10:31:05 -0400 Subject: [PATCH 10/17] added time to wellstate --- .../protocol_engine/state/state_summary.py | 3 +- .../opentrons/protocol_engine/state/wells.py | 42 +++++++++++-------- api/src/opentrons/protocol_engine/types.py | 11 ++++- .../protocol_engine/state/test_well_store.py | 8 ++-- .../protocol_engine/state/test_well_view.py | 18 ++++---- 5 files changed, 50 insertions(+), 32 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/state_summary.py b/api/src/opentrons/protocol_engine/state/state_summary.py index aa1c9ddae73..ca3458929dc 100644 --- a/api/src/opentrons/protocol_engine/state/state_summary.py +++ b/api/src/opentrons/protocol_engine/state/state_summary.py @@ -11,6 +11,7 @@ LoadedModule, LoadedPipette, Liquid, + LiquidHeightInfo, ) @@ -28,4 +29,4 @@ class StateSummary(BaseModel): startedAt: Optional[datetime] completedAt: Optional[datetime] liquids: List[Liquid] = Field(default_factory=list) - wells: List[float] + wells: List[LiquidHeightInfo] = Field(default_factor=list) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 91dab3ef7a8..1942b6bcbde 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,5 +1,6 @@ """Basic well data state and store.""" from dataclasses import dataclass +from datetime import datetime from typing import Dict, List, Optional from opentrons.protocol_engine.actions.actions import ( FailCommandAction, @@ -7,7 +8,7 @@ ) from opentrons.protocol_engine.commands.liquid_probe import LiquidProbe from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError -from opentrons.protocol_engine.types import WellDetails +from opentrons.protocol_engine.types import WellIdentifier, LiquidHeightInfo from .abstract_store import HasState, HandlesActions from ..actions import Action @@ -18,7 +19,7 @@ class WellState: """State of all wells.""" - current_liquid_heights: Dict[WellDetails, float] + measured_liquid_heights: Dict[WellIdentifier, LiquidHeightInfo] class WellStore(HasState[WellState], HandlesActions): @@ -28,7 +29,7 @@ class WellStore(HasState[WellState], HandlesActions): def __init__(self) -> None: """Initialize a well store and its state.""" - self._state = WellState(current_liquid_heights={}) + self._state = WellState(measured_liquid_heights={}) def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" @@ -39,24 +40,31 @@ def handle_action(self, action: Action) -> None: def _handle_succeded_command(self, command: Command) -> None: if isinstance(command, LiquidProbe): - well = WellDetails( + well = WellIdentifier( labware_id=command.params.labwareId, well_name=command.params.wellName ) if command.result is None: - self._set_liquid_height(well=well, height=None) + self._set_liquid_height(well=well, height=None, time=None) else: - self._set_liquid_height(well=well, height=command.result.z_position) + self._set_liquid_height( + well=well, + height=command.result.z_position, + time=command.createdAt, + ) def _handle_failed_command(self, action: FailCommandAction) -> None: if isinstance(action.error, LiquidNotFoundError): - self._set_liquid_height(None) + self._set_liquid_height(0) - def _set_liquid_height(self, well: WellDetails, height: Optional[float]) -> None: + def _set_liquid_height( + self, well: WellIdentifier, height: Optional[float], time: Optional[datetime] + ) -> None: """Set the liquid height of the well.""" - if height is None: - del self._state.current_liquid_heights[well] + if height is None or time is None: + del self._state.measured_liquid_heights[well] else: - self._state.current_liquid_heights[well] = height + lhi = LiquidHeightInfo(height=height, last_measured=time) + self._state.measured_liquid_heights[well] = lhi class WellView(HasState[WellState]): @@ -72,25 +80,25 @@ def __init__(self, state: WellState) -> None: """ self._state = state - def get_all(self) -> List[float]: + def get_all(self) -> List[LiquidHeightInfo]: """Get all well liquid heights.""" - return list(self._state.current_liquid_heights.values()) + return list(self._state.measured_liquid_heights.values()) - def get_last_measured_liquid_height(self, well: WellDetails) -> Optional[float]: + def get_last_measured_liquid_height(self, well: WellIdentifier) -> Optional[float]: """Returns the height of the liquid according to the most recent liquid level probe to this well. Returns None if no liquid probe has been done. """ try: - height = self._state.current_liquid_heights[well] + height = self._state.measured_liquid_heights[well].height return height except KeyError: return None - def has_measured_liquid_height(self, well: WellDetails) -> bool: + def has_measured_liquid_height(self, well: WellIdentifier) -> bool: """Returns True if the well has been liquid level probed previously.""" try: - height = self._state.current_liquid_heights[well] + height = self._state.measured_liquid_heights[well].height return height is not None except KeyError: return False diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 861b6cd43d2..1ea82c9be83 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -310,17 +310,24 @@ class CurrentWell: well_name: str -class WellDetails(BaseModel): +class WellIdentifier(BaseModel): """Identifying information needed for well state tracking.""" labware_id: str well_name: str def __hash__(self) -> int: - """Needed to make WellDetails a key in WellState dictionaries.""" + """Needed to make WellIdentifier a key in dictionaries.""" return hash((self.labware_id, self.well_name)) +class LiquidHeightInfo(BaseModel): + """Payload required to store recent measured liquid heights.""" + + height: float + last_measured: datetime + + @dataclass(frozen=True) class CurrentAddressableArea: """The latest addressable area the robot has accessed.""" diff --git a/api/tests/opentrons/protocol_engine/state/test_well_store.py b/api/tests/opentrons/protocol_engine/state/test_well_store.py index 58368104adc..93ab4a161bb 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -1,7 +1,7 @@ """Well state store tests.""" import pytest from opentrons.protocol_engine.state.wells import WellStore -from opentrons.protocol_engine.types import WellDetails +from opentrons.protocol_engine.types import WellIdentifier from opentrons.protocol_engine.actions.actions import SucceedCommandAction from .command_fixtures import create_liquid_probe_command @@ -15,7 +15,7 @@ def subject() -> WellStore: def test_handles_liquid_probe_success(subject: WellStore) -> None: """It should add the well to the state after a successful liquid probe.""" - well_details = WellDetails(labware_id="labware-id", well_name="well-name") + well_id = WellIdentifier(labware_id="labware-id", well_name="well-name") liquid_probe = create_liquid_probe_command() @@ -23,6 +23,6 @@ def test_handles_liquid_probe_success(subject: WellStore) -> None: SucceedCommandAction(private_result=None, command=liquid_probe) ) - assert len(subject.state.current_liquid_heights) == 1 + assert len(subject.state.measured_liquid_heights) == 1 - assert subject.state.current_liquid_heights[well_details] == 0.5 + assert subject.state.measured_liquid_heights[well_id].height == 0.5 diff --git a/api/tests/opentrons/protocol_engine/state/test_well_view.py b/api/tests/opentrons/protocol_engine/state/test_well_view.py index 102ac3ff4bb..5ee70b13bda 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_view.py @@ -1,5 +1,6 @@ """Well view tests.""" -from opentrons.protocol_engine.types import WellDetails +from datetime import datetime +from opentrons.protocol_engine.types import LiquidHeightInfo, WellIdentifier import pytest from opentrons.protocol_engine.state.wells import WellState, WellView @@ -7,22 +8,23 @@ @pytest.fixture def subject() -> WellView: """Get a well view test subject.""" - well_details = WellDetails(labware_id="labware-id", well_name="well-name") - state = WellState(current_liquid_heights={well_details: 0.5}) + well_id = WellIdentifier(labware_id="labware-id", well_name="well-name") + height_info = LiquidHeightInfo(height=0.5, last_measured=datetime.now()) + state = WellState(measured_liquid_heights={well_id: height_info}) return WellView(state) def test_get_all(subject: WellView) -> None: """Should return a list of well heights.""" - assert subject.get_all() == [0.5] + assert subject.get_all()[0].height == 0.5 def test_get_last_measured_liquid_height(subject: WellView) -> None: """Should return the height of a single well correctly for valid wells.""" - valid_details = WellDetails(labware_id="labware-id", well_name="well-name") + valid_details = WellIdentifier(labware_id="labware-id", well_name="well-name") - invalid_details = WellDetails( + invalid_details = WellIdentifier( labware_id="wrong-labware-id", well_name="wrong-well-name" ) assert subject.get_last_measured_liquid_height(invalid_details) is None @@ -31,9 +33,9 @@ def test_get_last_measured_liquid_height(subject: WellView) -> None: def test_has_measured_liquid_height(subject: WellView) -> None: """Should return True for measured wells and False for ones that have no measurements.""" - valid_details = WellDetails(labware_id="labware-id", well_name="well-name") + valid_details = WellIdentifier(labware_id="labware-id", well_name="well-name") - invalid_details = WellDetails( + invalid_details = WellIdentifier( labware_id="wrong-labware-id", well_name="wrong-well-name" ) assert subject.has_measured_liquid_height(invalid_details) is False From cd598c111715b9736fe8ed413c8bb7066d40fecb Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Wed, 17 Jul 2024 10:51:36 -0400 Subject: [PATCH 11/17] typo fix --- api/src/opentrons/protocol_engine/state/state_summary.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/state/state_summary.py b/api/src/opentrons/protocol_engine/state/state_summary.py index ca3458929dc..91ddcd40c62 100644 --- a/api/src/opentrons/protocol_engine/state/state_summary.py +++ b/api/src/opentrons/protocol_engine/state/state_summary.py @@ -29,4 +29,4 @@ class StateSummary(BaseModel): startedAt: Optional[datetime] completedAt: Optional[datetime] liquids: List[Liquid] = Field(default_factory=list) - wells: List[LiquidHeightInfo] = Field(default_factor=list) + wells: List[LiquidHeightInfo] = Field(default_factory=list) From 1862f94141820670bc041e92d7195e337314355e Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Wed, 17 Jul 2024 15:47:13 -0400 Subject: [PATCH 12/17] addressing comments. removing WellIdentifier --- .../opentrons/protocol_engine/state/wells.py | 72 +++++++++++-------- api/src/opentrons/protocol_engine/types.py | 11 --- .../protocol_engine/state/test_well_store.py | 6 +- .../protocol_engine/state/test_well_view.py | 35 +++++---- 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 1942b6bcbde..5da00a6b5bb 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,4 +1,5 @@ """Basic well data state and store.""" +import pdb from dataclasses import dataclass from datetime import datetime from typing import Dict, List, Optional @@ -6,9 +7,9 @@ FailCommandAction, SucceedCommandAction, ) -from opentrons.protocol_engine.commands.liquid_probe import LiquidProbe +from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError -from opentrons.protocol_engine.types import WellIdentifier, LiquidHeightInfo +from opentrons.protocol_engine.types import LiquidHeightInfo from .abstract_store import HasState, HandlesActions from ..actions import Action @@ -19,7 +20,8 @@ class WellState: """State of all wells.""" - measured_liquid_heights: Dict[WellIdentifier, LiquidHeightInfo] + # Dict[Labware: Dict[Wellname: [Height,TimeRecorded]]] + measured_liquid_heights: Dict[str, Dict[str, LiquidHeightInfo]] class WellStore(HasState[WellState], HandlesActions): @@ -34,37 +36,38 @@ def __init__(self) -> None: def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" if isinstance(action, SucceedCommandAction): - self._handle_succeded_command(action.command) + self._handle_succeeded_command(action.command) if isinstance(action, FailCommandAction): self._handle_failed_command(action) - def _handle_succeded_command(self, command: Command) -> None: - if isinstance(command, LiquidProbe): - well = WellIdentifier( - labware_id=command.params.labwareId, well_name=command.params.wellName + def _handle_succeeded_command(self, command: Command) -> None: + if isinstance(command.result, LiquidProbeResult): + self._set_liquid_height( + labware_id=command.params.labwareId, + well_name=command.params.wellName, + height=command.result.z_position, + time=command.createdAt, ) - if command.result is None: - self._set_liquid_height(well=well, height=None, time=None) - else: - self._set_liquid_height( - well=well, - height=command.result.z_position, - time=command.createdAt, - ) def _handle_failed_command(self, action: FailCommandAction) -> None: if isinstance(action.error, LiquidNotFoundError): - self._set_liquid_height(0) + self._set_liquid_height( + labware_id=action.error.private.labware_id, + well_name=action.error.private.well_name, + height=0, + time=action.failed_at, + ) def _set_liquid_height( - self, well: WellIdentifier, height: Optional[float], time: Optional[datetime] + self, labware_id: str, well_name: str, height: float, time: datetime ) -> None: """Set the liquid height of the well.""" - if height is None or time is None: - del self._state.measured_liquid_heights[well] - else: - lhi = LiquidHeightInfo(height=height, last_measured=time) - self._state.measured_liquid_heights[well] = lhi + lhi = LiquidHeightInfo(height=height, last_measured=time) + try: + self._state.measured_liquid_heights[labware_id] + except KeyError: + self._state.measured_liquid_heights[labware_id] = {} + self._state.measured_liquid_heights[labware_id][well_name] = lhi class WellView(HasState[WellState]): @@ -82,23 +85,32 @@ def __init__(self, state: WellState) -> None: def get_all(self) -> List[LiquidHeightInfo]: """Get all well liquid heights.""" - return list(self._state.measured_liquid_heights.values()) - - def get_last_measured_liquid_height(self, well: WellIdentifier) -> Optional[float]: + allHeights = [] # type: List[LiquidHeightInfo] + for val in self._state.measured_liquid_heights.values(): + allHeights.extend(a for a in val.values()) + return allHeights + + def get_wells_in_labware(self, labware_id: str) -> List[LiquidHeightInfo]: + """Get all well liquid heights for a particular labware.""" + return list(self._state.measured_liquid_heights[labware_id].values()) + + def get_last_measured_liquid_height( + self, labware_id: str, well_name: str + ) -> Optional[float]: """Returns the height of the liquid according to the most recent liquid level probe to this well. Returns None if no liquid probe has been done. """ try: - height = self._state.measured_liquid_heights[well].height + height = self._state.measured_liquid_heights[labware_id][well_name].height return height except KeyError: return None - def has_measured_liquid_height(self, well: WellIdentifier) -> bool: + def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: """Returns True if the well has been liquid level probed previously.""" try: - height = self._state.measured_liquid_heights[well].height - return height is not None + self._state.measured_liquid_heights[labware_id][well_name].height + return True except KeyError: return False diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 1ea82c9be83..648e6297328 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -310,17 +310,6 @@ class CurrentWell: well_name: str -class WellIdentifier(BaseModel): - """Identifying information needed for well state tracking.""" - - labware_id: str - well_name: str - - def __hash__(self) -> int: - """Needed to make WellIdentifier a key in dictionaries.""" - return hash((self.labware_id, self.well_name)) - - class LiquidHeightInfo(BaseModel): """Payload required to store recent measured liquid heights.""" diff --git a/api/tests/opentrons/protocol_engine/state/test_well_store.py b/api/tests/opentrons/protocol_engine/state/test_well_store.py index 93ab4a161bb..325021a9942 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -1,7 +1,6 @@ """Well state store tests.""" import pytest from opentrons.protocol_engine.state.wells import WellStore -from opentrons.protocol_engine.types import WellIdentifier from opentrons.protocol_engine.actions.actions import SucceedCommandAction from .command_fixtures import create_liquid_probe_command @@ -15,7 +14,8 @@ def subject() -> WellStore: def test_handles_liquid_probe_success(subject: WellStore) -> None: """It should add the well to the state after a successful liquid probe.""" - well_id = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" liquid_probe = create_liquid_probe_command() @@ -25,4 +25,4 @@ def test_handles_liquid_probe_success(subject: WellStore) -> None: assert len(subject.state.measured_liquid_heights) == 1 - assert subject.state.measured_liquid_heights[well_id].height == 0.5 + assert subject.state.measured_liquid_heights[labware_id][well_name].height == 0.5 diff --git a/api/tests/opentrons/protocol_engine/state/test_well_view.py b/api/tests/opentrons/protocol_engine/state/test_well_view.py index 5ee70b13bda..3bd86e9dcb9 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_view.py @@ -1,6 +1,6 @@ """Well view tests.""" from datetime import datetime -from opentrons.protocol_engine.types import LiquidHeightInfo, WellIdentifier +from opentrons.protocol_engine.types import LiquidHeightInfo import pytest from opentrons.protocol_engine.state.wells import WellState, WellView @@ -8,9 +8,10 @@ @pytest.fixture def subject() -> WellView: """Get a well view test subject.""" - well_id = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" height_info = LiquidHeightInfo(height=0.5, last_measured=datetime.now()) - state = WellState(measured_liquid_heights={well_id: height_info}) + state = WellState(measured_liquid_heights={labware_id: {well_name: height_info}}) return WellView(state) @@ -22,21 +23,29 @@ def test_get_all(subject: WellView) -> None: def test_get_last_measured_liquid_height(subject: WellView) -> None: """Should return the height of a single well correctly for valid wells.""" - valid_details = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" - invalid_details = WellIdentifier( - labware_id="wrong-labware-id", well_name="wrong-well-name" + invalid_labware_id = "invalid-labware-id" + invalid_well_name = "invalid-well-name" + + assert ( + subject.get_last_measured_liquid_height(invalid_labware_id, invalid_well_name) + is None ) - assert subject.get_last_measured_liquid_height(invalid_details) is None - assert subject.get_last_measured_liquid_height(valid_details) == 0.5 + assert subject.get_last_measured_liquid_height(labware_id, well_name) == 0.5 def test_has_measured_liquid_height(subject: WellView) -> None: """Should return True for measured wells and False for ones that have no measurements.""" - valid_details = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" + + invalid_labware_id = "invalid-labware-id" + invalid_well_name = "invalid-well-name" - invalid_details = WellIdentifier( - labware_id="wrong-labware-id", well_name="wrong-well-name" + assert ( + subject.has_measured_liquid_height(invalid_labware_id, invalid_well_name) + is False ) - assert subject.has_measured_liquid_height(invalid_details) is False - assert subject.has_measured_liquid_height(valid_details) is True + assert subject.has_measured_liquid_height(labware_id, well_name) is True From 6e2e2fb1cd2f9f76d7d4319c1b4b6d5142c384b7 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Wed, 17 Jul 2024 15:54:32 -0400 Subject: [PATCH 13/17] lint --- api/src/opentrons/protocol_engine/state/wells.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 5da00a6b5bb..6aa06cd4703 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,5 +1,4 @@ """Basic well data state and store.""" -import pdb from dataclasses import dataclass from datetime import datetime from typing import Dict, List, Optional @@ -90,7 +89,7 @@ def get_all(self) -> List[LiquidHeightInfo]: allHeights.extend(a for a in val.values()) return allHeights - def get_wells_in_labware(self, labware_id: str) -> List[LiquidHeightInfo]: + def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightInfo]: """Get all well liquid heights for a particular labware.""" return list(self._state.measured_liquid_heights[labware_id].values()) From 803eda827b4c143b91b38dc9262bd7f79dfd8407 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Mon, 22 Jul 2024 09:37:23 -0400 Subject: [PATCH 14/17] refactoring --- .../protocol_engine/state/state_summary.py | 4 +-- .../opentrons/protocol_engine/state/wells.py | 35 ++++++++++++++----- api/src/opentrons/protocol_engine/types.py | 9 +++++ 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/state_summary.py b/api/src/opentrons/protocol_engine/state/state_summary.py index 91ddcd40c62..96d6683edf8 100644 --- a/api/src/opentrons/protocol_engine/state/state_summary.py +++ b/api/src/opentrons/protocol_engine/state/state_summary.py @@ -6,12 +6,12 @@ from ..errors import ErrorOccurrence from ..types import ( EngineStatus, + LiquidHeightSummary, LoadedLabware, LabwareOffset, LoadedModule, LoadedPipette, Liquid, - LiquidHeightInfo, ) @@ -29,4 +29,4 @@ class StateSummary(BaseModel): startedAt: Optional[datetime] completedAt: Optional[datetime] liquids: List[Liquid] = Field(default_factory=list) - wells: List[LiquidHeightInfo] = Field(default_factory=list) + wells: List[LiquidHeightSummary] = Field(default_factory=list) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 6aa06cd4703..e50a23fdcf2 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -8,7 +8,7 @@ ) from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError -from opentrons.protocol_engine.types import LiquidHeightInfo +from opentrons.protocol_engine.types import LiquidHeightInfo, LiquidHeightSummary from .abstract_store import HasState, HandlesActions from ..actions import Action @@ -19,7 +19,6 @@ class WellState: """State of all wells.""" - # Dict[Labware: Dict[Wellname: [Height,TimeRecorded]]] measured_liquid_heights: Dict[str, Dict[str, LiquidHeightInfo]] @@ -82,16 +81,36 @@ def __init__(self, state: WellState) -> None: """ self._state = state - def get_all(self) -> List[LiquidHeightInfo]: + def get_all(self) -> List[LiquidHeightSummary]: """Get all well liquid heights.""" - allHeights = [] # type: List[LiquidHeightInfo] - for val in self._state.measured_liquid_heights.values(): - allHeights.extend(a for a in val.values()) + allHeights = [] # type: List[LiquidHeightSummary] + # for key, in self._state.measured_liquid_heights.items(): + # lhs = LiquidHeightSummary(labware_id=) + # allHeights.extend(a for a in val.values()) + # return allHeights + for labware in self._state.measured_liquid_heights.keys(): + for well, lhi in self._state.measured_liquid_heights[labware].items(): + lhs = LiquidHeightSummary( + labware_id=labware, + well_name=well, + height=lhi.height, + last_measured=lhi.last_measured, + ) + allHeights.append(lhs) return allHeights - def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightInfo]: + def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightSummary]: """Get all well liquid heights for a particular labware.""" - return list(self._state.measured_liquid_heights[labware_id].values()) + allHeights = [] # type: List[LiquidHeightSummary] + for well, lhi in self._state.measured_liquid_heights[labware_id].items(): + lhs = LiquidHeightSummary( + labware_id=labware_id, + well_name=well, + height=lhi.height, + last_measured=lhi.last_measured, + ) + allHeights.append(lhs) + return allHeights def get_last_measured_liquid_height( self, labware_id: str, well_name: str diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 648e6297328..be5e93982e2 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -317,6 +317,15 @@ class LiquidHeightInfo(BaseModel): last_measured: datetime +class LiquidHeightSummary(BaseModel): + """Payload for liquid state height in StateSummary.""" + + labware_id: str + well_name: str + height: float + last_measured: datetime + + @dataclass(frozen=True) class CurrentAddressableArea: """The latest addressable area the robot has accessed.""" From 56b81fc0c8cb9d643e5d2853e2eb0d5f417facb1 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Mon, 22 Jul 2024 10:25:33 -0400 Subject: [PATCH 15/17] addressing comments --- .../opentrons/protocol_engine/state/wells.py | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index e50a23fdcf2..e8c6559b058 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -61,9 +61,7 @@ def _set_liquid_height( ) -> None: """Set the liquid height of the well.""" lhi = LiquidHeightInfo(height=height, last_measured=time) - try: - self._state.measured_liquid_heights[labware_id] - except KeyError: + if labware_id not in self._state.measured_liquid_heights: self._state.measured_liquid_heights[labware_id] = {} self._state.measured_liquid_heights[labware_id][well_name] = lhi @@ -83,25 +81,21 @@ def __init__(self, state: WellState) -> None: def get_all(self) -> List[LiquidHeightSummary]: """Get all well liquid heights.""" - allHeights = [] # type: List[LiquidHeightSummary] - # for key, in self._state.measured_liquid_heights.items(): - # lhs = LiquidHeightSummary(labware_id=) - # allHeights.extend(a for a in val.values()) - # return allHeights - for labware in self._state.measured_liquid_heights.keys(): - for well, lhi in self._state.measured_liquid_heights[labware].items(): + all_heights: List[LiquidHeightSummary] = [] + for labware, wells in self._state.measured_liquid_heights.items(): + for well, lhi in wells.items(): lhs = LiquidHeightSummary( labware_id=labware, well_name=well, height=lhi.height, last_measured=lhi.last_measured, ) - allHeights.append(lhs) - return allHeights + all_heights.append(lhs) + return all_heights def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightSummary]: """Get all well liquid heights for a particular labware.""" - allHeights = [] # type: List[LiquidHeightSummary] + all_heights: List[LiquidHeightSummary] = [] for well, lhi in self._state.measured_liquid_heights[labware_id].items(): lhs = LiquidHeightSummary( labware_id=labware_id, @@ -109,8 +103,8 @@ def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightSummary]: height=lhi.height, last_measured=lhi.last_measured, ) - allHeights.append(lhs) - return allHeights + all_heights.append(lhs) + return all_heights def get_last_measured_liquid_height( self, labware_id: str, well_name: str @@ -128,7 +122,8 @@ def get_last_measured_liquid_height( def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: """Returns True if the well has been liquid level probed previously.""" try: - self._state.measured_liquid_heights[labware_id][well_name].height - return True + return bool( + self._state.measured_liquid_heights[labware_id][well_name].height + ) except KeyError: return False From f6eef7abc5700d4b071b4b423101a8ba31a0365e Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Tue, 23 Jul 2024 14:20:09 -0400 Subject: [PATCH 16/17] Revert "fix(analyses-snapshot-testing): exec-603-add-last-liquid-height snapshot failure capture (#15730)" This reverts commit 3b8fd27b949b85058cb989419d048399bb52ceb5. --- ...tialTipPickupThermocyclerLidConflict].json | 37 ++++++- ...6_P1000_96_TC_PartialTipPickupColumn].json | 102 +++++++++++++----- ..._pipetteCollisionWithThermocyclerLid].json | 37 ++++++- ...96_TC_PartialTipPickupTryToReturnTip].json | 41 ++++++- 4 files changed, 185 insertions(+), 32 deletions(-) diff --git a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[89a8226c4e][Flex_X_v2_16_P1000_96_TC_PartialTipPickupThermocyclerLidConflict].json b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[89a8226c4e][Flex_X_v2_16_P1000_96_TC_PartialTipPickupThermocyclerLidConflict].json index cf0293eee21..c30512b818b 100644 --- a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[89a8226c4e][Flex_X_v2_16_P1000_96_TC_PartialTipPickupThermocyclerLidConflict].json +++ b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[89a8226c4e][Flex_X_v2_16_P1000_96_TC_PartialTipPickupThermocyclerLidConflict].json @@ -4889,6 +4889,39 @@ }, "startedAt": "TIMESTAMP", "status": "succeeded" + }, + { + "commandType": "dispense", + "completedAt": "TIMESTAMP", + "createdAt": "TIMESTAMP", + "id": "UUID", + "key": "517162d1e8d73c035348a1870a8abc8a", + "notes": [], + "params": { + "flowRate": 160.0, + "labwareId": "UUID", + "pipetteId": "UUID", + "volume": 20.0, + "wellLocation": { + "offset": { + "x": 0.0, + "y": 0.0, + "z": -9.8 + }, + "origin": "top" + }, + "wellName": "A2" + }, + "result": { + "position": { + "x": 23.28, + "y": 181.18, + "z": 4.5 + }, + "volume": 20.0 + }, + "startedAt": "TIMESTAMP", + "status": "succeeded" } ], "config": { @@ -4902,7 +4935,7 @@ "errors": [ { "createdAt": "TIMESTAMP", - "detail": "PartialTipMovementNotAllowedError [line 24]: Error 2004 MOTION_PLANNING_FAILURE (PartialTipMovementNotAllowedError): Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", + "detail": "PartialTipMovementNotAllowedError [line 26]: Error 2004 MOTION_PLANNING_FAILURE (PartialTipMovementNotAllowedError): Moving to NEST 96 Well Plate 200 µL Flat in slot A2 with A12 nozzle partial configuration will result in collision with thermocycler lid in deck slot A1.", "errorCode": "4000", "errorInfo": {}, "errorType": "ExceptionInProtocolError", @@ -4911,7 +4944,7 @@ "wrappedErrors": [ { "createdAt": "TIMESTAMP", - "detail": "Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", + "detail": "Moving to NEST 96 Well Plate 200 µL Flat in slot A2 with A12 nozzle partial configuration will result in collision with thermocycler lid in deck slot A1.", "errorCode": "2004", "errorInfo": {}, "errorType": "PartialTipMovementNotAllowedError", diff --git a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[8fcfd2ced0][Flex_S_v2_16_P1000_96_TC_PartialTipPickupColumn].json b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[8fcfd2ced0][Flex_S_v2_16_P1000_96_TC_PartialTipPickupColumn].json index 02df13c1a33..10ee86bd162 100644 --- a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[8fcfd2ced0][Flex_S_v2_16_P1000_96_TC_PartialTipPickupColumn].json +++ b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[8fcfd2ced0][Flex_S_v2_16_P1000_96_TC_PartialTipPickupColumn].json @@ -3606,6 +3606,82 @@ }, "startedAt": "TIMESTAMP", "status": "succeeded" + }, + { + "commandType": "dispense", + "completedAt": "TIMESTAMP", + "createdAt": "TIMESTAMP", + "id": "UUID", + "key": "ddccee6754fe0092b9c66898d66b79a7", + "notes": [], + "params": { + "flowRate": 160.0, + "labwareId": "UUID", + "pipetteId": "UUID", + "volume": 20.0, + "wellLocation": { + "offset": { + "x": 0.0, + "y": 0.0, + "z": -9.8 + }, + "origin": "top" + }, + "wellName": "A2" + }, + "result": { + "position": { + "x": 23.28, + "y": 181.18, + "z": 4.5 + }, + "volume": 20.0 + }, + "startedAt": "TIMESTAMP", + "status": "succeeded" + }, + { + "commandType": "moveToAddressableAreaForDropTip", + "completedAt": "TIMESTAMP", + "createdAt": "TIMESTAMP", + "id": "UUID", + "key": "5287b77e909d217f4b05e5006cf9ff25", + "notes": [], + "params": { + "addressableAreaName": "movableTrashA3", + "alternateDropLocation": true, + "forceDirect": false, + "ignoreTipConfiguration": true, + "offset": { + "x": 0.0, + "y": 0.0, + "z": 0.0 + }, + "pipetteId": "UUID" + }, + "result": { + "position": { + "x": 466.25, + "y": 364.0, + "z": 40.0 + } + }, + "startedAt": "TIMESTAMP", + "status": "succeeded" + }, + { + "commandType": "dropTipInPlace", + "completedAt": "TIMESTAMP", + "createdAt": "TIMESTAMP", + "id": "UUID", + "key": "b81364c35c04784c34f571446e64484c", + "notes": [], + "params": { + "pipetteId": "UUID" + }, + "result": {}, + "startedAt": "TIMESTAMP", + "status": "succeeded" } ], "config": { @@ -3616,29 +3692,7 @@ "protocolType": "python" }, "createdAt": "TIMESTAMP", - "errors": [ - { - "createdAt": "TIMESTAMP", - "detail": "PartialTipMovementNotAllowedError [line 20]: Error 2004 MOTION_PLANNING_FAILURE (PartialTipMovementNotAllowedError): Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", - "errorCode": "4000", - "errorInfo": {}, - "errorType": "ExceptionInProtocolError", - "id": "UUID", - "isDefined": false, - "wrappedErrors": [ - { - "createdAt": "TIMESTAMP", - "detail": "Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", - "errorCode": "2004", - "errorInfo": {}, - "errorType": "PartialTipMovementNotAllowedError", - "id": "UUID", - "isDefined": false, - "wrappedErrors": [] - } - ] - } - ], + "errors": [], "files": [ { "name": "Flex_S_v2_16_P1000_96_TC_PartialTipPickupColumn.py", @@ -3681,7 +3735,7 @@ "pipetteName": "p1000_96" } ], - "result": "not-ok", + "result": "ok", "robotType": "OT-3 Standard", "runTimeParameters": [] } diff --git a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[adc0621263][Flex_X_v2_16_P1000_96_TC_pipetteCollisionWithThermocyclerLid].json b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[adc0621263][Flex_X_v2_16_P1000_96_TC_pipetteCollisionWithThermocyclerLid].json index a3cf2d44d05..66957b72660 100644 --- a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[adc0621263][Flex_X_v2_16_P1000_96_TC_pipetteCollisionWithThermocyclerLid].json +++ b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[adc0621263][Flex_X_v2_16_P1000_96_TC_pipetteCollisionWithThermocyclerLid].json @@ -6116,6 +6116,39 @@ }, "startedAt": "TIMESTAMP", "status": "succeeded" + }, + { + "commandType": "dispense", + "completedAt": "TIMESTAMP", + "createdAt": "TIMESTAMP", + "id": "UUID", + "key": "e1b16944e3d0ff8ae0a964f7e638c1b3", + "notes": [], + "params": { + "flowRate": 160.0, + "labwareId": "UUID", + "pipetteId": "UUID", + "volume": 20.0, + "wellLocation": { + "offset": { + "x": 0.0, + "y": 0.0, + "z": -9.8 + }, + "origin": "top" + }, + "wellName": "A2" + }, + "result": { + "position": { + "x": 23.28, + "y": 181.18, + "z": 4.5 + }, + "volume": 20.0 + }, + "startedAt": "TIMESTAMP", + "status": "succeeded" } ], "config": { @@ -6129,7 +6162,7 @@ "errors": [ { "createdAt": "TIMESTAMP", - "detail": "PartialTipMovementNotAllowedError [line 25]: Error 2004 MOTION_PLANNING_FAILURE (PartialTipMovementNotAllowedError): Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", + "detail": "PartialTipMovementNotAllowedError [line 28]: Error 2004 MOTION_PLANNING_FAILURE (PartialTipMovementNotAllowedError): Moving to NEST 96 Well Plate 200 µL Flat in slot A2 with A12 nozzle partial configuration will result in collision with thermocycler lid in deck slot A1.", "errorCode": "4000", "errorInfo": {}, "errorType": "ExceptionInProtocolError", @@ -6138,7 +6171,7 @@ "wrappedErrors": [ { "createdAt": "TIMESTAMP", - "detail": "Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", + "detail": "Moving to NEST 96 Well Plate 200 µL Flat in slot A2 with A12 nozzle partial configuration will result in collision with thermocycler lid in deck slot A1.", "errorCode": "2004", "errorInfo": {}, "errorType": "PartialTipMovementNotAllowedError", diff --git a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[b0ce7dde5d][Flex_X_v2_16_P1000_96_TC_PartialTipPickupTryToReturnTip].json b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[b0ce7dde5d][Flex_X_v2_16_P1000_96_TC_PartialTipPickupTryToReturnTip].json index 32e9e2f9294..cdb9d4235a9 100644 --- a/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[b0ce7dde5d][Flex_X_v2_16_P1000_96_TC_PartialTipPickupTryToReturnTip].json +++ b/analyses-snapshot-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[b0ce7dde5d][Flex_X_v2_16_P1000_96_TC_PartialTipPickupTryToReturnTip].json @@ -3606,6 +3606,39 @@ }, "startedAt": "TIMESTAMP", "status": "succeeded" + }, + { + "commandType": "dispense", + "completedAt": "TIMESTAMP", + "createdAt": "TIMESTAMP", + "id": "UUID", + "key": "ddccee6754fe0092b9c66898d66b79a7", + "notes": [], + "params": { + "flowRate": 160.0, + "labwareId": "UUID", + "pipetteId": "UUID", + "volume": 20.0, + "wellLocation": { + "offset": { + "x": 0.0, + "y": 0.0, + "z": -9.8 + }, + "origin": "top" + }, + "wellName": "A2" + }, + "result": { + "position": { + "x": 23.28, + "y": 181.18, + "z": 4.5 + }, + "volume": 20.0 + }, + "startedAt": "TIMESTAMP", + "status": "succeeded" } ], "config": { @@ -3619,7 +3652,7 @@ "errors": [ { "createdAt": "TIMESTAMP", - "detail": "PartialTipMovementNotAllowedError [line 21]: Error 2004 MOTION_PLANNING_FAILURE (PartialTipMovementNotAllowedError): Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", + "detail": "UnexpectedProtocolError [line 22]: Error 4000 GENERAL_ERROR (UnexpectedProtocolError): Cannot return tip to a tiprack while the pipette is configured for partial tip.", "errorCode": "4000", "errorInfo": {}, "errorType": "ExceptionInProtocolError", @@ -3628,10 +3661,10 @@ "wrappedErrors": [ { "createdAt": "TIMESTAMP", - "detail": "Requested motion with the A12 nozzle partial configuration is outside of robot bounds for the pipette.", - "errorCode": "2004", + "detail": "Cannot return tip to a tiprack while the pipette is configured for partial tip.", + "errorCode": "4000", "errorInfo": {}, - "errorType": "PartialTipMovementNotAllowedError", + "errorType": "UnexpectedProtocolError", "id": "UUID", "isDefined": false, "wrappedErrors": [] From ddfa84ae69a2622df55fd252f9912741e1f3659d Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Fri, 6 Sep 2024 16:13:20 -0400 Subject: [PATCH 17/17] lint fix --- api/src/opentrons/protocol_engine/state/wells.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index e8c6559b058..e6e19446c6f 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -10,7 +10,7 @@ from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError from opentrons.protocol_engine.types import LiquidHeightInfo, LiquidHeightSummary -from .abstract_store import HasState, HandlesActions +from ._abstract_store import HasState, HandlesActions from ..actions import Action from ..commands import Command