Opentrons · sfoster1 · Jul 8, 2022 · Jul 1, 2022 · Jul 1, 2022 · Jul 1, 2022
@@ -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,

@@ -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(

@@ -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:

@@ -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

@@ -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,

@@ -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
@@ -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,
             },

@@ -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():

@@ -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
-    )