From 544d695b57de6f706644a0379b0f1bfa38be9c55 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Fri, 18 Oct 2024 11:18:41 -0400 Subject: [PATCH 01/18] initial implementation for load_liquid volume-tracking --- .../protocol_engine/commands/load_liquid.py | 14 ++++++++++++++ .../opentrons/protocol_engine/state/geometry.py | 9 +++++++++ api/src/opentrons/protocol_engine/state/wells.py | 9 +++++++++ 3 files changed, 32 insertions(+) diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index 856cf3ee127..7b68bbaa856 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -3,6 +3,7 @@ from pydantic import BaseModel, Field from typing import Optional, Type, Dict, TYPE_CHECKING from typing_extensions import Literal +from datetime import datetime from .command import AbstractCommandImpl, BaseCommand, BaseCommandCreate, SuccessData from ..errors.error_occurrence import ErrorOccurrence @@ -53,6 +54,19 @@ async def execute( self._state_view.labware.validate_liquid_allowed_in_labware( labware_id=params.labwareId, wells=params.volumeByWell ) + # do this in WellStore?! + labware_id = params.labwareId + well_name = next(iter(params.volumeByWell)) + volume = next(iter(params.volumeByWell.values())) + height = self._state_view.geometry.get_well_height_at_volume( + labware_id=labware_id, well_name=well_name, volume=volume + ) + self._state_view.wells.set_liquid_height( + labware_id=labware_id, + well_name=well_name, + height=height, + time=datetime.now(), + ) return SuccessData(public=LoadLiquidResult(), private=None) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 125be3339a9..fd0359c8771 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1445,6 +1445,15 @@ def get_well_height_after_volume( target_volume=final_volume, well_geometry=well_geometry ) + def get_well_height_at_volume( + self, labware_id: str, well_name: str, volume: float + ) -> float: + """Convert well volume to height.""" + well_geometry = self._labware.get_well_geometry(labware_id, well_name) + return find_height_at_well_volume( + target_volume=volume, well_geometry=well_geometry + ) + def validate_dispense_volume_into_well( self, labware_id: str, diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index d74d94a1be0..04ea9e025f7 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -127,3 +127,12 @@ def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: ) except KeyError: return False + + def set_liquid_height( + self, labware_id: str, well_name: str, height: float, time: datetime + ) -> None: + """Set the liquid height of the well.""" + lhi = LiquidHeightInfo(height=height, last_measured=time) + 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 From aa5ff64613c910ba48b1f6750afdc8238d120d81 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Fri, 18 Oct 2024 11:34:31 -0400 Subject: [PATCH 02/18] updated naming --- .../protocol_engine/commands/load_liquid.py | 4 ++-- api/src/opentrons/protocol_engine/state/wells.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index 7b68bbaa856..09ddf1eb2da 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -54,14 +54,14 @@ async def execute( self._state_view.labware.validate_liquid_allowed_in_labware( labware_id=params.labwareId, wells=params.volumeByWell ) - # do this in WellStore?! + # do this in WellStore? Don't have access to GeometryView labware_id = params.labwareId well_name = next(iter(params.volumeByWell)) volume = next(iter(params.volumeByWell.values())) height = self._state_view.geometry.get_well_height_at_volume( labware_id=labware_id, well_name=well_name, volume=volume ) - self._state_view.wells.set_liquid_height( + self._state_view.wells.set_liquid_height_from_load( labware_id=labware_id, well_name=well_name, height=height, diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 04ea9e025f7..44df0000565 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -40,7 +40,7 @@ def handle_action(self, action: Action) -> None: def _handle_succeeded_command(self, command: Command) -> None: if isinstance(command.result, LiquidProbeResult): - self._set_liquid_height( + self._set_liquid_height_from_probe( labware_id=command.params.labwareId, well_name=command.params.wellName, height=command.result.z_position, @@ -49,17 +49,17 @@ def _handle_succeeded_command(self, command: Command) -> None: def _handle_failed_command(self, action: FailCommandAction) -> None: if isinstance(action.error, LiquidNotFoundError): - self._set_liquid_height( + self._set_liquid_height_from_probe( labware_id=action.error.private.labware_id, well_name=action.error.private.well_name, height=None, time=action.failed_at, ) - def _set_liquid_height( + def _set_liquid_height_from_probe( self, labware_id: str, well_name: str, height: float, time: datetime ) -> None: - """Set the liquid height of the well.""" + """Set the liquid height of the well from a LiquidProbe command.""" lhi = LiquidHeightInfo(height=height, last_measured=time) if labware_id not in self._state.measured_liquid_heights: self._state.measured_liquid_heights[labware_id] = {} @@ -128,10 +128,10 @@ def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: except KeyError: return False - def set_liquid_height( + def set_liquid_height_from_load( self, labware_id: str, well_name: str, height: float, time: datetime ) -> None: - """Set the liquid height of the well.""" + """Set the liquid height of the well from a LoadLiquid command.""" lhi = LiquidHeightInfo(height=height, last_measured=time) if labware_id not in self._state.measured_liquid_heights: self._state.measured_liquid_heights[labware_id] = {} From e7ee66eecf890e4de26bbfdf94bd0a9d907a3a47 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Fri, 18 Oct 2024 13:59:44 -0400 Subject: [PATCH 03/18] updated implementation to include Aspirate and Dispense results --- .../protocol_engine/commands/load_liquid.py | 13 ---- .../protocol_engine/state/geometry.py | 7 ++- .../opentrons/protocol_engine/state/wells.py | 61 +++++++++++++++---- 3 files changed, 53 insertions(+), 28 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index 09ddf1eb2da..86a78b94c62 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -54,19 +54,6 @@ async def execute( self._state_view.labware.validate_liquid_allowed_in_labware( labware_id=params.labwareId, wells=params.volumeByWell ) - # do this in WellStore? Don't have access to GeometryView - labware_id = params.labwareId - well_name = next(iter(params.volumeByWell)) - volume = next(iter(params.volumeByWell.values())) - height = self._state_view.geometry.get_well_height_at_volume( - labware_id=labware_id, well_name=well_name, volume=volume - ) - self._state_view.wells.set_liquid_height_from_load( - labware_id=labware_id, - well_name=well_name, - height=height, - time=datetime.now(), - ) return SuccessData(public=LoadLiquidResult(), private=None) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index fd0359c8771..1683a2670cb 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1386,9 +1386,9 @@ def get_well_offset_adjustment( volume = operation_volume or 0.0 if volume: - well_geometry = self._labware.get_well_geometry(labware_id, well_name) return self.get_well_height_after_volume( - well_geometry=well_geometry, + labware_id=labware_id, + well_name=well_name, initial_height=initial_handling_height, volume=volume, ) @@ -1431,12 +1431,13 @@ def get_well_handling_height( return float(handling_height) def get_well_height_after_volume( - self, well_geometry: InnerWellGeometry, initial_height: float, volume: float + self, labware_id: str, well_name: str, initial_height: float, volume: float ) -> float: """Return the height of liquid in a labware well after a given volume has been handled. This is given an initial handling height, with reference to the well bottom. """ + well_geometry = self._labware.get_well_geometry(labware_id=labware_id, well_name=well_name) initial_volume = find_volume_at_well_height( target_height=initial_height, well_geometry=well_geometry ) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 44df0000565..25d082ac362 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -7,12 +7,16 @@ SucceedCommandAction, ) from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult +from opentrons.protocol_engine.commands.load_liquid import LoadLiquidResult +from opentrons.protocol_engine.commands.aspirate import AspirateResult +from opentrons.protocol_engine.commands.dispense import DispenseResult from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError from opentrons.protocol_engine.types import LiquidHeightInfo, LiquidHeightSummary from ._abstract_store import HasState, HandlesActions from ..actions import Action from ..commands import Command +from .geometry import get_well_height_at_volume, get_well_height_after_volume @dataclass @@ -40,11 +44,32 @@ def handle_action(self, action: Action) -> None: def _handle_succeeded_command(self, command: Command) -> None: if isinstance(command.result, LiquidProbeResult): - self._set_liquid_height_from_probe( + self._set_liquid_height_after_probe( labware_id=command.params.labwareId, well_name=command.params.wellName, height=command.result.z_position, - time=command.createdAt, + time=command.completedAt, + ) + if isinstance(command.result, LoadLiquidResult): + self._set_liquid_height_after_load( + labware_id=command.params.labwareId, + well_name=next(iter(command.params.volumeByWell)), + volume=next(iter(command.params.volumeByWell.values())), + time=command.completedAt, + ) + if isinstance(command.result, AspirateResult): + self._update_liquid_height_after_operation( + labware_id=command.params.labwareId, + well_name=command.params.wellName, + volume=-command.result.volume, + time=command.completedAt, + ) + if isinstance(command.result, DispenseResult): + self._update_liquid_height_after_operation( + labware_id=command.params.labwareId, + well_name=command.params.wellName, + volume=command.result.volume, + time=command.completedAt, ) def _handle_failed_command(self, action: FailCommandAction) -> None: @@ -56,7 +81,7 @@ def _handle_failed_command(self, action: FailCommandAction) -> None: time=action.failed_at, ) - def _set_liquid_height_from_probe( + def _set_liquid_height_after_probe( self, labware_id: str, well_name: str, height: float, time: datetime ) -> None: """Set the liquid height of the well from a LiquidProbe command.""" @@ -65,6 +90,27 @@ def _set_liquid_height_from_probe( self._state.measured_liquid_heights[labware_id] = {} self._state.measured_liquid_heights[labware_id][well_name] = lhi + def _set_liquid_height_after_load( + self, labware_id: str, well_name: str, volume: float, time: datetime + ) -> None: + """Set the liquid height of the well from a LoadLiquid command.""" + height = get_well_height_at_volume(labware_id=labware_id, well_name=well_name, volume=volume) + lhi = LiquidHeightInfo(height=height, last_measured=time) + 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 + + def _update_liquid_height_after_operation( + self, labware_id: str, well_name: str, volume: float, time: datetime + ) -> None: + """Set the liquid height of the well from a LoadLiquid command.""" + initial_height = self._state.measured_liquid_heights[labware_id][well_name] + height = get_well_height_after_volume(labware_id=labware_id, well_name=well_name, initial_height=initial_height, volume=volume) + lhi = LiquidHeightInfo(height=height, last_measured=time) + 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 + class WellView(HasState[WellState]): """Read-only well state view.""" @@ -127,12 +173,3 @@ def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: ) except KeyError: return False - - def set_liquid_height_from_load( - self, labware_id: str, well_name: str, height: float, time: datetime - ) -> None: - """Set the liquid height of the well from a LoadLiquid command.""" - lhi = LiquidHeightInfo(height=height, last_measured=time) - 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 From 5959370866d05fdf2059861706266fde2fe35a14 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Fri, 18 Oct 2024 14:00:36 -0400 Subject: [PATCH 04/18] eliminated previously used import --- api/src/opentrons/protocol_engine/commands/load_liquid.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index 86a78b94c62..856cf3ee127 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -3,7 +3,6 @@ from pydantic import BaseModel, Field from typing import Optional, Type, Dict, TYPE_CHECKING from typing_extensions import Literal -from datetime import datetime from .command import AbstractCommandImpl, BaseCommand, BaseCommandCreate, SuccessData from ..errors.error_occurrence import ErrorOccurrence From 661e96a90aac08d9a1cc6a972cab748379ac73c9 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Fri, 18 Oct 2024 14:18:33 -0400 Subject: [PATCH 05/18] added operations_since_measurement to LiquidHeightInfo --- .../protocol_engine/state/geometry.py | 4 +- .../opentrons/protocol_engine/state/wells.py | 52 ++++++++++++++----- api/src/opentrons/protocol_engine/types.py | 1 + 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 1683a2670cb..9783508800c 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1437,7 +1437,9 @@ def get_well_height_after_volume( This is given an initial handling height, with reference to the well bottom. """ - well_geometry = self._labware.get_well_geometry(labware_id=labware_id, well_name=well_name) + well_geometry = self._labware.get_well_geometry( + labware_id=labware_id, well_name=well_name + ) initial_volume = find_volume_at_well_height( target_height=initial_height, well_geometry=well_geometry ) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 25d082ac362..75783d3308f 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -48,33 +48,35 @@ def _handle_succeeded_command(self, command: Command) -> None: labware_id=command.params.labwareId, well_name=command.params.wellName, height=command.result.z_position, - time=command.completedAt, + time=command.completedAt + if command.completedAt is not None + else command.createdAt, ) if isinstance(command.result, LoadLiquidResult): self._set_liquid_height_after_load( labware_id=command.params.labwareId, well_name=next(iter(command.params.volumeByWell)), volume=next(iter(command.params.volumeByWell.values())), - time=command.completedAt, + time=command.completedAt + if command.completedAt is not None + else command.createdAt, ) if isinstance(command.result, AspirateResult): self._update_liquid_height_after_operation( labware_id=command.params.labwareId, well_name=command.params.wellName, volume=-command.result.volume, - time=command.completedAt, ) if isinstance(command.result, DispenseResult): self._update_liquid_height_after_operation( labware_id=command.params.labwareId, well_name=command.params.wellName, volume=command.result.volume, - time=command.completedAt, ) def _handle_failed_command(self, action: FailCommandAction) -> None: if isinstance(action.error, LiquidNotFoundError): - self._set_liquid_height_from_probe( + self._set_liquid_height_after_probe( labware_id=action.error.private.labware_id, well_name=action.error.private.well_name, height=None, @@ -85,7 +87,9 @@ def _set_liquid_height_after_probe( self, labware_id: str, well_name: str, height: float, time: datetime ) -> None: """Set the liquid height of the well from a LiquidProbe command.""" - lhi = LiquidHeightInfo(height=height, last_measured=time) + lhi = LiquidHeightInfo( + height=height, last_measured=time, operations_since_measurement=0 + ) 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 @@ -94,19 +98,41 @@ def _set_liquid_height_after_load( self, labware_id: str, well_name: str, volume: float, time: datetime ) -> None: """Set the liquid height of the well from a LoadLiquid command.""" - height = get_well_height_at_volume(labware_id=labware_id, well_name=well_name, volume=volume) - lhi = LiquidHeightInfo(height=height, last_measured=time) + height = get_well_height_at_volume( + labware_id=labware_id, well_name=well_name, volume=volume + ) + lhi = LiquidHeightInfo( + height=height, last_measured=time, operations_since_measurement=0 + ) 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 def _update_liquid_height_after_operation( - self, labware_id: str, well_name: str, volume: float, time: datetime + self, labware_id: str, well_name: str, volume: float ) -> None: - """Set the liquid height of the well from a LoadLiquid command.""" - initial_height = self._state.measured_liquid_heights[labware_id][well_name] - height = get_well_height_after_volume(labware_id=labware_id, well_name=well_name, initial_height=initial_height, volume=volume) - lhi = LiquidHeightInfo(height=height, last_measured=time) + """Update the liquid height of the well after an Aspirate or Dispense command.""" + time = self._state.measured_liquid_heights[labware_id][well_name].last_measured + operations_since_measurement = ( + self._state.measured_liquid_heights[labware_id][ + well_name + ].operations_since_measurement + + 1 + ) + initial_height = self._state.measured_liquid_heights[labware_id][ + well_name + ].height + height = get_well_height_after_volume( + labware_id=labware_id, + well_name=well_name, + initial_height=initial_height, + volume=volume, + ) + lhi = LiquidHeightInfo( + height=height, + last_measured=time, + operations_since_measurement=operations_since_measurement, + ) 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 diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 72daafd3a52..d8b9345ab71 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -360,6 +360,7 @@ class LiquidHeightInfo(BaseModel): height: float last_measured: datetime + operations_since_measurement: int class LiquidHeightSummary(BaseModel): From 77f28d3350c4ba649297244368055ec9935dcb61 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Mon, 21 Oct 2024 18:13:21 -0400 Subject: [PATCH 06/18] refactor to use StateUpdate --- .../protocol_engine/commands/aspirate.py | 6 + .../protocol_engine/commands/dispense.py | 6 + .../protocol_engine/commands/liquid_probe.py | 41 +++- .../protocol_engine/commands/load_liquid.py | 19 +- .../protocol_engine/state/geometry.py | 9 + .../protocol_engine/state/update_types.py | 111 ++++++++-- .../opentrons/protocol_engine/state/wells.py | 190 +++++++----------- api/src/opentrons/protocol_engine/types.py | 27 ++- 8 files changed, 265 insertions(+), 144 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/aspirate.py b/api/src/opentrons/protocol_engine/commands/aspirate.py index 14b59248216..8e3dcde80eb 100644 --- a/api/src/opentrons/protocol_engine/commands/aspirate.py +++ b/api/src/opentrons/protocol_engine/commands/aspirate.py @@ -140,6 +140,7 @@ async def execute(self, params: AspirateParams) -> _ExecuteReturn: command_note_adder=self._command_note_adder, ) except PipetteOverpressureError as e: + # can we get aspirated_amount_prior_to_error? If not, can we assume no liquid was removed from well? Ask Ryan return DefinedErrorData( public=OverpressureError( id=self._model_utils.generate_id(), @@ -156,6 +157,11 @@ async def execute(self, params: AspirateParams) -> _ExecuteReturn: state_update=state_update, ) else: + state_update.set_operated_liquid( + labware_id=labware_id, + well_name=well_name, + volume=-volume_aspirated, + ) return SuccessData( public=AspirateResult( volume=volume_aspirated, diff --git a/api/src/opentrons/protocol_engine/commands/dispense.py b/api/src/opentrons/protocol_engine/commands/dispense.py index 7e18cc6560b..1285d159360 100644 --- a/api/src/opentrons/protocol_engine/commands/dispense.py +++ b/api/src/opentrons/protocol_engine/commands/dispense.py @@ -107,6 +107,7 @@ async def execute(self, params: DispenseParams) -> _ExecuteReturn: push_out=params.pushOut, ) except PipetteOverpressureError as e: + # can we get aspirated_amount_prior_to_error? If not, can we assume no liquid was removed from well? Ask Ryan return DefinedErrorData( public=OverpressureError( id=self._model_utils.generate_id(), @@ -123,6 +124,11 @@ async def execute(self, params: DispenseParams) -> _ExecuteReturn: state_update=state_update, ) else: + state_update.set_operated_liquid( + labware_id=labware_id, + well_name=well_name, + volume=volume, + ) return SuccessData( public=DispenseResult(volume=volume, position=deck_point), private=None, diff --git a/api/src/opentrons/protocol_engine/commands/liquid_probe.py b/api/src/opentrons/protocol_engine/commands/liquid_probe.py index 1a8597f9c03..b58fea92219 100644 --- a/api/src/opentrons/protocol_engine/commands/liquid_probe.py +++ b/api/src/opentrons/protocol_engine/commands/liquid_probe.py @@ -205,6 +205,13 @@ async def execute(self, params: _CommonParams) -> _LiquidProbeExecuteReturn: self._state_view, self._movement, self._pipetting, params ) if isinstance(z_pos_or_error, PipetteLiquidNotFoundError): + state_update.set_probed_liquid( + labware_id=params.labwareId, + well_name=params.wellName, + height=None, + volume=None, + last_probed=self._model_utils.get_timestamp(), + ) return DefinedErrorData( public=LiquidNotFoundError( id=self._model_utils.generate_id(), @@ -220,6 +227,18 @@ async def execute(self, params: _CommonParams) -> _LiquidProbeExecuteReturn: state_update=state_update, ) else: + well_volume = self._state_view.geometry.get_well_volume_at_height( + labware_id=params.labwareId, + well_name=params.wellName, + height=z_pos_or_error, + ) + state_update.set_probed_liquid( + labware_id=params.labwareId, + well_name=params.wellName, + height=z_pos_or_error, + volume=well_volume, + last_probed=self._model_utils.get_timestamp(), + ) return SuccessData( public=LiquidProbeResult( z_position=z_pos_or_error, position=deck_point @@ -239,11 +258,13 @@ def __init__( state_view: StateView, movement: MovementHandler, pipetting: PipettingHandler, + model_utils: ModelUtils, **kwargs: object, ) -> None: self._state_view = state_view self._movement = movement self._pipetting = pipetting + self._model_utils = model_utils async def execute(self, params: _CommonParams) -> _TryLiquidProbeExecuteReturn: """Execute a `tryLiquidProbe` command. @@ -256,11 +277,23 @@ async def execute(self, params: _CommonParams) -> _TryLiquidProbeExecuteReturn: self._state_view, self._movement, self._pipetting, params ) - z_pos = ( - None - if isinstance(z_pos_or_error, PipetteLiquidNotFoundError) - else z_pos_or_error + if isinstance(z_pos_or_error, PipetteLiquidNotFoundError): + z_pos = None + well_volume = None + else: + z_pos = z_pos_or_error + well_volume = self._state_view.geometry.get_well_volume_at_height( + labware_id=params.labwareId, well_name=params.wellName, height=z_pos + ) + + state_update.set_probed_liquid( + labware_id=params.labwareId, + well_name=params.wellName, + height=z_pos, + volume=well_volume, + last_probed=self._model_utils.get_timestamp(), ) + return SuccessData( public=TryLiquidProbeResult( z_position=z_pos, diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index 856cf3ee127..d5fdfd13ed2 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -4,11 +4,14 @@ from typing import Optional, Type, Dict, TYPE_CHECKING from typing_extensions import Literal +from opentrons.protocol_engine.state.update_types import StateUpdate + from .command import AbstractCommandImpl, BaseCommand, BaseCommandCreate, SuccessData from ..errors.error_occurrence import ErrorOccurrence if TYPE_CHECKING: from ..state.state import StateView + from ..resources import ModelUtils LoadLiquidCommandType = Literal["loadLiquid"] @@ -41,8 +44,11 @@ class LoadLiquidImplementation( ): """Load liquid command implementation.""" - def __init__(self, state_view: StateView, **kwargs: object) -> None: + def __init__( + self, state_view: StateView, model_utils: ModelUtils, **kwargs: object + ) -> None: self._state_view = state_view + self._model_utils = model_utils async def execute( self, params: LoadLiquidParams @@ -54,7 +60,16 @@ async def execute( labware_id=params.labwareId, wells=params.volumeByWell ) - return SuccessData(public=LoadLiquidResult(), private=None) + state_update = StateUpdate() + state_update.set_loaded_liquid( + labware_id=params.labwareId, + volumes=params.volumeByWell, + last_loaded=self._model_utils.get_timestamp(), + ) + + return SuccessData( + public=LoadLiquidResult(), private=None, state_update=state_update + ) class LoadLiquid(BaseCommand[LoadLiquidParams, LoadLiquidResult, ErrorOccurrence]): diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 9783508800c..0ebc5eca720 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1457,6 +1457,15 @@ def get_well_height_at_volume( target_volume=volume, well_geometry=well_geometry ) + def get_well_volume_at_height( + self, labware_id: str, well_name: str, height: float + ) -> float: + """Convert well height to volume.""" + well_geometry = self._labware.get_well_geometry(labware_id, well_name) + return find_volume_at_well_height( + target_height=height, well_geometry=well_geometry + ) + def validate_dispense_volume_into_well( self, labware_id: str, diff --git a/api/src/opentrons/protocol_engine/state/update_types.py b/api/src/opentrons/protocol_engine/state/update_types.py index 5d941d33933..91bc8c7d0d2 100644 --- a/api/src/opentrons/protocol_engine/state/update_types.py +++ b/api/src/opentrons/protocol_engine/state/update_types.py @@ -3,7 +3,8 @@ import dataclasses import enum -import typing +from typing import Final, TypeAlias, Literal, Dict, Optional, overload +from datetime import datetime from opentrons.hardware_control.nozzle_manager import NozzleMap from opentrons.protocol_engine.resources import pipette_data_provider @@ -17,14 +18,14 @@ class _NoChangeEnum(enum.Enum): NO_CHANGE = enum.auto() -NO_CHANGE: typing.Final = _NoChangeEnum.NO_CHANGE +NO_CHANGE: Final = _NoChangeEnum.NO_CHANGE """A sentinel value to indicate that a value shouldn't be changed. Useful when `None` is semantically unclear or already has some other meaning. """ -NoChangeType: typing.TypeAlias = typing.Literal[_NoChangeEnum.NO_CHANGE] +NoChangeType: TypeAlias = Literal[_NoChangeEnum.NO_CHANGE] """The type of `NO_CHANGE`, as `NoneType` is to `None`. Unfortunately, mypy doesn't let us write `Literal[NO_CHANGE]`. Use this instead. @@ -35,14 +36,14 @@ class _ClearEnum(enum.Enum): CLEAR = enum.auto() -CLEAR: typing.Final = _ClearEnum.CLEAR +CLEAR: Final = _ClearEnum.CLEAR """A sentinel value to indicate that a value should be cleared. Useful when `None` is semantically unclear or has some other meaning. """ -ClearType: typing.TypeAlias = typing.Literal[_ClearEnum.CLEAR] +ClearType: TypeAlias = Literal[_ClearEnum.CLEAR] """The type of `CLEAR`, as `NoneType` is to `None`. Unfortunately, mypy doesn't let us write `Literal[CLEAR]`. Use this instead. @@ -91,7 +92,7 @@ class LabwareLocationUpdate: new_location: LabwareLocation """The labware's new location.""" - offset_id: typing.Optional[str] + offset_id: Optional[str] """The ID of the labware's new offset, for its new location.""" @@ -105,10 +106,10 @@ class LoadedLabwareUpdate: new_location: LabwareLocation """The labware's initial location.""" - offset_id: typing.Optional[str] + offset_id: Optional[str] """The ID of the labware's offset.""" - display_name: typing.Optional[str] + display_name: Optional[str] definition: LabwareDefinition @@ -126,7 +127,7 @@ class LoadPipetteUpdate: pipette_name: PipetteNameType mount: MountType - liquid_presence_detection: typing.Optional[bool] + liquid_presence_detection: Optional[bool] @dataclasses.dataclass @@ -155,7 +156,7 @@ class PipetteTipStateUpdate: """Update pipette tip state.""" pipette_id: str - tip_geometry: typing.Optional[TipGeometry] + tip_geometry: Optional[TipGeometry] @dataclasses.dataclass @@ -175,6 +176,35 @@ class TipsUsedUpdate: """ +@dataclasses.dataclass +class LoadLiquidUpdate: + """An update from loading a liquid.""" + + labware_id: str + volumes: Dict[str, float] + last_loaded: datetime + + +@dataclasses.dataclass +class ProbeLiquidUpdate: + """An update from probing a liquid.""" + + labware_id: str + well_name: str + last_probed: datetime + height: Optional[float] = None + volume: Optional[float] = None + + +@dataclasses.dataclass +class OperateLiquidUpdate: + """An update from operating a liquid.""" + + labware_id: str + well_name: str + volume: float + + @dataclasses.dataclass class StateUpdate: """Represents an update to perform on engine state.""" @@ -195,10 +225,16 @@ class StateUpdate: tips_used: TipsUsedUpdate | NoChangeType = NO_CHANGE + loaded_liquid: LoadLiquidUpdate | NoChangeType = NO_CHANGE + + probed_liquid: ProbeLiquidUpdate | NoChangeType = NO_CHANGE + + operated_liquid: OperateLiquidUpdate | NoChangeType = NO_CHANGE + # These convenience functions let the caller avoid the boilerplate of constructing a # complicated dataclass tree. - @typing.overload + @overload def set_pipette_location( self, *, @@ -209,7 +245,7 @@ def set_pipette_location( ) -> None: """Schedule a pipette's location to be set to a well.""" - @typing.overload + @overload def set_pipette_location( self, *, @@ -270,8 +306,8 @@ def set_loaded_labware( self, definition: LabwareDefinition, labware_id: str, - offset_id: typing.Optional[str], - display_name: typing.Optional[str], + offset_id: Optional[str], + display_name: Optional[str], location: LabwareLocation, ) -> None: """Add a new labware to state. See `LoadedLabwareUpdate`.""" @@ -288,7 +324,7 @@ def set_load_pipette( pipette_id: str, pipette_name: PipetteNameType, mount: MountType, - liquid_presence_detection: typing.Optional[bool], + liquid_presence_detection: Optional[bool], ) -> None: """Add a new pipette to state. See `LoadPipetteUpdate`.""" self.loaded_pipette = LoadPipetteUpdate( @@ -316,7 +352,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] + self, pipette_id: str, tip_geometry: Optional[TipGeometry] ) -> None: """Update a pipette's tip state. See `PipetteTipStateUpdate`.""" self.pipette_tip_state = PipetteTipStateUpdate( @@ -330,3 +366,46 @@ def mark_tips_as_used( self.tips_used = TipsUsedUpdate( pipette_id=pipette_id, labware_id=labware_id, well_name=well_name ) + + def set_loaded_liquid( + self, + labware_id: str, + volumes: Dict[str, float], + last_loaded: datetime, + ) -> None: + """Add liquid volumes to well state. See `LoadLiquidUpdate`.""" + self.loaded_liquid = LoadLiquidUpdate( + labware_id=labware_id, + volumes=volumes, + last_loaded=last_loaded, + ) + + def set_probed_liquid( + self, + labware_id: str, + well_name: str, + last_probed: datetime, + height: Optional[float] = None, + volume: Optional[float] = None, + ) -> None: + """Add a liquid height and volume to well state. See `ProbeLiquidUpdate`.""" + self.probed_liquid = ProbeLiquidUpdate( + labware_id=labware_id, + well_name=well_name, + height=height, + volume=volume, + last_probed=last_probed, + ) + + def set_operated_liquid( + self, + labware_id: str, + well_name: str, + volume: float, + ) -> None: + """Update liquid volumes in well state. See `OperateLiquidUpdate`.""" + self.operated_liquid = OperateLiquidUpdate( + labware_id=labware_id, + well_name=well_name, + volume=volume, + ) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 75783d3308f..7a33a79d5ac 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,29 +1,29 @@ """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, SucceedCommandAction, ) -from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult -from opentrons.protocol_engine.commands.load_liquid import LoadLiquidResult -from opentrons.protocol_engine.commands.aspirate import AspirateResult -from opentrons.protocol_engine.commands.dispense import DispenseResult -from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError -from opentrons.protocol_engine.types import LiquidHeightInfo, LiquidHeightSummary +from opentrons.protocol_engine.types import ( + ProbedHeightInfo, + ProbedVolumeInfo, + LoadedVolumeInfo, + LiquidHeightSummary, +) +from . import update_types from ._abstract_store import HasState, HandlesActions -from ..actions import Action -from ..commands import Command -from .geometry import get_well_height_at_volume, get_well_height_after_volume +from ..actions import Action, get_state_update @dataclass class WellState: """State of all wells.""" - measured_liquid_heights: Dict[str, Dict[str, LiquidHeightInfo]] + loaded_volumes: Dict[str, Dict[str, LoadedVolumeInfo]] + probed_heights: Dict[str, Dict[str, ProbedHeightInfo]] + probed_volumes: Dict[str, Dict[str, ProbedVolumeInfo]] class WellStore(HasState[WellState], HandlesActions): @@ -33,109 +33,63 @@ class WellStore(HasState[WellState], HandlesActions): def __init__(self) -> None: """Initialize a well store and its state.""" - self._state = WellState(measured_liquid_heights={}) + self._state = WellState(loaded_volumes={}, probed_heights={}, probed_volumes={}) def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" - if isinstance(action, SucceedCommandAction): - self._handle_succeeded_command(action.command) - if isinstance(action, FailCommandAction): - self._handle_failed_command(action) - - def _handle_succeeded_command(self, command: Command) -> None: - if isinstance(command.result, LiquidProbeResult): - self._set_liquid_height_after_probe( - labware_id=command.params.labwareId, - well_name=command.params.wellName, - height=command.result.z_position, - time=command.completedAt - if command.completedAt is not None - else command.createdAt, - ) - if isinstance(command.result, LoadLiquidResult): - self._set_liquid_height_after_load( - labware_id=command.params.labwareId, - well_name=next(iter(command.params.volumeByWell)), - volume=next(iter(command.params.volumeByWell.values())), - time=command.completedAt - if command.completedAt is not None - else command.createdAt, - ) - if isinstance(command.result, AspirateResult): - self._update_liquid_height_after_operation( - labware_id=command.params.labwareId, - well_name=command.params.wellName, - volume=-command.result.volume, - ) - if isinstance(command.result, DispenseResult): - self._update_liquid_height_after_operation( - labware_id=command.params.labwareId, - well_name=command.params.wellName, - volume=command.result.volume, - ) + state_update = get_state_update(action) + if state_update is not None: + self._set_loaded_liquid(state_update) + self._set_probed_liquid(state_update) + self._set_operated_liquid(state_update) + + def _set_loaded_liquid(self, state_update: update_types.StateUpdate) -> None: + if state_update.loaded_liquid != update_types.NO_CHANGE: + labware_id = state_update.loaded_liquid.labware_id + for (well, volume) in state_update.loaded_liquid.volumes.items(): + self._state.loaded_volumes[labware_id][well] = LoadedVolumeInfo( + volume=volume, + last_loaded=state_update.loaded_liquid.last_loaded, + operations_since_load=0, + ) - def _handle_failed_command(self, action: FailCommandAction) -> None: - if isinstance(action.error, LiquidNotFoundError): - self._set_liquid_height_after_probe( - labware_id=action.error.private.labware_id, - well_name=action.error.private.well_name, - height=None, - time=action.failed_at, + def _set_probed_liquid(self, state_update: update_types.StateUpdate) -> None: + if state_update.probed_liquid != update_types.NO_CHANGE: + labware_id = state_update.probed_liquid.labware_id + well_name = state_update.probed_liquid.well_name + self._state.probed_heights[labware_id][well_name] = ProbedHeightInfo( + height=state_update.probed_liquid.height, + last_probed=state_update.probed_liquid.last_probed, + ) + self._state.probed_volumes[labware_id][well_name] = ProbedVolumeInfo( + volume=state_update.probed_liquid.volume, + last_probed=state_update.probed_liquid.last_probed, + operations_since_probe=0, ) - def _set_liquid_height_after_probe( - self, labware_id: str, well_name: str, height: float, time: datetime - ) -> None: - """Set the liquid height of the well from a LiquidProbe command.""" - lhi = LiquidHeightInfo( - height=height, last_measured=time, operations_since_measurement=0 - ) - 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 - - def _set_liquid_height_after_load( - self, labware_id: str, well_name: str, volume: float, time: datetime - ) -> None: - """Set the liquid height of the well from a LoadLiquid command.""" - height = get_well_height_at_volume( - labware_id=labware_id, well_name=well_name, volume=volume - ) - lhi = LiquidHeightInfo( - height=height, last_measured=time, operations_since_measurement=0 - ) - 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 - - def _update_liquid_height_after_operation( - self, labware_id: str, well_name: str, volume: float - ) -> None: - """Update the liquid height of the well after an Aspirate or Dispense command.""" - time = self._state.measured_liquid_heights[labware_id][well_name].last_measured - operations_since_measurement = ( - self._state.measured_liquid_heights[labware_id][ - well_name - ].operations_since_measurement - + 1 - ) - initial_height = self._state.measured_liquid_heights[labware_id][ - well_name - ].height - height = get_well_height_after_volume( - labware_id=labware_id, - well_name=well_name, - initial_height=initial_height, - volume=volume, - ) - lhi = LiquidHeightInfo( - height=height, - last_measured=time, - operations_since_measurement=operations_since_measurement, - ) - 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 + def _set_operated_liquid(self, state_update: update_types.StateUpdate) -> None: + if state_update.operated_liquid != update_types.NO_CHANGE: + labware_id = state_update.operated_liquid.labware_id + well_name = state_update.operated_liquid.well_name + # clear well probed_height info? Update types.py docstring + prev_loaded_vol_info = self._state.loaded_volumes[labware_id][well_name] + if prev_loaded_vol_info.volume: + self._state.loaded_volumes[labware_id][well_name] = LoadedVolumeInfo( + volume=prev_loaded_vol_info.volume + + state_update.operated_liquid.volume, + last_loaded=prev_loaded_vol_info.last_loaded, + operations_since_load=prev_loaded_vol_info.operations_since_load + + 1, + ) + prev_probed_vol_info = self._state.probed_volumes[labware_id][well_name] + if prev_probed_vol_info.volume: + self._state.probed_volumes[labware_id][well_name] = ProbedVolumeInfo( + volume=prev_probed_vol_info.volume + + state_update.operated_liquid.volume, + last_probed=prev_probed_vol_info.last_probed, + operations_since_probe=prev_probed_vol_info.operations_since_probe + + 1, + ) class WellView(HasState[WellState]): @@ -151,29 +105,35 @@ def __init__(self, state: WellState) -> None: """ self._state = state + # if volume requested, loaded_volumes or probed_volumes + # if height requested, probed_heights or loaded_vols_to_height or probed_vols_to_height + # to get height, call GeometryView.get_well_height, which does conversion if needed + + # update this def get_all(self) -> List[LiquidHeightSummary]: """Get all well liquid heights.""" all_heights: List[LiquidHeightSummary] = [] - for labware, wells in self._state.measured_liquid_heights.items(): + for labware, wells in self._state.probed_heights.items(): for well, lhi in wells.items(): lhs = LiquidHeightSummary( labware_id=labware, well_name=well, height=lhi.height, - last_measured=lhi.last_measured, + last_measured=lhi.last_probed, ) all_heights.append(lhs) return all_heights + # update this def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightSummary]: """Get all well liquid heights for a particular labware.""" all_heights: List[LiquidHeightSummary] = [] - for well, lhi in self._state.measured_liquid_heights[labware_id].items(): + for well, lhi in self._state.probed_heights[labware_id].items(): lhs = LiquidHeightSummary( labware_id=labware_id, well_name=well, height=lhi.height, - last_measured=lhi.last_measured, + last_measured=lhi.last_probed, ) all_heights.append(lhs) return all_heights @@ -186,7 +146,7 @@ def get_last_measured_liquid_height( Returns None if no liquid probe has been done. """ try: - height = self._state.measured_liquid_heights[labware_id][well_name].height + height = self._state.probed_heights[labware_id][well_name].height return height except KeyError: return None @@ -194,8 +154,6 @@ 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: - return bool( - self._state.measured_liquid_heights[labware_id][well_name].height - ) + return bool(self._state.probed_heights[labware_id][well_name].height) except KeyError: return False diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index d8b9345ab71..1bd4e7afd88 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -355,12 +355,27 @@ class CurrentWell: well_name: str -class LiquidHeightInfo(BaseModel): - """Payload required to store recent measured liquid heights.""" +class ProbedHeightInfo(BaseModel): + """A well's liquid height, initialized by a LiquidProbe, cleared by Aspirate and Dispense.""" - height: float - last_measured: datetime - operations_since_measurement: int + height: Optional[float] = None + last_probed: datetime + + +class ProbedVolumeInfo(BaseModel): + """A well's liquid volume, initialized by a LiquidProbe, updated by Aspirate and Dispense.""" + + volume: Optional[float] = None + last_probed: datetime + operations_since_probe: int + + +class LoadedVolumeInfo(BaseModel): + """A well's liquid volume, initialized by a LoadLiquid, updated by Aspirate and Dispense.""" + + volume: Optional[float] = None + last_loaded: datetime + operations_since_load: int class LiquidHeightSummary(BaseModel): @@ -368,8 +383,8 @@ class LiquidHeightSummary(BaseModel): labware_id: str well_name: str - height: float last_measured: datetime + height: Optional[float] = None @dataclass(frozen=True) From 52bc91e35aa858957601d32ed7b4e966e09b001e Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 11:53:40 -0400 Subject: [PATCH 07/18] updated setting and getting liquid info and linted --- .../protocol_engine/state/geometry.py | 34 ++++++++++++++----- .../protocol_engine/state/update_types.py | 6 ++-- .../opentrons/protocol_engine/state/wells.py | 22 ++++++++---- .../commands/test_load_liquid.py | 7 ++-- .../protocol_engine/state/test_well_store.py | 4 +-- .../protocol_engine/state/test_well_view.py | 10 ++++-- 6 files changed, 59 insertions(+), 24 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 0ebc5eca720..4172d6164cc 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -9,7 +9,6 @@ from opentrons.types import Point, DeckSlotName, StagingSlotName, MountType from opentrons_shared_data.labware.constants import WELL_NAME_PATTERN -from opentrons_shared_data.labware.labware_definition import InnerWellGeometry from opentrons_shared_data.deck.types import CutoutFixture from opentrons_shared_data.pipette import PIPETTE_X_SPAN from opentrons_shared_data.pipette.types import ChannelCount @@ -1372,6 +1371,7 @@ def get_well_offset_adjustment( Distance is with reference to the well bottom. """ + # refactor? Consider how many conversions are being done initial_handling_height = self.get_well_handling_height( labware_id=labware_id, well_name=well_name, @@ -1401,15 +1401,32 @@ def get_meniscus_height( well_name: str, ) -> float: """Returns stored meniscus height in specified well.""" - meniscus_height = self._wells.get_last_measured_liquid_height( - labware_id=labware_id, well_name=well_name - ) - if meniscus_height is None: - raise errors.LiquidHeightUnknownError( - "Must liquid probe before specifying WellOrigin.MENISCUS." + ( + loaded_volume_info, + probed_height_info, + probed_volume_info, + ) = self._wells.get_well_liquid_info(labware_id=labware_id, well_name=well_name) + if probed_height_info: + assert probed_height_info.height is not None + return probed_height_info.height + elif loaded_volume_info: + assert loaded_volume_info.volume is not None + return self.get_well_height_at_volume( + labware_id=labware_id, + well_name=well_name, + volume=loaded_volume_info.volume, + ) + elif probed_volume_info: + assert probed_volume_info.volume is not None + return self.get_well_height_at_volume( + labware_id=labware_id, + well_name=well_name, + volume=probed_volume_info.volume, ) else: - return meniscus_height + raise errors.LiquidHeightUnknownError( + "Must LiquidProbe or LoadLiquid before specifying WellOrigin.MENISCUS." + ) def get_well_handling_height( self, @@ -1474,6 +1491,7 @@ def validate_dispense_volume_into_well( volume: float, ) -> None: """Raise InvalidDispenseVolumeError if planned dispense volume will overflow well.""" + # update this (or where it's used)? Consider how many conversions are being done well_def = self._labware.get_well_definition(labware_id, well_name) well_volumetric_capacity = well_def.totalLiquidVolume if well_location.origin == WellOrigin.MENISCUS: diff --git a/api/src/opentrons/protocol_engine/state/update_types.py b/api/src/opentrons/protocol_engine/state/update_types.py index 91bc8c7d0d2..f37907976e0 100644 --- a/api/src/opentrons/protocol_engine/state/update_types.py +++ b/api/src/opentrons/protocol_engine/state/update_types.py @@ -243,7 +243,7 @@ def set_pipette_location( new_well_name: str, new_deck_point: DeckPoint, ) -> None: - """Schedule a pipette's location to be set to a well.""" + pass @overload def set_pipette_location( @@ -253,10 +253,9 @@ def set_pipette_location( new_addressable_area_name: str, new_deck_point: DeckPoint, ) -> None: - """Schedule a pipette's location to be set to an addressable area.""" pass - def set_pipette_location( # noqa: D102 + def set_pipette_location( self, *, pipette_id: str, @@ -265,6 +264,7 @@ def set_pipette_location( # noqa: D102 new_addressable_area_name: str | NoChangeType = NO_CHANGE, new_deck_point: DeckPoint, ) -> None: + """Schedule a pipette's location to be set to a well or an addressable area.""" if new_addressable_area_name != NO_CHANGE: self.pipette_location = PipetteLocationUpdate( pipette_id=pipette_id, diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 7a33a79d5ac..abdf4105707 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,10 +1,7 @@ """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 typing import Dict, List, Optional, Tuple + from opentrons.protocol_engine.types import ( ProbedHeightInfo, ProbedVolumeInfo, @@ -71,7 +68,7 @@ def _set_operated_liquid(self, state_update: update_types.StateUpdate) -> None: if state_update.operated_liquid != update_types.NO_CHANGE: labware_id = state_update.operated_liquid.labware_id well_name = state_update.operated_liquid.well_name - # clear well probed_height info? Update types.py docstring + del self._state.probed_heights[labware_id][well_name] prev_loaded_vol_info = self._state.loaded_volumes[labware_id][well_name] if prev_loaded_vol_info.volume: self._state.loaded_volumes[labware_id][well_name] = LoadedVolumeInfo( @@ -157,3 +154,16 @@ def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: return bool(self._state.probed_heights[labware_id][well_name].height) except KeyError: return False + + def get_well_liquid_info( + self, labware_id: str, well_name: str + ) -> Tuple[ + Optional[LoadedVolumeInfo], + Optional[ProbedHeightInfo], + Optional[ProbedVolumeInfo], + ]: + """Return all the liquid info for a well.""" + loaded_volume_info = self._state.loaded_volumes[labware_id][well_name] + probed_height_info = self._state.probed_heights[labware_id][well_name] + probed_volume_info = self._state.probed_volumes[labware_id][well_name] + return loaded_volume_info, probed_height_info, probed_volume_info diff --git a/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py b/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py index 3ccaaea15d0..5674dc057e8 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py +++ b/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py @@ -9,6 +9,7 @@ LoadLiquidParams, ) from opentrons.protocol_engine.state.state import StateView +from opentrons.protocol_engine.resources.model_utils import ModelUtils @pytest.fixture @@ -18,9 +19,11 @@ def mock_state_view(decoy: Decoy) -> StateView: @pytest.fixture -def subject(mock_state_view: StateView) -> LoadLiquidImplementation: +def subject( + mock_state_view: StateView, model_utils: ModelUtils +) -> LoadLiquidImplementation: """Load liquid implementation test subject.""" - return LoadLiquidImplementation(state_view=mock_state_view) + return LoadLiquidImplementation(state_view=mock_state_view, model_utils=model_utils) async def test_load_liquid_implementation( 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 325021a9942..6ae5387a218 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -23,6 +23,6 @@ def test_handles_liquid_probe_success(subject: WellStore) -> None: SucceedCommandAction(private_result=None, command=liquid_probe) ) - assert len(subject.state.measured_liquid_heights) == 1 + assert len(subject.state.probed_heights) == 1 - assert subject.state.measured_liquid_heights[labware_id][well_name].height == 0.5 + assert subject.state.probed_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 3bd86e9dcb9..fa2e19188d6 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 +from opentrons.protocol_engine.types import ProbedHeightInfo import pytest from opentrons.protocol_engine.state.wells import WellState, WellView @@ -10,8 +10,12 @@ def subject() -> WellView: """Get a well view test subject.""" labware_id = "labware-id" well_name = "well-name" - height_info = LiquidHeightInfo(height=0.5, last_measured=datetime.now()) - state = WellState(measured_liquid_heights={labware_id: {well_name: height_info}}) + height_info = ProbedHeightInfo(height=0.5, last_probed=datetime.now()) + state = WellState( + loaded_volumes={}, + probed_heights={labware_id: {well_name: height_info}}, + probed_volumes={}, + ) return WellView(state) From d5f021d814b60dd99377802b118dcd35281db761 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 13:51:16 -0400 Subject: [PATCH 08/18] for less diff --- api/src/opentrons/protocol_engine/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 1bd4e7afd88..78df5942033 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -383,8 +383,8 @@ class LiquidHeightSummary(BaseModel): labware_id: str well_name: str - last_measured: datetime height: Optional[float] = None + last_measured: datetime @dataclass(frozen=True) From 067b490071a22951b4d939d7b162f818651ccbfb Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 14:04:26 -0400 Subject: [PATCH 09/18] reverted typing changes --- .../protocol_engine/state/update_types.py | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/update_types.py b/api/src/opentrons/protocol_engine/state/update_types.py index f37907976e0..fbe3aa18173 100644 --- a/api/src/opentrons/protocol_engine/state/update_types.py +++ b/api/src/opentrons/protocol_engine/state/update_types.py @@ -3,7 +3,7 @@ import dataclasses import enum -from typing import Final, TypeAlias, Literal, Dict, Optional, overload +import typing # import Final, TypeAlias, Literal, Dict, Optional, overload from datetime import datetime from opentrons.hardware_control.nozzle_manager import NozzleMap @@ -18,14 +18,14 @@ class _NoChangeEnum(enum.Enum): NO_CHANGE = enum.auto() -NO_CHANGE: Final = _NoChangeEnum.NO_CHANGE +NO_CHANGE: typing.Final = _NoChangeEnum.NO_CHANGE """A sentinel value to indicate that a value shouldn't be changed. Useful when `None` is semantically unclear or already has some other meaning. """ -NoChangeType: TypeAlias = Literal[_NoChangeEnum.NO_CHANGE] +NoChangeType: typing.TypeAlias = typing.Literal[_NoChangeEnum.NO_CHANGE] """The type of `NO_CHANGE`, as `NoneType` is to `None`. Unfortunately, mypy doesn't let us write `Literal[NO_CHANGE]`. Use this instead. @@ -36,14 +36,14 @@ class _ClearEnum(enum.Enum): CLEAR = enum.auto() -CLEAR: Final = _ClearEnum.CLEAR +CLEAR: typing.Final = _ClearEnum.CLEAR """A sentinel value to indicate that a value should be cleared. Useful when `None` is semantically unclear or has some other meaning. """ -ClearType: TypeAlias = Literal[_ClearEnum.CLEAR] +ClearType: typing.TypeAlias = typing.Literal[_ClearEnum.CLEAR] """The type of `CLEAR`, as `NoneType` is to `None`. Unfortunately, mypy doesn't let us write `Literal[CLEAR]`. Use this instead. @@ -92,7 +92,7 @@ class LabwareLocationUpdate: new_location: LabwareLocation """The labware's new location.""" - offset_id: Optional[str] + offset_id: typing.Optional[str] """The ID of the labware's new offset, for its new location.""" @@ -106,10 +106,10 @@ class LoadedLabwareUpdate: new_location: LabwareLocation """The labware's initial location.""" - offset_id: Optional[str] + offset_id: typing.Optional[str] """The ID of the labware's offset.""" - display_name: Optional[str] + display_name: typing.Optional[str] definition: LabwareDefinition @@ -127,7 +127,7 @@ class LoadPipetteUpdate: pipette_name: PipetteNameType mount: MountType - liquid_presence_detection: Optional[bool] + liquid_presence_detection: typing.Optional[bool] @dataclasses.dataclass @@ -156,7 +156,7 @@ class PipetteTipStateUpdate: """Update pipette tip state.""" pipette_id: str - tip_geometry: Optional[TipGeometry] + tip_geometry: typing.Optional[TipGeometry] @dataclasses.dataclass @@ -181,7 +181,7 @@ class LoadLiquidUpdate: """An update from loading a liquid.""" labware_id: str - volumes: Dict[str, float] + volumes: typing.Dict[str, float] last_loaded: datetime @@ -192,8 +192,8 @@ class ProbeLiquidUpdate: labware_id: str well_name: str last_probed: datetime - height: Optional[float] = None - volume: Optional[float] = None + height: typing.Optional[float] = None + volume: typing.Optional[float] = None @dataclasses.dataclass @@ -234,7 +234,7 @@ class StateUpdate: # These convenience functions let the caller avoid the boilerplate of constructing a # complicated dataclass tree. - @overload + @typing.overload def set_pipette_location( self, *, @@ -243,9 +243,9 @@ def set_pipette_location( new_well_name: str, new_deck_point: DeckPoint, ) -> None: - pass + """Schedule a pipette's location to be set to a well.""" - @overload + @typing.overload def set_pipette_location( self, *, @@ -253,9 +253,10 @@ def set_pipette_location( new_addressable_area_name: str, new_deck_point: DeckPoint, ) -> None: + """Schedule a pipette's location to be set to an addressable area.""" pass - def set_pipette_location( + def set_pipette_location( # noqa: D102 self, *, pipette_id: str, @@ -264,7 +265,6 @@ def set_pipette_location( new_addressable_area_name: str | NoChangeType = NO_CHANGE, new_deck_point: DeckPoint, ) -> None: - """Schedule a pipette's location to be set to a well or an addressable area.""" if new_addressable_area_name != NO_CHANGE: self.pipette_location = PipetteLocationUpdate( pipette_id=pipette_id, @@ -306,8 +306,8 @@ def set_loaded_labware( self, definition: LabwareDefinition, labware_id: str, - offset_id: Optional[str], - display_name: Optional[str], + offset_id: typing.Optional[str], + display_name: typing.Optional[str], location: LabwareLocation, ) -> None: """Add a new labware to state. See `LoadedLabwareUpdate`.""" @@ -324,7 +324,7 @@ def set_load_pipette( pipette_id: str, pipette_name: PipetteNameType, mount: MountType, - liquid_presence_detection: Optional[bool], + liquid_presence_detection: typing.Optional[bool], ) -> None: """Add a new pipette to state. See `LoadPipetteUpdate`.""" self.loaded_pipette = LoadPipetteUpdate( @@ -352,7 +352,7 @@ def update_pipette_nozzle(self, pipette_id: str, nozzle_map: NozzleMap) -> None: ) def update_pipette_tip_state( - self, pipette_id: str, tip_geometry: Optional[TipGeometry] + self, pipette_id: str, tip_geometry: typing.Optional[TipGeometry] ) -> None: """Update a pipette's tip state. See `PipetteTipStateUpdate`.""" self.pipette_tip_state = PipetteTipStateUpdate( @@ -370,7 +370,7 @@ def mark_tips_as_used( def set_loaded_liquid( self, labware_id: str, - volumes: Dict[str, float], + volumes: typing.Dict[str, float], last_loaded: datetime, ) -> None: """Add liquid volumes to well state. See `LoadLiquidUpdate`.""" @@ -385,8 +385,8 @@ def set_probed_liquid( labware_id: str, well_name: str, last_probed: datetime, - height: Optional[float] = None, - volume: Optional[float] = None, + height: typing.Optional[float] = None, + volume: typing.Optional[float] = None, ) -> None: """Add a liquid height and volume to well state. See `ProbeLiquidUpdate`.""" self.probed_liquid = ProbeLiquidUpdate( From e8b8d6abea242e5bea7f65a743e4fd93d5dcd73c Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 14:04:58 -0400 Subject: [PATCH 10/18] 1 more revert --- api/src/opentrons/protocol_engine/state/update_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/state/update_types.py b/api/src/opentrons/protocol_engine/state/update_types.py index fbe3aa18173..2e105f73ace 100644 --- a/api/src/opentrons/protocol_engine/state/update_types.py +++ b/api/src/opentrons/protocol_engine/state/update_types.py @@ -3,7 +3,7 @@ import dataclasses import enum -import typing # import Final, TypeAlias, Literal, Dict, Optional, overload +import typing from datetime import datetime from opentrons.hardware_control.nozzle_manager import NozzleMap From 15dd2e0d756a877335727db925b462b096a6560e Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 14:20:14 -0400 Subject: [PATCH 11/18] updated naming --- .../opentrons/protocol_engine/state/wells.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index abdf4105707..df5a32260c0 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -36,11 +36,13 @@ 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_loaded_liquid(state_update) - self._set_probed_liquid(state_update) - self._set_operated_liquid(state_update) + self._handle_loaded_liquid_update(state_update) + self._handle_probed_liquid_update(state_update) + self._handle_operated_liquid_update(state_update) - def _set_loaded_liquid(self, state_update: update_types.StateUpdate) -> None: + def _handle_loaded_liquid_update( + self, state_update: update_types.StateUpdate + ) -> None: if state_update.loaded_liquid != update_types.NO_CHANGE: labware_id = state_update.loaded_liquid.labware_id for (well, volume) in state_update.loaded_liquid.volumes.items(): @@ -50,7 +52,9 @@ def _set_loaded_liquid(self, state_update: update_types.StateUpdate) -> None: operations_since_load=0, ) - def _set_probed_liquid(self, state_update: update_types.StateUpdate) -> None: + def _handle_probed_liquid_update( + self, state_update: update_types.StateUpdate + ) -> None: if state_update.probed_liquid != update_types.NO_CHANGE: labware_id = state_update.probed_liquid.labware_id well_name = state_update.probed_liquid.well_name @@ -64,7 +68,9 @@ def _set_probed_liquid(self, state_update: update_types.StateUpdate) -> None: operations_since_probe=0, ) - def _set_operated_liquid(self, state_update: update_types.StateUpdate) -> None: + def _handle_operated_liquid_update( + self, state_update: update_types.StateUpdate + ) -> None: if state_update.operated_liquid != update_types.NO_CHANGE: labware_id = state_update.operated_liquid.labware_id well_name = state_update.operated_liquid.well_name From f5b5302d863ad02994086cabf7f50feae01d833e Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 15:27:29 -0400 Subject: [PATCH 12/18] fixed WellStore dict handling --- .../opentrons/protocol_engine/state/wells.py | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index df5a32260c0..0e8862b3872 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -45,6 +45,8 @@ def _handle_loaded_liquid_update( ) -> None: if state_update.loaded_liquid != update_types.NO_CHANGE: labware_id = state_update.loaded_liquid.labware_id + if labware_id not in self._state.loaded_volumes: + self._state.loaded_volumes[labware_id] = {} for (well, volume) in state_update.loaded_liquid.volumes.items(): self._state.loaded_volumes[labware_id][well] = LoadedVolumeInfo( volume=volume, @@ -58,6 +60,10 @@ def _handle_probed_liquid_update( if state_update.probed_liquid != update_types.NO_CHANGE: labware_id = state_update.probed_liquid.labware_id well_name = state_update.probed_liquid.well_name + if labware_id not in self._state.probed_heights: + self._state.probed_heights[labware_id] = {} + if labware_id not in self._state.probed_volumes: + self._state.probed_volumes[labware_id] = {} self._state.probed_heights[labware_id][well_name] = ProbedHeightInfo( height=state_update.probed_liquid.height, last_probed=state_update.probed_liquid.last_probed, @@ -74,9 +80,12 @@ def _handle_operated_liquid_update( if state_update.operated_liquid != update_types.NO_CHANGE: labware_id = state_update.operated_liquid.labware_id well_name = state_update.operated_liquid.well_name - del self._state.probed_heights[labware_id][well_name] - prev_loaded_vol_info = self._state.loaded_volumes[labware_id][well_name] - if prev_loaded_vol_info.volume: + if ( + labware_id in self._state.loaded_volumes + and well_name in self._state.loaded_volumes[labware_id] + ): + prev_loaded_vol_info = self._state.loaded_volumes[labware_id][well_name] + assert prev_loaded_vol_info.volume is not None self._state.loaded_volumes[labware_id][well_name] = LoadedVolumeInfo( volume=prev_loaded_vol_info.volume + state_update.operated_liquid.volume, @@ -84,8 +93,17 @@ def _handle_operated_liquid_update( operations_since_load=prev_loaded_vol_info.operations_since_load + 1, ) - prev_probed_vol_info = self._state.probed_volumes[labware_id][well_name] - if prev_probed_vol_info.volume: + if ( + labware_id in self._state.probed_heights + and well_name in self._state.probed_heights[labware_id] + ): + del self._state.probed_heights[labware_id][well_name] + if ( + labware_id in self._state.probed_volumes + and well_name in self._state.probed_volumes[labware_id] + ): + prev_probed_vol_info = self._state.probed_volumes[labware_id][well_name] + assert prev_probed_vol_info.volume is not None self._state.probed_volumes[labware_id][well_name] = ProbedVolumeInfo( volume=prev_probed_vol_info.volume + state_update.operated_liquid.volume, From e2a6c3d82ad4cfb90cfa5b23dc011bdb4b8637c6 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Tue, 22 Oct 2024 16:54:51 -0400 Subject: [PATCH 13/18] renamed state update stuff and fixed tests --- .../protocol_engine/commands/aspirate.py | 2 +- .../protocol_engine/commands/dispense.py | 2 +- .../protocol_engine/commands/liquid_probe.py | 6 ++-- .../protocol_engine/commands/load_liquid.py | 2 +- .../protocol_engine/state/update_types.py | 24 +++++++------- .../opentrons/protocol_engine/state/wells.py | 32 +++++++++---------- .../protocol_engine/commands/test_aspirate.py | 21 ++++++++++-- .../protocol_engine/commands/test_dispense.py | 5 +++ .../commands/test_liquid_probe.py | 30 +++++++++++++++-- .../commands/test_load_liquid.py | 19 ++++++++++- 10 files changed, 103 insertions(+), 40 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/aspirate.py b/api/src/opentrons/protocol_engine/commands/aspirate.py index 8e3dcde80eb..2ea8cd821b5 100644 --- a/api/src/opentrons/protocol_engine/commands/aspirate.py +++ b/api/src/opentrons/protocol_engine/commands/aspirate.py @@ -157,7 +157,7 @@ async def execute(self, params: AspirateParams) -> _ExecuteReturn: state_update=state_update, ) else: - state_update.set_operated_liquid( + state_update.set_liquid_operated( labware_id=labware_id, well_name=well_name, volume=-volume_aspirated, diff --git a/api/src/opentrons/protocol_engine/commands/dispense.py b/api/src/opentrons/protocol_engine/commands/dispense.py index 1285d159360..0386a450e95 100644 --- a/api/src/opentrons/protocol_engine/commands/dispense.py +++ b/api/src/opentrons/protocol_engine/commands/dispense.py @@ -124,7 +124,7 @@ async def execute(self, params: DispenseParams) -> _ExecuteReturn: state_update=state_update, ) else: - state_update.set_operated_liquid( + state_update.set_liquid_operated( labware_id=labware_id, well_name=well_name, volume=volume, diff --git a/api/src/opentrons/protocol_engine/commands/liquid_probe.py b/api/src/opentrons/protocol_engine/commands/liquid_probe.py index b58fea92219..8054bcc3e75 100644 --- a/api/src/opentrons/protocol_engine/commands/liquid_probe.py +++ b/api/src/opentrons/protocol_engine/commands/liquid_probe.py @@ -205,7 +205,7 @@ async def execute(self, params: _CommonParams) -> _LiquidProbeExecuteReturn: self._state_view, self._movement, self._pipetting, params ) if isinstance(z_pos_or_error, PipetteLiquidNotFoundError): - state_update.set_probed_liquid( + state_update.set_liquid_probed( labware_id=params.labwareId, well_name=params.wellName, height=None, @@ -232,7 +232,7 @@ async def execute(self, params: _CommonParams) -> _LiquidProbeExecuteReturn: well_name=params.wellName, height=z_pos_or_error, ) - state_update.set_probed_liquid( + state_update.set_liquid_probed( labware_id=params.labwareId, well_name=params.wellName, height=z_pos_or_error, @@ -286,7 +286,7 @@ async def execute(self, params: _CommonParams) -> _TryLiquidProbeExecuteReturn: labware_id=params.labwareId, well_name=params.wellName, height=z_pos ) - state_update.set_probed_liquid( + state_update.set_liquid_probed( labware_id=params.labwareId, well_name=params.wellName, height=z_pos, diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index d5fdfd13ed2..d37bc537b2d 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -61,7 +61,7 @@ async def execute( ) state_update = StateUpdate() - state_update.set_loaded_liquid( + state_update.set_liquid_loaded( labware_id=params.labwareId, volumes=params.volumeByWell, last_loaded=self._model_utils.get_timestamp(), diff --git a/api/src/opentrons/protocol_engine/state/update_types.py b/api/src/opentrons/protocol_engine/state/update_types.py index 2e105f73ace..279fa7ec931 100644 --- a/api/src/opentrons/protocol_engine/state/update_types.py +++ b/api/src/opentrons/protocol_engine/state/update_types.py @@ -177,7 +177,7 @@ class TipsUsedUpdate: @dataclasses.dataclass -class LoadLiquidUpdate: +class LiquidLoadedUpdate: """An update from loading a liquid.""" labware_id: str @@ -186,7 +186,7 @@ class LoadLiquidUpdate: @dataclasses.dataclass -class ProbeLiquidUpdate: +class LiquidProbedUpdate: """An update from probing a liquid.""" labware_id: str @@ -197,7 +197,7 @@ class ProbeLiquidUpdate: @dataclasses.dataclass -class OperateLiquidUpdate: +class LiquidOperatedUpdate: """An update from operating a liquid.""" labware_id: str @@ -225,11 +225,11 @@ class StateUpdate: tips_used: TipsUsedUpdate | NoChangeType = NO_CHANGE - loaded_liquid: LoadLiquidUpdate | NoChangeType = NO_CHANGE + liquid_loaded: LiquidLoadedUpdate | NoChangeType = NO_CHANGE - probed_liquid: ProbeLiquidUpdate | NoChangeType = NO_CHANGE + liquid_probed: LiquidProbedUpdate | NoChangeType = NO_CHANGE - operated_liquid: OperateLiquidUpdate | NoChangeType = NO_CHANGE + liquid_operated: LiquidOperatedUpdate | NoChangeType = NO_CHANGE # These convenience functions let the caller avoid the boilerplate of constructing a # complicated dataclass tree. @@ -367,20 +367,20 @@ def mark_tips_as_used( pipette_id=pipette_id, labware_id=labware_id, well_name=well_name ) - def set_loaded_liquid( + def set_liquid_loaded( self, labware_id: str, volumes: typing.Dict[str, float], last_loaded: datetime, ) -> None: """Add liquid volumes to well state. See `LoadLiquidUpdate`.""" - self.loaded_liquid = LoadLiquidUpdate( + self.liquid_loaded = LiquidLoadedUpdate( labware_id=labware_id, volumes=volumes, last_loaded=last_loaded, ) - def set_probed_liquid( + def set_liquid_probed( self, labware_id: str, well_name: str, @@ -389,7 +389,7 @@ def set_probed_liquid( volume: typing.Optional[float] = None, ) -> None: """Add a liquid height and volume to well state. See `ProbeLiquidUpdate`.""" - self.probed_liquid = ProbeLiquidUpdate( + self.liquid_probed = LiquidProbedUpdate( labware_id=labware_id, well_name=well_name, height=height, @@ -397,14 +397,14 @@ def set_probed_liquid( last_probed=last_probed, ) - def set_operated_liquid( + def set_liquid_operated( self, labware_id: str, well_name: str, volume: float, ) -> None: """Update liquid volumes in well state. See `OperateLiquidUpdate`.""" - self.operated_liquid = OperateLiquidUpdate( + self.liquid_operated = LiquidOperatedUpdate( labware_id=labware_id, well_name=well_name, volume=volume, diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 0e8862b3872..e807d124b82 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -43,43 +43,43 @@ def handle_action(self, action: Action) -> None: def _handle_loaded_liquid_update( self, state_update: update_types.StateUpdate ) -> None: - if state_update.loaded_liquid != update_types.NO_CHANGE: - labware_id = state_update.loaded_liquid.labware_id + if state_update.liquid_loaded != update_types.NO_CHANGE: + labware_id = state_update.liquid_loaded.labware_id if labware_id not in self._state.loaded_volumes: self._state.loaded_volumes[labware_id] = {} - for (well, volume) in state_update.loaded_liquid.volumes.items(): + for (well, volume) in state_update.liquid_loaded.volumes.items(): self._state.loaded_volumes[labware_id][well] = LoadedVolumeInfo( volume=volume, - last_loaded=state_update.loaded_liquid.last_loaded, + last_loaded=state_update.liquid_loaded.last_loaded, operations_since_load=0, ) def _handle_probed_liquid_update( self, state_update: update_types.StateUpdate ) -> None: - if state_update.probed_liquid != update_types.NO_CHANGE: - labware_id = state_update.probed_liquid.labware_id - well_name = state_update.probed_liquid.well_name + if state_update.liquid_probed != update_types.NO_CHANGE: + labware_id = state_update.liquid_probed.labware_id + well_name = state_update.liquid_probed.well_name if labware_id not in self._state.probed_heights: self._state.probed_heights[labware_id] = {} if labware_id not in self._state.probed_volumes: self._state.probed_volumes[labware_id] = {} self._state.probed_heights[labware_id][well_name] = ProbedHeightInfo( - height=state_update.probed_liquid.height, - last_probed=state_update.probed_liquid.last_probed, + height=state_update.liquid_probed.height, + last_probed=state_update.liquid_probed.last_probed, ) self._state.probed_volumes[labware_id][well_name] = ProbedVolumeInfo( - volume=state_update.probed_liquid.volume, - last_probed=state_update.probed_liquid.last_probed, + volume=state_update.liquid_probed.volume, + last_probed=state_update.liquid_probed.last_probed, operations_since_probe=0, ) def _handle_operated_liquid_update( self, state_update: update_types.StateUpdate ) -> None: - if state_update.operated_liquid != update_types.NO_CHANGE: - labware_id = state_update.operated_liquid.labware_id - well_name = state_update.operated_liquid.well_name + if state_update.liquid_operated != update_types.NO_CHANGE: + labware_id = state_update.liquid_operated.labware_id + well_name = state_update.liquid_operated.well_name if ( labware_id in self._state.loaded_volumes and well_name in self._state.loaded_volumes[labware_id] @@ -88,7 +88,7 @@ def _handle_operated_liquid_update( assert prev_loaded_vol_info.volume is not None self._state.loaded_volumes[labware_id][well_name] = LoadedVolumeInfo( volume=prev_loaded_vol_info.volume - + state_update.operated_liquid.volume, + + state_update.liquid_operated.volume, last_loaded=prev_loaded_vol_info.last_loaded, operations_since_load=prev_loaded_vol_info.operations_since_load + 1, @@ -106,7 +106,7 @@ def _handle_operated_liquid_update( assert prev_probed_vol_info.volume is not None self._state.probed_volumes[labware_id][well_name] = ProbedVolumeInfo( volume=prev_probed_vol_info.volume - + state_update.operated_liquid.volume, + + state_update.liquid_operated.volume, last_probed=prev_probed_vol_info.last_probed, operations_since_probe=prev_probed_vol_info.operations_since_probe + 1, diff --git a/api/tests/opentrons/protocol_engine/commands/test_aspirate.py b/api/tests/opentrons/protocol_engine/commands/test_aspirate.py index 8d6f6d92179..5055d1ae185 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_aspirate.py +++ b/api/tests/opentrons/protocol_engine/commands/test_aspirate.py @@ -109,7 +109,12 @@ async def test_aspirate_implementation_no_prep( pipette_id="abc", new_location=update_types.Well(labware_id="123", well_name="A3"), new_deck_point=DeckPoint(x=1, y=2, z=3), - ) + ), + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id="123", + well_name="A3", + volume=-50, + ), ), ) @@ -178,7 +183,12 @@ async def test_aspirate_implementation_with_prep( pipette_id="abc", new_location=update_types.Well(labware_id="123", well_name="A3"), new_deck_point=DeckPoint(x=1, y=2, z=3), - ) + ), + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id="123", + well_name="A3", + volume=-50, + ), ), ) @@ -378,6 +388,11 @@ async def test_aspirate_implementation_meniscus( pipette_id="abc", new_location=update_types.Well(labware_id="123", well_name="A3"), new_deck_point=DeckPoint(x=1, y=2, z=3), - ) + ), + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id="123", + well_name="A3", + volume=-50, + ), ), ) diff --git a/api/tests/opentrons/protocol_engine/commands/test_dispense.py b/api/tests/opentrons/protocol_engine/commands/test_dispense.py index 167223e6d9d..0a227d1958a 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_dispense.py +++ b/api/tests/opentrons/protocol_engine/commands/test_dispense.py @@ -92,6 +92,11 @@ async def test_dispense_implementation( ), new_deck_point=DeckPoint.construct(x=1, y=2, z=3), ), + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id="labware-id-abc123", + well_name="A3", + volume=42, + ), ), ) diff --git a/api/tests/opentrons/protocol_engine/commands/test_liquid_probe.py b/api/tests/opentrons/protocol_engine/commands/test_liquid_probe.py index 6fb6ebc6935..fef29da18ce 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_liquid_probe.py +++ b/api/tests/opentrons/protocol_engine/commands/test_liquid_probe.py @@ -104,6 +104,7 @@ async def test_liquid_probe_implementation( subject: EitherImplementation, params_type: EitherParamsType, result_type: EitherResultType, + model_utils: ModelUtils, ) -> None: """It should move to the destination and do a liquid probe there.""" location = WellLocation(origin=WellOrigin.BOTTOM, offset=WellOffset(x=0, y=0, z=1)) @@ -137,6 +138,17 @@ async def test_liquid_probe_implementation( ), ).then_return(15.0) + decoy.when( + state_view.geometry.get_well_volume_at_height( + labware_id="123", + well_name="A3", + height=15.0, + ), + ).then_return(30.0) + + timestamp = datetime(year=2020, month=1, day=2) + decoy.when(model_utils.get_timestamp()).then_return(timestamp) + result = await subject.execute(data) assert type(result.public) is result_type # Pydantic v1 only compares the fields. @@ -148,7 +160,14 @@ async def test_liquid_probe_implementation( pipette_id="abc", new_location=update_types.Well(labware_id="123", well_name="A3"), new_deck_point=DeckPoint(x=1, y=2, z=3), - ) + ), + liquid_probed=update_types.LiquidProbedUpdate( + labware_id="123", + well_name="A3", + height=15.0, + volume=30.0, + last_probed=timestamp, + ), ), ) @@ -212,7 +231,14 @@ async def test_liquid_not_found_error( pipette_id=pipette_id, new_location=update_types.Well(labware_id=labware_id, well_name=well_name), new_deck_point=DeckPoint(x=position.x, y=position.y, z=position.z), - ) + ), + liquid_probed=update_types.LiquidProbedUpdate( + labware_id=labware_id, + well_name=well_name, + height=None, + volume=None, + last_probed=error_timestamp, + ), ) if isinstance(subject, LiquidProbeImplementation): assert result == DefinedErrorData( diff --git a/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py b/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py index 5674dc057e8..d8641b1eb4b 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py +++ b/api/tests/opentrons/protocol_engine/commands/test_load_liquid.py @@ -1,6 +1,7 @@ """Test load-liquid command.""" import pytest from decoy import Decoy +from datetime import datetime from opentrons.protocol_engine.commands.command import SuccessData from opentrons.protocol_engine.commands import ( @@ -10,6 +11,7 @@ ) from opentrons.protocol_engine.state.state import StateView from opentrons.protocol_engine.resources.model_utils import ModelUtils +from opentrons.protocol_engine.state import update_types @pytest.fixture @@ -30,6 +32,7 @@ async def test_load_liquid_implementation( decoy: Decoy, subject: LoadLiquidImplementation, mock_state_view: StateView, + model_utils: ModelUtils, ) -> None: """Test LoadLiquid command execution.""" data = LoadLiquidParams( @@ -37,9 +40,23 @@ async def test_load_liquid_implementation( liquidId="liquid-id", volumeByWell={"A1": 30, "B2": 100}, ) + + timestamp = datetime(year=2020, month=1, day=2) + decoy.when(model_utils.get_timestamp()).then_return(timestamp) + result = await subject.execute(data) - assert result == SuccessData(public=LoadLiquidResult(), private=None) + assert result == SuccessData( + public=LoadLiquidResult(), + private=None, + state_update=update_types.StateUpdate( + liquid_loaded=update_types.LiquidLoadedUpdate( + labware_id="labware-id", + volumes={"A1": 30, "B2": 100}, + last_loaded=timestamp, + ) + ), + ) decoy.verify(mock_state_view.liquid.validate_liquid_id("liquid-id")) From a6e23b02f2a43f3731481f75ccbd5781b022b555 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Wed, 23 Oct 2024 12:49:34 -0400 Subject: [PATCH 14/18] refactored getting liquid values, fixed and added tests --- .../protocol_engine/state/geometry.py | 25 +- .../opentrons/protocol_engine/state/wells.py | 18 +- .../protocol_engine/state/command_fixtures.py | 25 +- .../state/test_geometry_view.py | 94 ++++++-- .../protocol_engine/state/test_well_store.py | 216 +++++++++++++++++- 5 files changed, 334 insertions(+), 44 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 4172d6164cc..c6da945bc3a 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1402,26 +1402,25 @@ def get_meniscus_height( ) -> float: """Returns stored meniscus height in specified well.""" ( - loaded_volume_info, - probed_height_info, - probed_volume_info, - ) = self._wells.get_well_liquid_info(labware_id=labware_id, well_name=well_name) - if probed_height_info: - assert probed_height_info.height is not None - return probed_height_info.height - elif loaded_volume_info: - assert loaded_volume_info.volume is not None + loaded_volume, + probed_height, + probed_volume, + ) = self._wells.get_well_liquid_values( + labware_id=labware_id, well_name=well_name + ) + if probed_height: + return probed_height + elif loaded_volume: return self.get_well_height_at_volume( labware_id=labware_id, well_name=well_name, - volume=loaded_volume_info.volume, + volume=loaded_volume, ) - elif probed_volume_info: - assert probed_volume_info.volume is not None + elif probed_volume: return self.get_well_height_at_volume( labware_id=labware_id, well_name=well_name, - volume=probed_volume_info.volume, + volume=probed_volume, ) else: raise errors.LiquidHeightUnknownError( diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index e807d124b82..0bbfdd48d79 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -179,15 +179,11 @@ def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: except KeyError: return False - def get_well_liquid_info( + def get_well_liquid_values( self, labware_id: str, well_name: str - ) -> Tuple[ - Optional[LoadedVolumeInfo], - Optional[ProbedHeightInfo], - Optional[ProbedVolumeInfo], - ]: - """Return all the liquid info for a well.""" - loaded_volume_info = self._state.loaded_volumes[labware_id][well_name] - probed_height_info = self._state.probed_heights[labware_id][well_name] - probed_volume_info = self._state.probed_volumes[labware_id][well_name] - return loaded_volume_info, probed_height_info, probed_volume_info + ) -> Tuple[Optional[float], Optional[float], Optional[float]]: + """Return all the liquid values for a well.""" + loaded_volume = self._state.loaded_volumes[labware_id][well_name].volume + probed_height = self._state.probed_heights[labware_id][well_name].height + probed_volume = self._state.probed_volumes[labware_id][well_name].volume + return loaded_volume, probed_height, probed_volume diff --git a/api/tests/opentrons/protocol_engine/state/command_fixtures.py b/api/tests/opentrons/protocol_engine/state/command_fixtures.py index 9c4665d31a2..5ac522095f2 100644 --- a/api/tests/opentrons/protocol_engine/state/command_fixtures.py +++ b/api/tests/opentrons/protocol_engine/state/command_fixtures.py @@ -1,7 +1,7 @@ """Command factories to use in tests as data fixtures.""" from datetime import datetime from pydantic import BaseModel -from typing import Optional, cast +from typing import Optional, cast, Dict from opentrons_shared_data.pipette.types import PipetteNameType from opentrons.types import MountType @@ -338,6 +338,29 @@ def create_liquid_probe_command( ) +def create_load_liquid_command( + liquid_id: str = "liquid-id", + labware_id: str = "labware-id", + volume_by_well: Dict[str, float] = {"A1": 30, "B2": 100}, +) -> cmd.LoadLiquid: + """Get a completed Load Liquid command.""" + params = cmd.LoadLiquidParams( + liquidId=liquid_id, + labwareId=labware_id, + volumeByWell=volume_by_well, + ) + result = cmd.LoadLiquidResult() + + return cmd.LoadLiquid( + 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_geometry_view.py b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py index 7a94f06ca09..dc464966fa5 100644 --- a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py @@ -1539,9 +1539,9 @@ def test_get_well_position_with_meniscus_offset( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when( - mock_well_view.get_last_measured_liquid_height("labware-id", "B2") - ).then_return(70.5) + decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( + (None, 70.5, None) + ) decoy.when( mock_pipette_view.get_current_tip_lld_settings(pipette_id="pipette-id") ).then_return(0.5) @@ -1597,9 +1597,9 @@ def test_get_well_position_with_meniscus_and_literal_volume_offset( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when( - mock_well_view.get_last_measured_liquid_height("labware-id", "B2") - ).then_return(45.0) + decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( + (None, 45.0, None) + ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None inner_well_def = labware_def.innerLabwareGeometry["welldefinition1111"] @@ -1663,9 +1663,9 @@ def test_get_well_position_with_meniscus_and_float_volume_offset( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when( - mock_well_view.get_last_measured_liquid_height("labware-id", "B2") - ).then_return(45.0) + decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( + (None, 45.0, None) + ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None inner_well_def = labware_def.innerLabwareGeometry["welldefinition1111"] @@ -1728,9 +1728,9 @@ def test_get_well_position_raises_validation_error( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when( - mock_well_view.get_last_measured_liquid_height("labware-id", "B2") - ).then_return(40.0) + decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( + (None, 40.0, None) + ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None inner_well_def = labware_def.innerLabwareGeometry["welldefinition1111"] @@ -1755,6 +1755,70 @@ def test_get_well_position_raises_validation_error( ) +def test_get_meniscus_height( + decoy: Decoy, + well_plate_def: LabwareDefinition, + mock_labware_view: LabwareView, + mock_well_view: WellView, + mock_addressable_area_view: AddressableAreaView, + mock_pipette_view: PipetteView, + subject: GeometryView, +) -> None: + """It should be able to get the position of a well meniscus in a labware.""" + labware_data = LoadedLabware( + id="labware-id", + loadName="load-name", + definitionUri="definition-uri", + location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4), + offsetId="offset-id", + ) + calibration_offset = LabwareOffsetVector(x=1, y=-2, z=3) + slot_pos = Point(4, 5, 6) + well_def = well_plate_def.wells["B2"] + + decoy.when(mock_labware_view.get("labware-id")).then_return(labware_data) + decoy.when(mock_labware_view.get_definition("labware-id")).then_return( + well_plate_def + ) + decoy.when(mock_labware_view.get_labware_offset_vector("labware-id")).then_return( + calibration_offset + ) + decoy.when( + mock_addressable_area_view.get_addressable_area_position(DeckSlotName.SLOT_4.id) + ).then_return(slot_pos) + decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( + well_def + ) + decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( + (2000.0, None, None) + ) + labware_def = _load_labware_definition_data() + assert labware_def.innerLabwareGeometry is not None + inner_well_def = labware_def.innerLabwareGeometry["welldefinition1111"] + decoy.when(mock_labware_view.get_well_geometry("labware-id", "B2")).then_return( + inner_well_def + ) + decoy.when( + mock_pipette_view.get_current_tip_lld_settings(pipette_id="pipette-id") + ).then_return(0.5) + + result = subject.get_well_position( + labware_id="labware-id", + well_name="B2", + well_location=WellLocation( + origin=WellOrigin.MENISCUS, + offset=WellOffset(x=2, y=3, z=4), + ), + pipette_id="pipette-id", + ) + + assert result == Point( + x=slot_pos[0] + 1 + well_def.x + 2, + y=slot_pos[1] - 2 + well_def.y + 3, + z=slot_pos[2] + 3 + well_def.z + 4 + 39.2423, + ) + + def test_get_relative_well_location( decoy: Decoy, well_plate_def: LabwareDefinition, @@ -3133,9 +3197,9 @@ def test_validate_dispense_volume_into_well_meniscus( decoy.when(mock_labware_view.get_well_geometry("labware-id", "A1")).then_return( inner_well_def ) - decoy.when( - mock_well_view.get_last_measured_liquid_height("labware-id", "A1") - ).then_return(40.0) + decoy.when(mock_well_view.get_well_liquid_values("labware-id", "A1")).then_return( + (None, 40.0, None) + ) with pytest.raises(errors.InvalidDispenseVolumeError): subject.validate_dispense_volume_into_well( 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 6ae5387a218..823578f0eb4 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -1,9 +1,15 @@ """Well state store tests.""" import pytest +from datetime import datetime from opentrons.protocol_engine.state.wells import WellStore from opentrons.protocol_engine.actions.actions import SucceedCommandAction +from opentrons.protocol_engine.state import update_types -from .command_fixtures import create_liquid_probe_command +from .command_fixtures import ( + create_liquid_probe_command, + create_load_liquid_command, + create_aspirate_command, +) @pytest.fixture @@ -16,13 +22,215 @@ def test_handles_liquid_probe_success(subject: WellStore) -> None: """It should add the well to the state after a successful liquid probe.""" labware_id = "labware-id" well_name = "well-name" - liquid_probe = create_liquid_probe_command() + timestamp = datetime(year=2020, month=1, day=2) subject.handle_action( - SucceedCommandAction(private_result=None, command=liquid_probe) + SucceedCommandAction( + private_result=None, + command=liquid_probe, + state_update=update_types.StateUpdate( + liquid_probed=update_types.LiquidProbedUpdate( + labware_id="labware-id", + well_name="well-name", + height=15.0, + volume=30.0, + last_probed=timestamp, + ) + ), + ) ) assert len(subject.state.probed_heights) == 1 + assert len(subject.state.probed_volumes) == 1 + + assert subject.state.probed_heights[labware_id][well_name].height == 15.0 + assert subject.state.probed_heights[labware_id][well_name].last_probed == timestamp + assert subject.state.probed_volumes[labware_id][well_name].volume == 30.0 + assert subject.state.probed_volumes[labware_id][well_name].last_probed == timestamp + assert ( + subject.state.probed_volumes[labware_id][well_name].operations_since_probe == 0 + ) + + +def test_handles_load_liquid_success(subject: WellStore) -> None: + """It should add the well to the state after a successful load liquid.""" + labware_id = "labware-id" + well_name_1 = "well-name-1" + well_name_2 = "well-name-2" + load_liquid = create_load_liquid_command( + labware_id=labware_id, volume_by_well={well_name_1: 30, well_name_2: 100} + ) + timestamp = datetime(year=2020, month=1, day=2) + + subject.handle_action( + SucceedCommandAction( + private_result=None, + command=load_liquid, + state_update=update_types.StateUpdate( + liquid_loaded=update_types.LiquidLoadedUpdate( + labware_id=labware_id, + volumes={well_name_1: 30, well_name_2: 100}, + last_loaded=timestamp, + ) + ), + ) + ) + + assert len(subject.state.loaded_volumes) == 1 + assert len(subject.state.loaded_volumes[labware_id]) == 2 + + assert subject.state.loaded_volumes[labware_id][well_name_1].volume == 30.0 + assert ( + subject.state.loaded_volumes[labware_id][well_name_1].last_loaded == timestamp + ) + assert ( + subject.state.loaded_volumes[labware_id][well_name_1].operations_since_load == 0 + ) + assert subject.state.loaded_volumes[labware_id][well_name_2].volume == 100.0 + assert ( + subject.state.loaded_volumes[labware_id][well_name_2].last_loaded == timestamp + ) + assert ( + subject.state.loaded_volumes[labware_id][well_name_2].operations_since_load == 0 + ) + + +def test_handles_load_liquid_and_aspirate(subject: WellStore) -> None: + """It should populate the well state after load liquid and update the well state after aspirate.""" + pipette_id = "pipette-id" + labware_id = "labware-id" + well_name_1 = "well-name-1" + well_name_2 = "well-name-2" + aspirated_volume = 10.0 + load_liquid = create_load_liquid_command( + labware_id=labware_id, volume_by_well={well_name_1: 30, well_name_2: 100} + ) + aspirate_1 = create_aspirate_command( + pipette_id=pipette_id, + volume=aspirated_volume, + flow_rate=1.0, + labware_id=labware_id, + well_name=well_name_1, + ) + aspirate_2 = create_aspirate_command( + pipette_id=pipette_id, + volume=aspirated_volume, + flow_rate=1.0, + labware_id=labware_id, + well_name=well_name_2, + ) + timestamp = datetime(year=2020, month=1, day=2) + + subject.handle_action( + SucceedCommandAction( + private_result=None, + command=load_liquid, + state_update=update_types.StateUpdate( + liquid_loaded=update_types.LiquidLoadedUpdate( + labware_id=labware_id, + volumes={well_name_1: 30, well_name_2: 100}, + last_loaded=timestamp, + ) + ), + ) + ) + subject.handle_action( + SucceedCommandAction( + private_result=None, + command=aspirate_1, + state_update=update_types.StateUpdate( + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id=labware_id, + well_name=well_name_1, + volume=-aspirated_volume, + ) + ), + ) + ) + subject.handle_action( + SucceedCommandAction( + private_result=None, + command=aspirate_2, + state_update=update_types.StateUpdate( + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id=labware_id, + well_name=well_name_2, + volume=-aspirated_volume, + ) + ), + ) + ) + + assert len(subject.state.loaded_volumes) == 1 + assert len(subject.state.loaded_volumes[labware_id]) == 2 + + assert subject.state.loaded_volumes[labware_id][well_name_1].volume == 20.0 + assert ( + subject.state.loaded_volumes[labware_id][well_name_1].last_loaded == timestamp + ) + assert ( + subject.state.loaded_volumes[labware_id][well_name_1].operations_since_load == 1 + ) + assert subject.state.loaded_volumes[labware_id][well_name_2].volume == 90.0 + assert ( + subject.state.loaded_volumes[labware_id][well_name_2].last_loaded == timestamp + ) + assert ( + subject.state.loaded_volumes[labware_id][well_name_2].operations_since_load == 1 + ) + - assert subject.state.probed_heights[labware_id][well_name].height == 0.5 +def test_handles_liquid_probe_and_aspirate(subject: WellStore) -> None: + """It should populate the well state after liquid probe and update the well state after aspirate.""" + pipette_id = "pipette-id" + labware_id = "labware-id" + well_name = "well-name" + aspirated_volume = 10.0 + liquid_probe = create_liquid_probe_command() + aspirate = create_aspirate_command( + pipette_id=pipette_id, + volume=aspirated_volume, + flow_rate=1.0, + labware_id=labware_id, + well_name=well_name, + ) + timestamp = datetime(year=2020, month=1, day=2) + + subject.handle_action( + SucceedCommandAction( + private_result=None, + command=liquid_probe, + state_update=update_types.StateUpdate( + liquid_probed=update_types.LiquidProbedUpdate( + labware_id="labware-id", + well_name="well-name", + height=15.0, + volume=30.0, + last_probed=timestamp, + ) + ), + ) + ) + subject.handle_action( + SucceedCommandAction( + private_result=None, + command=aspirate, + state_update=update_types.StateUpdate( + liquid_operated=update_types.LiquidOperatedUpdate( + labware_id="labware-id", + well_name="well-name", + volume=-aspirated_volume, + ) + ), + ) + ) + + assert len(subject.state.probed_heights[labware_id]) == 0 + assert len(subject.state.probed_volumes) == 1 + + assert subject.state.probed_volumes[labware_id][well_name].volume == 20.0 + assert subject.state.probed_volumes[labware_id][well_name].last_probed == timestamp + assert ( + subject.state.probed_volumes[labware_id][well_name].operations_since_probe == 1 + ) From 2332aac2626e7b0c8e8dc802e190b13497ad505b Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Wed, 23 Oct 2024 17:13:35 -0400 Subject: [PATCH 15/18] updated WellView get methods, fixed LiquidProbe to prevent errors, added tests for IncompleteLabwareDefinitionError --- .../protocol_engine/commands/liquid_probe.py | 23 ++- .../protocol_engine/state/geometry.py | 24 ++- .../protocol_engine/state/state_summary.py | 10 +- .../opentrons/protocol_engine/state/wells.py | 137 ++++++++++-------- api/src/opentrons/protocol_engine/types.py | 30 ++-- .../state/test_geometry_view.py | 90 ++++++++++-- .../state/test_labware_view.py | 11 ++ .../protocol_engine/state/test_well_view.py | 69 +++++---- 8 files changed, 256 insertions(+), 138 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/liquid_probe.py b/api/src/opentrons/protocol_engine/commands/liquid_probe.py index 8054bcc3e75..057da787e78 100644 --- a/api/src/opentrons/protocol_engine/commands/liquid_probe.py +++ b/api/src/opentrons/protocol_engine/commands/liquid_probe.py @@ -11,6 +11,7 @@ MustHomeError, PipetteNotReadyToAspirateError, TipNotEmptyError, + IncompleteLabwareDefinitionError, ) from opentrons.types import MountType from opentrons_shared_data.errors.exceptions import ( @@ -227,11 +228,14 @@ async def execute(self, params: _CommonParams) -> _LiquidProbeExecuteReturn: state_update=state_update, ) else: - well_volume = self._state_view.geometry.get_well_volume_at_height( - labware_id=params.labwareId, - well_name=params.wellName, - height=z_pos_or_error, - ) + try: + well_volume = self._state_view.geometry.get_well_volume_at_height( + labware_id=params.labwareId, + well_name=params.wellName, + height=z_pos_or_error, + ) + except IncompleteLabwareDefinitionError: + well_volume = None state_update.set_liquid_probed( labware_id=params.labwareId, well_name=params.wellName, @@ -282,9 +286,12 @@ async def execute(self, params: _CommonParams) -> _TryLiquidProbeExecuteReturn: well_volume = None else: z_pos = z_pos_or_error - well_volume = self._state_view.geometry.get_well_volume_at_height( - labware_id=params.labwareId, well_name=params.wellName, height=z_pos - ) + try: + well_volume = self._state_view.geometry.get_well_volume_at_height( + labware_id=params.labwareId, well_name=params.wellName, height=z_pos + ) + except IncompleteLabwareDefinitionError: + well_volume = None state_update.set_liquid_probed( labware_id=params.labwareId, diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index c6da945bc3a..93e9c45e199 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1371,7 +1371,7 @@ def get_well_offset_adjustment( Distance is with reference to the well bottom. """ - # refactor? Consider how many conversions are being done + # TODO(pbm, 10-23-24): refactor to smartly reduce height/volume conversions initial_handling_height = self.get_well_handling_height( labware_id=labware_id, well_name=well_name, @@ -1402,25 +1402,23 @@ def get_meniscus_height( ) -> float: """Returns stored meniscus height in specified well.""" ( - loaded_volume, - probed_height, - probed_volume, - ) = self._wells.get_well_liquid_values( - labware_id=labware_id, well_name=well_name - ) - if probed_height: - return probed_height - elif loaded_volume: + loaded_volume_info, + probed_height_info, + probed_volume_info, + ) = self._wells.get_well_liquid_info(labware_id=labware_id, well_name=well_name) + if probed_height_info is not None and probed_height_info.height is not None: + return probed_height_info.height + elif loaded_volume_info is not None and loaded_volume_info.volume is not None: return self.get_well_height_at_volume( labware_id=labware_id, well_name=well_name, - volume=loaded_volume, + volume=loaded_volume_info.volume, ) - elif probed_volume: + elif probed_volume_info is not None and probed_volume_info.volume is not None: return self.get_well_height_at_volume( labware_id=labware_id, well_name=well_name, - volume=probed_volume, + volume=probed_volume_info.volume, ) else: raise errors.LiquidHeightUnknownError( diff --git a/api/src/opentrons/protocol_engine/state/state_summary.py b/api/src/opentrons/protocol_engine/state/state_summary.py index b1c4dd8f766..ddadd984baf 100644 --- a/api/src/opentrons/protocol_engine/state/state_summary.py +++ b/api/src/opentrons/protocol_engine/state/state_summary.py @@ -1,17 +1,19 @@ """Public protocol run data models.""" from pydantic import BaseModel, Field -from typing import List, Optional +from typing import List, Optional, Union from datetime import datetime from ..errors import ErrorOccurrence from ..types import ( EngineStatus, - LiquidHeightSummary, LoadedLabware, LabwareOffset, LoadedModule, LoadedPipette, Liquid, + LoadedVolumeSummary, + ProbedHeightSummary, + ProbedVolumeSummary, ) @@ -30,5 +32,7 @@ class StateSummary(BaseModel): startedAt: Optional[datetime] completedAt: Optional[datetime] liquids: List[Liquid] = Field(default_factory=list) - wells: List[LiquidHeightSummary] = Field(default_factory=list) + wells: List[ + Union[LoadedVolumeSummary, ProbedHeightSummary, ProbedVolumeSummary] + ] = Field(default_factory=list) files: List[str] = 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 0bbfdd48d79..91e19fcf5db 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,12 +1,14 @@ """Basic well data state and store.""" from dataclasses import dataclass -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Tuple, Union from opentrons.protocol_engine.types import ( ProbedHeightInfo, ProbedVolumeInfo, LoadedVolumeInfo, - LiquidHeightSummary, + ProbedHeightSummary, + ProbedVolumeSummary, + LoadedVolumeSummary, ) from . import update_types @@ -36,11 +38,11 @@ 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._handle_loaded_liquid_update(state_update) - self._handle_probed_liquid_update(state_update) - self._handle_operated_liquid_update(state_update) + self._handle_liquid_loaded_update(state_update) + self._handle_liquid_probed_update(state_update) + self._handle_liquid_operated_update(state_update) - def _handle_loaded_liquid_update( + def _handle_liquid_loaded_update( self, state_update: update_types.StateUpdate ) -> None: if state_update.liquid_loaded != update_types.NO_CHANGE: @@ -54,7 +56,7 @@ def _handle_loaded_liquid_update( operations_since_load=0, ) - def _handle_probed_liquid_update( + def _handle_liquid_probed_update( self, state_update: update_types.StateUpdate ) -> None: if state_update.liquid_probed != update_types.NO_CHANGE: @@ -74,7 +76,7 @@ def _handle_probed_liquid_update( operations_since_probe=0, ) - def _handle_operated_liquid_update( + def _handle_liquid_operated_update( self, state_update: update_types.StateUpdate ) -> None: if state_update.liquid_operated != update_types.NO_CHANGE: @@ -130,60 +132,71 @@ def __init__(self, state: WellState) -> None: # if height requested, probed_heights or loaded_vols_to_height or probed_vols_to_height # to get height, call GeometryView.get_well_height, which does conversion if needed - # update this - def get_all(self) -> List[LiquidHeightSummary]: - """Get all well liquid heights.""" - all_heights: List[LiquidHeightSummary] = [] - for labware, wells in self._state.probed_heights.items(): - for well, lhi in wells.items(): - lhs = LiquidHeightSummary( - labware_id=labware, - well_name=well, - height=lhi.height, - last_measured=lhi.last_probed, + def get_all( + self, + ) -> List[Union[ProbedHeightSummary, ProbedVolumeSummary, LoadedVolumeSummary]]: + """Get all well liquid info summaries.""" + all_summaries: List[ + Union[ProbedHeightSummary, ProbedVolumeSummary, LoadedVolumeSummary] + ] = [] + for lv_labware, lv_wells in self._state.loaded_volumes.items(): + for lv_well, lvi in lv_wells.items(): + lvs = LoadedVolumeSummary( + labware_id=lv_labware, + well_name=lv_well, + volume=lvi.volume, + last_loaded=lvi.last_loaded, + operations_since_load=lvi.operations_since_load, ) - all_heights.append(lhs) - return all_heights - - # update this - def get_all_in_labware(self, labware_id: str) -> List[LiquidHeightSummary]: - """Get all well liquid heights for a particular labware.""" - all_heights: List[LiquidHeightSummary] = [] - for well, lhi in self._state.probed_heights[labware_id].items(): - lhs = LiquidHeightSummary( - labware_id=labware_id, - well_name=well, - height=lhi.height, - last_measured=lhi.last_probed, - ) - all_heights.append(lhs) - return all_heights - - 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. + all_summaries.append(lvs) + for ph_labware, ph_wells in self._state.probed_heights.items(): + for ph_well, phi in ph_wells.items(): + phs = ProbedHeightSummary( + labware_id=ph_labware, + well_name=ph_well, + height=phi.height, + last_probed=phi.last_probed, + ) + all_summaries.append(phs) + for pv_labware, pv_wells in self._state.probed_volumes.items(): + for pv_well, pvi in pv_wells.items(): + pvs = ProbedVolumeSummary( + labware_id=pv_labware, + well_name=pv_well, + volume=pvi.volume, + last_probed=pvi.last_probed, + operations_since_probe=pvi.operations_since_probe, + ) + all_summaries.append(pvs) + return all_summaries - Returns None if no liquid probe has been done. - """ - try: - height = self._state.probed_heights[labware_id][well_name].height - return height - except KeyError: - return None - - 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: - return bool(self._state.probed_heights[labware_id][well_name].height) - except KeyError: - return False - - def get_well_liquid_values( + def get_well_liquid_info( self, labware_id: str, well_name: str - ) -> Tuple[Optional[float], Optional[float], Optional[float]]: - """Return all the liquid values for a well.""" - loaded_volume = self._state.loaded_volumes[labware_id][well_name].volume - probed_height = self._state.probed_heights[labware_id][well_name].height - probed_volume = self._state.probed_volumes[labware_id][well_name].volume - return loaded_volume, probed_height, probed_volume + ) -> Tuple[ + Optional[LoadedVolumeInfo], + Optional[ProbedHeightInfo], + Optional[ProbedVolumeInfo], + ]: + """Return all the liquid info for a well.""" + if ( + labware_id not in self._state.loaded_volumes + or well_name not in self._state.loaded_volumes[labware_id] + ): + loaded_volume_info = None + else: + loaded_volume_info = self._state.loaded_volumes[labware_id][well_name] + if ( + labware_id not in self._state.probed_heights + or well_name not in self._state.probed_heights[labware_id] + ): + probed_height_info = None + else: + probed_height_info = self._state.probed_heights[labware_id][well_name] + if ( + labware_id not in self._state.probed_volumes + or well_name not in self._state.probed_volumes[labware_id] + ): + probed_volume_info = None + else: + probed_volume_info = self._state.probed_volumes[labware_id][well_name] + return loaded_volume_info, probed_height_info, probed_volume_info diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 78df5942033..0a46a6544c3 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -355,6 +355,14 @@ class CurrentWell: well_name: str +class LoadedVolumeInfo(BaseModel): + """A well's liquid volume, initialized by a LoadLiquid, updated by Aspirate and Dispense.""" + + volume: Optional[float] = None + last_loaded: datetime + operations_since_load: int + + class ProbedHeightInfo(BaseModel): """A well's liquid height, initialized by a LiquidProbe, cleared by Aspirate and Dispense.""" @@ -370,21 +378,25 @@ class ProbedVolumeInfo(BaseModel): operations_since_probe: int -class LoadedVolumeInfo(BaseModel): - """A well's liquid volume, initialized by a LoadLiquid, updated by Aspirate and Dispense.""" +class LoadedVolumeSummary(LoadedVolumeInfo): + """Payload for a well's loaded volume in StateSummary.""" - volume: Optional[float] = None - last_loaded: datetime - operations_since_load: int + labware_id: str + well_name: str -class LiquidHeightSummary(BaseModel): - """Payload for liquid state height in StateSummary.""" +class ProbedHeightSummary(ProbedHeightInfo): + """Payload for a well's probed height in StateSummary.""" + + labware_id: str + well_name: str + + +class ProbedVolumeSummary(ProbedVolumeInfo): + """Payload for a well's probed volume in StateSummary.""" labware_id: str well_name: str - height: Optional[float] = None - last_measured: datetime @dataclass(frozen=True) diff --git a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py index dc464966fa5..66bdd457b8a 100644 --- a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py @@ -55,6 +55,8 @@ LoadedPipette, TipGeometry, ModuleDefinition, + ProbedHeightInfo, + LoadedVolumeInfo, ) from opentrons.protocol_engine.commands import ( CommandStatus, @@ -1539,8 +1541,8 @@ def test_get_well_position_with_meniscus_offset( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( - (None, 70.5, None) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return( + (None, ProbedHeightInfo(height=70.5, last_probed=datetime.now()), None) ) decoy.when( mock_pipette_view.get_current_tip_lld_settings(pipette_id="pipette-id") @@ -1563,6 +1565,64 @@ def test_get_well_position_with_meniscus_offset( ) +def test_get_well_position_with_volume_offset_raises_error( + decoy: Decoy, + well_plate_def: LabwareDefinition, + mock_labware_view: LabwareView, + mock_well_view: WellView, + mock_addressable_area_view: AddressableAreaView, + mock_pipette_view: PipetteView, + subject: GeometryView, +) -> None: + """Calling get_well_position with any volume offset should raise an error when there's no innerLabwareGeometry.""" + labware_data = LoadedLabware( + id="labware-id", + loadName="load-name", + definitionUri="definition-uri", + location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4), + offsetId="offset-id", + ) + calibration_offset = LabwareOffsetVector(x=1, y=-2, z=3) + slot_pos = Point(4, 5, 6) + well_def = well_plate_def.wells["B2"] + + decoy.when(mock_labware_view.get("labware-id")).then_return(labware_data) + decoy.when(mock_labware_view.get_definition("labware-id")).then_return( + well_plate_def + ) + decoy.when(mock_labware_view.get_labware_offset_vector("labware-id")).then_return( + calibration_offset + ) + decoy.when( + mock_addressable_area_view.get_addressable_area_position(DeckSlotName.SLOT_4.id) + ).then_return(slot_pos) + decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( + well_def + ) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return( + (None, ProbedHeightInfo(height=45.0, last_probed=datetime.now()), None) + ) + decoy.when( + mock_pipette_view.get_current_tip_lld_settings(pipette_id="pipette-id") + ).then_return(0.5) + decoy.when(mock_labware_view.get_well_geometry("labware-id", "B2")).then_raise( + errors.IncompleteLabwareDefinitionError("Woops!") + ) + + with pytest.raises(errors.IncompleteLabwareDefinitionError): + subject.get_well_position( + labware_id="labware-id", + well_name="B2", + well_location=LiquidHandlingWellLocation( + origin=WellOrigin.MENISCUS, + offset=WellOffset(x=2, y=3, z=4), + volumeOffset="operationVolume", + ), + operation_volume=-1245.833, + pipette_id="pipette-id", + ) + + def test_get_well_position_with_meniscus_and_literal_volume_offset( decoy: Decoy, well_plate_def: LabwareDefinition, @@ -1597,8 +1657,8 @@ def test_get_well_position_with_meniscus_and_literal_volume_offset( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( - (None, 45.0, None) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return( + (None, ProbedHeightInfo(height=45.0, last_probed=datetime.now()), None) ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None @@ -1663,8 +1723,8 @@ def test_get_well_position_with_meniscus_and_float_volume_offset( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( - (None, 45.0, None) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return( + (None, ProbedHeightInfo(height=45.0, last_probed=datetime.now()), None) ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None @@ -1728,8 +1788,8 @@ def test_get_well_position_raises_validation_error( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( - (None, 40.0, None) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return( + (None, ProbedHeightInfo(height=40.0, last_probed=datetime.now()), None) ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None @@ -1789,8 +1849,14 @@ def test_get_meniscus_height( decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return( well_def ) - decoy.when(mock_well_view.get_well_liquid_values("labware-id", "B2")).then_return( - (2000.0, None, None) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return( + ( + LoadedVolumeInfo( + volume=2000.0, last_loaded=datetime.now(), operations_since_load=0 + ), + None, + None, + ) ) labware_def = _load_labware_definition_data() assert labware_def.innerLabwareGeometry is not None @@ -3197,8 +3263,8 @@ def test_validate_dispense_volume_into_well_meniscus( decoy.when(mock_labware_view.get_well_geometry("labware-id", "A1")).then_return( inner_well_def ) - decoy.when(mock_well_view.get_well_liquid_values("labware-id", "A1")).then_return( - (None, 40.0, None) + decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return( + (None, ProbedHeightInfo(height=40.0, last_probed=datetime.now()), None) ) with pytest.raises(errors.InvalidDispenseVolumeError): diff --git a/api/tests/opentrons/protocol_engine/state/test_labware_view.py b/api/tests/opentrons/protocol_engine/state/test_labware_view.py index d461ddda4e6..d6b05b7b027 100644 --- a/api/tests/opentrons/protocol_engine/state/test_labware_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_labware_view.py @@ -460,6 +460,17 @@ def test_get_well_definition_get_first(well_plate_def: LabwareDefinition) -> Non assert result == expected_well_def +def test_get_well_geometry_raises_error(well_plate_def: LabwareDefinition) -> None: + """It should raise an IncompleteLabwareDefinitionError when there's no innerLabwareGeometry.""" + subject = get_labware_view( + labware_by_id={"plate-id": plate}, + definitions_by_uri={"some-plate-uri": well_plate_def}, + ) + + with pytest.raises(errors.IncompleteLabwareDefinitionError): + subject.get_well_geometry(labware_id="plate-id") + + def test_get_well_size_circular(well_plate_def: LabwareDefinition) -> None: """It should return the well dimensions of a circular well.""" subject = get_labware_view( 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 fa2e19188d6..6cdb3715052 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,13 @@ """Well view tests.""" from datetime import datetime -from opentrons.protocol_engine.types import ProbedHeightInfo +from opentrons.protocol_engine.types import ( + LoadedVolumeInfo, + ProbedHeightInfo, + ProbedVolumeInfo, + LoadedVolumeSummary, + ProbedHeightSummary, + ProbedVolumeSummary, +) import pytest from opentrons.protocol_engine.state.wells import WellState, WellView @@ -10,46 +17,46 @@ def subject() -> WellView: """Get a well view test subject.""" labware_id = "labware-id" well_name = "well-name" - height_info = ProbedHeightInfo(height=0.5, last_probed=datetime.now()) + loaded_volume_info = LoadedVolumeInfo( + volume=30.0, last_loaded=datetime.now(), operations_since_load=0 + ) + probed_height_info = ProbedHeightInfo(height=5.5, last_probed=datetime.now()) + probed_volume_info = ProbedVolumeInfo( + volume=25.0, last_probed=datetime.now(), operations_since_probe=0 + ) state = WellState( - loaded_volumes={}, - probed_heights={labware_id: {well_name: height_info}}, - probed_volumes={}, + loaded_volumes={labware_id: {well_name: loaded_volume_info}}, + probed_heights={labware_id: {well_name: probed_height_info}}, + probed_volumes={labware_id: {well_name: probed_volume_info}}, ) return WellView(state) def test_get_all(subject: WellView) -> None: - """Should return a list of well heights.""" - assert subject.get_all()[0].height == 0.5 - + """Should return a list of well summaries.""" + all_summaries = subject.get_all() -def test_get_last_measured_liquid_height(subject: WellView) -> None: - """Should return the height of a single well correctly for valid wells.""" - labware_id = "labware-id" - well_name = "well-name" + assert len(all_summaries) == 3 + assert isinstance(all_summaries[0], LoadedVolumeSummary) + assert all_summaries[0].volume == 30.0 + assert isinstance(all_summaries[1], ProbedHeightSummary) + assert all_summaries[1].height == 5.5 + assert isinstance(all_summaries[2], ProbedVolumeSummary) + assert all_summaries[2].volume == 25.0 - 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(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.""" +def test_get_well_liquid_info(subject: WellView) -> None: + """Should return a tuple of well infos.""" labware_id = "labware-id" well_name = "well-name" - - invalid_labware_id = "invalid-labware-id" - invalid_well_name = "invalid-well-name" - - assert ( - subject.has_measured_liquid_height(invalid_labware_id, invalid_well_name) - is False + lvi, phi, pvi = subject.get_well_liquid_info( + labware_id=labware_id, well_name=well_name ) - assert subject.has_measured_liquid_height(labware_id, well_name) is True + + assert lvi is not None + assert phi is not None + assert pvi is not None + assert lvi.volume == 30.0 + assert phi.height == 5.5 + assert pvi.volume == 25.0 From 5bafa09ab998a59a8d76d94e10fb88ee8c24a870 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Wed, 23 Oct 2024 17:44:00 -0400 Subject: [PATCH 16/18] removed comments --- api/src/opentrons/protocol_engine/state/geometry.py | 2 +- api/src/opentrons/protocol_engine/state/wells.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index 93e9c45e199..3b592e62083 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -1488,10 +1488,10 @@ def validate_dispense_volume_into_well( volume: float, ) -> None: """Raise InvalidDispenseVolumeError if planned dispense volume will overflow well.""" - # update this (or where it's used)? Consider how many conversions are being done well_def = self._labware.get_well_definition(labware_id, well_name) well_volumetric_capacity = well_def.totalLiquidVolume if well_location.origin == WellOrigin.MENISCUS: + # TODO(pbm, 10-23-24): refactor to smartly reduce height/volume conversions well_geometry = self._labware.get_well_geometry(labware_id, well_name) meniscus_height = self.get_meniscus_height( labware_id=labware_id, well_name=well_name diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 91e19fcf5db..bc292a743ca 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -128,10 +128,6 @@ def __init__(self, state: WellState) -> None: """ self._state = state - # if volume requested, loaded_volumes or probed_volumes - # if height requested, probed_heights or loaded_vols_to_height or probed_vols_to_height - # to get height, call GeometryView.get_well_height, which does conversion if needed - def get_all( self, ) -> List[Union[ProbedHeightSummary, ProbedVolumeSummary, LoadedVolumeSummary]]: From 4a36e9a144984bba5d47ccfbae38dd875ccf089e Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Wed, 23 Oct 2024 18:08:24 -0400 Subject: [PATCH 17/18] added comments --- api/src/opentrons/protocol_api/instrument_context.py | 2 +- api/src/opentrons/protocol_engine/commands/blow_out.py | 2 ++ api/src/opentrons/protocol_engine/commands/dispense.py | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/protocol_api/instrument_context.py b/api/src/opentrons/protocol_api/instrument_context.py index 880626b53c9..7460cda6911 100644 --- a/api/src/opentrons/protocol_api/instrument_context.py +++ b/api/src/opentrons/protocol_api/instrument_context.py @@ -765,7 +765,7 @@ def air_gap( raise RuntimeError("No previous Well cached to perform air gap") target = loc.labware.as_well().top(height) self.move_to(target, publish=False) - self.aspirate(volume) + self.aspirate(volume) # add (is_liquid: bool = True) parameter to aspirate? Or deduce location is not in well in Aspirate command? return self @publisher.publish(command=cmds.return_tip) diff --git a/api/src/opentrons/protocol_engine/commands/blow_out.py b/api/src/opentrons/protocol_engine/commands/blow_out.py index c8a6b65ce63..a0e85a063a1 100644 --- a/api/src/opentrons/protocol_engine/commands/blow_out.py +++ b/api/src/opentrons/protocol_engine/commands/blow_out.py @@ -93,6 +93,7 @@ async def execute(self, params: BlowOutParams) -> _ExecuteReturn: pipette_id=params.pipetteId, flow_rate=params.flowRate ) except PipetteOverpressureError as e: + # can we get blown_out_amount_prior_to_error? If not, can we assume no liquid was added to well? Ask Ryan return DefinedErrorData( public=OverpressureError( id=self._model_utils.generate_id(), @@ -114,6 +115,7 @@ async def execute(self, params: BlowOutParams) -> _ExecuteReturn: ), ) else: + # if blow out over a well, can we get pipette tip liquid volume and add that to well liquid volume? return SuccessData( public=BlowOutResult(position=deck_point), private=None, diff --git a/api/src/opentrons/protocol_engine/commands/dispense.py b/api/src/opentrons/protocol_engine/commands/dispense.py index 0386a450e95..fb3b5b0ffaa 100644 --- a/api/src/opentrons/protocol_engine/commands/dispense.py +++ b/api/src/opentrons/protocol_engine/commands/dispense.py @@ -107,7 +107,7 @@ async def execute(self, params: DispenseParams) -> _ExecuteReturn: push_out=params.pushOut, ) except PipetteOverpressureError as e: - # can we get aspirated_amount_prior_to_error? If not, can we assume no liquid was removed from well? Ask Ryan + # can we get dispensed_amount_prior_to_error? If not, can we assume no liquid was added to well? Ask Ryan return DefinedErrorData( public=OverpressureError( id=self._model_utils.generate_id(), From 53e0e04c78731707403647ccdd9c38a60042d6c3 Mon Sep 17 00:00:00 2001 From: pmoegenburg Date: Wed, 23 Oct 2024 18:15:01 -0400 Subject: [PATCH 18/18] fixed lint error --- api/src/opentrons/protocol_api/instrument_context.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_api/instrument_context.py b/api/src/opentrons/protocol_api/instrument_context.py index 7460cda6911..694c0bee7c0 100644 --- a/api/src/opentrons/protocol_api/instrument_context.py +++ b/api/src/opentrons/protocol_api/instrument_context.py @@ -765,7 +765,8 @@ def air_gap( raise RuntimeError("No previous Well cached to perform air gap") target = loc.labware.as_well().top(height) self.move_to(target, publish=False) - self.aspirate(volume) # add (is_liquid: bool = True) parameter to aspirate? Or deduce location is not in well in Aspirate command? + # add (is_liquid: bool = True) parameter to aspirate? Or deduce location is not in well in Aspirate command? + self.aspirate(volume) return self @publisher.publish(command=cmds.return_tip)