Skip to content

Commit 541de46

Browse files
authored
fix(app): stacker error recovery shouldn't call manualRetrieve pre-maturely (#18824)
This PR fixes the following error recovery issues: 1. We skipped a step in the `STACKER_SHUTTLE_EMPTY_SKIP` route by routing to the wrong step in `HoldingLabware` 2. The `flexStackerLabwareRetrieveFailed` error was not caught in our failedLabwareUtil so we've been rendering incorrect labware info for the `STACKER_SHUTTLE_EMPTY_SKIP/RETRY` routes 3. The re-engage latch PNG was not rendered properly because we rendered it as a video instead of an image 4. `ReleaseLabware` screen now renders a latch release animation instead of gripper for stacker-related flows 5. We prematurely called `manualRetrieve` and changed the stacker state before exiting the flow. When a user tries to go back to a previous step, the rest of the flow would always end up failing. To fix this, the `manualRetrieve` happens in the last step of the flow. 6. Updating the hopper labware info screen to reflect the correct number after a potential manual retrieval, since now we won't get the state change to occur until the flow is completed
1 parent 780114e commit 541de46

File tree

12 files changed

+209
-92
lines changed

12 files changed

+209
-92
lines changed

app/src/organisms/ErrorRecoveryFlows/hooks/useFailedLabwareUtils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ export function getRelevantLabwareIdFromFailedCmd(
374374
'flexStackerStallOrCollision',
375375
'flexStackerShuttleMissing',
376376
'flexStackerHopperLabwareFailed',
377+
'flexStackerLabwareRetrieveFailed',
377378
].includes(error.errorType)
378379
if (recentRelevantFailedLabwareCmd == null) {
379380
return null

app/src/organisms/ErrorRecoveryFlows/shared/HoldingLabware.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export function HoldingLabware({
9090
case STACKER_SHUTTLE_EMPTY_SKIP.ROUTE:
9191
return proceedToRouteAndStep(
9292
STACKER_SHUTTLE_EMPTY_SKIP.ROUTE,
93-
STACKER_SHUTTLE_EMPTY_SKIP.STEPS.FILL_HOPPER
93+
STACKER_SHUTTLE_EMPTY_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE
9494
)
9595
default: {
9696
console.error('Unexpected recovery option for gripper routing.')

app/src/organisms/ErrorRecoveryFlows/shared/LeftColumnLabwareInfo.tsx

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type LeftColumnLabwareInfoProps = RecoveryContentProps & {
1212
layout: ComponentProps<typeof InterventionContent>['infoProps']['layout']
1313
/* Renders a warning InlineNotification if provided. */
1414
bannerText?: string | null
15+
showQuantity?: boolean
1516
}
1617
// TODO(jh, 06-12-24): EXEC-500 & EXEC-501.
1718
// The left column component adjacent to RecoveryDeckMap/TipSelection.
@@ -22,6 +23,7 @@ export function LeftColumnLabwareInfo({
2223
layout,
2324
bannerText,
2425
recoveryMap,
26+
showQuantity = true,
2527
}: LeftColumnLabwareInfoProps): JSX.Element {
2628
const { step, route } = recoveryMap
2729
const {
@@ -69,6 +71,7 @@ export function LeftColumnLabwareInfo({
6971
switch (step) {
7072
case STACKER_STALLED_RETRY.STEPS.CHECK_HOPPER:
7173
case STACKER_STALLED_SKIP.STEPS.CHECK_HOPPER:
74+
case STACKER_SHUTTLE_EMPTY_SKIP.STEPS.FILL_HOPPER:
7275
return {
7376
labwareName: failedLabwareNames.name ?? '',
7477
labwareNickname: failedLabwareNames.nickName,
@@ -78,7 +81,7 @@ export function LeftColumnLabwareInfo({
7881
}
7982
case STACKER_STALLED_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE:
8083
case STACKER_HOPPER_EMPTY_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE:
81-
case STACKER_SHUTTLE_EMPTY_SKIP.STEPS.FILL_HOPPER:
84+
case STACKER_SHUTTLE_EMPTY_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE:
8285
return {
8386
labwareName: failedLabwareNames.name ?? '',
8487
labwareNickname: failedLabwareNames.nickName,
@@ -99,16 +102,22 @@ export function LeftColumnLabwareInfo({
99102
}
100103

101104
const buildQuantity = (): number | null => {
102-
switch (step) {
103-
case STACKER_STALLED_RETRY.STEPS.CHECK_HOPPER:
104-
case STACKER_STALLED_SKIP.STEPS.CHECK_HOPPER:
105-
case STACKER_HOPPER_EMPTY_SKIP.STEPS.FILL_HOPPER:
106-
return labwareQuantity
107-
case STACKER_STALLED_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE:
108-
case STACKER_HOPPER_EMPTY_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE:
109-
return null
110-
default:
111-
return labwareQuantity
105+
if (!showQuantity) {
106+
return null
107+
}
108+
if (
109+
(route === RECOVERY_MAP.STACKER_HOPPER_EMPTY_SKIP.ROUTE &&
110+
step === RECOVERY_MAP.STACKER_HOPPER_EMPTY_SKIP.STEPS.FILL_HOPPER) ||
111+
(route === RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_SKIP.ROUTE &&
112+
step === RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_SKIP.STEPS.FILL_HOPPER) ||
113+
(route === RECOVERY_MAP.STACKER_STALLED_SKIP.ROUTE &&
114+
step === RECOVERY_MAP.STACKER_STALLED_SKIP.STEPS.CHECK_HOPPER)
115+
) {
116+
return labwareQuantity != null && labwareQuantity > 0
117+
? labwareQuantity - 1 // one has been moved manually onto the shuttle
118+
: null
119+
} else {
120+
return labwareQuantity ?? null
112121
}
113122
}
114123

app/src/organisms/ErrorRecoveryFlows/shared/ReleaseLabware.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
StyledText,
1313
} from '@opentrons/components'
1414

15+
import stackerReleaseLatchAnimation from '/app/assets/videos/error-recovery/FlexStacker_ReleaseLatch.webm'
1516
import gripperReleaseAnimation from '/app/assets/videos/error-recovery/Gripper_Release.webm'
1617
import { TwoColumn } from '/app/molecules/InterventionModal'
1718
import { RECOVERY_MAP } from '/app/organisms/ErrorRecoveryFlows/constants'
@@ -72,6 +73,12 @@ export function ReleaseLabware({
7273
return t('take_any_necessary_precautions')
7374
}
7475
}
76+
const animationSrc =
77+
route === STACKER_SHUTTLE_EMPTY_RETRY.ROUTE ||
78+
route === STACKER_SHUTTLE_EMPTY_SKIP.ROUTE
79+
? stackerReleaseLatchAnimation
80+
: gripperReleaseAnimation
81+
7582
return (
7683
<RecoverySingleColumnContentWrapper>
7784
<TwoColumn>
@@ -95,10 +102,7 @@ export function ReleaseLabware({
95102
</Flex>
96103
<Flex css={ANIMATION_CONTAINER_STYLE}>
97104
<AnimationVideo role="presentation" css={ANIMATION_STYLE}>
98-
<source
99-
src={gripperReleaseAnimation}
100-
data-testid="gripper-animation"
101-
/>
105+
<source src={animationSrc} data-testid="release-animation" />
102106
</AnimationVideo>
103107
</Flex>
104108
</TwoColumn>

app/src/organisms/ErrorRecoveryFlows/shared/SkipStepInfo.tsx

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,28 @@ export function SkipStepInfo(props: RecoveryContentProps): JSX.Element {
2424
} = RECOVERY_MAP
2525
const { selectedRecoveryOption } = currentRecoveryOptionUtils
2626
const { skipFailedCommand } = recoveryCommands
27-
const { moveLabwareWithoutPause } = recoveryCommands
27+
const { moveLabwareWithoutPause, manualRetrieve } = recoveryCommands
2828
const { handleMotionRouting } = routeUpdateActions
2929
const { ROBOT_SKIPPING_STEP } = RECOVERY_MAP
3030
const { t } = useTranslation('error_recovery')
3131

3232
const primaryBtnOnClick = (): Promise<void> => {
3333
return handleMotionRouting(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => {
34-
if (selectedRecoveryOption === MANUAL_MOVE_AND_SKIP.ROUTE) {
35-
void moveLabwareWithoutPause().then(() => {
34+
switch (selectedRecoveryOption) {
35+
case MANUAL_MOVE_AND_SKIP.ROUTE:
36+
void moveLabwareWithoutPause().then(() => {
37+
skipFailedCommand()
38+
})
39+
break
40+
case STACKER_HOPPER_EMPTY_SKIP.ROUTE:
41+
case STACKER_SHUTTLE_EMPTY_SKIP.ROUTE:
42+
case STACKER_STALLED_SKIP.ROUTE:
43+
void manualRetrieve().then(() => {
44+
skipFailedCommand()
45+
})
46+
break
47+
default:
3648
skipFailedCommand()
37-
})
38-
} else {
39-
skipFailedCommand()
4049
}
4150
})
4251
}

app/src/organisms/ErrorRecoveryFlows/shared/StackerReengageLatch.tsx

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { useTranslation } from 'react-i18next'
2+
import { css } from 'styled-components'
3+
4+
import { JUSTIFY_CENTER, RESPONSIVENESS } from '@opentrons/components'
25

36
import ReengageLatch from '/app/assets/images/flex_stacker_reengage_latch.png'
47
import { DescriptionContent, TwoColumn } from '/app/molecules/InterventionModal'
58

69
import { RecoverySingleColumnContentWrapper } from './RecoveryContentWrapper'
710
import { RecoveryFooterButtons } from './RecoveryFooterButtons'
8-
import { RightColumnAnimation } from './RightColumnAnimation'
911

1012
import type { RecoveryContentProps } from '../types'
1113

@@ -22,7 +24,7 @@ export function StackerReengageLatch(props: RecoveryContentProps): JSX.Element {
2224
headline={t('prepare_for_stacker_latch_reengage')}
2325
message={t('stacker_latch_will_reengage')}
2426
/>
25-
<RightColumnAnimation animationSrc={ReengageLatch} />
27+
<img src={ReengageLatch} css={IMAGE_STYLE} />
2628
</TwoColumn>
2729
<RecoveryFooterButtons
2830
primaryBtnOnClick={proceedNextStep}
@@ -31,3 +33,14 @@ export function StackerReengageLatch(props: RecoveryContentProps): JSX.Element {
3133
</RecoverySingleColumnContentWrapper>
3234
)
3335
}
36+
37+
const IMAGE_STYLE = css`
38+
justify-content: ${JUSTIFY_CENTER};
39+
overflow: hidden;
40+
max-height: 13.25rem;
41+
42+
@media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} {
43+
width: 27rem;
44+
height: 20.25rem;
45+
}
46+
`

app/src/organisms/ErrorRecoveryFlows/shared/StackerShuttleLwInfo.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,8 @@ import { RightColumnAnimation } from './RightColumnAnimation'
1111
import type { RecoveryContentProps } from '../types'
1212

1313
export function StackerShuttleLwInfo(props: RecoveryContentProps): JSX.Element {
14-
const { recoveryCommands, routeUpdateActions } = props
15-
const { manualRetrieve } = recoveryCommands
16-
const { proceedNextStep } = routeUpdateActions
17-
1814
const { t } = useTranslation('error_recovery')
1915

20-
const primaryOnClick = (): void => {
21-
void manualRetrieve().then(() => proceedNextStep())
22-
}
23-
2416
return (
2517
<RecoverySingleColumnContentWrapper>
2618
<TwoColumn>
@@ -29,11 +21,12 @@ export function StackerShuttleLwInfo(props: RecoveryContentProps): JSX.Element {
2921
title={t('load_labware_into_labware_shuttle')}
3022
type={'location'}
3123
layout={'stacked'}
24+
showQuantity={false}
3225
/>
3326
<RightColumnAnimation animationSrc={ShuttleLabware} />
3427
</TwoColumn>
3528
<RecoveryFooterButtons
36-
primaryBtnOnClick={primaryOnClick}
29+
primaryBtnOnClick={props.routeUpdateActions.proceedNextStep}
3730
secondaryBtnOnClick={props.routeUpdateActions.goBackPrevStep}
3831
/>
3932
</RecoverySingleColumnContentWrapper>

app/src/organisms/ErrorRecoveryFlows/shared/__tests__/HoldingLabware.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ describe('HoldingLabware', () => {
193193
await waitFor(() => {
194194
expect(mockProceedToRouteAndStep).toHaveBeenCalledWith(
195195
RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_SKIP.ROUTE,
196-
RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_SKIP.STEPS.FILL_HOPPER
196+
RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_SKIP.STEPS.PLACE_LABWARE_ON_SHUTTLE
197197
)
198198
})
199199
})

app/src/organisms/ErrorRecoveryFlows/shared/__tests__/ReleaseLabware.test.tsx

Lines changed: 99 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ vi.mock('/app/assets/videos/error-recovery/Gripper_Release.webm', () => ({
1616
default: 'mocked-animation-path.webm',
1717
}))
1818

19+
vi.mock(
20+
'/app/assets/videos/error-recovery/FlexStacker_ReleaseLatch.webm',
21+
() => ({
22+
default: 'mocked-stacker-latch-path.webm',
23+
})
24+
)
25+
1926
const render = (props: ComponentProps<typeof ReleaseLabware>) => {
2027
return renderWithProviders(<ReleaseLabware {...props} />, {
2128
i18nInstance: i18n,
@@ -26,58 +33,99 @@ describe('ReleaseLabware', () => {
2633
let props: ComponentProps<typeof ReleaseLabware>
2734
let mockHandleMotionRouting: Mock
2835

29-
beforeEach(() => {
30-
mockHandleMotionRouting = vi.fn(() => Promise.resolve())
31-
32-
props = {
33-
...mockRecoveryContentProps,
34-
routeUpdateActions: {
35-
handleMotionRouting: mockHandleMotionRouting,
36-
goBackPrevStep: vi.fn(),
37-
} as any,
38-
}
36+
describe('For gripper flows', () => {
37+
beforeEach(() => {
38+
mockHandleMotionRouting = vi.fn(() => Promise.resolve())
39+
40+
props = {
41+
...mockRecoveryContentProps,
42+
routeUpdateActions: {
43+
handleMotionRouting: mockHandleMotionRouting,
44+
goBackPrevStep: vi.fn(),
45+
} as any,
46+
}
47+
})
48+
49+
it('renders gripper copy', () => {
50+
render(props)
51+
52+
screen.getByText('Release labware from gripper')
53+
screen.getByText(
54+
'Take any necessary precautions before positioning yourself to stabilize or catch the labware. Once confirmed, a countdown will begin before the gripper releases.'
55+
)
56+
screen.getByText('The labware will be released from its current height.')
57+
})
58+
59+
it(`clicking the primary button routes to ${RECOVERY_MAP.ROBOT_RELEASING_LABWARE.ROUTE}`, () => {
60+
render(props)
61+
62+
clickButtonLabeled('Release')
63+
64+
expect(mockHandleMotionRouting).toHaveBeenCalledWith(
65+
true,
66+
RECOVERY_MAP.ROBOT_RELEASING_LABWARE.ROUTE
67+
)
68+
})
69+
70+
it('renders gripper animation', () => {
71+
render(props)
72+
73+
screen.getByRole('presentation', { hidden: true })
74+
expect(screen.getByTestId('release-animation')).toHaveAttribute(
75+
'src',
76+
'mocked-animation-path.webm'
77+
)
78+
})
3979
})
4080

41-
it('renders gripper copy', () => {
42-
render(props)
43-
44-
screen.getByText('Release labware from gripper')
45-
screen.getByText(
46-
'Take any necessary precautions before positioning yourself to stabilize or catch the labware. Once confirmed, a countdown will begin before the gripper releases.'
47-
)
48-
screen.getByText('The labware will be released from its current height.')
49-
})
50-
51-
it('renders latch copy', () => {
52-
props.recoveryMap = {
53-
route: RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_RETRY.ROUTE,
54-
step:
55-
RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_RETRY.STEPS.CONFIRM_LABWARE_IN_LATCH,
56-
}
57-
render(props)
58-
59-
screen.getByText('Release labware from latch')
60-
screen.getByText(
61-
'Take any necessary precautions before positioning yourself to stabilize or catch the labware if needed. Once confirmed, a countdown will begin before the latch releases.'
62-
)
63-
screen.getByText('The labware will be released from its current height.')
64-
})
65-
66-
it('clicking the primary button has correct behavior', () => {
67-
render(props)
68-
69-
clickButtonLabeled('Release')
70-
71-
expect(mockHandleMotionRouting).toHaveBeenCalled()
72-
})
73-
74-
it('renders gripper animation', () => {
75-
render(props)
76-
77-
screen.getByRole('presentation', { hidden: true })
78-
expect(screen.getByTestId('gripper-animation')).toHaveAttribute(
79-
'src',
80-
'mocked-animation-path.webm'
81-
)
81+
describe('For stacker flows', () => {
82+
beforeEach(() => {
83+
mockHandleMotionRouting = vi.fn(() => Promise.resolve())
84+
85+
props = {
86+
...mockRecoveryContentProps,
87+
recoveryMap: {
88+
route: RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_RETRY.ROUTE,
89+
step:
90+
RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_RETRY.STEPS
91+
.CONFIRM_LABWARE_IN_LATCH,
92+
},
93+
routeUpdateActions: {
94+
handleMotionRouting: mockHandleMotionRouting,
95+
goBackPrevStep: vi.fn(),
96+
} as any,
97+
}
98+
})
99+
100+
it('renders latch copy', () => {
101+
render(props)
102+
103+
screen.getByText('Release labware from latch')
104+
screen.getByText(
105+
'Take any necessary precautions before positioning yourself to stabilize or catch the labware if needed. Once confirmed, a countdown will begin before the latch releases.'
106+
)
107+
screen.getByText('The labware will be released from its current height.')
108+
})
109+
110+
it(`clicking the primary button routes to ${RECOVERY_MAP.ROBOT_RELEASING_LABWARE_LATCH.ROUTE}`, () => {
111+
render(props)
112+
113+
clickButtonLabeled('Release')
114+
115+
expect(mockHandleMotionRouting).toHaveBeenCalledWith(
116+
true,
117+
RECOVERY_MAP.ROBOT_RELEASING_LABWARE_LATCH.ROUTE
118+
)
119+
})
120+
121+
it('renders latch animation', () => {
122+
render(props)
123+
124+
screen.getByRole('presentation', { hidden: true })
125+
expect(screen.getByTestId('release-animation')).toHaveAttribute(
126+
'src',
127+
'mocked-stacker-latch-path.webm'
128+
)
129+
})
82130
})
83131
})

0 commit comments

Comments
 (0)