Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mjhuff committed May 20, 2024
1 parent 7f119a1 commit a4f5c28
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,6 @@ export function useInitialPipetteHome(
React.useLayoutEffect(() => {
void setRobotInMotion(true)
.then(() => homePipetteZAxes())
.then(() => setRobotInMotion(false))
.finally(() => setRobotInMotion(false))
}, [])
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('useErrorRecovery', () => {
useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_RUNNING)
)

expect(result.current.isEREnabled).toBe(false)
expect(result.current.isERActive).toBe(false)
})

it('should toggle the value of isEREnabled properly', () => {
Expand All @@ -41,13 +41,13 @@ describe('useErrorRecovery', () => {
result.current.toggleER()
})

expect(result.current.isEREnabled).toBe(true)
expect(result.current.isERActive).toBe(true)

act(() => {
result.current.toggleER()
})

expect(result.current.isEREnabled).toBe(false)
expect(result.current.isERActive).toBe(false)
})

it('should disable error recovery when runStatus is not "awaiting-recovery"', () => {
Expand All @@ -65,15 +65,15 @@ describe('useErrorRecovery', () => {
// @ts-expect-error "running" is a valid status here
rerender(RUN_STATUS_RUNNING)

expect(result.current.isEREnabled).toBe(false)
expect(result.current.isERActive).toBe(false)

act(() => {
result.current.toggleER()
})

rerender(RUN_STATUS_AWAITING_RECOVERY)

expect(result.current.isEREnabled).toBe(false)
expect(result.current.isERActive).toBe(false)
})

it('should return the failed run command', () => {
Expand Down
14 changes: 7 additions & 7 deletions app/src/organisms/ErrorRecoveryFlows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { RunStatus } from '@opentrons/api-client'
import type { FailedCommand } from './types'

interface UseErrorRecoveryResult {
isEREnabled: boolean
isERActive: boolean
failedCommand: FailedCommand | null
toggleER: () => void
}
Expand All @@ -18,22 +18,22 @@ export function useErrorRecoveryFlows(
runId: string,
runStatus: RunStatus | null
): UseErrorRecoveryResult {
const [isEREnabled, setIsEREnabled] = React.useState(false)
const [isERActive, setIsERActive] = React.useState(false)
const failedCommand = useCurrentlyFailedRunCommand(runId, runStatus)

const toggleER = (): void => {
setIsEREnabled(!isEREnabled)
setIsERActive(!isERActive)
}

// Because multiple ER flows may occur per run, disable ER when the status is not "awaiting-recovery."
React.useEffect(() => {
if (isEREnabled && runStatus !== RUN_STATUS_AWAITING_RECOVERY) {
setIsEREnabled(false)
if (isERActive && runStatus !== RUN_STATUS_AWAITING_RECOVERY) {
setIsERActive(false)
}
}, [isEREnabled, runStatus])
}, [isERActive, runStatus])

return {
isEREnabled,
isERActive,
failedCommand,
toggleER,
}
Expand Down
4 changes: 1 addition & 3 deletions app/src/organisms/ErrorRecoveryFlows/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@ import type {

// TODO(jh, 05-09-24): Migrate utils, useRecoveryCommands.ts, and respective tests to a utils dir, and make each util a separate file.

// TODO(jh, 05-09-24): It is possible for a desktop app not to detect the most recently failed run command if there are
// too many FIXIT commands. See PR associated with TODO for explanation.

// While the run is "awaiting-recovery", return the most recently failed run command with a protocol intent.
// Otherwise, returns null.
const ALL_COMMANDS_POLL_MS = 5000

// TODO(jh, 05-20-24): Update the logic for returning the failed run command once EXEC-458 merges.
export function useCurrentlyFailedRunCommand(
runId: string,
runStatus: RunStatus | null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe('RunningProtocol', () => {
<div>MOCK ERROR RECOVERY</div>
)
vi.mocked(useErrorRecoveryFlows).mockReturnValue({
isEREnabled: false,
isERActive: false,
toggleER: vi.fn(),
failedCommand: {} as any,
})
Expand Down Expand Up @@ -207,7 +207,7 @@ describe('RunningProtocol', () => {
expect(screen.queryByText('MOCK ERROR RECOVERY')).not.toBeInTheDocument()

vi.mocked(useErrorRecoveryFlows).mockReturnValue({
isEREnabled: true,
isERActive: true,
toggleER: vi.fn(),
failedCommand: {} as any,
})
Expand Down
4 changes: 2 additions & 2 deletions app/src/pages/RunningProtocol/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export function RunningProtocol(): JSX.Element {
const robotAnalyticsData = useRobotAnalyticsData(robotName)
const robotType = useRobotType(robotName)
const enableRunNotes = useFeatureFlag('enableRunNotes')
const { isEREnabled, failedCommand, toggleER } = useErrorRecoveryFlows(
const { isERActive, failedCommand, toggleER } = useErrorRecoveryFlows(
runId,
runStatus
)
Expand Down Expand Up @@ -166,7 +166,7 @@ export function RunningProtocol(): JSX.Element {

return (
<>
{isEREnabled ? (
{isERActive ? (
<ErrorRecoveryFlows runId={runId} failedCommand={failedCommand} />
) : null}
{runStatus === RUN_STATUS_AWAITING_RECOVERY && enableRunNotes ? (
Expand Down

0 comments on commit a4f5c28

Please sign in to comment.