readbility changes (#14178)

api/src/opentrons/hardware_control/instruments/ot3/gripper.py

@@ -24,7 +24,6 @@
 from opentrons.hardware_control.dev_types import AttachedGripper, GripperDict
 from opentrons_shared_data.errors.exceptions import (
     CommandPreconditionViolated,
-    FailedGripperPickupError,
 )
 
 from opentrons_shared_data.gripper import (
@@ -203,23 +202,6 @@ def check_calibration_pin_location_is_accurate(self) -> None:
                 },
             )
 
-    def check_labware_pickup(self, labware_width: float) -> None:
-        """Ensure that a gripper pickup succeeded."""
-        # check if the gripper is at an acceptable position after attempting to
-        #  pick up labware
-        expected_gripper_position = labware_width
-        current_gripper_position = self.jaw_width
-        if (
-            abs(current_gripper_position - expected_gripper_position)
-            > self.max_allowed_grip_error
-        ):
-            raise FailedGripperPickupError(
-                details={
-                    "expected jaw width": expected_gripper_position,
-                    "actual jaw width": current_gripper_position,
-                },
-            )
-
     def critical_point(self, cp_override: Optional[CriticalPoint] = None) -> Point:
         """
         The vector from the gripper mount to the critical point, which is selectable

api/src/opentrons/hardware_control/ot3api.py

@@ -36,6 +36,7 @@
 from opentrons_shared_data.robot.dev_types import RobotType
 from opentrons_shared_data.errors.exceptions import (
     StallOrCollisionDetectedError,
+    FailedGripperPickupError,
 )
 
 from opentrons import types as top_types
@@ -1323,6 +1324,24 @@ async def idle_gripper(self) -> None:
         except GripperNotPresentError:
             pass
 
+    def raise_error_if_gripper_pickup_failed(self, labware_width: float) -> None:
+        """Ensure that a gripper pickup succeeded."""
+        # check if the gripper is at an acceptable position after attempting to
+        #  pick up labware
+        assert self.hardware_gripper
+        expected_gripper_position = labware_width
+        current_gripper_position = self.hardware_gripper.jaw_width
+        if (
+            abs(current_gripper_position - expected_gripper_position)
+            > self.hardware_gripper.max_allowed_grip_error
+        ):
+            raise FailedGripperPickupError(
+                details={
+                    "expected jaw width": expected_gripper_position,
+                    "actual jaw width": current_gripper_position,
+                },
+            )
+
     def gripper_jaw_can_home(self) -> bool:
         return self._gripper_handler.is_ready_for_jaw_home()
 

api/src/opentrons/hardware_control/protocols/gripper_controller.py

@@ -33,6 +33,9 @@ def gripper_jaw_can_home(self) -> bool:
         """
         ...
 
+    def raise_error_if_gripper_pickup_failed(self, labware_width: float) -> None:
+        """Ensure that a gripper pickup succeeded."""
+
     @property
     def attached_gripper(self) -> Optional[GripperDict]:
         """Get a dict of all attached grippers."""

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

@@ -135,7 +135,7 @@ async def move_labware_with_gripper(
                 post_drop_slide_offset=post_drop_slide_offset,
             )
             labware_grip_force = self._state_store.labware.get_grip_force(labware_id)
-            gripper_opened = False
+            holding_labware = False
             for waypoint_data in movement_waypoints:
                 if waypoint_data.jaw_open:
                     if waypoint_data.dropping:
@@ -146,7 +146,7 @@ async def move_labware_with_gripper(
                         # on the side of a falling tiprack catches the jaw.
                         await ot3api.disengage_axes([Axis.Z_G])
                     await ot3api.ungrip()
-                    gripper_opened = True
+                    holding_labware = True
                     if waypoint_data.dropping:
                         # We lost the position estimation after disengaging the axis, so
                         # it is necessary to home it next
@@ -155,9 +155,8 @@ async def move_labware_with_gripper(
                     await ot3api.grip(force_newtons=labware_grip_force)
                     # we only want to check position after the gripper has opened and
                     # should be holding labware
-                    if gripper_opened:
-                        assert ot3api.hardware_gripper
-                        ot3api.hardware_gripper.check_labware_pickup(
+                    if holding_labware:
+                        ot3api.raise_error_if_gripper_pickup_failed(
                             labware_width=self._state_store.labware.get_dimensions(
                                 labware_id
                             ).y

api/tests/opentrons/hardware_control/test_gripper.py

@@ -1,15 +1,12 @@
-from typing import Optional, Callable, TYPE_CHECKING, Any, Generator
+from typing import Optional, Callable, TYPE_CHECKING
 import pytest
-from contextlib import nullcontext
-from unittest.mock import MagicMock, patch, PropertyMock
 
 from opentrons.types import Point
 from opentrons.calibration_storage import types as cal_types
 from opentrons.hardware_control.instruments.ot3 import gripper, instrument_calibration
 from opentrons.hardware_control.types import CriticalPoint
 from opentrons.config import gripper_config
 from opentrons_shared_data.gripper import GripperModel
-from opentrons_shared_data.errors.exceptions import FailedGripperPickupError
 
 if TYPE_CHECKING:
     from opentrons.hardware_control.instruments.ot3.instrument_calibration import (
@@ -29,24 +26,6 @@ def fake_offset() -> "GripperCalibrationOffset":
     return load_gripper_calibration_offset("fakeid123")
 
 
-@pytest.fixture
-def mock_jaw_width() -> Generator[MagicMock, None, None]:
-    with patch(
-        "opentrons.hardware_control.instruments.ot3.gripper.Gripper.jaw_width",
-        new_callable=PropertyMock,
-    ) as jaw_width:
-        yield jaw_width
-
-
-@pytest.fixture
-def mock_max_grip_error() -> Generator[MagicMock, None, None]:
-    with patch(
-        "opentrons.hardware_control.instruments.ot3.gripper.Gripper.max_allowed_grip_error",
-        new_callable=PropertyMock,
-    ) as max_error:
-        yield max_error
-
-
 @pytest.mark.ot3_only
 def test_id_get_added_to_dict(fake_offset: "GripperCalibrationOffset") -> None:
     gripr = gripper.Gripper(fake_gripper_conf, fake_offset, "fakeid123")
@@ -88,32 +67,6 @@ def test_load_gripper_cal_offset(fake_offset: "GripperCalibrationOffset") -> Non
     )
 
 
-@pytest.mark.ot3_only
-@pytest.mark.parametrize(
-    argnames=["jaw_width_val", "error_context"],
-    argvalues=[
-        (89, nullcontext()),
-        (100, pytest.raises(FailedGripperPickupError)),
-        (50, pytest.raises(FailedGripperPickupError)),
-        (85, nullcontext()),
-    ],
-)
-def test_check_labware_pickup(
-    mock_jaw_width: Any,
-    mock_max_grip_error: Any,
-    jaw_width_val: float,
-    error_context: Any,
-) -> None:
-    """Test that FailedGripperPickupError is raised correctly."""
-    #  This should only be triggered when the difference between the
-    #  gripper jaw and labware widths is greater than the max allowed error.
-    gripr = gripper.Gripper(fake_gripper_conf, fake_offset, "fakeid123")
-    mock_jaw_width.return_value = jaw_width_val
-    mock_max_grip_error.return_value = 6
-    with error_context:
-        gripr.check_labware_pickup(85)
-
-
 @pytest.mark.ot3_only
 def test_reload_instrument_cal_ot3(fake_offset: "GripperCalibrationOffset") -> None:
     old_gripper = gripper.Gripper(

api/tests/opentrons/hardware_control/test_ot3_api.py

@@ -7,6 +7,7 @@
 from decoy import Decoy
 from mock import AsyncMock, patch, Mock, PropertyMock, MagicMock
 from hypothesis import given, strategies, settings, HealthCheck, assume, example
+from contextlib import nullcontext
 
 from opentrons.calibration_storage.types import CalibrationStatus, SourceType
 from opentrons.config.types import (
@@ -63,6 +64,7 @@
     GripperNotPresentError,
     CommandPreconditionViolated,
     CommandParameterLimitViolated,
+    FailedGripperPickupError,
 )
 from opentrons_shared_data.gripper.gripper_definition import GripperModel
 from opentrons_shared_data.pipette.types import (
@@ -297,6 +299,24 @@ async def mock_reset(ot3_hardware: ThreadManager[OT3API]) -> Iterator[AsyncMock]
         yield mock_reset
 
 
+@pytest.fixture
+def mock_jaw_width() -> Iterator[MagicMock]:
+    with patch(
+        "opentrons.hardware_control.instruments.ot3.gripper.Gripper.jaw_width",
+        new_callable=PropertyMock,
+    ) as jaw_width:
+        yield jaw_width
+
+
+@pytest.fixture
+def mock_max_grip_error() -> Iterator[MagicMock]:
+    with patch(
+        "opentrons.hardware_control.instruments.ot3.gripper.Gripper.max_allowed_grip_error",
+        new_callable=PropertyMock,
+    ) as max_error:
+        yield max_error
+
+
 @pytest.fixture
 async def mock_instrument_handlers(
     ot3_hardware: ThreadManager[OT3API],
@@ -1014,6 +1034,39 @@ async def test_gripper_action_fails_with_no_gripper(
     mock_ungrip.assert_not_called()
 
 
+@pytest.mark.parametrize(
+    argnames=["jaw_width_val", "error_context"],
+    argvalues=[
+        (89, nullcontext()),
+        (100, pytest.raises(FailedGripperPickupError)),
+        (50, pytest.raises(FailedGripperPickupError)),
+        (85, nullcontext()),
+    ],
+)
+async def test_raise_error_if_gripper_pickup_failed(
+    ot3_hardware: ThreadManager[OT3API],
+    mock_jaw_width: Any,
+    mock_max_grip_error: Any,
+    jaw_width_val: float,
+    error_context: Any,
+) -> None:
+    """Test that FailedGripperPickupError is raised correctly."""
+    #  This should only be triggered when the difference between the
+    #  gripper jaw and labware widths is greater than the max allowed error.
+
+    gripper_config = gc.load(GripperModel.v1)
+    instr_data = AttachedGripper(config=gripper_config, id="test")
+    ot3_hardware._backend._attached_instruments[OT3Mount.GRIPPER] = {
+        "model": GripperModel.v1,
+        "id": "test",
+    }
+    await ot3_hardware.cache_gripper(instr_data)
+    mock_jaw_width.return_value = jaw_width_val
+    mock_max_grip_error.return_value = 6
+    with error_context:
+        ot3_hardware.raise_error_if_gripper_pickup_failed(85)
+
+
 async def test_gripper_action_works_with_gripper(
     ot3_hardware: ThreadManager[OT3API],
     mock_grip: AsyncMock,

api/tests/opentrons/protocol_engine/execution/test_labware_movement_handler.py

@@ -177,7 +177,7 @@ def subject(
 
 
 @pytest.mark.ot3_only
-async def test_check_labware_pickup(
+async def test_raise_error_if_gripper_pickup_failed(
     decoy: Decoy,
     state_store: StateStore,
     thermocycler_plate_lifter: ThermocyclerPlateLifter,
@@ -226,7 +226,7 @@ async def test_check_labware_pickup(
         )
     ).then_return(Point(201, 202, 219.5))
 
-    decoy.when(ot3_hardware_api.hardware_gripper.check_labware_pickup(85)).then_return()
+    decoy.when(ot3_hardware_api.raise_error_if_gripper_pickup_failed(85)).then_return()
 
     await subject.move_labware_with_gripper(
         labware_id="my-teleporting-labware",
@@ -242,11 +242,11 @@ async def test_check_labware_pickup(
         await ot3_hardware_api.grip(force_newtons=100),
         await ot3_hardware_api.ungrip(),
         await ot3_hardware_api.grip(force_newtons=100),
-        ot3_hardware_api.hardware_gripper.check_labware_pickup(labware_width=85),
+        ot3_hardware_api.raise_error_if_gripper_pickup_failed(labware_width=85),
         await ot3_hardware_api.grip(force_newtons=100),
-        ot3_hardware_api.hardware_gripper.check_labware_pickup(labware_width=85),
+        ot3_hardware_api.raise_error_if_gripper_pickup_failed(labware_width=85),
         await ot3_hardware_api.grip(force_newtons=100),
-        ot3_hardware_api.hardware_gripper.check_labware_pickup(labware_width=85),
+        ot3_hardware_api.raise_error_if_gripper_pickup_failed(labware_width=85),
         await ot3_hardware_api.disengage_axes([Axis.Z_G]),
         await ot3_hardware_api.ungrip(),
         await ot3_hardware_api.ungrip(),