fix(hardware_control): Fixing liquid probe bugs found in end to end t…

…esting (#15619)

<!--
Thanks for taking the time to open a pull request! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview

<!--
Use this section to describe your pull-request at a high level. If the
PR addresses any open issues, please tag the issues here.
-->

These changes came from testing the end to end implementation of liquid
level detection on the ot3 and fixing the bugs that came up.

# Test Plan

<!--
Use this section to describe the steps that you took to test your Pull
Request.
If you did not perform any testing provide justification why.

OT-3 Developers: You should default to testing on actual physical
hardware.
Once again, if you did not perform testing against hardware, justify
why.

Note: It can be helpful to write a test plan before doing development

Example Test Plan (HTTP API Change)

- Verified that new optional argument `dance-party` causes the robot to
flash its lights, move the pipettes,
then home.
- Verified that when you omit the `dance-party` option the robot homes
normally
- Added protocol that uses `dance-party` argument to G-Code Testing
Suite
- Ran protocol that did not use `dance-party` argument and everything
was successful
- Added unit tests to validate that changes to pydantic model are
correct

-->
Changed the unit tests to match new functionality.
Tested all new methods for liquid level detection on the ot3. 

# Changelog

<!--
List out the changes to the code in this PR. Please try your best to
categorize your changes and describe what has changed and why.

Example changelog:
- Fixed app crash when trying to calibrate an illegal pipette
- Added state to API to track pipette usage
- Updated API docs to mention only two pipettes are supported

IMPORTANT: MAKE SURE ANY BREAKING CHANGES ARE PROPERLY COMMUNICATED
-->

Changes include:
* Deleting unnecessary checks in ot3controller.py
* Editing ot3api.py to issue the correct probe movements
* Standardizing the attribute names for liquid_presence_detection
* Conditionally adding a require_liquid_presence command before
aspirates
* Redoing movement in liquid_probe so that it actually takes a sensible
path
* Editing tests accordingly

# Review requests

<!--
Describe any requests for your reviewers here.
-->

We are aware of a bug that even though we handle LLD exceptions at the
protocol_context level, protocol_engine commands will still be marked as
failed, so all future commands will fail. As far as I know for this PR,
this only affects detect_liquid_presence on a well with no liquid.
Another PR will be coming soon that addresses this.

# Risk assessment

<!--
Carefully go over your pull request and look at the other parts of the
codebase it may affect. Look for the possibility, even if you think it's
small, that your change may affect some other part of the system - for
instance, changing return tip behavior in protocol may also change the
behavior of labware calibration.

Identify the other parts of the system your codebase may affect, so that
in addition to your own review and testing, other people who may not
have the system internalized as much as you can focus their attention
and testing there.
-->

Medium? I'm not sure.

---------

Co-authored-by: aaron-kulkarni <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: aaron-kulkarni <[email protected]>

api/src/opentrons/config/defaults_ot3.py

@@ -23,8 +23,8 @@
 DEFAULT_MODULE_OFFSET = [0.0, 0.0, 0.0]
 
 DEFAULT_LIQUID_PROBE_SETTINGS: Final[LiquidProbeSettings] = LiquidProbeSettings(
-    mount_speed=10,
-    plunger_speed=5,
+    mount_speed=5,
+    plunger_speed=20,
     plunger_impulse_time=0.2,
     sensor_threshold_pascals=15,
     output_option=OutputOptions.sync_buffer_to_csv,
@@ -328,7 +328,7 @@ def _build_default_liquid_probe(
         or output_option is OutputOptions.stream_to_csv
     ):
         data_files = _build_log_files_with_default(
-            from_conf.get("data_files", {}), default.data_files
+            from_conf.get("data_files", None), default.data_files
         )
     return LiquidProbeSettings(
         mount_speed=from_conf.get("mount_speed", default.mount_speed),

api/src/opentrons/hardware_control/backends/ot3controller.py

@@ -194,7 +194,6 @@
     PipetteLiquidNotFoundError,
     CommunicationError,
     PythonException,
-    UnsupportedHardwareCommand,
 )
 
 from .subsystem_manager import SubsystemManager
@@ -1363,17 +1362,6 @@ async def liquid_probe(
         probe: InstrumentProbeType = InstrumentProbeType.PRIMARY,
         force_both_sensors: bool = False,
     ) -> float:
-        if output_option == OutputOptions.sync_buffer_to_csv:
-            if (
-                self._subsystem_manager.device_info[
-                    SubSystem.of_mount(mount)
-                ].revision.tertiary
-                != "1"
-            ):
-                raise UnsupportedHardwareCommand(
-                    "Liquid Probe not supported on this pipette firmware"
-                )
-
         head_node = axis_to_node(Axis.by_mount(mount))
         tool = sensor_node_for_pipette(OT3Mount(mount.value))
         csv_output = bool(output_option.value & OutputOptions.stream_to_csv.value)

api/src/opentrons/hardware_control/ot3api.py

@@ -2636,10 +2636,13 @@ async def liquid_probe(
         pos = await self.gantry_position(checked_mount, refresh=True)
         while (probe_start_pos.z - pos.z) < max_z_dist:
             # safe distance so we don't accidentally aspirate liquid if we're already close to liquid
-            safe_plunger_pos = pos._replace(z=(pos.z + 2))
+            safe_plunger_pos = top_types.Point(pos.x, pos.y, pos.z + 2)
             # overlap amount we want to use between passes
-            pass_start_pos = pos._replace(z=(pos.z + 0.5))
-
+            pass_start_pos = top_types.Point(pos.x, pos.y, pos.z + 0.5)
+            max_z_time = (
+                max_z_dist - (probe_start_pos.z - safe_plunger_pos.z)
+            ) / probe_settings.mount_speed
+            pass_travel = min(max_z_time * probe_settings.plunger_speed, p_travel)
             # Prep the plunger
             await self.move_to(checked_mount, safe_plunger_pos)
             if probe_settings.aspirate_while_sensing:
@@ -2655,7 +2658,7 @@ async def liquid_probe(
                     checked_mount,
                     probe_settings,
                     probe if probe else InstrumentProbeType.PRIMARY,
-                    p_travel,
+                    pass_travel,
                 )
                 # if we made it here without an error we found the liquid
                 error = None

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

@@ -838,13 +838,12 @@ 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 liquid_probe_with_recovery(self, well_core: WellCore) -> 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()
         well_location = WellLocation(
             origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0)
         )
-
         self._engine_client.execute_command(
             cmd.LiquidProbeParams(
                 labwareId=labware_id,
@@ -854,13 +853,16 @@ def liquid_probe_with_recovery(self, well_core: WellCore) -> None:
             )
         )
 
-    def liquid_probe_without_recovery(self, well_core: WellCore) -> float:
+        self._protocol_core.set_last_location(location=loc, mount=self.get_mount())
+
+    def liquid_probe_without_recovery(
+        self, well_core: WellCore, loc: Location
+    ) -> 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,
@@ -870,5 +872,7 @@ def liquid_probe_without_recovery(self, well_core: WellCore) -> float:
             )
         )
 
+        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

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

@@ -306,12 +306,16 @@ def retract(self) -> None:
         ...
 
     @abstractmethod
-    def liquid_probe_with_recovery(self, well_core: WellCoreType) -> None:
+    def liquid_probe_with_recovery(
+        self, well_core: WellCoreType, loc: types.Location
+    ) -> 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:
+    def liquid_probe_without_recovery(
+        self, well_core: WellCoreType, loc: types.Location
+    ) -> float:
         """Do a liquid probe to find the level of the liquid in the well."""
         ...
 

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

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

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

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

api/src/opentrons/protocol_api/instrument_context.py

@@ -9,7 +9,6 @@
     CommandParameterLimitViolated,
     UnexpectedTipRemovalError,
 )
-from opentrons.protocol_engine.errors.exceptions import WellDoesNotExistError
 from opentrons.legacy_broker import LegacyBroker
 from opentrons.hardware_control.dev_types import PipetteDict
 from opentrons import types
@@ -226,7 +225,6 @@ def aspirate(
 
         well: Optional[labware.Well] = None
         move_to_location: types.Location
-
         last_location = self._get_last_location_by_api_version()
         try:
             target = validation.validate_location(
@@ -264,6 +262,14 @@ def aspirate(
             c_vol = self._core.get_available_volume() if not volume else volume
         flow_rate = self._core.get_aspirate_flow_rate(rate)
 
+        if (
+            self.api_version >= APIVersion(2, 20)
+            and well is not None
+            and self.liquid_presence_detection
+        ):
+            self.require_liquid_presence(well=well)
+            self.prepare_to_aspirate()
+
         with publisher.publish_context(
             broker=self.broker,
             command=cmds.aspirate(
@@ -2105,11 +2111,11 @@ def detect_liquid_presence(self, well: labware.Well) -> bool:
 
         :returns: A boolean.
         """
-        if not isinstance(well, labware.Well):
-            raise WellDoesNotExistError("You must provide a valid well to check.")
+        loc = well.top()
         try:
-            self._core.liquid_probe_without_recovery(well._core)
+            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
@@ -2122,10 +2128,8 @@ def require_liquid_presence(self, well: labware.Well) -> None:
 
         :returns: None.
         """
-        if not isinstance(well, labware.Well):
-            raise WellDoesNotExistError("You must provide a valid well to check.")
-
-        self._core.liquid_probe_with_recovery(well._core)
+        loc = well.top()
+        self._core.liquid_probe_with_recovery(well._core, loc)
 
     @requires_version(2, 20)
     def measure_liquid_height(self, well: labware.Well) -> float:
@@ -2137,8 +2141,6 @@ def measure_liquid_height(self, well: labware.Well) -> float:
 
         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.liquid_probe_without_recovery(well._core)
+        loc = well.top()
+        height = self._core.liquid_probe_without_recovery(well._core, loc)
         return height

api/src/opentrons/protocol_engine/commands/liquid_probe.py

@@ -10,7 +10,7 @@
 
 from pydantic import Field
 
-from ..types import CurrentWell, DeckPoint
+from ..types import DeckPoint
 from .pipetting_common import (
     LiquidNotFoundError,
     LiquidNotFoundErrorInternalData,
@@ -77,8 +77,9 @@ async def execute(self, params: LiquidProbeParams) -> _ExecuteReturn:
         Return the z-position of the found liquid.
 
         Raises:
-            TipNotAttachedError: if there is not tip attached to the pipette
+            TipNotAttachedError: if there is no tip attached to the pipette
             MustHomeError: if the plunger is not in a valid position
+            TipNotEmptyError: if the tip starts with liquid in it
             LiquidNotFoundError: if liquid is not found during the probe process.
         """
         pipette_id = params.pipetteId
@@ -99,19 +100,12 @@ async def execute(self, params: LiquidProbeParams) -> _ExecuteReturn:
                 message="Current position of pipette is invalid. Please home."
             )
 
-        current_well = CurrentWell(
-            pipette_id=pipette_id,
-            labware_id=labware_id,
-            well_name=well_name,
-        )
-
         # liquid_probe process start position
         position = await self._movement.move_to_well(
             pipette_id=pipette_id,
             labware_id=labware_id,
             well_name=well_name,
             well_location=params.wellLocation,
-            current_well=current_well,
         )
 
         try:

api/src/opentrons/protocol_engine/execution/pipetting.py

@@ -30,7 +30,7 @@ class PipettingHandler(TypingProtocol):
     """Liquid handling commands."""
 
     def get_is_empty(self, pipette_id: str) -> bool:
-        """Get whether a pipette has a working volume equal to 0."""
+        """Get whether a pipette has an aspirated volume equal to 0."""
 
     def get_is_ready_to_aspirate(self, pipette_id: str) -> bool:
         """Get whether a pipette is ready to aspirate."""
@@ -81,8 +81,8 @@ def __init__(self, state_view: StateView, hardware_api: HardwareControlAPI) -> N
         self._hardware_api = hardware_api
 
     def get_is_empty(self, pipette_id: str) -> bool:
-        """Get whether a pipette has a working volume equal to 0."""
-        return self._state_view.pipettes.get_working_volume(pipette_id) == 0
+        """Get whether a pipette has an aspirated volume equal to 0."""
+        return self._state_view.pipettes.get_aspirated_volume(pipette_id) == 0
 
     def get_is_ready_to_aspirate(self, pipette_id: str) -> bool:
         """Get whether a pipette is ready to aspirate."""
@@ -234,8 +234,8 @@ def __init__(
         self._state_view = state_view
 
     def get_is_empty(self, pipette_id: str) -> bool:
-        """Get whether a pipette has a working volume equal to 0."""
-        return self._state_view.pipettes.get_working_volume(pipette_id) == 0
+        """Get whether a pipette has an aspirated volume equal to 0."""
+        return self._state_view.pipettes.get_aspirated_volume(pipette_id) == 0
 
     def get_is_ready_to_aspirate(self, pipette_id: str) -> bool:
         """Get whether a pipette is ready to aspirate."""

api/src/opentrons/protocol_engine/state/pipettes.py

@@ -637,7 +637,7 @@ def get_current_tip_lld_settings(self, pipette_id: str) -> float:
         if attached_tip is None or attached_tip.volume is None:
             return 0
         lld_settings = self.get_pipette_lld_settings(pipette_id)
-        tipVolume = str(attached_tip.volume)
+        tipVolume = "t" + str(int(attached_tip.volume))
         if (
             lld_settings is None
             or lld_settings[tipVolume] is None

api/tests/opentrons/hardware_control/test_ot3_api.py

@@ -806,9 +806,6 @@ async def test_liquid_probe(
     )
     await ot3_hardware.cache_pipette(mount, instr_data, None)
     pipette = ot3_hardware.hardware_pipettes[mount.to_mount()]
-    plunger_positions = ot3_hardware._pipette_handler.get_pipette(
-        mount
-    ).plunger_positions
 
     assert pipette
     await ot3_hardware.add_tip(mount, 100)
@@ -841,7 +838,7 @@ async def test_liquid_probe(
         mock_move_to_plunger_bottom.assert_called_once()
         mock_liquid_probe.assert_called_once_with(
             mount,
-            plunger_positions.bottom - plunger_positions.top,
+            3.0,
             fake_settings_aspirate.mount_speed,
             (fake_settings_aspirate.plunger_speed * -1),
             fake_settings_aspirate.sensor_threshold_pascals,

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

@@ -1,7 +1,6 @@
 """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
@@ -1340,30 +1339,14 @@ def test_liquid_probe_without_recovery(
             )
         )
     ).then_raise(PipetteLiquidNotFoundError())
+    loc = Location(Point(0, 0, 0), None)
     try:
-        subject.liquid_probe_without_recovery(well_core=well_core)
+        subject.liquid_probe_without_recovery(well_core=well_core, loc=loc)
     except PipetteLiquidNotFoundError:
         assert True
     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(
-                    origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=0)
-                ),
-                wellName=well_core.get_name(),
-                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(
@@ -1377,7 +1360,8 @@ def test_liquid_probe_with_recovery(
     well_core = WellCore(
         name="my cool well", labware_id="123abc", engine_client=mock_engine_client
     )
-    subject.liquid_probe_with_recovery(well_core=well_core)
+    loc = Location(Point(0, 0, 0), None)
+    subject.liquid_probe_with_recovery(well_core=well_core, loc=loc)
     decoy.verify(
         mock_engine_client.execute_command(
             cmd.LiquidProbeParams(