Skip to content

Commit

Permalink
fix(api): home all gripper axis when a stall is detected (#16579)
Browse files Browse the repository at this point in the history
  • Loading branch information
TamarZanzouri authored Oct 24, 2024
1 parent 1734ff5 commit 565865d
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 30 deletions.
4 changes: 2 additions & 2 deletions app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
import { RecoveryInProgress } from './RecoveryInProgress'
import { getErrorKind } from './utils'
import { RECOVERY_MAP } from './constants'
import { useHomeGripperZAxis } from './hooks'
import { useHomeGripper } from './hooks'

import type { LabwareDefinition2, RobotType } from '@opentrons/shared-data'
import type { RecoveryRoute, RouteStep, RecoveryContentProps } from './types'
Expand Down Expand Up @@ -90,7 +90,7 @@ export function ErrorRecoveryWizard(
routeUpdateActions,
})

useHomeGripperZAxis(props)
useHomeGripper(props)

return <ErrorRecoveryComponent errorKind={errorKind} {...props} />
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { renderHook, act } from '@testing-library/react'
import { describe, it, expect, vi, beforeEach } from 'vitest'

import { useHomeGripperZAxis } from '../useHomeGripperZAxis'
import { useHomeGripper } from '../useHomeGripper'
import { RECOVERY_MAP } from '/app/organisms/ErrorRecoveryFlows/constants'

describe('useHomeGripperZAxis', () => {
describe('useHomeGripper', () => {
const mockRecoveryCommands = {
homeGripperZAxis: vi.fn().mockResolvedValue(undefined),
updatePositionEstimatorsAndHomeGripper: vi
.fn()
.mockResolvedValue(undefined),
}

const mockRouteUpdateActions = {
Expand All @@ -28,7 +30,7 @@ describe('useHomeGripperZAxis', () => {

it('should home gripper Z axis when in manual gripper step and door is closed', async () => {
renderHook(() => {
useHomeGripperZAxis({
useHomeGripper({
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
recoveryMap: mockRecoveryMap,
Expand All @@ -43,15 +45,17 @@ describe('useHomeGripperZAxis', () => {
expect(mockRouteUpdateActions.handleMotionRouting).toHaveBeenCalledWith(
true
)
expect(mockRecoveryCommands.homeGripperZAxis).toHaveBeenCalled()
expect(
mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
).toHaveBeenCalled()
expect(mockRouteUpdateActions.handleMotionRouting).toHaveBeenCalledWith(
false
)
})

it('should go back to previous step when door is open', () => {
renderHook(() => {
useHomeGripperZAxis({
useHomeGripper({
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
recoveryMap: mockRecoveryMap,
Expand All @@ -60,12 +64,14 @@ describe('useHomeGripperZAxis', () => {
})

expect(mockRouteUpdateActions.goBackPrevStep).toHaveBeenCalled()
expect(mockRecoveryCommands.homeGripperZAxis).not.toHaveBeenCalled()
expect(
mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
).not.toHaveBeenCalled()
})

it('should not home again if already homed once', async () => {
const { rerender } = renderHook(() => {
useHomeGripperZAxis({
useHomeGripper({
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
recoveryMap: mockRecoveryMap,
Expand All @@ -77,17 +83,21 @@ describe('useHomeGripperZAxis', () => {
await new Promise(resolve => setTimeout(resolve, 0))
})

expect(mockRecoveryCommands.homeGripperZAxis).toHaveBeenCalledTimes(1)
expect(
mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
).toHaveBeenCalledTimes(1)

rerender()

expect(mockRecoveryCommands.homeGripperZAxis).toHaveBeenCalledTimes(1)
expect(
mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
).toHaveBeenCalledTimes(1)
})

it('should reset hasHomedOnce when step changes to non-manual gripper step and back', async () => {
const { rerender } = renderHook(
({ recoveryMap }) => {
useHomeGripperZAxis({
useHomeGripper({
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
recoveryMap,
Expand All @@ -103,7 +113,9 @@ describe('useHomeGripperZAxis', () => {
await new Promise(resolve => setTimeout(resolve, 0))
})

expect(mockRecoveryCommands.homeGripperZAxis).toHaveBeenCalledTimes(1)
expect(
mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
).toHaveBeenCalledTimes(1)

rerender({ recoveryMap: { step: 'SOME_OTHER_STEP' } as any })

Expand All @@ -117,6 +129,8 @@ describe('useHomeGripperZAxis', () => {
await new Promise(resolve => setTimeout(resolve, 0))
})

expect(mockRecoveryCommands.homeGripperZAxis).toHaveBeenCalledTimes(2)
expect(
mockRecoveryCommands.updatePositionEstimatorsAndHomeGripper
).toHaveBeenCalledTimes(2)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
RELEASE_GRIPPER_JAW,
buildPickUpTips,
buildIgnorePolicyRules,
HOME_GRIPPER_Z_AXIS,
UPDATE_ESTIMATORS_EXCEPT_PLUNGERS,
HOME_GRIPPER_Z,
} from '../useRecoveryCommands'
import { RECOVERY_MAP } from '../../constants'

Expand Down Expand Up @@ -264,15 +265,15 @@ describe('useRecoveryCommands', () => {
)
})

it('should call homeGripperZAxis and resolve the promise', async () => {
it('should call useUpdatePositionEstimators and resolve the promise', async () => {
const { result } = renderHook(() => useRecoveryCommands(props))

await act(async () => {
await result.current.homeGripperZAxis()
await result.current.updatePositionEstimatorsAndHomeGripper()
})

expect(mockChainRunCommands).toHaveBeenCalledWith(
[HOME_GRIPPER_Z_AXIS],
[UPDATE_ESTIMATORS_EXCEPT_PLUNGERS, HOME_GRIPPER_Z],
false
)
})
Expand Down
2 changes: 1 addition & 1 deletion app/src/organisms/ErrorRecoveryFlows/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export { useRouteUpdateActions } from './useRouteUpdateActions'
export { useERUtils } from './useERUtils'
export { useRecoveryTakeover } from './useRecoveryTakeover'
export { useRetainedFailedCommandBySource } from './useRetainedFailedCommandBySource'
export { useHomeGripperZAxis } from './useHomeGripperZAxis'
export { useHomeGripper } from './useHomeGripper'

export type { ERUtilsProps } from './useERUtils'
export type { UseRouteUpdateActionsResult } from './useRouteUpdateActions'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { RECOVERY_MAP } from '/app/organisms/ErrorRecoveryFlows/constants'

import type { ErrorRecoveryWizardProps } from '/app/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard'

// Home the gripper z-axis implicitly. Because the z-home is not tied to a CTA, it must be handled here.
export function useHomeGripperZAxis({
// Home the gripper implicitly. Because the home is not tied to a CTA, it must be handled here.
export function useHomeGripper({
recoveryCommands,
routeUpdateActions,
recoveryMap,
Expand All @@ -20,15 +20,15 @@ export function useHomeGripperZAxis({

useLayoutEffect(() => {
const { handleMotionRouting, goBackPrevStep } = routeUpdateActions
const { homeGripperZAxis } = recoveryCommands
const { updatePositionEstimatorsAndHomeGripper } = recoveryCommands

if (!hasHomedOnce) {
if (isManualGripperStep) {
if (isDoorOpen) {
void goBackPrevStep()
} else {
void handleMotionRouting(true)
.then(() => homeGripperZAxis())
.then(() => updatePositionEstimatorsAndHomeGripper())
.then(() => {
setHasHomedOnce(true)
})
Expand Down
22 changes: 16 additions & 6 deletions app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export interface UseRecoveryCommandsResult {
/* A non-terminal recovery command */
releaseGripperJaws: () => Promise<CommandData[]>
/* A non-terminal recovery command */
homeGripperZAxis: () => Promise<CommandData[]>
updatePositionEstimatorsAndHomeGripper: () => Promise<CommandData[]>
/* A non-terminal recovery command */
moveLabwareWithoutPause: () => Promise<CommandData[]>
}
Expand Down Expand Up @@ -269,8 +269,13 @@ export function useRecoveryCommands({
return chainRunRecoveryCommands([RELEASE_GRIPPER_JAW])
}, [chainRunRecoveryCommands])

const homeGripperZAxis = useCallback((): Promise<CommandData[]> => {
return chainRunRecoveryCommands([HOME_GRIPPER_Z_AXIS])
const updatePositionEstimatorsAndHomeGripper = useCallback((): Promise<
CommandData[]
> => {
return chainRunRecoveryCommands([
UPDATE_ESTIMATORS_EXCEPT_PLUNGERS,
HOME_GRIPPER_Z,
])
}, [chainRunRecoveryCommands])

const moveLabwareWithoutPause = useCallback((): Promise<CommandData[]> => {
Expand All @@ -291,7 +296,7 @@ export function useRecoveryCommands({
homePipetteZAxes,
pickUpTips,
releaseGripperJaws,
homeGripperZAxis,
updatePositionEstimatorsAndHomeGripper,
moveLabwareWithoutPause,
skipFailedCommand,
ignoreErrorKindThisRun,
Expand All @@ -310,10 +315,15 @@ export const RELEASE_GRIPPER_JAW: CreateCommand = {
intent: 'fixit',
}

export const HOME_GRIPPER_Z_AXIS: CreateCommand = {
// in case the gripper does not know the position after a stall/collision we must update the position.
export const UPDATE_ESTIMATORS_EXCEPT_PLUNGERS: CreateCommand = {
commandType: 'unsafe/updatePositionEstimators',
params: { axes: ['x', 'y', 'extensionZ'] },
}

export const HOME_GRIPPER_Z: CreateCommand = {
commandType: 'home',
params: { axes: ['extensionZ'] },
intent: 'fixit',
}

const buildMoveLabwareWithoutPause = (
Expand Down

0 comments on commit 565865d

Please sign in to comment.