From f0d880a79e19da8f90cc73b9f64d5e3d99f4b2a8 Mon Sep 17 00:00:00 2001
From: Alise Au <20424172+ahiuchingau@users.noreply.github.com>
Date: Mon, 29 Apr 2024 17:15:58 -0400
Subject: [PATCH] fix(api): pass the calibrated jaw max offset value when we
 resetting instrument (#15032)

<!--
Thanks for taking the time to open a pull request! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview
We really only need to calibrate the gripper's max jaw offset once every
time we attach a new gripper. This means that when we call
`ot3api.reset()`, which is pretty often because it's called by
`ot3api.stop()`, we should just reuse the calibrated value so we don't
see the jaw opening and closing all the time.
---
 .../instruments/ot3/gripper.py                |  7 +++--
 .../instruments/ot3/gripper_handler.py        |  1 +
 .../hardware_control/test_gripper.py          | 28 +++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/api/src/opentrons/hardware_control/instruments/ot3/gripper.py b/api/src/opentrons/hardware_control/instruments/ot3/gripper.py
index 3f120f972a6..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
 
@@ -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:
@@ -325,11 +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,
+                    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..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,6 +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,
         )
         self._gripper = new_gripper
 
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")