fix(app): fix attachment flow firmware step filter (#13823) (#13902)

Closes RQA-1827

* fix(app): fix attachment flow firmware step filter

Fixes the logic for determining whether or not the firmware step is necessary during an attachment
flow. Because it's not possible to check whether an instrument or module needs a firmware update
until it is attached, do not filter out the step entirely. Instead, if the instrument is known to
have the latest firmware, gracefully proceed to the next step.

app/src/assets/localization/en/gripper_wizard_flows.json

@@ -12,6 +12,7 @@
   "continue_calibration": "Continue calibration",
   "detach_gripper": "Detach Gripper",
   "firmware_updating": "A firmware update is required, instrument is updating...",
+  "firmware_up_to_date": "Firmware is up to date.",
   "get_started": "Get started",
   "gripper_calibration": "Gripper Calibration",
   "gripper_recalibration": "Gripper Recalibration",

app/src/assets/localization/en/module_wizard_flows.json

@@ -20,6 +20,7 @@
   "error_prepping_module": "Error prepping module for calibration",
   "exit": "Exit",
   "firmware_updated": "{{module}} firmware updated!",
+  "firmware_up_to_date": "{{module}} firmware up to date.",
   "get_started": "<block>To get started, remove labware from the deck and clean up the working area to make the calibration easier. Also gather the needed equipment shown to the right.</block><block>The calibration adapter came with your module. The pipette probe came with your Flex pipette.</block>",
   "install_probe_8_channel": "<block>Take the calibration probe from its storage location. Ensure its collar is unlocked. Push the pipette ejector up and press the probe firmly onto the <strong>backmost</strong> pipette nozzle. Twist the collar to lock the probe. Test that the probe is secure by gently pulling it back and forth.</block>",
   "install_probe_96_channel": "<block>Take the calibration probe from its storage location. Ensure its collar is unlocked. Push the pipette ejector up and press the probe firmly onto the <strong>A1 (back left corner)</strong> pipette nozzle. Twist the collar to lock the probe. Test that the probe is secure by gently pulling it back and forth.</block>",

app/src/assets/localization/en/pipette_wizard_flows.json

@@ -38,6 +38,7 @@
   "detach": "Detaching Pipette",
   "exit_cal": "Exit calibration",
   "firmware_updating": "A firmware update is required, instrument is updating...",
+  "firmware_up_to_date": "Firmware is up to date.",
   "gantry_empty_for_96_channel_success": "Now that both mounts are empty, you can begin the 96-Channel Pipette attachment process.",
   "get_started_detach": "<block>To get started, remove labware from the deck and clean up the working area to make detachment easier. Also gather the needed equipment shown to the right.</block>",
   "grab_screwdriver": "While continuing to hold in place, grab your 2.5mm driver and tighten screws as shown in the animation. Test the pipette attachment by giving it a wiggle before pressing continue",

app/src/organisms/Devices/InstrumentsAndModules.tsx

@@ -153,6 +153,8 @@ export function InstrumentsAndModules({
             subsystem={subsystemToUpdate}
             proceed={() => setSubsystemToUpdate(null)}
             description={t('updating_firmware')}
+            proceedDescription={t('firmware_up_to_date')}
+            isOnDevice={false}
           />
         </ModalShell>
       )}

app/src/organisms/FirmwareUpdateModal/__tests__/FirmwareUpdateModal.test.tsx

@@ -41,6 +41,8 @@ describe('FirmwareUpdateModal', () => {
       proceed: jest.fn(),
       description: 'A firmware update is required, instrument is updating',
       subsystem: 'pipette_left',
+      proceedDescription: 'Firmware is up to date.',
+      isOnDevice: true,
     }
     mockUseInstrumentQuery.mockReturnValue({
       data: {
@@ -72,7 +74,7 @@ describe('FirmwareUpdateModal', () => {
       updateSubsystem,
     } as any)
   })
-  it('calls proceed if no update is needed', () => {
+  it('initially renders a spinner and text', () => {
     mockUseInstrumentQuery.mockReturnValue({
       data: {
         data: [
@@ -82,15 +84,81 @@ describe('FirmwareUpdateModal', () => {
           } as PipetteData,
         ],
       },
+      refetch,
+    } as any)
+    mockUseSubsystemUpdateQuery.mockReturnValue({
+      data: {
+        data: {
+          id: 'update id',
+          updateStatus: null,
+        } as any,
+      } as SubsystemUpdateProgressData,
+    } as any)
+    jest.useFakeTimers()
+    const { getByText, getByLabelText } = render(props)
+    getByLabelText('spinner')
+    getByText('Checking for updates...')
+  })
+  it('calls proceed if no update is needed', async () => {
+    mockUseInstrumentQuery.mockReturnValue({
+      data: {
+        data: [
+          {
+            subsystem: 'pipette_left',
+            ok: true,
+          } as PipetteData,
+        ],
+      },
+      refetch,
+    } as any)
+    mockUseSubsystemUpdateQuery.mockReturnValue({
+      data: {
+        data: {
+          id: 'update id',
+          updateStatus: null,
+        } as any,
+      } as SubsystemUpdateProgressData,
     } as any)
-    mockUseSubsystemUpdateQuery.mockReturnValue({} as any)
+    jest.useFakeTimers()
     const { getByText } = render(props)
-    getByText('A firmware update is required, instrument is updating')
-    expect(props.proceed).toHaveBeenCalled()
+    act(() => {
+      jest.advanceTimersByTime(3000)
+    })
+    getByText('Firmware is up to date.')
+    await waitFor(() => expect(props.proceed).toHaveBeenCalled())
+  })
+  it('does not render text or a progress bar until instrument update status is known', () => {
+    mockUseSubsystemUpdateQuery.mockReturnValue({
+      data: {
+        data: {
+          id: 'update id',
+          updateStatus: undefined,
+        } as any,
+      } as SubsystemUpdateProgressData,
+    } as any)
+    mockUseInstrumentQuery.mockReturnValue({
+      data: undefined,
+      refetch,
+    } as any)
+    const { queryByText } = render(props)
+    expect(
+      queryByText('A firmware update is required, instrument is updating')
+    ).not.toBeInTheDocument()
   })
   it('calls update subsystem if update is needed', () => {
-    mockUseSubsystemUpdateQuery.mockReturnValue({} as any)
+    mockUseSubsystemUpdateQuery.mockReturnValue({
+      data: {
+        data: {
+          id: 'update id',
+          updateStatus: 'in progress',
+        } as any,
+      } as SubsystemUpdateProgressData,
+    } as any)
+    jest.useFakeTimers()
     const { getByText } = render(props)
+    act(() => {
+      jest.advanceTimersByTime(3000)
+    })
     getByText('A firmware update is required, instrument is updating')
     expect(updateSubsystem).toHaveBeenCalled()
   })

app/src/organisms/FirmwareUpdateModal/index.tsx

@@ -6,6 +6,7 @@ import {
   TYPOGRAPHY,
   SPACING,
   Flex,
+  Icon,
   RESPONSIVENESS,
   JUSTIFY_CENTER,
   BORDERS,
@@ -22,8 +23,10 @@ import { BadGripper, BadPipette, Subsystem } from '@opentrons/api-client'
 
 interface FirmwareUpdateModalProps {
   description: string
+  proceedDescription: string
   proceed: () => void
   subsystem: Subsystem
+  isOnDevice: boolean
 }
 
 const DESCRIPTION_STYLE = css`
@@ -58,11 +61,27 @@ const OUTER_STYLES = css`
   width: 13.374rem;
 `
 
+const SPINNER_STYLE = css`
+  color: ${COLORS.darkGreyEnabled};
+  opacity: 100%;
+  @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} {
+    color: ${COLORS.darkBlackEnabled};
+    opacity: 70%;
+  }
+`
+
 export const FirmwareUpdateModal = (
   props: FirmwareUpdateModalProps
 ): JSX.Element => {
-  const { proceed, subsystem, description } = props
+  const {
+    proceed,
+    proceedDescription,
+    subsystem,
+    description,
+    isOnDevice,
+  } = props
   const [updateId, setUpdateId] = React.useState('')
+  const [firmwareText, setFirmwareText] = React.useState('')
   const {
     data: attachedInstruments,
     refetch: refetchInstruments,
@@ -79,18 +98,27 @@ export const FirmwareUpdateModal = (
     attachedInstruments?.data?.some(
       (i): i is BadGripper | BadPipette => !i.ok && i.subsystem === subsystem
     ) ?? false
+
   React.useEffect(() => {
-    if (!updateNeeded) {
-      proceed()
-    } else {
-      updateSubsystem(subsystem)
-    }
+    setTimeout(() => {
+      if (!updateNeeded) {
+        setFirmwareText(proceedDescription)
+        setTimeout(() => {
+          proceed()
+        }, 2000)
+      } else {
+        updateSubsystem(subsystem)
+      }
+    }, 2000)
   }, [])
   const { data: updateData } = useSubsystemUpdateQuery(updateId)
   const status = updateData?.data.updateStatus
   const percentComplete = updateData?.data.updateProgress ?? 0
 
   React.useEffect(() => {
+    if ((status != null || updateNeeded) && firmwareText !== description) {
+      setFirmwareText(description)
+    }
     if (status === 'done') {
       refetchInstruments()
         .then(() => {
@@ -108,15 +136,28 @@ export const FirmwareUpdateModal = (
           proceed()
         })
     }
-  }, [status, proceed, refetchInstruments, instrumentToUpdate])
+  }, [status, proceed, refetchInstruments, instrumentToUpdate, updateNeeded])
 
   return (
     <Flex css={MODAL_STYLE}>
-      <StyledText css={DESCRIPTION_STYLE}>{description}</StyledText>
-      <ProgressBar
-        percentComplete={percentComplete}
-        outerStyles={OUTER_STYLES}
-      />
+      <StyledText css={DESCRIPTION_STYLE}>
+        {firmwareText.length ? firmwareText : 'Checking for updates...'}
+      </StyledText>
+      {status != null || updateNeeded ? (
+        <ProgressBar
+          percentComplete={percentComplete}
+          outerStyles={OUTER_STYLES}
+        />
+      ) : null}
+      {firmwareText.length ? null : (
+        <Icon
+          name="ot-spinner"
+          aria-label="spinner"
+          size={isOnDevice ? '6.25rem' : '5.125rem'}
+          css={SPINNER_STYLE}
+          spin
+        />
+      )}
     </Flex>
   )
 }

app/src/organisms/GripperWizardFlows/getGripperWizardSteps.ts

@@ -12,8 +12,7 @@ import {
 import type { GripperWizardStep, GripperWizardFlowType } from './types'
 
 export const getGripperWizardSteps = (
-  flowType: GripperWizardFlowType,
-  requiresFirmwareUpdate: boolean
+  flowType: GripperWizardFlowType
 ): GripperWizardStep[] => {
   switch (flowType) {
     case GRIPPER_FLOW_TYPES.RECALIBRATE: {
@@ -32,7 +31,7 @@ export const getGripperWizardSteps = (
       ]
     }
     case GRIPPER_FLOW_TYPES.ATTACH: {
-      const ALL_STEPS = [
+      return [
         { section: SECTIONS.BEFORE_BEGINNING },
         { section: SECTIONS.MOUNT_GRIPPER },
         { section: SECTIONS.FIRMWARE_UPDATE },
@@ -48,10 +47,6 @@ export const getGripperWizardSteps = (
           successfulAction: SUCCESSFULLY_ATTACHED_AND_CALIBRATED,
         },
       ]
-
-      return requiresFirmwareUpdate
-        ? ALL_STEPS
-        : ALL_STEPS.filter(step => step.section !== SECTIONS.FIRMWARE_UPDATE)
     }
     case GRIPPER_FLOW_TYPES.DETACH: {
       return [

app/src/organisms/GripperWizardFlows/index.tsx

@@ -209,11 +209,7 @@ export const GripperWizard = (
   } = props
   const isOnDevice = useSelector(getIsOnDevice)
   const { t } = useTranslation('gripper_wizard_flows')
-  const requiresFirmwareUpdate = !attachedGripper?.ok
-  const gripperWizardSteps = getGripperWizardSteps(
-    flowType,
-    requiresFirmwareUpdate
-  )
+  const gripperWizardSteps = getGripperWizardSteps(flowType)
   const [currentStepIndex, setCurrentStepIndex] = React.useState<number>(0)
   const [
     frontJawOffset,
@@ -309,6 +305,8 @@ export const GripperWizard = (
         proceed={handleProceed}
         subsystem="gripper"
         description={t('firmware_updating')}
+        proceedDescription={t('firmware_up_to_date')}
+        isOnDevice={isOnDevice}
       />
     )
   } else if (currentStep.section === SECTIONS.UNMOUNT_GRIPPER) {

app/src/organisms/ModuleWizardFlows/getModuleCalibrationSteps.ts

@@ -1,10 +1,8 @@
 import { SECTIONS } from './constants'
 import type { ModuleCalibrationWizardStep } from './types'
 
-export const getModuleCalibrationSteps = (
-  requiresFirmwareUpdate: boolean
-): ModuleCalibrationWizardStep[] => {
-  const ALL_STEPS = [
+export const getModuleCalibrationSteps = (): ModuleCalibrationWizardStep[] => {
+  return [
     { section: SECTIONS.BEFORE_BEGINNING },
     { section: SECTIONS.FIRMWARE_UPDATE },
     { section: SECTIONS.SELECT_LOCATION },
@@ -13,8 +11,4 @@ export const getModuleCalibrationSteps = (
     { section: SECTIONS.DETACH_PROBE },
     { section: SECTIONS.SUCCESS },
   ]
-
-  return requiresFirmwareUpdate
-    ? ALL_STEPS
-    : ALL_STEPS.filter(step => step.section !== SECTIONS.FIRMWARE_UPDATE)
 }

app/src/organisms/ModuleWizardFlows/index.tsx

@@ -8,9 +8,9 @@ import {
 import { COLORS } from '@opentrons/components'
 import {
   CreateCommand,
-  LEFT,
   getModuleType,
   getModuleDisplayName,
+  LEFT,
 } from '@opentrons/shared-data'
 import { LegacyModalShell } from '../../molecules/LegacyModal'
 import { Portal } from '../../App/portal'
@@ -61,14 +61,7 @@ export const ModuleWizardFlows = (
   const { t } = useTranslation('module_wizard_flows')
   const attachedPipettes = useAttachedPipettesFromInstrumentsQuery()
   const attachedPipette = attachedPipettes.left ?? attachedPipettes.right
-  const requiresFirmwareUpdate = !attachedPipette?.ok
-  const attachedPipetteMount =
-    attachedPipette?.mount === LEFT ? 'pipette_left' : 'pipette_right'
-
-  const moduleCalibrationSteps = getModuleCalibrationSteps(
-    requiresFirmwareUpdate
-  )
-
+  const moduleCalibrationSteps = getModuleCalibrationSteps()
   const availableSlotNames =
     FLEX_SLOT_NAMES_BY_MOD_TYPE[getModuleType(attachedModule.moduleModel)] ?? []
   const [slotName, setSlotName] = React.useState(
@@ -269,8 +262,14 @@ export const ModuleWizardFlows = (
     modalContent = (
       <FirmwareUpdateModal
         proceed={proceed}
-        subsystem={attachedPipetteMount}
+        subsystem={
+          attachedPipette.mount === LEFT ? 'pipette_left' : 'pipette_right'
+        }
         description={t('firmware_update')}
+        proceedDescription={t('firmware_up_to_date', {
+          module: getModuleDisplayName(attachedModule.moduleModel),
+        })}
+        isOnDevice={isOnDevice}
       />
     )
   } else if (currentStep.section === SECTIONS.SELECT_LOCATION) {