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.
Opentrons · Oct 16, 2020 · d5ae8cd · d5ae8cd
1 parent d51b958
commit d5ae8cd
Show file tree

Hide file tree

Showing 12 changed files with 206 additions and 22 deletions.
diff --git 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(
         <CalibrateDeck
           robotName="robot-name"
-          session={mockDeckCalSession}
+          session={session}
           closeWizard={() => {}}
-          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
@@ -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<SessionCommandParams>) {
-    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
@@ -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
@@ -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(
         <CalibratePipetteOffset
           robotName="robot-name"
-          session={mockPipOffsetCalSession}
+          session={session}
           closeWizard={jest.fn()}
-          dispatchRequests={jest.fn()}
+          dispatchRequests={dispatchRequests}
           showSpinner={showSpinner}
+          isJogging={isJogging}
         />,
         {
           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
@@ -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<SessionCommandParams>) {
-    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
@@ -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
@@ -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) => {
@@ -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
 
@@ -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<PipetteOffsetCalibrationSessionParams> | void = {}
+  ) => {
     dispatchRequests(
       Sessions.ensureSession(
         robotName,
@@ -132,6 +148,7 @@ export function useCalibratePipetteOffset(
           closeWizard={closeWizard}
           showSpinner={showSpinner}
           dispatchRequests={dispatchRequests}
+          isJogging={isJogging}
         />
       </Portal>
     ) : null,

diff --git 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(
         <CalibrateTipLength
           robotName="robot-name"
-          session={mockTipLengthSession}
+          session={session}
           closeWizard={() => {}}
-          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
@@ -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<SessionCommandParams>) {
-    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
@@ -11,4 +11,5 @@ export type CalibrateTipLengthParentProps = {|
     ...Array<{ ...Action, meta: { requestId: string } }>
   ) => void,
   showSpinner: boolean,
+  isJogging: boolean,
 |}