refactor(api): ot3: add gripper critical points (#11024)

For now, add some gripper-specific critical points to the enum that the
gripper handles, and use them to calculate position offsets for motion.
JAW_CENTER is the default, and will be what gets used in most calls when
a gripper is present; the calibration points will be used for calibration.

These new critical points will cause an error if used with pipettes, and the pipette critical points will cause an error if used with the gripper.

It's no longer valid to move the mount critical point for the gripper, since that criticalpoint is attached directly to the carriage.

These values are tested on a proto build.

api/src/opentrons/config/defaults_ot3.py

@@ -56,7 +56,7 @@
 DEFAULT_CARRIAGE_OFFSET: Final[Offset] = (436.605, 484.975, 233.475)
 DEFAULT_LEFT_MOUNT_OFFSET: Final[Offset] = (-21.0, -63.05, 256.175)
 DEFAULT_RIGHT_MOUNT_OFFSET: Final[Offset] = (33, -63.05, 256.175)
-DEFAULT_GRIPPER_MOUNT_OFFSET: Final[Offset] = (82.15, 16, 92.55)
+DEFAULT_GRIPPER_MOUNT_OFFSET: Final[Offset] = (82.15, -16, 92.55)
 DEFAULT_Z_RETRACT_DISTANCE: Final = 2
 
 DEFAULT_MAX_SPEEDS: Final[ByGantryLoad[Dict[OT3AxisKind, float]]] = ByGantryLoad(
@@ -225,6 +225,7 @@
     two_low_throughput={
         OT3AxisKind.X: 1.0,
         OT3AxisKind.Y: 1.0,
+        OT3AxisKind.Z: 1.4,
     },
     gripper={
         OT3AxisKind.Z: 1.4,

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

@@ -9,7 +9,11 @@
 from opentrons.types import Point
 from opentrons.calibration_storage.types import GripperCalibrationOffset
 from opentrons.config import gripper_config
-from opentrons.hardware_control.types import CriticalPoint, GripperJawState
+from opentrons.hardware_control.types import (
+    CriticalPoint,
+    GripperJawState,
+    InvalidMoveError,
+)
 from .instrument_abc import AbstractInstrument
 from opentrons.hardware_control.dev_types import AttachedGripper, GripperDict
 
@@ -37,7 +41,24 @@ def __init__(
         self._config = config
         self._name = self._config.name
         self._model = self._config.model
+        base_offset = Point(*self._config.base_offset_from_mount)
+        self._jaw_center_offset = (
+            Point(*self._config.jaw_center_offset_from_base) + base_offset
+        )
+        #: the distance between the gripper mount and the jaw center at home
+        self._front_calibration_pin_offset = (
+            Point(*self._config.pin_one_offset_from_base) + base_offset
+        )
+        #: the distance between the gripper mount and the front calibration pin
+        #: at home
+        self._back_calibration_pin_offset = (
+            Point(*self._config.pin_two_offset_from_base) + base_offset
+        )
+        #: the distance between the gripper mount and the back calibration pin
+        #: at home
         self._calibration_offset = gripper_cal_offset
+        #: The output value of calibration - the additional vector added into
+        #: the critical point geometry based on gripper mount calibration
         self._gripper_id = gripper_id
         self._state = GripperJawState.UNHOMED
         self._log = mod_log.getChild(self._gripper_id)
@@ -80,12 +101,23 @@ def update_calibration_offset(self, cal_offset: GripperCalibrationOffset) -> Non
 
     def critical_point(self, cp_override: Optional[CriticalPoint] = None) -> Point:
         """
-        The vector from the gripper's origin to its critical point. The
-        critical point for a pipette is the end of the nozzle if no tip is
-        attached, or the end of the tip if a tip is attached.
+        The vector from the gripper mount to the critical point, which is selectable
+        between the center of the gripper engagement volume and the calibration pins.
         """
-        # TODO: add critical point implementation
-        return Point(0, 0, 0)
+        if cp_override == CriticalPoint.GRIPPER_FRONT_CALIBRATION_PIN:
+            return self._front_calibration_pin_offset + Point(
+                *self._calibration_offset.offset
+            )
+        elif cp_override == CriticalPoint.GRIPPER_BACK_CALIBRATION_PIN:
+            return self._back_calibration_pin_offset + Point(
+                *self._calibration_offset.offset
+            )
+        elif cp_override == CriticalPoint.GRIPPER_JAW_CENTER or not cp_override:
+            return self._jaw_center_offset + Point(*self._calibration_offset.offset)
+        else:
+            raise InvalidMoveError(
+                f"Critical point {cp_override.name} is not valid for a gripper"
+            )
 
     def duty_cycle_by_force(self, newton: float) -> float:
         return gripper_config.piecewise_force_conversion(

api/src/opentrons/hardware_control/instruments/gripper_handler.py

@@ -3,16 +3,15 @@
 from opentrons.types import Point
 from opentrons.hardware_control.robot_calibration import load_gripper_calibration_offset
 from opentrons.hardware_control.dev_types import GripperDict
-from opentrons.hardware_control.types import CriticalPoint, GripperJawState
+from opentrons.hardware_control.types import (
+    CriticalPoint,
+    GripperJawState,
+    InvalidMoveError,
+    GripperNotAttachedError,
+)
 from .gripper import Gripper
 
 
-class GripperNotAttachedError(Exception):
-    """An error raised if a gripper is accessed that is not attached"""
-
-    pass
-
-
 class GripError(Exception):
     """An error raised if a gripper action is blocked"""
 
@@ -63,9 +62,10 @@ def gripper(self, gripper: Optional[Gripper] = None) -> None:
 
     def get_critical_point(self, cp_override: Optional[CriticalPoint] = None) -> Point:
         if not self._gripper:
-            return Point(0, 0, 0)
-        else:
-            return self._gripper.critical_point(cp_override)
+            raise GripperNotAttachedError()
+        if cp_override == CriticalPoint.MOUNT:
+            raise InvalidMoveError("The gripper mount may not be moved directly.")
+        return self._gripper.critical_point(cp_override)
 
     def get_gripper_dict(self) -> Optional[GripperDict]:
         if not self._gripper:

api/src/opentrons/hardware_control/instruments/pipette.py

@@ -16,7 +16,12 @@
 from opentrons.config.types import RobotConfig, OT3Config
 from opentrons.drivers.types import MoveSplit
 from .instrument_abc import AbstractInstrument
-from opentrons.hardware_control.types import CriticalPoint, BoardRevision, OT3AxisKind
+from opentrons.hardware_control.types import (
+    CriticalPoint,
+    BoardRevision,
+    OT3AxisKind,
+    InvalidMoveError,
+)
 
 
 if TYPE_CHECKING:
@@ -146,6 +151,15 @@ def critical_point(self, cp_override: Optional[CriticalPoint] = None) -> Point:
         instr = Point(*self._pipette_offset.offset)
         offsets = self.nozzle_offset
 
+        if cp_override in [
+            CriticalPoint.GRIPPER_JAW_CENTER,
+            CriticalPoint.GRIPPER_FRONT_CALIBRATION_PIN,
+            CriticalPoint.GRIPPER_BACK_CALIBRATION_PIN,
+        ]:
+            raise InvalidMoveError(
+                f"Critical point {cp_override.name} is not valid for a pipette"
+            )
+
         if not self.has_tip or cp_override == CriticalPoint.NOZZLE:
             cp_type = CriticalPoint.NOZZLE
             tip_length = 0.0

api/src/opentrons/hardware_control/ot3api.py

@@ -64,6 +64,7 @@
     OT3AxisMap,
     OT3SubSystem,
     GripperJawState,
+    GripperNotAttachedError,
 )
 from . import modules
 from .robot_calibration import (
@@ -76,7 +77,7 @@
 
 from .protocols import HardwareControlAPI
 from .instruments.pipette_handler import OT3PipetteHandler, InstrumentsByMount
-from .instruments.gripper_handler import GripperHandler, GripperNotAttachedError
+from .instruments.gripper_handler import GripperHandler
 from .motion_utilities import (
     target_position_from_absolute,
     target_position_from_relative,

api/src/opentrons/hardware_control/types.py

@@ -387,6 +387,25 @@ class CriticalPoint(enum.Enum):
     Only relevant when a multichannel pipette is present.
     """
 
+    GRIPPER_JAW_CENTER = enum.auto()
+    """
+    The center of the gripper jaw engagement zone, such that if this critical
+    point is moved to the center of a labware the gripper will be ready to
+    grip it.
+    """
+
+    GRIPPER_FRONT_CALIBRATION_PIN = enum.auto()
+    """
+    The center of the bottom face of a calibration pin inserted in the gripper's
+    front calibration pin slot.
+    """
+
+    GRIPPER_BACK_CALIBRATION_PIN = enum.auto()
+    """
+    The center of the bottom face of a calibration pin inserted in the gripper's
+    back calibration pin slot.
+    """
+
 
 class ExecutionState(enum.Enum):
     RUNNING = enum.auto()
@@ -462,3 +481,13 @@ class GripperJawState(enum.Enum):
     @property
     def ready_for_grip(self) -> bool:
         return self in [GripperJawState.HOMED_READY, GripperJawState.HOLDING_OPENED]
+
+
+class InvalidMoveError(ValueError):
+    pass
+
+
+class GripperNotAttachedError(Exception):
+    """An error raised if a gripper is accessed that is not attached"""
+
+    pass

api/tests/opentrons/config/ot3_settings.py

@@ -129,10 +129,7 @@
                 "Z": 0.7,
                 "P": 0.8,
             },
-            "two_low_throughput": {
-                "X": 0.7,
-                "Y": 0.7,
-            },
+            "two_low_throughput": {"X": 0.7, "Y": 0.7, "Z": 0.6},
             "gripper": {
                 "Z": 0.7,
             },
@@ -156,10 +153,7 @@
                 "Z": 0.4,
                 "P": 2.0,
             },
-            "two_low_throughput": {
-                "X": 9,
-                "Y": 0.1,
-            },
+            "two_low_throughput": {"X": 9, "Y": 0.1, "Z": 0.6},
             "gripper": {
                 "Z": 10,
             },

api/tests/opentrons/hardware_control/test_gripper.py

@@ -1,6 +1,10 @@
+from typing import Optional, Callable
+import pytest
+
 from opentrons.types import Point
 from opentrons.hardware_control.robot_calibration import load_gripper_calibration_offset
 from opentrons.hardware_control.instruments import gripper
+from opentrons.hardware_control.types import CriticalPoint
 from opentrons.calibration_storage.delete import clear_gripper_calibration_offsets
 from opentrons.config import gripper_config
 from opentrons_shared_data.gripper.dev_types import GripperModel
@@ -26,10 +30,27 @@ def test_id_get_added_to_dict():
     assert gripr.as_dict()["gripper_id"] == "fakeid123"
 
 
-def test_critical_point():
+@pytest.mark.parametrize(
+    "override,result_accessor",
+    [
+        (None, lambda g: g._jaw_center_offset),
+        (CriticalPoint.GRIPPER_JAW_CENTER, lambda g: g._jaw_center_offset),
+        (
+            CriticalPoint.GRIPPER_FRONT_CALIBRATION_PIN,
+            lambda g: g._front_calibration_pin_offset,
+        ),
+        (
+            CriticalPoint.GRIPPER_BACK_CALIBRATION_PIN,
+            lambda g: g._back_calibration_pin_offset,
+        ),
+    ],
+)
+def test_critical_point(
+    override: Optional[CriticalPoint],
+    result_accessor: Callable[[gripper.Gripper], Point],
+):
     gripr = gripper.Gripper(fake_gripper_conf, FAKE_OFFSET, "fakeid123")
-    # TODO: update test when critical_point() is fully implemented
-    assert gripr.critical_point() == Point(0, 0, 0)
+    assert gripr.critical_point(override) == result_accessor(gripr)
 
 
 def test_load_gripper_cal_offset():

api/tests/opentrons/hardware_control/test_moves.py

@@ -14,9 +14,10 @@
     OutOfBoundsMove,
     MotionChecks,
     MustHomeError,
+    InvalidMoveError,
 )
 from opentrons.hardware_control.robot_calibration import RobotCalibration
-from opentrons.hardware_control.types import OT3Axis, OT3Mount
+from opentrons.hardware_control.types import OT3Axis
 
 
 async def test_controller_must_home(hardware_api):
@@ -59,16 +60,6 @@ async def test_retract(hardware_api):
     }
 
 
-@pytest.fixture
-async def mock_backend_move(ot3_hardware):
-    with mock.patch.object(
-        ot3_hardware.managed_obj._backend,
-        "move",
-        mock.AsyncMock(spec=ot3_hardware.managed_obj._backend.move),
-    ) as mock_move:
-        yield mock_move
-
-
 @pytest.fixture
 def mock_home(ot3_hardware):
     with mock.patch.object(ot3_hardware._backend, "home") as mock_home:
@@ -179,6 +170,29 @@ async def test_mount_offset_applied(hardware_api, is_robot):
     assert hardware_api._current_position == target_position
 
 
+@pytest.mark.parametrize(
+    "critical_point",
+    [
+        CriticalPoint.GRIPPER_JAW_CENTER,
+        CriticalPoint.GRIPPER_FRONT_CALIBRATION_PIN,
+        CriticalPoint.GRIPPER_BACK_CALIBRATION_PIN,
+    ],
+)
+async def test_gripper_critical_points_fail_on_pipettes(
+    hardware_api, is_robot, critical_point
+):
+    await hardware_api.home()
+    hardware_api._backend._attached_instruments = {
+        types.Mount.LEFT: {"model": None, "id": None},
+        types.Mount.RIGHT: {"model": "p10_single_v1", "id": "testyness"},
+    }
+    await hardware_api.cache_instruments()
+    with pytest.raises(InvalidMoveError):
+        await hardware_api.move_to(
+            types.Mount.RIGHT, types.Point(0, 0, 0), critical_point=critical_point
+        )
+
+
 async def test_critical_point_applied(hardware_api, monkeypatch, is_robot):
     await hardware_api.home()
     hardware_api._backend._attached_instruments = {
@@ -516,34 +530,3 @@ async def test_current_position_homing_failures(hardware_api):
         mount=types.Mount.RIGHT,
         fail_on_not_homed=True,
     )
-
-
-async def test_gripper_move_to(ot3_hardware, mock_backend_move):
-    # Moving the gripper should, well, work
-    await ot3_hardware.move_to(OT3Mount.GRIPPER, types.Point(0, 0, 0))
-    origin, moves = mock_backend_move.call_args_list[0][0]
-    # The moves that it emits should move only x, y, and the gripper z
-    assert origin == {
-        OT3Axis.X: 0,
-        OT3Axis.Y: 0,
-        OT3Axis.Z_L: 0,
-        OT3Axis.Z_R: 0,
-        OT3Axis.P_L: 0,
-        OT3Axis.P_R: 0,
-        OT3Axis.Z_G: 0,
-        OT3Axis.G: 0,
-    }
-    for move in moves:
-        assert list(sorted(move.unit_vector.keys(), key=lambda elem: elem.value)) == [
-            OT3Axis.X,
-            OT3Axis.Y,
-            OT3Axis.Z_G,
-        ]
-
-
-async def test_gripper_position(ot3_hardware):
-    await ot3_hardware.home()
-    position = await ot3_hardware.gantry_position(OT3Mount.GRIPPER)
-    assert position == types.Point(*ot3_hardware.config.carriage_offset) + types.Point(
-        *ot3_hardware.config.gripper_mount_offset
-    )