Skip to content

Commit

Permalink
refactor(app): do pipette homing via onclick
Browse files Browse the repository at this point in the history
  • Loading branch information
mjhuff committed Nov 22, 2024
1 parent 5d0bb60 commit fe13ce4
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 117 deletions.
46 changes: 7 additions & 39 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 @@ -75,21 +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,
})

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

export function ErrorRecoveryComponent(
Expand Down Expand Up @@ -280,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])
}
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)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,14 @@ describe('RecoverySplash', () => {
let props: React.ComponentProps<typeof RecoverySplash>
const mockToggleERWiz = vi.fn(() => Promise.resolve())
const mockProceedToRouteAndStep = vi.fn()
const mockHandleMotionRouting = vi.fn(() => Promise.resolve())
const mockRouteUpdateActions = {
proceedToRouteAndStep: mockProceedToRouteAndStep,
handleMotionRouting: mockHandleMotionRouting,
} as any
const mockMakeToast = vi.fn()
const mockResumeRecovery = vi.fn()
const mockHomePipetteZAxes = vi.fn(() => Promise.resolve())

beforeEach(() => {
props = {
Expand All @@ -96,6 +99,7 @@ describe('RecoverySplash', () => {
resumeRecovery: mockResumeRecovery,
} as any,
resumePausedRecovery: true,
recoveryCommands: { homePipetteZAxes: mockHomePipetteZAxes } as any,
}

vi.mocked(StepInfo).mockReturnValue(<div>MOCK STEP INFO</div>)
Expand Down Expand Up @@ -162,6 +166,13 @@ describe('RecoverySplash', () => {
await waitFor(() => {
expect(mockToggleERWiz).toHaveBeenCalledWith(true, true)
})

expect(mockHandleMotionRouting).toHaveBeenNthCalledWith(1, true)
expect(mockHandleMotionRouting).toHaveBeenNthCalledWith(2, false)

await waitFor(() => {
expect(props.recoveryCommands.homePipetteZAxes).toHaveBeenCalled()
})
})

it('should render a door open toast if the door is open', () => {
Expand Down

0 comments on commit fe13ce4

Please sign in to comment.