From 597364654d628325d6aee143c29975ca89ba98eb Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 5 Nov 2024 14:51:17 -0500 Subject: [PATCH] feat(api): add load-empty-liquid (#16676) We have a loadLiquid command and associated python protocol API method, which allow users to specify that a well contains a given amount of a given (predefined) liquid. This was all that we needed for a while because we didn't really do anything with that information except display the starting deck state based on it. Now that we track liquid, however, this isn't enough, because we also need a way for users to specify that a well is known to be empty. To that end, introduce a special EMPTY sentinel value for liquid ID and restrict loadLiquid commands that use it to setting a 0 volume. I think we probably should have a better API for setting multiple wells but that's not this PR. Closes EXEC-801 ## reviews - seem like a valid thing to do? Didn't make sense to have a whole different command ## testing - make sure empty liquids get loaded ## notes - [x] will rebase this PR to edge once the current target is merged --------- Co-authored-by: Ed Cormany --- .../protocol_api/core/engine/well.py | 21 ++++++++++- .../core/legacy/legacy_well_core.py | 4 +++ api/src/opentrons/protocol_api/core/well.py | 4 +++ api/src/opentrons/protocol_api/labware.py | 8 +++++ .../protocol_engine/commands/load_liquid.py | 14 ++++++-- .../protocol_engine/errors/__init__.py | 2 ++ .../protocol_engine/errors/exceptions.py | 13 +++++++ .../protocol_engine/protocol_engine.py | 7 ++-- .../protocol_engine/state/liquids.py | 18 ++++++++-- api/src/opentrons/protocol_engine/types.py | 4 +++ api/tests/opentrons/protocol_api/test_well.py | 9 +++++ .../commands/test_load_liquid.py | 35 +++++++++++++++++++ .../protocol_engine/state/test_liquid_view.py | 21 ++++++++++- .../protocol_engine/test_protocol_engine.py | 15 ++++---- shared-data/command/schemas/10.json | 14 ++++++-- 15 files changed, 167 insertions(+), 22 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/well.py b/api/src/opentrons/protocol_api/core/engine/well.py index 6743a8a39c5..dba1dc6c840 100644 --- a/api/src/opentrons/protocol_api/core/engine/well.py +++ b/api/src/opentrons/protocol_api/core/engine/well.py @@ -130,7 +130,10 @@ def load_liquid( liquid: Liquid, volume: float, ) -> None: - """Load liquid into a well.""" + """Load liquid into a well. + + If the well is known to be empty, use ``load_empty()`` instead of calling this with a 0.0 volume. + """ self._engine_client.execute_command( cmd.LoadLiquidParams( labwareId=self._labware_id, @@ -139,6 +142,22 @@ def load_liquid( ) ) + def load_empty( + self, + ) -> None: + """Inform the system that a well is known to be empty. + + This should be done early in the protocol, at the same time as a load_liquid command might + be used. + """ + self._engine_client.execute_command( + cmd.LoadLiquidParams( + labwareId=self._labware_id, + liquidId="EMPTY", + volumeByWell={self._name: 0.0}, + ) + ) + def from_center_cartesian(self, x: float, y: float, z: float) -> Point: """Gets point in deck coordinates based on percentage of the radius of each axis.""" well_size = self._engine_client.state.labware.get_well_size( diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_well_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_well_core.py index a88dd2eee80..891f0f1b681 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_well_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_well_core.py @@ -114,6 +114,10 @@ def load_liquid( """Load liquid into a well.""" raise APIVersionError(api_element="Loading a liquid") + def load_empty(self) -> None: + """Mark a well as empty.""" + assert False, "load_empty only supported on engine core" + def from_center_cartesian(self, x: float, y: float, z: float) -> Point: """Gets point in deck coordinates based on percentage of the radius of each axis.""" return self._geometry.from_center_cartesian(x, y, z) diff --git a/api/src/opentrons/protocol_api/core/well.py b/api/src/opentrons/protocol_api/core/well.py index bd58963a59c..24489bb04e7 100644 --- a/api/src/opentrons/protocol_api/core/well.py +++ b/api/src/opentrons/protocol_api/core/well.py @@ -79,6 +79,10 @@ def load_liquid( ) -> None: """Load liquid into a well.""" + @abstractmethod + def load_empty(self) -> None: + """Mark a well as containing no liquid.""" + @abstractmethod def from_center_cartesian(self, x: float, y: float, z: float) -> Point: """Gets point in deck coordinates based on percentage of the radius of each axis.""" diff --git a/api/src/opentrons/protocol_api/labware.py b/api/src/opentrons/protocol_api/labware.py index 0e8a17d07d3..825cc19668a 100644 --- a/api/src/opentrons/protocol_api/labware.py +++ b/api/src/opentrons/protocol_api/labware.py @@ -280,12 +280,20 @@ def load_liquid(self, liquid: Liquid, volume: float) -> None: :param Liquid liquid: The liquid to load into the well. :param float volume: The volume of liquid to load, in µL. + + .. note:: + In API version 2.22 and later, use :py:meth:`~.Well.load_empty()` to mark a well as empty at the beginning of a protocol, rather than using this method with ``volume=0``. """ self._core.load_liquid( liquid=liquid, volume=volume, ) + @requires_version(2, 22) + def load_empty(self) -> None: + """Mark a well as empty.""" + self._core.load_empty() + def _from_center_cartesian(self, x: float, y: float, z: float) -> Point: """ Private version of from_center_cartesian. Present only for backward diff --git a/api/src/opentrons/protocol_engine/commands/load_liquid.py b/api/src/opentrons/protocol_engine/commands/load_liquid.py index 5dd4737410e..f6aa037fa01 100644 --- a/api/src/opentrons/protocol_engine/commands/load_liquid.py +++ b/api/src/opentrons/protocol_engine/commands/load_liquid.py @@ -5,6 +5,8 @@ from typing_extensions import Literal from opentrons.protocol_engine.state.update_types import StateUpdate +from opentrons.protocol_engine.types import LiquidId +from opentrons.protocol_engine.errors import InvalidLiquidError from .command import AbstractCommandImpl, BaseCommand, BaseCommandCreate, SuccessData from ..errors.error_occurrence import ErrorOccurrence @@ -19,9 +21,9 @@ class LoadLiquidParams(BaseModel): """Payload required to load a liquid into a well.""" - liquidId: str = Field( + liquidId: LiquidId = Field( ..., - description="Unique identifier of the liquid to load.", + description="Unique identifier of the liquid to load. If this is the sentinel value EMPTY, all values of volumeByWell must be 0.", ) labwareId: str = Field( ..., @@ -29,7 +31,7 @@ class LoadLiquidParams(BaseModel): ) volumeByWell: Dict[str, float] = Field( ..., - description="Volume of liquid, in µL, loaded into each well by name, in this labware.", + description="Volume of liquid, in µL, loaded into each well by name, in this labware. If the liquid id is the sentinel value EMPTY, all volumes must be 0.", ) @@ -57,6 +59,12 @@ async def execute(self, params: LoadLiquidParams) -> SuccessData[LoadLiquidResul self._state_view.labware.validate_liquid_allowed_in_labware( labware_id=params.labwareId, wells=params.volumeByWell ) + if params.liquidId == "EMPTY": + for well_name, volume in params.volumeByWell.items(): + if volume != 0.0: + raise InvalidLiquidError( + 'loadLiquid commands that specify the special liquid "EMPTY" must set volume to be 0.0, but the volume for {well_name} is {volume}' + ) state_update = StateUpdate() state_update.set_liquid_loaded( diff --git a/api/src/opentrons/protocol_engine/errors/__init__.py b/api/src/opentrons/protocol_engine/errors/__init__.py index b25dfdb2d0e..e9f1acddeed 100644 --- a/api/src/opentrons/protocol_engine/errors/__init__.py +++ b/api/src/opentrons/protocol_engine/errors/__init__.py @@ -77,6 +77,7 @@ OperationLocationNotInWellError, InvalidDispenseVolumeError, StorageLimitReachedError, + InvalidLiquidError, ) from .error_occurrence import ErrorOccurrence, ProtocolCommandFailedError @@ -137,6 +138,7 @@ "InvalidTargetSpeedError", "InvalidBlockVolumeError", "InvalidHoldTimeError", + "InvalidLiquidError", "CannotPerformModuleAction", "ResumeFromRecoveryNotAllowedError", "PauseNotAllowedError", diff --git a/api/src/opentrons/protocol_engine/errors/exceptions.py b/api/src/opentrons/protocol_engine/errors/exceptions.py index 12f45f4936d..36b0d2ccbef 100644 --- a/api/src/opentrons/protocol_engine/errors/exceptions.py +++ b/api/src/opentrons/protocol_engine/errors/exceptions.py @@ -244,6 +244,19 @@ def __init__( super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping) +class InvalidLiquidError(ProtocolEngineError): + """Raised when attempting to add a liquid with an invalid property.""" + + def __init__( + self, + message: Optional[str] = None, + details: Optional[Dict[str, Any]] = None, + wrapping: Optional[Sequence[EnumeratedError]] = None, + ) -> None: + """Build an InvalidLiquidError.""" + super().__init__(ErrorCodes.INVALID_PROTOCOL_DATA, message, details, wrapping) + + class LabwareDefinitionDoesNotExistError(ProtocolEngineError): """Raised when referencing a labware definition that does not exist.""" diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index ced32b20cc3..574c3d076f9 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -566,9 +566,12 @@ def add_liquid( description=(description or ""), displayColor=color, ) + validated_liquid = self._state_store.liquid.validate_liquid_allowed( + liquid=liquid + ) - self._action_dispatcher.dispatch(AddLiquidAction(liquid=liquid)) - return liquid + self._action_dispatcher.dispatch(AddLiquidAction(liquid=validated_liquid)) + return validated_liquid def add_addressable_area(self, addressable_area_name: str) -> None: """Add an addressable area to state.""" diff --git a/api/src/opentrons/protocol_engine/state/liquids.py b/api/src/opentrons/protocol_engine/state/liquids.py index 9394e4261b1..775223c6a60 100644 --- a/api/src/opentrons/protocol_engine/state/liquids.py +++ b/api/src/opentrons/protocol_engine/state/liquids.py @@ -1,11 +1,11 @@ """Basic liquid data state and store.""" from dataclasses import dataclass from typing import Dict, List -from opentrons.protocol_engine.types import Liquid +from opentrons.protocol_engine.types import Liquid, LiquidId from ._abstract_store import HasState, HandlesActions from ..actions import Action, AddLiquidAction -from ..errors import LiquidDoesNotExistError +from ..errors import LiquidDoesNotExistError, InvalidLiquidError @dataclass @@ -51,11 +51,23 @@ def get_all(self) -> List[Liquid]: """Get all protocol liquids.""" return list(self._state.liquids_by_id.values()) - def validate_liquid_id(self, liquid_id: str) -> str: + def validate_liquid_id(self, liquid_id: LiquidId) -> LiquidId: """Check if liquid_id exists in liquids.""" + is_empty = liquid_id == "EMPTY" + if is_empty: + return liquid_id has_liquid = liquid_id in self._state.liquids_by_id if not has_liquid: raise LiquidDoesNotExistError( f"Supplied liquidId: {liquid_id} does not exist in the loaded liquids." ) return liquid_id + + def validate_liquid_allowed(self, liquid: Liquid) -> Liquid: + """Validate that a liquid is legal to load.""" + is_empty = liquid.id == "EMPTY" + if is_empty: + raise InvalidLiquidError( + message='Protocols may not define a liquid with the special id "EMPTY".' + ) + return liquid diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 780b02d2129..5aa4c8c26e9 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -828,6 +828,10 @@ def _color_is_a_valid_hex(cls, v: str) -> str: return v +EmptyLiquidId = Literal["EMPTY"] +LiquidId = str | EmptyLiquidId + + class Liquid(BaseModel): """Payload required to create a liquid.""" diff --git a/api/tests/opentrons/protocol_api/test_well.py b/api/tests/opentrons/protocol_api/test_well.py index ef1eed84c62..b4817567dde 100644 --- a/api/tests/opentrons/protocol_api/test_well.py +++ b/api/tests/opentrons/protocol_api/test_well.py @@ -8,6 +8,8 @@ from opentrons.protocol_api._liquid import Liquid from opentrons.types import Point, Location +from . import versions_at_or_above + @pytest.fixture def mock_well_core(decoy: Decoy) -> WellCore: @@ -140,6 +142,13 @@ def test_load_liquid(decoy: Decoy, mock_well_core: WellCore, subject: Well) -> N ) +@pytest.mark.parametrize("api_version", versions_at_or_above(APIVersion(2, 22))) +def test_load_empty(decoy: Decoy, mock_well_core: WellCore, subject: Well) -> None: + """It should mark a location as empty.""" + subject.load_empty() + decoy.verify(mock_well_core.load_empty(), times=1) + + def test_diameter(decoy: Decoy, mock_well_core: WellCore, subject: Well) -> None: """It should get the diameter from the core.""" decoy.when(mock_well_core.diameter).then_return(12.3) 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 dbc584ae2a3..6bd61061f3c 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 @@ LoadLiquidImplementation, LoadLiquidParams, ) +from opentrons.protocol_engine.errors import InvalidLiquidError 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 @@ -64,3 +65,37 @@ async def test_load_liquid_implementation( "labware-id", {"A1": 30.0, "B2": 100.0} ) ) + + +async def test_load_empty_liquid_requires_zero_volume( + decoy: Decoy, + subject: LoadLiquidImplementation, + mock_state_view: StateView, + model_utils: ModelUtils, +) -> None: + """Test that loadLiquid requires empty liquids to have 0 volume.""" + data = LoadLiquidParams( + labwareId="labware-id", liquidId="EMPTY", volumeByWell={"A1": 1.0} + ) + timestamp = datetime(year=2020, month=1, day=2) + decoy.when(model_utils.get_timestamp()).then_return(timestamp) + + with pytest.raises(InvalidLiquidError): + await subject.execute(data) + + decoy.verify(mock_state_view.liquid.validate_liquid_id("EMPTY")) + + data2 = LoadLiquidParams( + labwareId="labware-id", liquidId="EMPTY", volumeByWell={"A1": 0.0} + ) + result = await subject.execute(data2) + assert result == SuccessData( + public=LoadLiquidResult(), + state_update=update_types.StateUpdate( + liquid_loaded=update_types.LiquidLoadedUpdate( + labware_id="labware-id", + volumes=data2.volumeByWell, + last_loaded=timestamp, + ) + ), + ) diff --git a/api/tests/opentrons/protocol_engine/state/test_liquid_view.py b/api/tests/opentrons/protocol_engine/state/test_liquid_view.py index f3424932b0e..db1e6f274a1 100644 --- a/api/tests/opentrons/protocol_engine/state/test_liquid_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_liquid_view.py @@ -3,7 +3,7 @@ from opentrons.protocol_engine.state.liquids import LiquidState, LiquidView from opentrons.protocol_engine import Liquid -from opentrons.protocol_engine.errors import LiquidDoesNotExistError +from opentrons.protocol_engine.errors import LiquidDoesNotExistError, InvalidLiquidError @pytest.fixture @@ -33,3 +33,22 @@ def test_has_liquid(subject: LiquidView) -> None: with pytest.raises(LiquidDoesNotExistError): subject.validate_liquid_id("no-id") + + +def test_validate_liquid_prevents_empty(subject: LiquidView) -> None: + """It should not allow loading a liquid with the special id EMPTY.""" + with pytest.raises(InvalidLiquidError): + subject.validate_liquid_allowed( + Liquid(id="EMPTY", displayName="empty", description="nothing") + ) + + +def test_validate_liquid_allows_non_empty(subject: LiquidView) -> None: + """It should allow a valid liquid.""" + valid_liquid = Liquid( + id="some-id", + displayName="some-display-name", + description="some-description", + displayColor=None, + ) + assert subject.validate_liquid_allowed(valid_liquid) == valid_liquid diff --git a/api/tests/opentrons/protocol_engine/test_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_protocol_engine.py index ac83e987153..bc581114ab2 100644 --- a/api/tests/opentrons/protocol_engine/test_protocol_engine.py +++ b/api/tests/opentrons/protocol_engine/test_protocol_engine.py @@ -1133,21 +1133,18 @@ def test_add_liquid( decoy: Decoy, action_dispatcher: ActionDispatcher, subject: ProtocolEngine, + state_store: StateStore, ) -> None: """It should dispatch an AddLiquidAction action.""" + liquid_obj = Liquid(id="water-id", displayName="water", description="water desc") + decoy.when( + state_store.liquid.validate_liquid_allowed(liquid=liquid_obj) + ).then_return(liquid_obj) subject.add_liquid( id="water-id", name="water", description="water desc", color=None ) - decoy.verify( - action_dispatcher.dispatch( - AddLiquidAction( - liquid=Liquid( - id="water-id", displayName="water", description="water desc" - ) - ) - ) - ) + decoy.verify(action_dispatcher.dispatch(AddLiquidAction(liquid=liquid_obj))) async def test_use_attached_temp_and_mag_modules( diff --git a/shared-data/command/schemas/10.json b/shared-data/command/schemas/10.json index b87d0dcf0fd..61521073fac 100644 --- a/shared-data/command/schemas/10.json +++ b/shared-data/command/schemas/10.json @@ -1621,8 +1621,16 @@ "properties": { "liquidId": { "title": "Liquidid", - "description": "Unique identifier of the liquid to load.", - "type": "string" + "description": "Unique identifier of the liquid to load. If this is the sentinel value EMPTY, all values of volumeByWell must be 0.", + "anyOf": [ + { + "type": "string" + }, + { + "enum": ["EMPTY"], + "type": "string" + } + ] }, "labwareId": { "title": "Labwareid", @@ -1631,7 +1639,7 @@ }, "volumeByWell": { "title": "Volumebywell", - "description": "Volume of liquid, in \u00b5L, loaded into each well by name, in this labware.", + "description": "Volume of liquid, in \u00b5L, loaded into each well by name, in this labware. If the liquid id is the sentinel value EMPTY, all volumes must be 0.", "type": "object", "additionalProperties": { "type": "number"