From d5ae8cddcf0701debd6f01f5d80a886e9df1f46a Mon Sep 17 00:00:00 2001 From: Brian Arthur Cooper Date: Fri, 16 Oct 2020 15:54:34 -0400 Subject: [PATCH] fix(app): debounce calibration jog requests while others are in flight (#6794) In order to prevent jog commands from queueing up, which can be sometimes considerable given network latency, do not register jog commands that are fired while another is still bending on the robot. Note: at the moment this is not attached to a visual change in the ui, as the requests generally clear in under 2 seconds, and it is often considered bad UX to change the visual for any action that takes less than two seconds. --- .../__tests__/CalibrateDeck.test.js | 50 ++++++++++++++++- app/src/components/CalibrateDeck/index.js | 3 +- app/src/components/CalibrateDeck/types.js | 1 + .../__tests__/CalibratePipetteOffset.test.js | 56 ++++++++++++++++--- .../CalibratePipetteOffset/index.js | 4 +- .../CalibratePipetteOffset/types.js | 1 + .../useCalibratePipetteOffset.js | 27 +++++++-- .../__tests__/CalibrateTipLength.test.js | 51 ++++++++++++++++- .../components/CalibrateTipLength/index.js | 3 +- .../components/CalibrateTipLength/types.js | 1 + .../RobotSettings/DeckCalibrationControl.js | 15 +++++ .../Calibrate/CalibrateTipLengthControl.js | 16 ++++++ 12 files changed, 206 insertions(+), 22 deletions(-) diff --git a/app/src/components/CalibrateDeck/__tests__/CalibrateDeck.test.js b/app/src/components/CalibrateDeck/__tests__/CalibrateDeck.test.js index ae0cf1a7849..24f9b5f0541 100644 --- a/app/src/components/CalibrateDeck/__tests__/CalibrateDeck.test.js +++ b/app/src/components/CalibrateDeck/__tests__/CalibrateDeck.test.js @@ -42,6 +42,7 @@ describe('CalibrateDeck', () => { let mockStore let render let dispatch + let dispatchRequests let mockDeckCalSession: Sessions.DeckCalibrationSession = { id: 'fake_session_id', ...mockDeckCalibrationSessionAttributes, @@ -74,6 +75,7 @@ describe('CalibrateDeck', () => { beforeEach(() => { dispatch = jest.fn() + dispatchRequests = jest.fn() mockStore = { subscribe: () => {}, getState: () => ({ @@ -89,14 +91,19 @@ describe('CalibrateDeck', () => { } render = (props = {}) => { - const { showSpinner = false } = props + const { + showSpinner = false, + isJogging = false, + session = mockDeckCalSession, + } = props return mount( {}} - dispatchRequests={jest.fn()} + dispatchRequests={dispatchRequests} showSpinner={showSpinner} + isJogging={isJogging} />, { wrappingComponent: Provider, @@ -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] }, + }) + ) + }) }) diff --git a/app/src/components/CalibrateDeck/index.js b/app/src/components/CalibrateDeck/index.js index 7d00e306f00..ad150ba4d50 100644 --- a/app/src/components/CalibrateDeck/index.js +++ b/app/src/components/CalibrateDeck/index.js @@ -98,6 +98,7 @@ export function CalibrateDeck(props: CalibrateDeckParentProps): React.Node { closeWizard, dispatchRequests, showSpinner, + isJogging, } = props const { currentStep, instrument, labware } = session?.details || {} @@ -115,7 +116,7 @@ export function CalibrateDeck(props: CalibrateDeckParentProps): React.Node { }, [instrument]) function sendCommands(...commands: Array) { - if (session?.id) { + if (session?.id && !isJogging) { const sessionCommandActions = commands.map(c => Sessions.createSessionCommand(robotName, session.id, { command: c.command, diff --git a/app/src/components/CalibrateDeck/types.js b/app/src/components/CalibrateDeck/types.js index 3294fa8594d..19c6e14efed 100644 --- a/app/src/components/CalibrateDeck/types.js +++ b/app/src/components/CalibrateDeck/types.js @@ -10,4 +10,5 @@ export type CalibrateDeckParentProps = {| ...Array<{ ...Action, meta: { requestId: string } }> ) => void, showSpinner: boolean, + isJogging: boolean, |} diff --git a/app/src/components/CalibratePipetteOffset/__tests__/CalibratePipetteOffset.test.js b/app/src/components/CalibratePipetteOffset/__tests__/CalibratePipetteOffset.test.js index 5fe639a053f..896af13c198 100644 --- a/app/src/components/CalibratePipetteOffset/__tests__/CalibratePipetteOffset.test.js +++ b/app/src/components/CalibratePipetteOffset/__tests__/CalibratePipetteOffset.test.js @@ -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') @@ -72,6 +70,7 @@ describe('CalibratePipetteOffset', () => { beforeEach(() => { dispatch = jest.fn() + dispatchRequests = jest.fn() mockStore = { subscribe: () => {}, getState: () => ({ @@ -87,14 +86,19 @@ describe('CalibratePipetteOffset', () => { } render = (props = {}) => { - const { showSpinner = false } = props + const { + showSpinner = false, + isJogging = false, + session = mockPipOffsetCalSession, + } = props return mount( , { wrappingComponent: Provider, @@ -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] }, + }) + ) + }) }) diff --git a/app/src/components/CalibratePipetteOffset/index.js b/app/src/components/CalibratePipetteOffset/index.js index 37be3654b5b..0dc4e60533c 100644 --- a/app/src/components/CalibratePipetteOffset/index.js +++ b/app/src/components/CalibratePipetteOffset/index.js @@ -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 { @@ -120,7 +120,7 @@ export function CalibratePipetteOffset( }, [instrument]) function sendCommands(...commands: Array) { - if (session?.id) { + if (session?.id && !isJogging) { const sessionCommandActions = commands.map(c => Sessions.createSessionCommand(robotName, session.id, { command: c.command, diff --git a/app/src/components/CalibratePipetteOffset/types.js b/app/src/components/CalibratePipetteOffset/types.js index 3724c4565e8..0a7572f5571 100644 --- a/app/src/components/CalibratePipetteOffset/types.js +++ b/app/src/components/CalibratePipetteOffset/types.js @@ -16,6 +16,7 @@ export type CalibratePipetteOffsetParentProps = {| ...Array<{ ...Action, meta: { requestId: string } }> ) => void, showSpinner: boolean, + isJogging: boolean, |} export type CalibratePipetteOffsetChildProps = {| diff --git a/app/src/components/CalibratePipetteOffset/useCalibratePipetteOffset.js b/app/src/components/CalibratePipetteOffset/useCalibratePipetteOffset.js index 223055ec33e..6b2a2512e06 100644 --- a/app/src/components/CalibratePipetteOffset/useCalibratePipetteOffset.js +++ b/app/src/components/CalibratePipetteOffset/useCalibratePipetteOffset.js @@ -33,9 +33,10 @@ export function useCalibratePipetteOffset( ): [Invoker, React.Node | null] { const [showWizard, setShowWizard] = React.useState(false) - const trackedRequestId = React.useRef(null) const deleteRequestId = React.useRef(null) const createRequestId = React.useRef(null) + const jogRequestId = React.useRef(null) + const spinnerRequestId = React.useRef(null) const pipOffsetCalSession: PipetteOffsetCalibrationSession | null = useSelector( (state: State) => { @@ -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 => - trackedRequestId.current - ? RobotApi.getRequestById(state, trackedRequestId.current) + spinnerRequestId.current + ? RobotApi.getRequestById(state, spinnerRequestId.current) : null )?.status === RobotApi.PENDING @@ -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) @@ -106,7 +120,9 @@ export function useCalibratePipetteOffset( hasCalibrationBlock = false, tipRackDefinition = null, } = sessionParams - const handleStartPipOffsetCalSession: Invoker = overrideParams => { + const handleStartPipOffsetCalSession: Invoker = ( + overrideParams: $Shape | void = {} + ) => { dispatchRequests( Sessions.ensureSession( robotName, @@ -132,6 +148,7 @@ export function useCalibratePipetteOffset( closeWizard={closeWizard} showSpinner={showSpinner} dispatchRequests={dispatchRequests} + isJogging={isJogging} /> ) : null, diff --git a/app/src/components/CalibrateTipLength/__tests__/CalibrateTipLength.test.js b/app/src/components/CalibrateTipLength/__tests__/CalibrateTipLength.test.js index 00539501312..0bf14a2e60e 100644 --- a/app/src/components/CalibrateTipLength/__tests__/CalibrateTipLength.test.js +++ b/app/src/components/CalibrateTipLength/__tests__/CalibrateTipLength.test.js @@ -42,6 +42,7 @@ describe('CalibrateTipLength', () => { let mockStore let render let dispatch + let dispatchRequests let mockTipLengthSession: Sessions.TipLengthCalibrationSession = { id: 'fake_session_id', ...mockTipLengthCalibrationSessionAttributes, @@ -72,6 +73,7 @@ describe('CalibrateTipLength', () => { beforeEach(() => { dispatch = jest.fn() + dispatchRequests = jest.fn() mockStore = { subscribe: () => {}, getState: () => ({ @@ -87,14 +89,19 @@ describe('CalibrateTipLength', () => { } render = (props = {}) => { - const { showSpinner = false } = props + const { + showSpinner = false, + isJogging = false, + session = mockTipLengthSession, + } = props return mount( {}} - dispatchRequests={jest.fn()} + dispatchRequests={dispatchRequests} showSpinner={showSpinner} + isJogging={isJogging} />, { wrappingComponent: Provider, @@ -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] }, + }) + ) + }) }) diff --git a/app/src/components/CalibrateTipLength/index.js b/app/src/components/CalibrateTipLength/index.js index 589c092af6d..85b6f891731 100644 --- a/app/src/components/CalibrateTipLength/index.js +++ b/app/src/components/CalibrateTipLength/index.js @@ -99,6 +99,7 @@ export function CalibrateTipLength( closeWizard, showSpinner, dispatchRequests, + isJogging, } = props const { currentStep, instrument, labware } = session?.details || {} @@ -114,7 +115,7 @@ export function CalibrateTipLength( : null function sendCommands(...commands: Array) { - if (session?.id) { + if (session?.id && !isJogging) { const sessionCommandActions = commands.map(c => Sessions.createSessionCommand(robotName, session.id, { command: c.command, diff --git a/app/src/components/CalibrateTipLength/types.js b/app/src/components/CalibrateTipLength/types.js index 39c5be30b57..5d3e8c56ff4 100644 --- a/app/src/components/CalibrateTipLength/types.js +++ b/app/src/components/CalibrateTipLength/types.js @@ -11,4 +11,5 @@ export type CalibrateTipLengthParentProps = {| ...Array<{ ...Action, meta: { requestId: string } }> ) => void, showSpinner: boolean, + isJogging: boolean, |} diff --git a/app/src/components/RobotSettings/DeckCalibrationControl.js b/app/src/components/RobotSettings/DeckCalibrationControl.js index 9b2e064e65a..2d1ee512adb 100644 --- a/app/src/components/RobotSettings/DeckCalibrationControl.js +++ b/app/src/components/RobotSettings/DeckCalibrationControl.js @@ -82,6 +82,7 @@ export function DeckCalibrationControl(props: Props): React.Node { const trackedRequestId = React.useRef(null) const deleteRequestId = React.useRef(null) const createRequestId = React.useRef(null) + const jogRequestId = React.useRef(null) const [dispatchRequests] = RobotApi.useDispatchApiRequests( dispatchedAction => { @@ -92,6 +93,12 @@ export function DeckCalibrationControl(props: Props): React.Node { deckCalibrationSession?.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( @@ -124,6 +131,13 @@ export function DeckCalibrationControl(props: Props): React.Node { : null )?.status === RobotApi.SUCCESS + const isJogging = + useSelector((state: State) => + jogRequestId.current + ? RobotApi.getRequestById(state, jogRequestId.current) + : null + )?.status === RobotApi.PENDING + React.useEffect(() => { if (shouldOpen) { setShowWizard(true) @@ -214,6 +228,7 @@ export function DeckCalibrationControl(props: Props): React.Node { closeWizard={() => setShowWizard(false)} dispatchRequests={dispatchRequests} showSpinner={showSpinner} + isJogging={isJogging} /> )} diff --git a/app/src/pages/Calibrate/CalibrateTipLengthControl.js b/app/src/pages/Calibrate/CalibrateTipLengthControl.js index 292eaf0a1a4..115cd042199 100644 --- a/app/src/pages/Calibrate/CalibrateTipLengthControl.js +++ b/app/src/pages/Calibrate/CalibrateTipLengthControl.js @@ -58,6 +58,7 @@ export function CalibrateTipLengthControl({ const trackedRequestId = React.useRef(null) const deleteRequestId = React.useRef(null) const createRequestId = React.useRef(null) + const jogRequestId = React.useRef(null) const [dispatchRequests] = RobotApi.useDispatchApiRequests( dispatchedAction => { @@ -68,6 +69,12 @@ export function CalibrateTipLengthControl({ calibrationSession?.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( @@ -142,6 +149,13 @@ export function CalibrateTipLengthControl({ : null )?.status === RobotApi.SUCCESS + const isJogging = + useSelector((state: State) => + jogRequestId.current + ? RobotApi.getRequestById(state, jogRequestId.current) + : null + )?.status === RobotApi.PENDING + React.useEffect(() => { if (shouldOpen) { setShowWizard(true) @@ -201,6 +215,7 @@ export function CalibrateTipLengthControl({ closeWizard={handleCloseWizard} showSpinner={showSpinner} dispatchRequests={dispatchRequests} + isJogging={isJogging} /> ) : ( )}