-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(app, react-api-client): add "retry failed command" during Error Recovery #15170
Changes from 1 commit
0ff7b51
cd46e87
0878f4e
7f119a1
a4f5c28
79bd847
c7a7fb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
import * as React from 'react' | ||
import { createPortal } from 'react-dom' | ||
import { useSelector } from 'react-redux' | ||
|
||
import { | ||
BORDERS, | ||
COLORS, | ||
DIRECTION_COLUMN, | ||
Flex, | ||
POSITION_ABSOLUTE, | ||
} from '@opentrons/components' | ||
|
||
import { getIsOnDevice } from '../../redux/config' | ||
import { getTopPortalEl } from '../../App/portal' | ||
import { BeforeBeginning } from './BeforeBeginning' | ||
import { SelectRecoveryOption, ResumeRun } from './RecoveryOptions' | ||
import { ErrorRecoveryHeader } from './ErrorRecoveryHeader' | ||
import { RecoveryInProgress } from './RecoveryInProgress' | ||
import { getErrorKind, useRouteUpdateActions } from './utils' | ||
import { useRecoveryCommands } from './useRecoveryCommands' | ||
import { RECOVERY_MAP } from './constants' | ||
|
||
import type { FailedCommand, IRecoveryMap, RecoveryContentProps } from './types' | ||
|
||
export interface ErrorRecoveryFlowsProps { | ||
runId: string | ||
failedCommand: FailedCommand | null | ||
} | ||
|
||
export function ErrorRecoveryWizard({ | ||
runId, | ||
failedCommand, | ||
}: ErrorRecoveryFlowsProps): JSX.Element { | ||
/** | ||
* Recovery Route: A logically-related collection of recovery steps or a single step if unrelated to any existing recovery route. | ||
* Recovery Step: Analogous to a "step" in other wizard flows. | ||
*/ | ||
const [recoveryMap, setRecoveryMap] = React.useState<IRecoveryMap>({ | ||
route: RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE, // Initialize the route to "in motion", since wizard always start with a home command. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we expect the robot to already be in motion when we start this wizard? If it is already in motion, then this makes sense, if it is not, should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good Q - doing it this way prevents us initially rendering some other screen for one render cycle before the in-motion screen. This problem is really only noticeable if you test it on the physical ODD, and it looks bad enough IMO that I'd rather eagerly set the robot to 'in motion' immediately and not have the screen immediately changing as soon as we launch ER. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DerekMaggio After further investigation, I found a better React way of doing this while initializing the route to something more sensible, so thank you for calling this out! |
||
step: RECOVERY_MAP.ROBOT_IN_MOTION.STEPS.IN_MOTION, | ||
}) | ||
|
||
const errorKind = getErrorKind(failedCommand?.error?.errorType) | ||
const isOnDevice = useSelector(getIsOnDevice) | ||
const routeUpdateActions = useRouteUpdateActions({ | ||
recoveryMap, | ||
setRecoveryMap, | ||
}) | ||
const recoveryCommands = useRecoveryCommands({ | ||
runId, | ||
failedCommand, | ||
}) | ||
|
||
useInitialPipetteHome(recoveryCommands, routeUpdateActions) | ||
|
||
return ( | ||
<ErrorRecoveryComponent | ||
failedCommand={failedCommand} | ||
errorKind={errorKind} | ||
isOnDevice={isOnDevice} | ||
recoveryMap={recoveryMap} | ||
routeUpdateActions={routeUpdateActions} | ||
recoveryCommands={recoveryCommands} | ||
/> | ||
) | ||
} | ||
|
||
function ErrorRecoveryComponent(props: RecoveryContentProps): JSX.Element { | ||
return createPortal( | ||
<Flex | ||
flexDirection={DIRECTION_COLUMN} | ||
width="992px" | ||
height="568px" | ||
left="14.5px" | ||
top="16px" | ||
borderRadius={BORDERS.borderRadius12} | ||
position={POSITION_ABSOLUTE} | ||
backgroundColor={COLORS.white} | ||
> | ||
<ErrorRecoveryHeader errorKind={props.errorKind} /> | ||
<ErrorRecoveryContent {...props} /> | ||
</Flex>, | ||
getTopPortalEl() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably wants to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yep, great point. Given hi-fi designs, it looks like we may have some modal-within-modals as well, so we'd want the lower |
||
) | ||
} | ||
|
||
export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element { | ||
const buildBeforeBeginning = (): JSX.Element => { | ||
return <BeforeBeginning {...props} /> | ||
} | ||
|
||
const buildSelectRecoveryOption = (): JSX.Element => { | ||
return <SelectRecoveryOption {...props} /> | ||
} | ||
|
||
const buildRecoveryInProgress = (): JSX.Element => { | ||
return <RecoveryInProgress {...props} /> | ||
} | ||
|
||
const buildResumeRun = (): JSX.Element => { | ||
return <ResumeRun {...props} /> | ||
} | ||
|
||
switch (props.recoveryMap.route) { | ||
case RECOVERY_MAP.BEFORE_BEGINNING.ROUTE: | ||
return buildBeforeBeginning() | ||
case RECOVERY_MAP.OPTION_SELECTION.ROUTE: | ||
return buildSelectRecoveryOption() | ||
case RECOVERY_MAP.RESUME.ROUTE: | ||
return buildResumeRun() | ||
case RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE: | ||
case RECOVERY_MAP.ROBOT_RESUMING.ROUTE: | ||
case RECOVERY_MAP.ROBOT_RETRYING_COMMAND.ROUTE: | ||
return buildRecoveryInProgress() | ||
default: | ||
return buildSelectRecoveryOption() | ||
} | ||
} | ||
|
||
// Home the Z-axis of all attached pipettes on Error Recovery launch. | ||
export function useInitialPipetteHome( | ||
recoveryCommands: ReturnType<typeof useRecoveryCommands>, | ||
routeUpdateActions: ReturnType<typeof useRouteUpdateActions> | ||
): void { | ||
const { homePipetteZAxes } = recoveryCommands | ||
const { setRobotInMotion } = routeUpdateActions | ||
|
||
React.useEffect(() => { | ||
void setRobotInMotion(true) | ||
.then(() => homePipetteZAxes()) | ||
.then(() => setRobotInMotion(false)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably wants to be |
||
}, []) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diff this against the old
index.tsx
to see changes.The differences are the new route for the "retrying command" view, the
useInitialPipetteHome
, and the new name,ErrorRecoveryWizard
.