Skip to content
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

fix(app): make cal commands chain instead of race #6561

Merged
merged 13 commits into from
Sep 18, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { act } from 'react-dom/test-utils'

import { getDeckDefinitions } from '@opentrons/components/src/deck/getDeckDefinitions'

import { getRequestById } from '../../../robot-api'
import * as Sessions from '../../../sessions'
import { mockPipetteOffsetCalibrationSessionAttributes } from '../../../sessions/__fixtures__'

Expand All @@ -21,7 +20,6 @@ import {
CompleteConfirmation,
} from '../../CalibrationPanels'

import type { State } from '../../../types'
import type { PipetteOffsetCalibrationStep } from '../../../sessions/types'

jest.mock('@opentrons/components/src/deck/getDeckDefinitions')
Expand All @@ -40,11 +38,6 @@ const mockGetDeckDefinitions: JestMockFn<
$Call<typeof getDeckDefinitions, any>
> = getDeckDefinitions

const mockGetRequestById: JestMockFn<
[State, string],
$Call<typeof getRequestById, State, string>
> = getRequestById

describe('CalibratePipetteOffset', () => {
let mockStore
let render
Expand Down Expand Up @@ -93,12 +86,15 @@ describe('CalibratePipetteOffset', () => {
...mockPipetteOffsetCalibrationSessionAttributes,
}

render = () => {
render = (props = {}) => {
const { showSpinner = false } = props
return mount(
<CalibratePipetteOffset
robotName="robot-name"
session={mockPipOffsetCalSession}
closeWizard={() => {}}
closeWizard={jest.fn()}
dispatchRequests={jest.fn()}
showSpinner={showSpinner}
/>,
{
wrappingComponent: Provider,
Expand Down Expand Up @@ -142,12 +138,13 @@ describe('CalibratePipetteOffset', () => {
expect(wrapper.find('ConfirmExitModal').exists()).toBe(true)
})

it('renders spinner when last tracked request is pending, and not present otherwise', () => {
const wrapper = render()
it('does not render spinner when showSpinner is false', () => {
const wrapper = render({ showSpinner: false })
expect(wrapper.find('SpinnerModalPage').exists()).toBe(false)
})

mockGetRequestById.mockReturnValue({ status: 'pending' })
wrapper.setProps({})
it('renders spinner when showSpinner is true', () => {
const wrapper = render({ showSpinner: true })
expect(wrapper.find('SpinnerModalPage').exists()).toBe(true)
})
})
80 changes: 37 additions & 43 deletions app/src/components/CalibratePipetteOffset/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// @flow
// Pipette Offset Calibration Orchestration Component
import * as React from 'react'
import { useSelector, useDispatch } from 'react-redux'
import last from 'lodash/last'

import { getPipetteModelSpecs } from '@opentrons/shared-data'
import {
Expand All @@ -11,15 +9,11 @@ import {
useConditionalConfirm,
} from '@opentrons/components'

import type { State } from '../../types'
import type {
SessionCommandString,
SessionCommandData,
DeckCalibrationLabware,
SessionCommandParams,
} from '../../sessions/types'
import * as Sessions from '../../sessions'
import { useDispatchApiRequest, getRequestById, PENDING } from '../../robot-api'
import type { RequestState } from '../../robot-api/types'
import {
Introduction,
DeckSetup,
Expand Down Expand Up @@ -64,53 +58,53 @@ const PANEL_STYLE_BY_STEP: {
export function CalibratePipetteOffset(
props: CalibratePipetteOffsetParentProps
): React.Node {
const { session, robotName, closeWizard } = props
const {
session,
robotName,
closeWizard,
dispatchRequests,
showSpinner,
} = props
const { currentStep, instrument, labware } = session?.details || {}
const [dispatchRequest, requestIds] = useDispatchApiRequest()
const dispatch = useDispatch()

const requestStatus = useSelector<State, RequestState | null>(state =>
getRequestById(state, last(requestIds))
)?.status

function sendCommand(
command: SessionCommandString,
data: SessionCommandData = {},
loadingSpinner: boolean = true
) {
if (session === null) return
const sessionCommand = Sessions.createSessionCommand(
robotName,
session.id,
{ command, data }
)
if (loadingSpinner) {
dispatchRequest(sessionCommand)
} else {
dispatch(sessionCommand)
}
}

function deleteSession() {
session?.id &&
dispatchRequest(Sessions.deleteSession(robotName, session.id))
closeWizard()
}

const {
showConfirmation: showConfirmExit,
confirm: confirmExit,
cancel: cancelExit,
} = useConditionalConfirm(() => {
sendCommand(Sessions.deckCalCommands.EXIT)
deleteSession()
cleanUpAndExit()
}, true)

const isMulti = React.useMemo(() => {
const spec = instrument && getPipetteModelSpecs(instrument.model)
return spec ? spec.channels > 1 : false
}, [instrument])

function sendCommands(...commands: Array<SessionCommandParams>) {
if (session?.id) {
const sessionCommandActions = commands.map(c =>
Sessions.createSessionCommand(robotName, session.id, {
command: c.command,
data: c.data || {},
})
)
dispatchRequests(...sessionCommandActions)
}
}

function cleanUpAndExit() {
if (session?.id) {
dispatchRequests(
Sessions.createSessionCommand(robotName, session.id, {
command: Sessions.sharedCalCommands.EXIT,
data: {},
}),
Sessions.deleteSession(robotName, session.id)
)
}
closeWizard()
}

const tipRack: DeckCalibrationLabware | null =
(labware && labware.find(l => l.isTiprack)) ?? null

Expand All @@ -123,7 +117,7 @@ export function CalibratePipetteOffset(
back: { onClick: confirmExit, title: EXIT, children: EXIT },
}

if (requestStatus === PENDING) {
if (showSpinner) {
return <SpinnerModalPage titleBar={titleBarProps} />
}

Expand All @@ -135,8 +129,8 @@ export function CalibratePipetteOffset(
contentsClassName={PANEL_STYLE_BY_STEP[currentStep]}
>
<Panel
sendSessionCommand={sendCommand}
deleteSession={deleteSession}
sendCommands={sendCommands}
cleanUpAndExit={cleanUpAndExit}
tipRack={tipRack}
isMulti={isMulti}
mount={instrument?.mount.toLowerCase()}
Expand Down
14 changes: 7 additions & 7 deletions app/src/components/CalibratePipetteOffset/types.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @flow
import type { Action } from '../../types'
import type {
SessionCommandString,
SessionCommandData,
SessionCommandParams,
PipetteOffsetCalibrationSession,
} from '../../sessions/types'

Expand All @@ -14,14 +14,14 @@ export type CalibratePipetteOffsetParentProps = {|
robotName: string,
session: PipetteOffsetCalibrationSession | null,
closeWizard: () => void,
dispatchRequests: (
...Array<{ ...Action, meta: { requestId: string } }>
) => void,
showSpinner: boolean,
|}

export type CalibratePipetteOffsetChildProps = {|
sendSessionCommand: (
command: SessionCommandString,
data?: SessionCommandData,
loadingSpinner?: boolean
) => void,
sendSessionCommands: (...Array<SessionCommandParams>) => void,
deleteSession: () => void,
tipRack: PipetteOffsetCalibrationLabware,
isMulti: boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,8 @@ export function CompleteConfirmation(props: CalibrationPanelProps): React.Node {
const { sessionType } = props
const { headerText } = contentsBySessionType[sessionType]

// TODO: BC 2020-09-04 avoid potential race condition by having an epic send the delete
// session command upon a successful exit response
const exitSession = () => {
props.sendSessionCommand(Sessions.sharedCalCommands.EXIT)
// TODO: IMMEDIATELY use actualy epic for managing chained dependent commands
setTimeout(() => {
props.deleteSession()
}, 300)
props.cleanUpAndExit()
}
return (
<Flex
Expand Down
4 changes: 2 additions & 2 deletions app/src/components/CalibrationPanels/DeckSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ const DECK_SETUP_BUTTON_TEXT = 'Confirm placement and continue'
export function DeckSetup(props: CalibrationPanelProps): React.Node {
const deckDef = React.useMemo(() => getDeckDefinitions()['ot2_standard'], [])

const { tipRack, sendSessionCommand } = props
const { tipRack, sendCommands } = props

const proceed = () => {
sendSessionCommand(Sessions.sharedCalCommands.MOVE_TO_TIP_RACK)
sendCommands({ command: Sessions.sharedCalCommands.MOVE_TO_TIP_RACK })
}

return (
Expand Down
4 changes: 2 additions & 2 deletions app/src/components/CalibrationPanels/Introduction.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ const contentsBySessionType: {
}

export function Introduction(props: CalibrationPanelProps): React.Node {
const { tipRack, sendSessionCommand, sessionType } = props
const { tipRack, sendCommands, sessionType } = props

const { showConfirmation, confirm: proceed, cancel } = useConditionalConfirm(
() => {
sendSessionCommand(Sessions.sharedCalCommands.LOAD_LABWARE)
sendCommands({ command: Sessions.sharedCalCommands.LOAD_LABWARE })
},
true
)
Expand Down
22 changes: 11 additions & 11 deletions app/src/components/CalibrationPanels/SaveXYPoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ const contentsBySessionTypeByCurrentStep: {
}

export function SaveXYPoint(props: CalibrationPanelProps): React.Node {
const { isMulti, mount, sendSessionCommand, currentStep, sessionType } = props
const { isMulti, mount, sendCommands, currentStep, sessionType } = props

const {
slotNumber,
Expand All @@ -136,21 +136,21 @@ export function SaveXYPoint(props: CalibrationPanelProps): React.Node {
)

const jog = (axis: JogAxis, dir: JogDirection, step: JogStep) => {
sendSessionCommand(
Sessions.deckCalCommands.JOG,
{
sendCommands({
command: Sessions.sharedCalCommands.JOG,
data: {
vector: formatJogVector(axis, dir, step),
},
false
)
})
}

const savePoint = () => {
sendSessionCommand(Sessions.sharedCalCommands.SAVE_OFFSET)
// TODO: IMMEDIATELY use actualy epic for managing chained dependent commands
setTimeout(() => {
moveCommandString && sendSessionCommand(moveCommandString)
}, 300)
let commands = [{ command: Sessions.sharedCalCommands.SAVE_OFFSET }]

if (moveCommandString) {
commands = [...commands, { command: moveCommandString }]
}
sendCommands(...commands)
}

return (
Expand Down
20 changes: 9 additions & 11 deletions app/src/components/CalibrationPanels/SaveZPoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const contentsBySessionType: {
}

export function SaveZPoint(props: CalibrationPanelProps): React.Node {
const { isMulti, mount, sendSessionCommand, sessionType } = props
const { isMulti, mount, sendCommands, sessionType } = props

const { headerText, buttonText } = contentsBySessionType[sessionType]

Expand All @@ -79,21 +79,19 @@ export function SaveZPoint(props: CalibrationPanelProps): React.Node {
)

const jog = (axis: JogAxis, dir: JogDirection, step: JogStep) => {
sendSessionCommand(
Sessions.deckCalCommands.JOG,
{
sendCommands({
command: Sessions.sharedCalCommands.JOG,
data: {
vector: formatJogVector(axis, dir, step),
},
false
)
})
}

const savePoint = () => {
sendSessionCommand(Sessions.sharedCalCommands.SAVE_OFFSET)
// TODO: IMMEDIATELY use actualy epic for managing chained dependent commands
setTimeout(() => {
sendSessionCommand(Sessions.sharedCalCommands.MOVE_TO_POINT_ONE)
}, 300)
sendCommands(
{ command: Sessions.sharedCalCommands.SAVE_OFFSET },
{ command: Sessions.sharedCalCommands.MOVE_TO_POINT_ONE }
)
}

return (
Expand Down
6 changes: 3 additions & 3 deletions app/src/components/CalibrationPanels/TipConfirmation.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ const CONFIRM_TIP_YES_BUTTON_TEXT = 'Yes, move to slot 5'
const CONFIRM_TIP_NO_BUTTON_TEXT = 'No, try again'

export function TipConfirmation(props: CalibrationPanelProps): React.Node {
const { sendSessionCommand } = props
const { sendCommands } = props

const invalidateTip = () => {
sendSessionCommand(Sessions.sharedCalCommands.INVALIDATE_TIP)
sendCommands({ command: Sessions.sharedCalCommands.INVALIDATE_TIP })
}
const confirmTip = () => {
sendSessionCommand(Sessions.sharedCalCommands.MOVE_TO_DECK)
sendCommands({ command: Sessions.sharedCalCommands.MOVE_TO_DECK })
}

return (
Expand Down
13 changes: 6 additions & 7 deletions app/src/components/CalibrationPanels/TipPickUp.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const ASSET_MAP = {
single: singleDemoAsset,
}
export function TipPickUp(props: CalibrationPanelProps): React.Node {
const { sendSessionCommand, tipRack, isMulti } = props
const { sendCommands, tipRack, isMulti } = props

const tipRackDef = React.useMemo(
() => getLatestLabwareDef(tipRack?.loadName),
Expand All @@ -70,17 +70,16 @@ export function TipPickUp(props: CalibrationPanelProps): React.Node {
)

const pickUpTip = () => {
sendSessionCommand(Sessions.sharedCalCommands.PICK_UP_TIP)
sendCommands({ command: Sessions.sharedCalCommands.PICK_UP_TIP })
}

const jog = (axis: JogAxis, dir: JogDirection, step: JogStep) => {
sendSessionCommand(
Sessions.sharedCalCommands.JOG,
{
sendCommands({
command: Sessions.sharedCalCommands.JOG,
data: {
vector: formatJogVector(axis, dir, step),
},
false
)
})
}

return (
Expand Down
Loading