Skip to content

Commit

Permalink
refactor(app): Refactor effect-driven commands in Error Recovery (#16948
Browse files Browse the repository at this point in the history
)

Closes RQA-3664
  • Loading branch information
mjhuff authored Nov 22, 2024
1 parent 6297c7b commit 15ef0ee
Show file tree
Hide file tree
Showing 11 changed files with 293 additions and 469 deletions.
49 changes: 7 additions & 42 deletions app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect, useLayoutEffect } from 'react'
import { useState, useEffect } from 'react'
import { useTranslation } from 'react-i18next'
import { css } from 'styled-components'

Expand Down Expand Up @@ -29,7 +29,6 @@ import {
import { RecoveryInProgress } from './RecoveryInProgress'
import { getErrorKind } from './utils'
import { RECOVERY_MAP } from './constants'
import { useHomeGripper } from './hooks'

import type { LabwareDefinition2, RobotType } from '@opentrons/shared-data'
import type { RecoveryRoute, RouteStep, RecoveryContentProps } from './types'
Expand Down Expand Up @@ -76,23 +75,12 @@ export type ErrorRecoveryWizardProps = ErrorRecoveryFlowsProps &
export function ErrorRecoveryWizard(
props: ErrorRecoveryWizardProps
): JSX.Element {
const {
hasLaunchedRecovery,
failedCommand,
recoveryCommands,
routeUpdateActions,
} = props
const errorKind = getErrorKind(failedCommand)

useInitialPipetteHome({
hasLaunchedRecovery,
recoveryCommands,
routeUpdateActions,
})

useHomeGripper(props)

return <ErrorRecoveryComponent errorKind={errorKind} {...props} />
return (
<ErrorRecoveryComponent
errorKind={getErrorKind(props.failedCommand)}
{...props}
/>
)
}

export function ErrorRecoveryComponent(
Expand Down Expand Up @@ -283,26 +271,3 @@ export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element {
return buildSelectRecoveryOption()
}
}
interface UseInitialPipetteHomeParams {
hasLaunchedRecovery: ErrorRecoveryWizardProps['hasLaunchedRecovery']
recoveryCommands: ErrorRecoveryWizardProps['recoveryCommands']
routeUpdateActions: ErrorRecoveryWizardProps['routeUpdateActions']
}
// Home the Z-axis of all attached pipettes on Error Recovery launch.
export function useInitialPipetteHome({
hasLaunchedRecovery,
recoveryCommands,
routeUpdateActions,
}: UseInitialPipetteHomeParams): void {
const { homePipetteZAxes } = recoveryCommands
const { handleMotionRouting } = routeUpdateActions

// Synchronously set the recovery route to "robot in motion" before initial render to prevent screen flicker on ER launch.
useLayoutEffect(() => {
if (hasLaunchedRecovery) {
void handleMotionRouting(true)
.then(() => homePipetteZAxes())
.finally(() => handleMotionRouting(false))
}
}, [hasLaunchedRecovery])
}
98 changes: 53 additions & 45 deletions app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function useGripperRelease({
doorStatusUtils,
recoveryMap,
}: UseGripperReleaseProps): number {
const { releaseGripperJaws } = recoveryCommands
const { releaseGripperJaws, homeExceptPlungers } = recoveryCommands
const { selectedRecoveryOption } = currentRecoveryOptionUtils
const {
proceedToRouteAndStep,
Expand All @@ -112,49 +112,47 @@ export function useGripperRelease({
const { MANUAL_MOVE_AND_SKIP, MANUAL_REPLACE_AND_RETRY } = RECOVERY_MAP
const [countdown, setCountdown] = useState(GRIPPER_RELEASE_COUNTDOWN_S)

const proceedToValidNextStep = (): void => {
if (isDoorOpen) {
switch (selectedRecoveryOption) {
case MANUAL_MOVE_AND_SKIP.ROUTE:
void proceedToRouteAndStep(
MANUAL_MOVE_AND_SKIP.ROUTE,
MANUAL_MOVE_AND_SKIP.STEPS.CLOSE_DOOR_GRIPPER_Z_HOME
)
break
case MANUAL_REPLACE_AND_RETRY.ROUTE:
void proceedToRouteAndStep(
MANUAL_REPLACE_AND_RETRY.ROUTE,
MANUAL_REPLACE_AND_RETRY.STEPS.CLOSE_DOOR_GRIPPER_Z_HOME
)
break
default: {
console.error(
'Unhandled post grip-release routing when door is open.'
)
void proceedToRouteAndStep(RECOVERY_MAP.OPTION_SELECTION.ROUTE)
}
}
} else {
switch (selectedRecoveryOption) {
case MANUAL_MOVE_AND_SKIP.ROUTE:
void proceedToRouteAndStep(
MANUAL_MOVE_AND_SKIP.ROUTE,
MANUAL_MOVE_AND_SKIP.STEPS.MANUAL_MOVE
)
break
case MANUAL_REPLACE_AND_RETRY.ROUTE:
void proceedToRouteAndStep(
MANUAL_REPLACE_AND_RETRY.ROUTE,
MANUAL_REPLACE_AND_RETRY.STEPS.MANUAL_REPLACE
)
break
default:
console.error('Unhandled post grip-release routing.')
void proceedNextStep()
const proceedToDoorStep = (): void => {
switch (selectedRecoveryOption) {
case MANUAL_MOVE_AND_SKIP.ROUTE:
void proceedToRouteAndStep(
MANUAL_MOVE_AND_SKIP.ROUTE,
MANUAL_MOVE_AND_SKIP.STEPS.CLOSE_DOOR_GRIPPER_Z_HOME
)
break
case MANUAL_REPLACE_AND_RETRY.ROUTE:
void proceedToRouteAndStep(
MANUAL_REPLACE_AND_RETRY.ROUTE,
MANUAL_REPLACE_AND_RETRY.STEPS.CLOSE_DOOR_GRIPPER_Z_HOME
)
break
default: {
console.error('Unhandled post grip-release routing when door is open.')
void proceedToRouteAndStep(RECOVERY_MAP.OPTION_SELECTION.ROUTE)
}
}
}

const proceedToValidNextStep = (): void => {
switch (selectedRecoveryOption) {
case MANUAL_MOVE_AND_SKIP.ROUTE:
void proceedToRouteAndStep(
MANUAL_MOVE_AND_SKIP.ROUTE,
MANUAL_MOVE_AND_SKIP.STEPS.MANUAL_MOVE
)
break
case MANUAL_REPLACE_AND_RETRY.ROUTE:
void proceedToRouteAndStep(
MANUAL_REPLACE_AND_RETRY.ROUTE,
MANUAL_REPLACE_AND_RETRY.STEPS.MANUAL_REPLACE
)
break
default:
console.error('Unhandled post grip-release routing.')
void proceedNextStep()
}
}

useEffect(() => {
let intervalId: NodeJS.Timeout | null = null

Expand All @@ -167,11 +165,21 @@ export function useGripperRelease({
if (intervalId != null) {
clearInterval(intervalId)
}
void releaseGripperJaws()
.finally(() => handleMotionRouting(false))
.then(() => {
proceedToValidNextStep()
})

void releaseGripperJaws().then(() => {
if (isDoorOpen) {
return handleMotionRouting(false).then(() => {
proceedToDoorStep()
})
}

return handleMotionRouting(true)
.then(() => homeExceptPlungers())
.then(() => handleMotionRouting(false))
.then(() => {
proceedToValidNextStep()
})
})
}

return updatedCountdown
Expand Down
16 changes: 12 additions & 4 deletions app/src/organisms/ErrorRecoveryFlows/RecoverySplash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,14 @@ export function RecoverySplash(props: RecoverySplashProps): JSX.Element | null {
runStatus,
recoveryActionMutationUtils,
resumePausedRecovery,
recoveryCommands,
} = props
const { t } = useTranslation('error_recovery')
const errorKind = getErrorKind(failedCommand)
const title = useErrorName(errorKind)
const { makeToast } = useToaster()

const { proceedToRouteAndStep } = routeUpdateActions
const { proceedToRouteAndStep, handleMotionRouting } = routeUpdateActions
const { reportErrorEvent } = analytics

const buildTitleHeadingDesktop = (): JSX.Element => {
Expand Down Expand Up @@ -138,9 +139,16 @@ export function RecoverySplash(props: RecoverySplashProps): JSX.Element | null {

const onLaunchERClick = (): void => {
const onClick = (): void => {
void toggleERWizAsActiveUser(true, true).then(() => {
reportErrorEvent(failedCommand?.byRunRecord ?? null, 'launch-recovery')
})
void toggleERWizAsActiveUser(true, true)
.then(() => {
reportErrorEvent(
failedCommand?.byRunRecord ?? null,
'launch-recovery'
)
})
.then(() => handleMotionRouting(true))
.then(() => recoveryCommands.homePipetteZAxes())
.finally(() => handleMotionRouting(false))
}
handleConditionalClick(onClick)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import type * as React from 'react'
import { vi, describe, it, expect, beforeEach } from 'vitest'
import { renderHook, act, screen, waitFor } from '@testing-library/react'
import { renderHook, act, screen } from '@testing-library/react'

import { renderWithProviders } from '/app/__testing-utils__'
import { i18n } from '/app/i18n'
import { mockRecoveryContentProps } from '../__fixtures__'
import {
ErrorRecoveryContent,
useInitialPipetteHome,
useERWizard,
ErrorRecoveryComponent,
} from '../ErrorRecoveryWizard'
Expand Down Expand Up @@ -35,8 +34,6 @@ import {
RecoveryDoorOpenSpecial,
} from '../shared'

import type { Mock } from 'vitest'

vi.mock('../RecoveryOptions')
vi.mock('../RecoveryInProgress')
vi.mock('../RecoveryError')
Expand Down Expand Up @@ -509,73 +506,3 @@ describe('ErrorRecoveryContent', () => {
screen.getByText('MOCK_DOOR_OPEN_SPECIAL')
})
})

describe('useInitialPipetteHome', () => {
let mockZHomePipetteZAxes: Mock
let mockhandleMotionRouting: Mock
let mockRecoveryCommands: any
let mockRouteUpdateActions: any

beforeEach(() => {
mockZHomePipetteZAxes = vi.fn()
mockhandleMotionRouting = vi.fn()

mockhandleMotionRouting.mockResolvedValue(() => mockZHomePipetteZAxes())
mockZHomePipetteZAxes.mockResolvedValue(() => mockhandleMotionRouting())

mockRecoveryCommands = {
homePipetteZAxes: mockZHomePipetteZAxes,
} as any
mockRouteUpdateActions = {
handleMotionRouting: mockhandleMotionRouting,
} as any
})

it('does not z-home the pipettes if error recovery was not launched', () => {
renderHook(() =>
useInitialPipetteHome({
hasLaunchedRecovery: false,
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
})
)

expect(mockhandleMotionRouting).not.toHaveBeenCalled()
})

it('sets the motion screen properly and z-homes all pipettes only on the initial render of Error Recovery', async () => {
const { rerender } = renderHook(() =>
useInitialPipetteHome({
hasLaunchedRecovery: true,
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
})
)

await waitFor(() => {
expect(mockhandleMotionRouting).toHaveBeenCalledWith(true)
})
await waitFor(() => {
expect(mockZHomePipetteZAxes).toHaveBeenCalledTimes(1)
})
await waitFor(() => {
expect(mockhandleMotionRouting).toHaveBeenCalledWith(false)
})

expect(mockhandleMotionRouting.mock.invocationCallOrder[0]).toBeLessThan(
mockZHomePipetteZAxes.mock.invocationCallOrder[0]
)
expect(mockZHomePipetteZAxes.mock.invocationCallOrder[0]).toBeLessThan(
mockhandleMotionRouting.mock.invocationCallOrder[1]
)

rerender()

await waitFor(() => {
expect(mockhandleMotionRouting).toHaveBeenCalledTimes(2)
})
await waitFor(() => {
expect(mockZHomePipetteZAxes).toHaveBeenCalledTimes(1)
})
})
})
Loading

0 comments on commit 15ef0ee

Please sign in to comment.