fix(app): Update gripper homing behavior during gripper Error Recovery (

#16691) Closes RQA-3489 and RQA-3487 Although Opentrons/ot3-firmware#813 fixes the current behavior and unblocks gripper recovery, updating the position estimators isn't sufficiently accurate for post-recovery, protocol run commands. Instead, we should home everything minus the pipette plungers. There is some minor copy update on one view only per discussion w/ design.
Opentrons · Nov 5, 2024 · 3cf6f34 · 3cf6f34
1 parent c5b323d
commit 3cf6f34
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 50 deletions.
diff --git a/app/src/assets/localization/en/error_recovery.json b/app/src/assets/localization/en/error_recovery.json
@@ -19,7 +19,7 @@
   "continue_run_now": "Continue run now",
   "continue_to_drop_tip": "Continue to drop tip",
   "do_you_need_to_blowout": "First, do you need to blow out aspirated liquid?",
-  "door_open_gripper_home": "The robot door must be closed for the gripper to home its Z-axis before you can continue manually moving labware.",
+  "door_open_robot_home": "The robot needs to safely move to its home location before you manually move the labware.",
   "ensure_lw_is_accurately_placed": "Ensure labware is accurately placed in the slot to prevent further errors.",
   "error": "Error",
   "error_details": "Error details",

diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useHomeGripper.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useHomeGripper.test.ts
@@ -6,9 +6,7 @@ import { RECOVERY_MAP } from '/app/organisms/ErrorRecoveryFlows/constants'
 
 describe('useHomeGripper', () => {
   const mockRecoveryCommands = {
-    updatePositionEstimatorsAndHomeGripper: vi
-      .fn()
-      .mockResolvedValue(undefined),
+    homeExceptPlungers: vi.fn().mockResolvedValue(undefined),
   }
 
   const mockRouteUpdateActions = {
@@ -45,9 +43,7 @@ describe('useHomeGripper', () => {
     expect(mockRouteUpdateActions.handleMotionRouting).toHaveBeenCalledWith(
       true
     )
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).toHaveBeenCalled()
+    expect(mockRecoveryCommands.homeExceptPlungers).toHaveBeenCalled()
     expect(mockRouteUpdateActions.handleMotionRouting).toHaveBeenCalledWith(
       false
     )
@@ -64,9 +60,7 @@ describe('useHomeGripper', () => {
     })
 
     expect(mockRouteUpdateActions.goBackPrevStep).toHaveBeenCalled()
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).not.toHaveBeenCalled()
+    expect(mockRecoveryCommands.homeExceptPlungers).not.toHaveBeenCalled()
   })
 
   it('should not home again if already homed once', async () => {
@@ -83,15 +77,11 @@ describe('useHomeGripper', () => {
       await new Promise(resolve => setTimeout(resolve, 0))
     })
 
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).toHaveBeenCalledTimes(1)
+    expect(mockRecoveryCommands.homeExceptPlungers).toHaveBeenCalledTimes(1)
 
     rerender()
 
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).toHaveBeenCalledTimes(1)
+    expect(mockRecoveryCommands.homeExceptPlungers).toHaveBeenCalledTimes(1)
   })
 
   it('should only reset hasHomedOnce when step changes to non-manual gripper step', async () => {
@@ -113,28 +103,22 @@ describe('useHomeGripper', () => {
       await new Promise(resolve => setTimeout(resolve, 0))
     })
 
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).toHaveBeenCalledTimes(1)
+    expect(mockRecoveryCommands.homeExceptPlungers).toHaveBeenCalledTimes(1)
 
     rerender({ recoveryMap: { step: 'SOME_OTHER_STEP' } as any })
 
     await act(async () => {
       await new Promise(resolve => setTimeout(resolve, 0))
     })
 
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).toHaveBeenCalledTimes(1)
+    expect(mockRecoveryCommands.homeExceptPlungers).toHaveBeenCalledTimes(1)
 
     rerender({ recoveryMap: mockRecoveryMap })
 
     await act(async () => {
       await new Promise(resolve => setTimeout(resolve, 0))
     })
 
-    expect(
-      mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
-    ).toHaveBeenCalledTimes(2)
+    expect(mockRecoveryCommands.homeExceptPlungers).toHaveBeenCalledTimes(1)
   })
 })
diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts
@@ -16,8 +16,7 @@ import {
   buildPickUpTips,
   buildIgnorePolicyRules,
   isAssumeFalsePositiveResumeKind,
-  UPDATE_ESTIMATORS_EXCEPT_PLUNGERS,
-  HOME_GRIPPER_Z,
+  HOME_EXCEPT_PLUNGERS,
 } from '../useRecoveryCommands'
 import { RECOVERY_MAP, ERROR_KINDS } from '../../constants'
 import { getErrorKind } from '/app/organisms/ErrorRecoveryFlows/utils'
@@ -290,11 +289,11 @@ describe('useRecoveryCommands', () => {
     const { result } = renderHook(() => useRecoveryCommands(props))
 
     await act(async () => {
-      await result.current.updatePositionEstimatorsAndHomeGripper()
+      await result.current.homeExceptPlungers()
     })
 
     expect(mockChainRunCommands).toHaveBeenCalledWith(
-      [UPDATE_ESTIMATORS_EXCEPT_PLUNGERS, HOME_GRIPPER_Z],
+      [HOME_EXCEPT_PLUNGERS],
       false
     )
   })

diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useHomeGripper.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useHomeGripper.ts
@@ -20,25 +20,21 @@ export function useHomeGripper({
 
   useLayoutEffect(() => {
     const { handleMotionRouting, goBackPrevStep } = routeUpdateActions
-    const { updatePositionEstimatorsAndHomeGripper } = recoveryCommands
+    const { homeExceptPlungers } = recoveryCommands
 
     if (!hasHomedOnce) {
       if (isManualGripperStep) {
         if (isDoorOpen) {
           void goBackPrevStep()
         } else {
           void handleMotionRouting(true)
-            .then(() => updatePositionEstimatorsAndHomeGripper())
-            .finally(() => {
-              handleMotionRouting(false)
+            .then(() => homeExceptPlungers())
+            .then(() => handleMotionRouting(false))
+            .then(() => {
               setHasHomedOnce(true)
             })
         }
       }
-    } else {
-      if (!isManualGripperStep && hasHomedOnce) {
-        setHasHomedOnce(false)
-      }
     }
   }, [step, hasHomedOnce, isDoorOpen, isManualGripperStep])
 }
diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts
@@ -69,7 +69,7 @@ export interface UseRecoveryCommandsResult {
   /* A non-terminal recovery command */
   releaseGripperJaws: () => Promise<CommandData[]>
   /* A non-terminal recovery command */
-  updatePositionEstimatorsAndHomeGripper: () => Promise<CommandData[]>
+  homeExceptPlungers: () => Promise<CommandData[]>
   /* A non-terminal recovery command */
   moveLabwareWithoutPause: () => Promise<CommandData[]>
 }
@@ -294,13 +294,8 @@ export function useRecoveryCommands({
     return chainRunRecoveryCommands([RELEASE_GRIPPER_JAW])
   }, [chainRunRecoveryCommands])
 
-  const updatePositionEstimatorsAndHomeGripper = useCallback((): Promise<
-    CommandData[]
-  > => {
-    return chainRunRecoveryCommands([
-      UPDATE_ESTIMATORS_EXCEPT_PLUNGERS,
-      HOME_GRIPPER_Z,
-    ])
+  const homeExceptPlungers = useCallback((): Promise<CommandData[]> => {
+    return chainRunRecoveryCommands([HOME_EXCEPT_PLUNGERS])
   }, [chainRunRecoveryCommands])
 
   const moveLabwareWithoutPause = useCallback((): Promise<CommandData[]> => {
@@ -321,7 +316,7 @@ export function useRecoveryCommands({
     homePipetteZAxes,
     pickUpTips,
     releaseGripperJaws,
-    updatePositionEstimatorsAndHomeGripper,
+    homeExceptPlungers,
     moveLabwareWithoutPause,
     skipFailedCommand,
     ignoreErrorKindThisRun,
@@ -360,9 +355,11 @@ export const UPDATE_ESTIMATORS_EXCEPT_PLUNGERS: CreateCommand = {
   params: { axes: ['x', 'y', 'extensionZ'] },
 }
 
-export const HOME_GRIPPER_Z: CreateCommand = {
+export const HOME_EXCEPT_PLUNGERS: CreateCommand = {
   commandType: 'home',
-  params: { axes: ['extensionZ'] },
+  params: {
+    axes: ['extensionJaw', 'extensionZ', 'leftZ', 'rightZ', 'x', 'y'],
+  },
 }
 
 const buildMoveLabwareWithoutPause = (

diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryDoorOpenSpecial.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryDoorOpenSpecial.tsx
@@ -46,7 +46,7 @@ export function RecoveryDoorOpenSpecial({
     switch (selectedRecoveryOption) {
       case RECOVERY_MAP.MANUAL_REPLACE_AND_RETRY.ROUTE:
       case RECOVERY_MAP.MANUAL_MOVE_AND_SKIP.ROUTE:
-        return t('door_open_gripper_home')
+        return t('door_open_robot_home')
       default: {
         console.error(
           `Unhandled special-cased door open subtext on route ${selectedRecoveryOption}.`

diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/RecoveryDoorOpenSpecial.test.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/RecoveryDoorOpenSpecial.test.tsx
@@ -66,7 +66,7 @@ describe('RecoveryDoorOpenSpecial', () => {
     render(props)
     screen.getByText('Close the robot door')
     screen.getByText(
-      'The robot door must be closed for the gripper to home its Z-axis before you can continue manually moving labware.'
+      'The robot needs to safely move to its home location before you manually move the labware.'
     )
   })