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): debounce calibration jog requests while others are in flight #6794

Merged
merged 3 commits into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions app/src/components/CalibrateDeck/__tests__/CalibrateDeck.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('CalibrateDeck', () => {
let mockStore
let render
let dispatch
let dispatchRequests
let mockDeckCalSession: Sessions.DeckCalibrationSession = {
id: 'fake_session_id',
...mockDeckCalibrationSessionAttributes,
Expand Down Expand Up @@ -74,6 +75,7 @@ describe('CalibrateDeck', () => {

beforeEach(() => {
dispatch = jest.fn()
dispatchRequests = jest.fn()
mockStore = {
subscribe: () => {},
getState: () => ({
Expand All @@ -89,14 +91,19 @@ describe('CalibrateDeck', () => {
}

render = (props = {}) => {
const { showSpinner = false } = props
const {
showSpinner = false,
isJogging = false,
session = mockDeckCalSession,
} = props
return mount(
<CalibrateDeck
robotName="robot-name"
session={mockDeckCalSession}
session={session}
closeWizard={() => {}}
dispatchRequests={jest.fn()}
dispatchRequests={dispatchRequests}
showSpinner={showSpinner}
isJogging={isJogging}
/>,
{
wrappingComponent: Provider,
Expand Down Expand Up @@ -149,4 +156,41 @@ describe('CalibrateDeck', () => {
const wrapper = render({ showSpinner: true })
expect(wrapper.find('SpinnerModalPage').exists()).toBe(true)
})
it('does dispatch jog requests when not isJogging', () => {
const session = {
id: 'fake_session_id',
...mockDeckCalibrationSessionAttributes,
details: {
...mockDeckCalibrationSessionAttributes.details,
currentStep: Sessions.DECK_STEP_PREPARING_PIPETTE,
},
}
const wrapper = render({ isJogging: false, session })
wrapper.find('button[title="forward"]').invoke('onClick')()
expect(dispatchRequests).toHaveBeenCalledWith(
Sessions.createSessionCommand('robot-name', session.id, {
command: Sessions.sharedCalCommands.JOG,
data: { vector: [0, -0.1, 0] },
})
)
})

it('does not dispatch jog requests when isJogging', () => {
const session = {
id: 'fake_session_id',
...mockDeckCalibrationSessionAttributes,
details: {
...mockDeckCalibrationSessionAttributes.details,
currentStep: Sessions.DECK_STEP_PREPARING_PIPETTE,
},
}
const wrapper = render({ isJogging: true, session })
wrapper.find('button[title="forward"]').invoke('onClick')()
expect(dispatchRequests).not.toHaveBeenCalledWith(
Sessions.createSessionCommand('robot-name', session.id, {
command: Sessions.sharedCalCommands.JOG,
data: { vector: [0, -0.1, 0] },
})
)
})
})
3 changes: 2 additions & 1 deletion app/src/components/CalibrateDeck/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export function CalibrateDeck(props: CalibrateDeckParentProps): React.Node {
closeWizard,
dispatchRequests,
showSpinner,
isJogging,
} = props
const { currentStep, instrument, labware } = session?.details || {}

Expand All @@ -115,7 +116,7 @@ export function CalibrateDeck(props: CalibrateDeckParentProps): React.Node {
}, [instrument])

function sendCommands(...commands: Array<SessionCommandParams>) {
if (session?.id) {
if (session?.id && !isJogging) {
const sessionCommandActions = commands.map(c =>
Sessions.createSessionCommand(robotName, session.id, {
command: c.command,
Expand Down
1 change: 1 addition & 0 deletions app/src/components/CalibrateDeck/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ export type CalibrateDeckParentProps = {|
...Array<{ ...Action, meta: { requestId: string } }>
) => void,
showSpinner: boolean,
isJogging: boolean,
|}
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,8 @@ describe('CalibratePipetteOffset', () => {
let mockStore
let render
let dispatch
let mockPipOffsetCalSession: Sessions.PipetteOffsetCalibrationSession = {
id: 'fake_session_id',
...mockPipetteOffsetCalibrationSessionAttributes,
}
let dispatchRequests
let mockPipOffsetCalSession: Sessions.PipetteOffsetCalibrationSession

const getExitButton = wrapper =>
wrapper.find({ title: 'exit' }).find('button')
Expand All @@ -72,6 +70,7 @@ describe('CalibratePipetteOffset', () => {

beforeEach(() => {
dispatch = jest.fn()
dispatchRequests = jest.fn()
mockStore = {
subscribe: () => {},
getState: () => ({
Expand All @@ -87,14 +86,19 @@ describe('CalibratePipetteOffset', () => {
}

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

it('does dispatch jog requests when not isJogging', () => {
const session = {
id: 'fake_session_id',
...mockPipetteOffsetCalibrationSessionAttributes,
details: {
...mockPipetteOffsetCalibrationSessionAttributes.details,
currentStep: Sessions.PIP_OFFSET_STEP_PREPARING_PIPETTE,
},
}
const wrapper = render({ isJogging: false, session })
wrapper.find('button[title="forward"]').invoke('onClick')()
expect(dispatchRequests).toHaveBeenCalledWith(
Sessions.createSessionCommand('robot-name', session.id, {
command: Sessions.sharedCalCommands.JOG,
data: { vector: [0, -0.1, 0] },
})
)
})

it('does not dispatch jog requests when isJogging', () => {
const session = {
id: 'fake_session_id',
...mockPipetteOffsetCalibrationSessionAttributes,
details: {
...mockPipetteOffsetCalibrationSessionAttributes.details,
currentStep: Sessions.PIP_OFFSET_STEP_PREPARING_PIPETTE,
},
}
const wrapper = render({ isJogging: true, session })
wrapper.find('button[title="forward"]').invoke('onClick')()
expect(dispatchRequests).not.toHaveBeenCalledWith(
Sessions.createSessionCommand('robot-name', session.id, {
command: Sessions.sharedCalCommands.JOG,
data: { vector: [0, -0.1, 0] },
})
)
})
})
4 changes: 2 additions & 2 deletions app/src/components/CalibratePipetteOffset/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const PANEL_STYLE_PROPS_BY_STEP: {
export function CalibratePipetteOffset(
props: CalibratePipetteOffsetParentProps
): React.Node {
const { session, robotName, dispatchRequests, showSpinner } = props
const { session, robotName, dispatchRequests, showSpinner, isJogging } = props
const { currentStep, instrument, labware } = session?.details || {}

const {
Expand All @@ -120,7 +120,7 @@ export function CalibratePipetteOffset(
}, [instrument])

function sendCommands(...commands: Array<SessionCommandParams>) {
if (session?.id) {
if (session?.id && !isJogging) {
const sessionCommandActions = commands.map(c =>
Sessions.createSessionCommand(robotName, session.id, {
command: c.command,
Expand Down
1 change: 1 addition & 0 deletions app/src/components/CalibratePipetteOffset/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type CalibratePipetteOffsetParentProps = {|
...Array<{ ...Action, meta: { requestId: string } }>
) => void,
showSpinner: boolean,
isJogging: boolean,
|}

export type CalibratePipetteOffsetChildProps = {|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ export function useCalibratePipetteOffset(
): [Invoker, React.Node | null] {
const [showWizard, setShowWizard] = React.useState(false)

const trackedRequestId = React.useRef<string | null>(null)
const deleteRequestId = React.useRef<string | null>(null)
const createRequestId = React.useRef<string | null>(null)
const jogRequestId = React.useRef<string | null>(null)
const spinnerRequestId = React.useRef<string | null>(null)

const pipOffsetCalSession: PipetteOffsetCalibrationSession | null = useSelector(
(state: State) => {
Expand All @@ -52,21 +53,27 @@ export function useCalibratePipetteOffset(
pipOffsetCalSession?.id === dispatchedAction.payload.sessionId
) {
deleteRequestId.current = dispatchedAction.meta.requestId
} else if (
dispatchedAction.type === Sessions.CREATE_SESSION_COMMAND &&
dispatchedAction.payload.command.command ===
Sessions.sharedCalCommands.JOG
) {
jogRequestId.current = dispatchedAction.meta.requestId
} else if (
dispatchedAction.type !== Sessions.CREATE_SESSION_COMMAND ||
!spinnerCommandBlockList.includes(
dispatchedAction.payload.command.command
)
) {
trackedRequestId.current = dispatchedAction.meta.requestId
spinnerRequestId.current = dispatchedAction.meta.requestId
}
}
)

const showSpinner =
useSelector<State, RequestState | null>(state =>
trackedRequestId.current
? RobotApi.getRequestById(state, trackedRequestId.current)
spinnerRequestId.current
? RobotApi.getRequestById(state, spinnerRequestId.current)
: null
)?.status === RobotApi.PENDING

Expand All @@ -84,6 +91,13 @@ export function useCalibratePipetteOffset(
: null
)?.status === RobotApi.SUCCESS

const isJogging =
useSelector((state: State) =>
jogRequestId.current
? RobotApi.getRequestById(state, jogRequestId.current)
: null
)?.status === RobotApi.PENDING

const closeWizard = React.useCallback(() => {
onComplete && onComplete()
setShowWizard(false)
Expand All @@ -106,7 +120,9 @@ export function useCalibratePipetteOffset(
hasCalibrationBlock = false,
tipRackDefinition = null,
} = sessionParams
const handleStartPipOffsetCalSession: Invoker = overrideParams => {
const handleStartPipOffsetCalSession: Invoker = (
overrideParams: $Shape<PipetteOffsetCalibrationSessionParams> | void = {}
) => {
dispatchRequests(
Sessions.ensureSession(
robotName,
Expand All @@ -132,6 +148,7 @@ export function useCalibratePipetteOffset(
closeWizard={closeWizard}
showSpinner={showSpinner}
dispatchRequests={dispatchRequests}
isJogging={isJogging}
/>
</Portal>
) : null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('CalibrateTipLength', () => {
let mockStore
let render
let dispatch
let dispatchRequests
let mockTipLengthSession: Sessions.TipLengthCalibrationSession = {
id: 'fake_session_id',
...mockTipLengthCalibrationSessionAttributes,
Expand Down Expand Up @@ -72,6 +73,7 @@ describe('CalibrateTipLength', () => {

beforeEach(() => {
dispatch = jest.fn()
dispatchRequests = jest.fn()
mockStore = {
subscribe: () => {},
getState: () => ({
Expand All @@ -87,14 +89,19 @@ describe('CalibrateTipLength', () => {
}

render = (props = {}) => {
const { showSpinner = false } = props
const {
showSpinner = false,
isJogging = false,
session = mockTipLengthSession,
} = props
return mount(
<CalibrateTipLength
robotName="robot-name"
session={mockTipLengthSession}
session={session}
closeWizard={() => {}}
dispatchRequests={jest.fn()}
dispatchRequests={dispatchRequests}
showSpinner={showSpinner}
isJogging={isJogging}
/>,
{
wrappingComponent: Provider,
Expand Down Expand Up @@ -147,4 +154,42 @@ describe('CalibrateTipLength', () => {
const wrapper = render({ showSpinner: true })
expect(wrapper.find('SpinnerModalPage').exists()).toBe(true)
})

it('does dispatch jog requests when not isJogging', () => {
const session = {
id: 'fake_session_id',
...mockTipLengthCalibrationSessionAttributes,
details: {
...mockTipLengthCalibrationSessionAttributes.details,
currentStep: Sessions.TIP_LENGTH_STEP_PREPARING_PIPETTE,
},
}
const wrapper = render({ isJogging: false, session })
wrapper.find('button[title="forward"]').invoke('onClick')()
expect(dispatchRequests).toHaveBeenCalledWith(
Sessions.createSessionCommand('robot-name', session.id, {
command: Sessions.sharedCalCommands.JOG,
data: { vector: [0, -0.1, 0] },
})
)
})

it('does not dispatch jog requests when isJogging', () => {
const session = {
id: 'fake_session_id',
...mockTipLengthCalibrationSessionAttributes,
details: {
...mockTipLengthCalibrationSessionAttributes.details,
currentStep: Sessions.TIP_LENGTH_STEP_PREPARING_PIPETTE,
},
}
const wrapper = render({ isJogging: true, session })
wrapper.find('button[title="forward"]').invoke('onClick')()
expect(dispatchRequests).not.toHaveBeenCalledWith(
Sessions.createSessionCommand('robot-name', session.id, {
command: Sessions.sharedCalCommands.JOG,
data: { vector: [0, -0.1, 0] },
})
)
})
})
3 changes: 2 additions & 1 deletion app/src/components/CalibrateTipLength/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export function CalibrateTipLength(
closeWizard,
showSpinner,
dispatchRequests,
isJogging,
} = props
const { currentStep, instrument, labware } = session?.details || {}

Expand All @@ -114,7 +115,7 @@ export function CalibrateTipLength(
: null

function sendCommands(...commands: Array<SessionCommandParams>) {
if (session?.id) {
if (session?.id && !isJogging) {
const sessionCommandActions = commands.map(c =>
Sessions.createSessionCommand(robotName, session.id, {
command: c.command,
Expand Down
1 change: 1 addition & 0 deletions app/src/components/CalibrateTipLength/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ export type CalibrateTipLengthParentProps = {|
...Array<{ ...Action, meta: { requestId: string } }>
) => void,
showSpinner: boolean,
isJogging: boolean,
|}
Loading