diff --git a/api/src/opentrons/config/defaults_ot3.py b/api/src/opentrons/config/defaults_ot3.py index 68866c12e70..7b267850e6e 100644 --- a/api/src/opentrons/config/defaults_ot3.py +++ b/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, diff --git a/api/src/opentrons/hardware_control/instruments/gripper.py b/api/src/opentrons/hardware_control/instruments/gripper.py index 19bf6f25d75..d09c8dc3be8 100644 --- a/api/src/opentrons/hardware_control/instruments/gripper.py +++ b/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( diff --git a/api/src/opentrons/hardware_control/instruments/gripper_handler.py b/api/src/opentrons/hardware_control/instruments/gripper_handler.py index b6724d24445..f33f48a4a9b 100644 --- a/api/src/opentrons/hardware_control/instruments/gripper_handler.py +++ b/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: diff --git a/api/src/opentrons/hardware_control/instruments/pipette.py b/api/src/opentrons/hardware_control/instruments/pipette.py index 3ee84ab538f..0052771193e 100644 --- a/api/src/opentrons/hardware_control/instruments/pipette.py +++ b/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 diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index a8cd163e7fe..ef5f3d42b88 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/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, diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index f300e3778ff..c07809c2ff9 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/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 diff --git a/api/tests/opentrons/config/ot3_settings.py b/api/tests/opentrons/config/ot3_settings.py index 657c25ade5f..990a0f12ddc 100644 --- a/api/tests/opentrons/config/ot3_settings.py +++ b/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, }, diff --git a/api/tests/opentrons/hardware_control/test_gripper.py b/api/tests/opentrons/hardware_control/test_gripper.py index 1d41aeab9d4..2fe06a663c5 100644 --- a/api/tests/opentrons/hardware_control/test_gripper.py +++ b/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(): diff --git a/api/tests/opentrons/hardware_control/test_moves.py b/api/tests/opentrons/hardware_control/test_moves.py index 698bc3fa2da..8ba2c130b6c 100644 --- a/api/tests/opentrons/hardware_control/test_moves.py +++ b/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 - ) diff --git a/api/tests/opentrons/hardware_control/test_ot3_api.py b/api/tests/opentrons/hardware_control/test_ot3_api.py index ff4f972711b..91dfd7e126e 100644 --- a/api/tests/opentrons/hardware_control/test_ot3_api.py +++ b/api/tests/opentrons/hardware_control/test_ot3_api.py @@ -11,10 +11,15 @@ AttachedGripper, ) from opentrons.hardware_control.instruments.gripper_handler import ( - GripperNotAttachedError, GripError, ) -from opentrons.hardware_control.types import OT3Mount, OT3Axis +from opentrons.hardware_control.types import ( + OT3Mount, + OT3Axis, + GripperNotAttachedError, + InvalidMoveError, + CriticalPoint, +) from opentrons.hardware_control.ot3api import OT3API from opentrons.hardware_control import ThreadManager from opentrons.hardware_control.backends.ot3utils import axis_to_node @@ -86,6 +91,16 @@ def mock_ungrip(ot3_hardware: ThreadManager[OT3API]) -> Iterator[AsyncMock]: yield mock_move +@pytest.fixture +async def mock_backend_move(ot3_hardware: ThreadManager[OT3API]) -> Iterator[AsyncMock]: + with patch.object( + ot3_hardware.managed_obj._backend, + "move", + AsyncMock(spec=ot3_hardware.managed_obj._backend.move), + ) as mock_move: + yield mock_move + + @pytest.mark.parametrize( "attached,load", ( @@ -381,3 +396,89 @@ async def test_gripper_action( await ot3_hardware.ungrip() mock_ungrip.assert_called_once() + + +async def test_gripper_move_fails_with_no_gripper( + ot3_hardware: ThreadManager[OT3API], +) -> None: + assert not ot3_hardware._gripper_handler.gripper + with pytest.raises(GripperNotAttachedError): + await ot3_hardware.move_to(OT3Mount.GRIPPER, Point(0, 0, 0)) + + +async def test_gripper_mount_not_movable( + ot3_hardware: ThreadManager[OT3API], +) -> None: + gripper_config = gc.load(GripperModel.V1, "g12345") + instr_data = AttachedGripper(config=gripper_config, id="g12345") + await ot3_hardware.cache_gripper(instr_data) + assert ot3_hardware._gripper_handler.gripper + with pytest.raises(InvalidMoveError): + await ot3_hardware.move_to( + OT3Mount.GRIPPER, Point(0, 0, 0), critical_point=CriticalPoint.MOUNT + ) + + +@pytest.mark.parametrize( + "critical_point", + [ + CriticalPoint.NOZZLE, + CriticalPoint.TIP, + CriticalPoint.XY_CENTER, + CriticalPoint.FRONT_NOZZLE, + ], +) +async def test_gripper_fails_for_pipette_cps( + ot3_hardware: ThreadManager[OT3API], critical_point: CriticalPoint +) -> None: + gripper_config = gc.load(GripperModel.V1, "g12345") + instr_data = AttachedGripper(config=gripper_config, id="g12345") + await ot3_hardware.cache_gripper(instr_data) + assert ot3_hardware._gripper_handler.gripper + with pytest.raises(InvalidMoveError): + await ot3_hardware.move_to( + OT3Mount.GRIPPER, Point(0, 0, 0), critical_point=critical_point + ) + + +async def test_gripper_position(ot3_hardware: ThreadManager[OT3API]): + gripper_config = gc.load(GripperModel.V1, "g12345") + instr_data = AttachedGripper(config=gripper_config, id="g12345") + await ot3_hardware.cache_gripper(instr_data) + await ot3_hardware.home() + position = await ot3_hardware.gantry_position(OT3Mount.GRIPPER) + assert ( + position + == Point(*ot3_hardware.config.carriage_offset) + + Point(*ot3_hardware.config.gripper_mount_offset) + + ot3_hardware._gripper_handler.gripper._jaw_center_offset + ) + + +async def test_gripper_move_to( + ot3_hardware: ThreadManager[OT3API], mock_backend_move: AsyncMock +): + # Moving the gripper should, well, work + gripper_config = gc.load(GripperModel.V1, "g12345") + instr_data = AttachedGripper(config=gripper_config, id="g12345") + await ot3_hardware.cache_gripper(instr_data) + + await ot3_hardware.move_to(OT3Mount.GRIPPER, 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, + ] diff --git a/shared-data/gripper/definitions/1/gripperV1.json b/shared-data/gripper/definitions/1/gripperV1.json index 838cd58c014..7880fb51eb9 100644 --- a/shared-data/gripper/definitions/1/gripperV1.json +++ b/shared-data/gripper/definitions/1/gripperV1.json @@ -39,23 +39,23 @@ ], "baseOffsetFromMount": { "x": 6.775, - "y": 87.325, - "z": 32.05 + "y": -87.325, + "z": -32.05 }, "jawCenterOffsetFromBase": { - "x": 8.5, + "x": -8.5, "y": 2.5, - "z": 86 + "z": -86 }, "pinOneOffsetFromBase": { "x": 23, - "y": 73.37920159, - "z": 95 + "y": -73.37920159, + "z": -95 }, "pinTwoOffsetFromBase": { "x": 23, "y": 78.37920159, - "z": 95 + "z": -95 }, "quirks": [] } diff --git a/shared-data/python/Makefile b/shared-data/python/Makefile index fd49c55f858..734564400c6 100644 --- a/shared-data/python/Makefile +++ b/shared-data/python/Makefile @@ -86,7 +86,7 @@ $(wheel_file): setup.py $(py_sources) $(json_sources) $(SHX) ls $(BUILD_DIR) -$(sdist_file): setup.py $(py_sources) +$(sdist_file): setup.py $(py_sources) $(json_sources) $(SHX) mkdir -p build $(python) setup.py sdist $(SHX) ls $(BUILD_DIR)