fix(api): Do not enter recovery mode if detect_liquid_presence() de…

…tects no liquid (#15698)

api/src/opentrons/protocol_api/core/engine/instrument.py

@@ -2,7 +2,6 @@
 from __future__ import annotations
 
 from typing import Optional, TYPE_CHECKING, cast, Union
-from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult
 from opentrons.protocols.api_support.types import APIVersion
 
 from opentrons.types import Location, Mount
@@ -838,6 +837,44 @@ 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 detect_liquid_presence(self, well_core: WellCore, loc: Location) -> bool:
+        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)
+        )
+
+        # The error handling here is a bit nuanced and also a bit broken:
+        #
+        # - If the hardware detects liquid, the `tryLiquidProbe` engine command will
+        #   succeed and return a height, which we'll convert to a `True` return.
+        #   Okay so far.
+        #
+        # - If the hardware detects no liquid, the `tryLiquidProbe` engine command will
+        #   succeed and return `None`, which we'll convert to a `False` return.
+        #   Still okay so far.
+        #
+        # - If there is any other error within the `tryLiquidProbe` command, things get
+        #   messy. It may kick the run into recovery mode. At that point, all bets are
+        #   off--we lose our guarantee of having a `tryLiquidProbe` command whose
+        #   `result` we can inspect. We don't know how to deal with that here, so we
+        #   currently propagate the exception up, which will quickly kill the protocol,
+        #   after a potential split second of recovery mode. It's unclear what would
+        #   be good user-facing behavior here, but it's unfortunate to kill the protocol
+        #   for an error that the engine thinks should be recoverable.
+        result = self._engine_client.execute_command_without_recovery(
+            cmd.TryLiquidProbeParams(
+                labwareId=labware_id,
+                wellName=well_name,
+                wellLocation=well_location,
+                pipetteId=self.pipette_id,
+            )
+        )
+
+        self._protocol_core.set_last_location(location=loc, mount=self.get_mount())
+
+        return result.z_position is not None
+
     def liquid_probe_with_recovery(self, well_core: WellCore, loc: Location) -> None:
         labware_id = well_core.labware_id
         well_name = well_core.get_name()
@@ -874,5 +911,4 @@ def liquid_probe_without_recovery(
 
         self._protocol_core.set_last_location(location=loc, mount=self.get_mount())
 
-        if result is not None and isinstance(result, LiquidProbeResult):
-            return result.z_position
+        return result.z_position

api/src/opentrons/protocol_api/core/instrument.py

@@ -305,6 +305,12 @@ def retract(self) -> None:
         """Retract this instrument to the top of the gantry."""
         ...
 
+    @abstractmethod
+    def detect_liquid_presence(
+        self, well_core: WellCoreType, loc: types.Location
+    ) -> bool:
+        """Do a liquid probe to detect whether there is liquid in the well."""
+
     @abstractmethod
     def liquid_probe_with_recovery(
         self, well_core: WellCoreType, loc: types.Location

api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py

@@ -566,6 +566,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 detect_liquid_presence(self, well_core: WellCore, loc: types.Location) -> bool:
+        """This will never be called because it was added in API 2.20."""
+        assert False, "detect_liquid_presence only supported in API 2.20 & later"
+
     def liquid_probe_with_recovery(
         self, well_core: WellCore, loc: types.Location
     ) -> None:

api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py

@@ -484,6 +484,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 detect_liquid_presence(self, well_core: WellCore, loc: types.Location) -> bool:
+        """This will never be called because it was added in API 2.20."""
+        assert False, "detect_liquid_presence only supported in API 2.20 & later"
+
     def liquid_probe_with_recovery(
         self, well_core: WellCore, loc: types.Location
     ) -> None:

api/src/opentrons/protocol_api/instrument_context.py

@@ -2,8 +2,6 @@
 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,
@@ -2112,15 +2110,7 @@ def detect_liquid_presence(self, well: labware.Well) -> bool:
         :returns: A boolean.
         """
         loc = well.top()
-        try:
-            self._core.liquid_probe_without_recovery(well._core, loc)
-        except ProtocolCommandFailedError as e:
-            # if we handle the error, we change the protocl state from error to valid
-            if isinstance(e.original_error, LiquidNotFoundError):
-                return False
-            raise e
-        else:
-            return True
+        return self._core.detect_liquid_presence(well._core, loc)
 
     @requires_version(2, 20)
     def require_liquid_presence(self, well: labware.Well) -> None:

api/src/opentrons/protocol_engine/clients/sync_client.py

@@ -83,6 +83,12 @@ def execute_command_without_recovery(
     ) -> commands.LiquidProbeResult:
         pass
 
+    @overload
+    def execute_command_without_recovery(
+        self, params: commands.TryLiquidProbeParams
+    ) -> commands.TryLiquidProbeResult:
+        pass
+
     def execute_command_without_recovery(
         self, params: commands.CommandParams
     ) -> commands.CommandResult:

api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py

@@ -1315,13 +1315,55 @@ def test_configure_for_volume_post_219(
         )
 
 
-@pytest.mark.parametrize("version", versions_at_or_above(APIVersion(2, 20)))
+@pytest.mark.parametrize(
+    ("returned_from_engine", "expected_return_from_core"),
+    [
+        (None, False),
+        (0, True),
+        (1, True),
+    ],
+)
+def test_detect_liquid_presence(
+    returned_from_engine: Optional[float],
+    expected_return_from_core: bool,
+    decoy: Decoy,
+    mock_protocol_core: ProtocolCore,
+    mock_engine_client: EngineClient,
+    subject: InstrumentCore,
+) -> None:
+    """It should convert a height result from the engine to True/False."""
+    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.TryLiquidProbeParams(
+                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_return(
+        cmd.TryLiquidProbeResult.construct(
+            z_position=returned_from_engine,
+            position=object(),  # type: ignore[arg-type]
+        )
+    )
+    loc = Location(Point(0, 0, 0), None)
+
+    result = subject.detect_liquid_presence(well_core=well_core, loc=loc)
+    assert result == expected_return_from_core
+
+    decoy.verify(mock_protocol_core.set_last_location(loc, mount=subject.get_mount()))
+
+
 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 and return a float on a valid well."""
     well_core = WellCore(
@@ -1332,24 +1374,22 @@ def test_liquid_probe_without_recovery(
             cmd.LiquidProbeParams(
                 pipetteId=subject.pipette_id,
                 wellLocation=WellLocation(
-                    origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0)
+                    origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=2)
                 ),
                 wellName=well_core.get_name(),
                 labwareId=well_core.labware_id,
             )
         )
     ).then_raise(PipetteLiquidNotFoundError())
     loc = Location(Point(0, 0, 0), None)
-    subject.liquid_probe_without_recovery(well_core=well_core, loc=loc)
+    with pytest.raises(PipetteLiquidNotFoundError):
+        subject.liquid_probe_without_recovery(well_core=well_core, loc=loc)
 
 
-@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(

api/tests/opentrons/protocol_api/test_instrument_context.py

@@ -1,18 +1,21 @@
 """Tests for the InstrumentContext public interface."""
+import inspect
+import pytest
 from collections import OrderedDict
+from contextlib import nullcontext as does_not_raise
 from datetime import datetime
-import inspect
+from typing import ContextManager, Optional
+from unittest.mock import sentinel
+
+from decoy import Decoy
+from pytest_lazyfixture import lazy_fixture  # type: ignore[import-untyped]
+
 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
 
 from opentrons.legacy_broker import LegacyBroker
-from typing import ContextManager, Optional
-from contextlib import nullcontext as does_not_raise
 
 from opentrons.protocols.api_support import instrument as mock_instrument_support
 from opentrons.protocols.api_support.types import APIVersion
@@ -1289,19 +1292,11 @@ 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.liquid_probe_without_recovery(
-            mock_well._core, mock_well.top()
-        )
-    ).then_raise(errorToRaise)
-    result = subject.detect_liquid_presence(mock_well)
-    assert isinstance(result, bool)
-    assert not result
+        mock_instrument_core.detect_liquid_presence(mock_well._core, mock_well.top())
+    ).then_return(sentinel.inner_result)
+    outer_result = subject.detect_liquid_presence(mock_well)
+    assert outer_result is sentinel.inner_result
 
 
 @pytest.mark.parametrize("api_version", [APIVersion(2, 20)])