From ec181c6d566d8301cbbf495c2050045cc40a7a55 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Fri, 26 Apr 2024 17:19:35 -0400 Subject: [PATCH 1/3] pass the calibrated jaw max offset value when we reset the gripper --- .../opentrons/hardware_control/instruments/ot3/gripper.py | 6 +++++- .../hardware_control/instruments/ot3/gripper_handler.py | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py index 3f120f972a6..db34501da08 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py @@ -52,6 +52,7 @@ def __init__( config: GripperDefinition, gripper_cal_offset: GripperCalibrationOffset, gripper_id: str, + jaw_max_offset: Optional[float] = None, ) -> None: self._config = config self._model = config.model @@ -83,7 +84,7 @@ def __init__( self._log.info( f"loaded: {self._model}, gripper offset: {self._calibration_offset}" ) - self._jaw_max_offset: Optional[float] = None + self._jaw_max_offset = jaw_max_offset @property def grip_force_profile(self) -> GripForceProfile: @@ -330,6 +331,9 @@ def _reload_gripper( new_config, cal_offset, attached_instr._gripper_id, + attached_instr._jaw_max_offset + if attached_instr.has_jaw_width_calibration + else None, ), False, ) diff --git a/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py b/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py index cf2ba55e23d..ad0c442e44c 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py @@ -51,6 +51,9 @@ def reset_gripper(self) -> None: og_gripper.config, load_gripper_calibration_offset(og_gripper.gripper_id), og_gripper.gripper_id, + og_gripper._jaw_max_offset + if og_gripper.has_jaw_width_calibration + else None, ) self._gripper = new_gripper From b3bdfbd429e829f45666b0da33532030f72f7066 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:13:06 -0400 Subject: [PATCH 2/3] add tests --- .../hardware_control/instruments/ot3/gripper.py | 4 +--- .../instruments/ot3/gripper_handler.py | 4 +--- .../opentrons/hardware_control/test_ot3_api.py | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py index db34501da08..4ea3651a63a 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py @@ -331,9 +331,7 @@ def _reload_gripper( new_config, cal_offset, attached_instr._gripper_id, - attached_instr._jaw_max_offset - if attached_instr.has_jaw_width_calibration - else None, + attached_instr._jaw_max_offset, ), False, ) diff --git a/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py b/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py index ad0c442e44c..e327306c19f 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/gripper_handler.py @@ -51,9 +51,7 @@ def reset_gripper(self) -> None: og_gripper.config, load_gripper_calibration_offset(og_gripper.gripper_id), og_gripper.gripper_id, - og_gripper._jaw_max_offset - if og_gripper.has_jaw_width_calibration - else None, + og_gripper._jaw_max_offset, ) self._gripper = new_gripper diff --git a/api/tests/opentrons/hardware_control/test_ot3_api.py b/api/tests/opentrons/hardware_control/test_ot3_api.py index 7ab0a2f1c00..500327a99c5 100644 --- a/api/tests/opentrons/hardware_control/test_ot3_api.py +++ b/api/tests/opentrons/hardware_control/test_ot3_api.py @@ -1957,3 +1957,18 @@ async def test_stop_only_home_necessary_axes( await ot3_hardware.stop(home_after=True) if jaw_state == GripperJawState.GRIPPING: mock_home.assert_called_once_with(skip=[Axis.G]) + + +async def test_gripper_max_offset_persists_after_reset( + ot3_hardware: ThreadManager[OT3API], +) -> None: + gripper_config = gc.load(GripperModel.v1) + instr_data = AttachedGripper(config=gripper_config, id="test") + await ot3_hardware.cache_gripper(instr_data) + ot3_hardware._gripper_handler.get_gripper()._jaw_max_offset = 10 + + await ot3_hardware.reset() + reset_gripper = ot3_hardware._gripper_handler.get_gripper() + assert reset_gripper._config == gripper_config + assert reset_gripper._gripper_id == "test" + assert reset_gripper._jaw_max_offset == 10 From bf806cc6af4e259014fbdfc2158cf9a68ee2e4a0 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:32:37 -0400 Subject: [PATCH 3/3] reload config should force us to recalibrate jaw --- .../instruments/ot3/gripper.py | 5 ++-- .../hardware_control/test_gripper.py | 28 +++++++++++++++++++ .../hardware_control/test_ot3_api.py | 15 ---------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py index 4ea3651a63a..ba49ea7d5e7 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py @@ -33,7 +33,7 @@ Geometry, ) -RECONFIG_KEYS = {"quirks"} +RECONFIG_KEYS = {"quirks", "grip_force_profile"} MAX_ACCEPTABLE_JAW_DISPLACEMENT: Final = 20 @@ -326,12 +326,13 @@ def _reload_gripper( changed.add(k) if changed.intersection(RECONFIG_KEYS): # Something has changed that requires reconfig + # we shoud recalibrate the jaw as well return ( Gripper( new_config, cal_offset, attached_instr._gripper_id, - attached_instr._jaw_max_offset, + None, ), False, ) diff --git a/api/tests/opentrons/hardware_control/test_gripper.py b/api/tests/opentrons/hardware_control/test_gripper.py index 0d01b225752..6066b8a74a1 100644 --- a/api/tests/opentrons/hardware_control/test_gripper.py +++ b/api/tests/opentrons/hardware_control/test_gripper.py @@ -74,6 +74,7 @@ def test_reload_instrument_cal_ot3(fake_offset: "GripperCalibrationOffset") -> N fake_gripper_conf, fake_offset, "fakeid123", + jaw_max_offset=15, ) # if only calibration is changed new_cal = instrument_calibration.GripperCalibrationOffset( @@ -87,10 +88,37 @@ def test_reload_instrument_cal_ot3(fake_offset: "GripperCalibrationOffset") -> N # it's the same gripper assert new_gripper == old_gripper + # jaw offset should persists as well + assert new_gripper._jaw_max_offset == old_gripper._jaw_max_offset # we said upstream could skip assert skip +@pytest.mark.ot3_only +def test_reload_instrument_cal_ot3_conf_changed( + fake_offset: "GripperCalibrationOffset", +) -> None: + old_gripper = gripper.Gripper( + fake_gripper_conf, + fake_offset, + "fakeid123", + jaw_max_offset=15, + ) + new_conf = fake_gripper_conf.copy( + update={"grip_force_profile": {"default_grip_force": 1}} + ) + assert new_conf != old_gripper.config + + new_gripper, skip = gripper._reload_gripper(new_conf, old_gripper, fake_offset) + + # it's not the same gripper + assert new_gripper != old_gripper + # do not pass in the old jaw max offse + assert not new_gripper._jaw_max_offset + # we said upstream could skip + assert not skip + + @pytest.mark.ot3_only def test_jaw_calibration_error_checking() -> None: subject = gripper.Gripper(fake_gripper_conf, fake_offset, "fakeid123") diff --git a/api/tests/opentrons/hardware_control/test_ot3_api.py b/api/tests/opentrons/hardware_control/test_ot3_api.py index 500327a99c5..7ab0a2f1c00 100644 --- a/api/tests/opentrons/hardware_control/test_ot3_api.py +++ b/api/tests/opentrons/hardware_control/test_ot3_api.py @@ -1957,18 +1957,3 @@ async def test_stop_only_home_necessary_axes( await ot3_hardware.stop(home_after=True) if jaw_state == GripperJawState.GRIPPING: mock_home.assert_called_once_with(skip=[Axis.G]) - - -async def test_gripper_max_offset_persists_after_reset( - ot3_hardware: ThreadManager[OT3API], -) -> None: - gripper_config = gc.load(GripperModel.v1) - instr_data = AttachedGripper(config=gripper_config, id="test") - await ot3_hardware.cache_gripper(instr_data) - ot3_hardware._gripper_handler.get_gripper()._jaw_max_offset = 10 - - await ot3_hardware.reset() - reset_gripper = ot3_hardware._gripper_handler.get_gripper() - assert reset_gripper._config == gripper_config - assert reset_gripper._gripper_id == "test" - assert reset_gripper._jaw_max_offset == 10