diff --git a/api/src/opentrons/protocol_api/core/engine/instrument.py b/api/src/opentrons/protocol_api/core/engine/instrument.py index f39f315366c..7b6400bc561 100644 --- a/api/src/opentrons/protocol_api/core/engine/instrument.py +++ b/api/src/opentrons/protocol_api/core/engine/instrument.py @@ -31,8 +31,6 @@ from opentrons.protocol_engine.errors.exceptions import TipNotAttachedError from opentrons.protocol_engine.clients import SyncClient as EngineClient from opentrons.protocols.api_support.definitions import MAX_SUPPORTED_VERSION - -from opentrons_shared_data.errors.exceptions import PipetteLiquidNotFoundError from opentrons_shared_data.pipette.dev_types import PipetteNameType from opentrons.protocol_api._nozzle_layout import NozzleLayout from opentrons.hardware_control.nozzle_manager import NozzleConfigurationType @@ -846,34 +844,37 @@ def retract(self) -> None: z_axis = self._engine_client.state.pipettes.get_z_axis(self._pipette_id) self._engine_client.execute_command(cmd.HomeParams(axes=[z_axis])) - def find_liquid_level(self, well_core: WellCore, error_recovery: bool) -> float: + def liquid_probe_with_recovery(self, well_core: WellCore) -> None: labware_id = well_core.labware_id well_name = well_core.get_name() well_location = WellLocation( origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0) ) - if error_recovery: - result = self._engine_client.execute_command_with_result( - cmd.LiquidProbeParams( - labwareId=labware_id, - wellName=well_name, - wellLocation=well_location, - pipetteId=self.pipette_id, - ) + + self._engine_client.execute_command( + cmd.LiquidProbeParams( + labwareId=labware_id, + wellName=well_name, + wellLocation=well_location, + pipetteId=self.pipette_id, ) - else: - result = self._engine_client.execute_command_without_recovery( - cmd.LiquidProbeParams( - labwareId=labware_id, - wellName=well_name, - wellLocation=well_location, - pipetteId=self.pipette_id, - ) + ) + + def liquid_probe_without_recovery(self, well_core: WellCore) -> float: + labware_id = well_core.labware_id + well_name = well_core.get_name() + well_location = WellLocation( + origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0) + ) + + result = self._engine_client.execute_command_without_recovery( + cmd.LiquidProbeParams( + labwareId=labware_id, + wellName=well_name, + wellLocation=well_location, + pipetteId=self.pipette_id, ) + ) if result is not None and isinstance(result, LiquidProbeResult): return result.z_position - # should never get here - raise PipetteLiquidNotFoundError( - "Error while trying to find liquid level.", - ) diff --git a/api/src/opentrons/protocol_api/core/instrument.py b/api/src/opentrons/protocol_api/core/instrument.py index 77967023714..2ad70c7274b 100644 --- a/api/src/opentrons/protocol_api/core/instrument.py +++ b/api/src/opentrons/protocol_api/core/instrument.py @@ -304,7 +304,12 @@ def retract(self) -> None: ... @abstractmethod - def find_liquid_level(self, well_core: WellCoreType, error_recovery: bool) -> float: + def liquid_probe_with_recovery(self, well_core: WellCoreType) -> None: + """Do a liquid probe to detect the presence of liquid in the well.""" + ... + + @abstractmethod + def liquid_probe_without_recovery(self, well_core: WellCoreType) -> float: """Do a liquid probe to find the level of the liquid in the well.""" ... diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py index 2e2aeba3ed9..02f0bef1b0a 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py @@ -571,6 +571,10 @@ def retract(self) -> None: """Retract this instrument to the top of the gantry.""" self._protocol_interface.get_hardware.retract(self._mount) # type: ignore [attr-defined] - def find_liquid_level(self, well_core: WellCore, error_recovery: bool) -> float: + def liquid_probe_with_recovery(self, well_core: WellCore) -> None: """This will never be called because it was added in API 2.20.""" - assert False, "find_liquid_level only supported in API 2.20 & later" + assert False, "liquid_probe_with_recovery only supported in API 2.20 & later" + + def liquid_probe_without_recovery(self, well_core: WellCore) -> float: + """This will never be called because it was added in API 2.20.""" + assert False, "liquid_probe_without_recovery only supported in API 2.20 & later" diff --git a/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py b/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py index 27b964b4d61..f53279334a4 100644 --- a/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py +++ b/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py @@ -489,6 +489,10 @@ def retract(self) -> None: """Retract this instrument to the top of the gantry.""" self._protocol_interface.get_hardware.retract(self._mount) # type: ignore [attr-defined] - def find_liquid_level(self, well_core: WellCore, error_recovery: bool) -> float: + def liquid_probe_with_recovery(self, well_core: WellCore) -> None: """This will never be called because it was added in API 2.20.""" - assert False, "find_liquid_level only supported in API 2.20 & later" + assert False, "liquid_probe_with_recovery only supported in API 2.20 & later" + + def liquid_probe_without_recovery(self, well_core: WellCore) -> float: + """This will never be called because it was added in API 2.20.""" + assert False, "liquid_probe_without_recovery only supported in API 2.20 & later" diff --git a/api/src/opentrons/protocol_api/instrument_context.py b/api/src/opentrons/protocol_api/instrument_context.py index 4b77340bc7e..2c5b1c58160 100644 --- a/api/src/opentrons/protocol_api/instrument_context.py +++ b/api/src/opentrons/protocol_api/instrument_context.py @@ -2,10 +2,11 @@ import logging from contextlib import ExitStack from typing import Any, List, Optional, Sequence, Union, cast, Dict +from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError +from opentrons.protocol_engine.errors.error_occurrence import ProtocolCommandFailedError from opentrons_shared_data.errors.exceptions import ( CommandPreconditionViolated, CommandParameterLimitViolated, - PipetteLiquidNotFoundError, UnexpectedTipRemovalError, ) from opentrons.protocol_engine.errors.exceptions import WellDoesNotExistError @@ -2057,16 +2058,13 @@ def detect_liquid_presence(self, well: labware.Well) -> bool: if not isinstance(well, labware.Well): raise WellDoesNotExistError("You must provide a valid well to check.") try: - height = self._core.find_liquid_level( - well_core=well._core, error_recovery=False - ) - if height > 0: - return True - return False # it should never get here - except PipetteLiquidNotFoundError: - return False - except Exception as e: + self._core.liquid_probe_without_recovery(well._core) + except ProtocolCommandFailedError as e: + if isinstance(e.original_error, LiquidNotFoundError): + return False raise e + else: + return True @requires_version(2, 20) def require_liquid_presence(self, well: labware.Well) -> None: @@ -2077,18 +2075,20 @@ def require_liquid_presence(self, well: labware.Well) -> None: if not isinstance(well, labware.Well): raise WellDoesNotExistError("You must provide a valid well to check.") - self._core.find_liquid_level(well_core=well._core, error_recovery=True) + self._core.liquid_probe_with_recovery(well._core) @requires_version(2, 20) def measure_liquid_height(self, well: labware.Well) -> float: """Check the height of the liquid within a well. :returns: The height, in mm, of the liquid from the deck. + + :meta private: + + This is intended for Opentrons internal use only and is not a guaranteed API. """ if not isinstance(well, labware.Well): raise WellDoesNotExistError("You must provide a valid well to check.") - height = self._core.find_liquid_level( - well_core=well._core, error_recovery=False - ) - return float(height) + height = self._core.liquid_probe_without_recovery(well._core) + return height diff --git a/api/src/opentrons/protocol_engine/clients/sync_client.py b/api/src/opentrons/protocol_engine/clients/sync_client.py index 3eed8e2d263..f772d81cea8 100644 --- a/api/src/opentrons/protocol_engine/clients/sync_client.py +++ b/api/src/opentrons/protocol_engine/clients/sync_client.py @@ -2,7 +2,6 @@ from typing import cast, Any, Optional, overload -from opentrons.protocol_engine.errors.error_occurrence import ProtocolCommandFailedError from opentrons_shared_data.labware.dev_types import LabwareUri from opentrons_shared_data.labware.labware_definition import LabwareDefinition @@ -96,26 +95,6 @@ def execute_command_without_recovery( create_request = CreateType(params=cast(Any, params)) return self._transport.execute_command(create_request) - def execute_command_with_result( - self, params: commands.CommandParams - ) -> Optional[commands.CommandResult]: - """Execute a ProtocolEngine command, including error recovery, and return a result. - - See `ChildThreadTransport.execute_command_wait_for_recovery()` for exact - behavior. - """ - CreateType = CREATE_TYPES_BY_PARAMS_TYPE[type(params)] - create_request = CreateType(params=cast(Any, params)) - result = self._transport.execute_command_wait_for_recovery(create_request) - if result.error is None: - return result.result - if isinstance(result.error, BaseException): # necessary to pass lint - raise result.error - raise ProtocolCommandFailedError( - original_error=result.error, - message=f"{result.error.errorType}: {result.error.detail}", - ) - @property def state(self) -> StateView: """Get a view of the engine's state.""" diff --git a/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py b/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py index a38ffc0a234..3ca12bc004f 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py @@ -1,6 +1,7 @@ """Test for the ProtocolEngine-based instrument API core.""" from typing import cast, Optional, Union +from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult from opentrons_shared_data.errors.exceptions import PipetteLiquidNotFoundError import pytest from decoy import Decoy @@ -1292,23 +1293,41 @@ def test_configure_for_volume_post_219( @pytest.mark.parametrize("version", versions_at_or_above(APIVersion(2, 20))) -def test_find_liquid_level( +def test_liquid_probe_without_recovery( decoy: Decoy, mock_engine_client: EngineClient, mock_protocol_core: ProtocolCore, subject: InstrumentCore, version: APIVersion, ) -> None: - """It should raise an exception on an empty well.""" + """It should raise an exception on an empty well and return a float on a valid well.""" well_core = WellCore( name="my cool well", labware_id="123abc", engine_client=mock_engine_client ) + decoy.when( + mock_engine_client.execute_command_without_recovery( + cmd.LiquidProbeParams( + pipetteId=subject.pipette_id, + wellLocation=WellLocation( + origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0) + ), + wellName=well_core.get_name(), + labwareId=well_core.labware_id, + ) + ) + ).then_raise(PipetteLiquidNotFoundError()) try: - subject.find_liquid_level(well_core=well_core, error_recovery=True) + subject.liquid_probe_without_recovery(well_core=well_core) except PipetteLiquidNotFoundError: assert True - decoy.verify( - mock_engine_client.execute_command_with_result( + else: + assert False + + decoy.reset() + + lpr = LiquidProbeResult(z_position=5.0) + decoy.when( + mock_engine_client.execute_command_without_recovery( cmd.LiquidProbeParams( pipetteId=subject.pipette_id, wellLocation=WellLocation( @@ -1318,13 +1337,25 @@ def test_find_liquid_level( labwareId=well_core.labware_id, ) ) + ).then_return(lpr) + assert subject.liquid_probe_without_recovery(well_core=well_core) == 5.0 + + +@pytest.mark.parametrize("version", versions_at_or_above(APIVersion(2, 20))) +def test_liquid_probe_with_recovery( + decoy: Decoy, + mock_engine_client: EngineClient, + mock_protocol_core: ProtocolCore, + subject: InstrumentCore, + version: APIVersion, +) -> None: + """It should not raise an exception on an empty well.""" + well_core = WellCore( + name="my cool well", labware_id="123abc", engine_client=mock_engine_client ) - try: - subject.find_liquid_level(well_core=well_core, error_recovery=False) - except PipetteLiquidNotFoundError: - assert True + subject.liquid_probe_with_recovery(well_core=well_core) decoy.verify( - mock_engine_client.execute_command_without_recovery( + mock_engine_client.execute_command( cmd.LiquidProbeParams( pipetteId=subject.pipette_id, wellLocation=WellLocation( diff --git a/api/tests/opentrons/protocol_api/test_instrument_context.py b/api/tests/opentrons/protocol_api/test_instrument_context.py index ab5d099460e..0d69d462098 100644 --- a/api/tests/opentrons/protocol_api/test_instrument_context.py +++ b/api/tests/opentrons/protocol_api/test_instrument_context.py @@ -1,6 +1,11 @@ """Tests for the InstrumentContext public interface.""" from collections import OrderedDict +from datetime import datetime import inspect +from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError +from opentrons.protocol_engine.errors.error_occurrence import ( + ProtocolCommandFailedError, +) import pytest from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] from decoy import Decoy @@ -38,7 +43,6 @@ from opentrons_shared_data.errors.exceptions import ( CommandPreconditionViolated, - PipetteLiquidNotFoundError, ) @@ -1279,11 +1283,17 @@ def test_detect_liquid_presence( ) -> None: """It should only return booleans. Not raise an exception.""" mock_well = decoy.mock(cls=Well) + lnfe = LiquidNotFoundError(id="1234", createdAt=datetime.now()) + errorToRaise = ProtocolCommandFailedError( + original_error=lnfe, + message=f"{lnfe.errorType}: {lnfe.detail}", + ) decoy.when( - mock_instrument_core.find_liquid_level(mock_well._core, False) - ).then_raise(PipetteLiquidNotFoundError()) + mock_instrument_core.liquid_probe_without_recovery(mock_well._core) + ).then_raise(errorToRaise) result = subject.detect_liquid_presence(mock_well) assert isinstance(result, bool) + assert not result @pytest.mark.parametrize("api_version", [APIVersion(2, 20)]) @@ -1295,13 +1305,19 @@ def test_require_liquid_presence( ) -> None: """It should raise an exception when called.""" mock_well = decoy.mock(cls=Well) - decoy.when(mock_instrument_core.find_liquid_level(mock_well._core, True)) + lnfe = LiquidNotFoundError(id="1234", createdAt=datetime.now()) + errorToRaise = ProtocolCommandFailedError( + original_error=lnfe, + message=f"{lnfe.errorType}: {lnfe.detail}", + ) + decoy.when(mock_instrument_core.liquid_probe_with_recovery(mock_well._core)) subject.require_liquid_presence(mock_well) decoy.when( - mock_instrument_core.find_liquid_level(mock_well._core, True) - ).then_raise(PipetteLiquidNotFoundError()) - with pytest.raises(PipetteLiquidNotFoundError): + mock_instrument_core.liquid_probe_with_recovery(mock_well._core) + ).then_raise(errorToRaise) + with pytest.raises(ProtocolCommandFailedError) as pcfe: subject.require_liquid_presence(mock_well) + assert pcfe.value is errorToRaise @pytest.mark.parametrize("api_version", [APIVersion(2, 20)]) @@ -1313,8 +1329,14 @@ def test_measure_liquid_height( ) -> None: """It should raise an exception when called.""" mock_well = decoy.mock(cls=Well) + lnfe = LiquidNotFoundError(id="1234", createdAt=datetime.now()) + errorToRaise = ProtocolCommandFailedError( + original_error=lnfe, + message=f"{lnfe.errorType}: {lnfe.detail}", + ) decoy.when( - mock_instrument_core.find_liquid_level(mock_well._core, False) - ).then_raise(PipetteLiquidNotFoundError()) - with pytest.raises(PipetteLiquidNotFoundError): + mock_instrument_core.liquid_probe_without_recovery(mock_well._core) + ).then_raise(errorToRaise) + with pytest.raises(ProtocolCommandFailedError) as pcfe: subject.measure_liquid_height(mock_well) + assert pcfe.value is errorToRaise