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

feat(app): oDD banners for pipette calibration warning #13624

Merged
merged 3 commits into from
Sep 21, 2023
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
1 change: 1 addition & 0 deletions app/src/assets/localization/en/instruments_dashboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"firmware_version": "firmware version",
"last_calibrated": "last calibrated",
"no_cal_data": "no calibration data",
"pipette_calibrations_differ": "<bold>Pipette recalibration recommended</bold> The attached pipettes have very different calibration values. When properly calibrated, the values should be similar.",
"recalibrate": "recalibrate",
"serial_number": "serial number"
}
17 changes: 4 additions & 13 deletions app/src/organisms/Devices/InstrumentsAndModules.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import { getPipetteModelSpecs, LEFT, RIGHT } from '@opentrons/shared-data'
import { INCONSISTENT_PIPETTE_OFFSET } from '@opentrons/api-client'
import {
useAllPipetteOffsetCalibrationsQuery,
useModulesQuery,
Expand Down Expand Up @@ -32,6 +31,7 @@ import { useIsOT3, useIsRobotViewable, useRunStatuses } from './hooks'
import {
getIs96ChannelPipetteAttached,
getOffsetCalibrationForMount,
getShowPipetteCalibrationWarning,
} from './utils'
import { PipetteCard } from './PipetteCard'
import { GripperCard } from '../GripperCard'
Expand Down Expand Up @@ -140,16 +140,6 @@ export function InstrumentsAndModules({
attachedPipettes,
RIGHT
)
const pipetteCalibrationWarning =
attachedInstruments?.data.some((i): i is PipetteData => {
const failuresList =
i.ok && i.data.calibratedOffset?.reasonability_check_failures != null
? i.data.calibratedOffset?.reasonability_check_failures
: []
if (failuresList.length > 0) {
return failuresList[0]?.kind === INCONSISTENT_PIPETTE_OFFSET
} else return false
}) ?? false

return (
<Flex
Expand Down Expand Up @@ -192,11 +182,12 @@ export function InstrumentsAndModules({
<Banner type="warning">{t('robot_control_not_available')}</Banner>
</Flex>
)}
{pipetteCalibrationWarning && (currentRunId == null || isRunTerminal) && (
{getShowPipetteCalibrationWarning(attachedInstruments) &&
(currentRunId == null || isRunTerminal) ? (
<Flex paddingBottom={SPACING.spacing16}>
<PipetteRecalibrationWarning />
</Flex>
)}
) : null}
{isRobotViewable ? (
<Flex gridGap={SPACING.spacing8} width="100%">
<Flex
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import {
SPACING,
TYPOGRAPHY,
} from '@opentrons/components'
import { INCONSISTENT_PIPETTE_OFFSET } from '@opentrons/api-client'
import { StyledText } from '../../../atoms/text'
import * as PipetteConstants from '../../../redux/pipettes/constants'
import { getShowPipetteCalibrationWarning } from '../utils'
import { PipetteRecalibrationWarning } from '../PipetteCard/PipetteRecalibrationWarning'
import {
useRunPipetteInfoByMount,
Expand All @@ -24,7 +24,7 @@ import { useMostRecentCompletedAnalysis } from '../../LabwarePositionCheck/useMo
import { useInstrumentsQuery } from '@opentrons/react-api-client'
import { isGripperInCommands } from '../../../resources/protocols/utils'

import type { GripperData, PipetteData } from '@opentrons/api-client'
import type { GripperData } from '@opentrons/api-client'
import { i18n } from '../../../i18n'

const EQUIPMENT_POLL_MS = 5000
Expand Down Expand Up @@ -56,20 +56,12 @@ export function SetupInstrumentCalibration({
(i): i is GripperData => i.instrumentType === 'gripper'
) ?? null
: null
const pipetteCalibrationWarning =
instrumentsQueryData?.data.some((i): i is PipetteData => {
const failuresList =
i.ok && i.data.calibratedOffset?.reasonability_check_failures != null
? i.data.calibratedOffset?.reasonability_check_failures
: []
if (failuresList.length > 0) {
return failuresList[0]?.kind === INCONSISTENT_PIPETTE_OFFSET
} else return false
}) ?? false

return (
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
{pipetteCalibrationWarning && <PipetteRecalibrationWarning />}
{getShowPipetteCalibrationWarning(instrumentsQueryData) && (
<PipetteRecalibrationWarning />
)}
Comment on lines +62 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{getShowPipetteCalibrationWarning(instrumentsQueryData) && (
<PipetteRecalibrationWarning />
)}
{getShowPipetteCalibrationWarning(instrumentsQueryData) ? (
<PipetteRecalibrationWarning />
): null}

<StyledText
color={COLORS.black}
css={TYPOGRAPHY.pSemiBold}
Expand Down
23 changes: 22 additions & 1 deletion app/src/organisms/Devices/utils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { format, parseISO } from 'date-fns'
import { INCONSISTENT_PIPETTE_OFFSET } from '@opentrons/api-client'
import type {
FetchPipettesResponseBody,
FetchPipettesResponsePipette,
Mount,
} from '../../redux/pipettes/types'
import type { PipetteOffsetCalibration } from '@opentrons/api-client'
import type {
Instruments,
PipetteData,
PipetteOffsetCalibration,
} from '@opentrons/api-client'

/**
* formats a string if it is in ISO 8601 date format
Expand Down Expand Up @@ -68,3 +73,19 @@ export function getOffsetCalibrationForMount(
)
}
}

export function getShowPipetteCalibrationWarning(
attachedInstruments?: Instruments
): boolean {
return (
attachedInstruments?.data.some((i): i is PipetteData => {
const failuresList =
i.ok && i.data.calibratedOffset?.reasonability_check_failures != null
? i.data.calibratedOffset?.reasonability_check_failures
: []
if (failuresList.length > 0) {
return failuresList[0]?.kind === INCONSISTENT_PIPETTE_OFFSET
} else return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
} else return false
} else { return false }

}) ?? false
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,7 @@ import { FLOWS } from '../PipetteWizardFlows/constants'
import { PipetteWizardFlows } from '../PipetteWizardFlows'
import { GripperWizardFlows } from '../GripperWizardFlows'

import type {
InstrumentData,
PipetteOffsetCalibration,
GripperData,
} from '@opentrons/api-client'
import type { InstrumentData } from '@opentrons/api-client'
import type { GripperModel } from '@opentrons/shared-data'
import type { Mount } from '../../redux/pipettes/types'

Expand All @@ -53,10 +49,6 @@ interface ProtocolInstrumentMountItemProps {
mount: Mount | 'extension'
mostRecentAnalysis?: CompletedProtocolAnalysis | null
attachedInstrument: InstrumentData | null
attachedCalibrationData:
| PipetteOffsetCalibration
| GripperData['data']['calibratedOffset']
| null
speccedName: PipetteName | GripperModel
instrumentsRefetch?: () => void
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ describe('ProtocolInstrumentMountItem', () => {
props = {
mount: LEFT,
attachedInstrument: null,
attachedCalibrationData: null,
speccedName: 'p1000_multi_flex',
}
mockPipetteWizardFlows.mockReturnValue(<div>pipette wizard flow</div>)
Expand Down
27 changes: 8 additions & 19 deletions app/src/organisms/ProtocolSetupInstruments/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ import {
SPACING,
TYPOGRAPHY,
} from '@opentrons/components'
import {
useAllPipetteOffsetCalibrationsQuery,
useInstrumentsQuery,
} from '@opentrons/react-api-client'
import { useInstrumentsQuery } from '@opentrons/react-api-client'
import { ODDBackButton } from '../../molecules/ODDBackButton'
import { PipetteRecalibrationODDWarning } from '../../pages/OnDeviceDisplay/PipetteRealibrationODDWarning'
import { getShowPipetteCalibrationWarning } from '../Devices/utils'
import { useMostRecentCompletedAnalysis } from '../LabwarePositionCheck/useMostRecentCompletedAnalysis'
import { ProtocolInstrumentMountItem } from '../InstrumentMountItem'

Expand All @@ -34,9 +33,6 @@ export function ProtocolSetupInstruments({
}: ProtocolSetupInstrumentsProps): JSX.Element {
const { t, i18n } = useTranslation('protocol_setup')
const { data: attachedInstruments, refetch } = useInstrumentsQuery()
const {
data: allPipettesCalibrationData,
} = useAllPipetteOffsetCalibrationsQuery()
const mostRecentAnalysis = useMostRecentCompletedAnalysis(runId)

const usesGripper =
Expand All @@ -59,6 +55,11 @@ export function ProtocolSetupInstruments({
label={t('instruments')}
onClick={() => setSetupScreen('prepare to run')}
/>
{getShowPipetteCalibrationWarning(attachedInstruments) && (
<Flex paddingBottom={SPACING.spacing16}>
<PipetteRecalibrationODDWarning />
</Flex>
)}
<Flex
justifyContent={JUSTIFY_SPACE_BETWEEN}
alignItems={ALIGN_CENTER}
Expand All @@ -85,15 +86,6 @@ export function ProtocolSetupInstruments({
speccedName={loadedPipette.pipetteName}
attachedInstrument={attachedPipetteMatch}
mostRecentAnalysis={mostRecentAnalysis}
attachedCalibrationData={
attachedPipetteMatch != null
? allPipettesCalibrationData?.data.find(
cal =>
cal.mount === attachedPipetteMatch.mount &&
cal.pipette === attachedPipetteMatch.instrumentName
) ?? null
: null
}
instrumentsRefetch={refetch}
/>
)
Expand All @@ -104,9 +96,6 @@ export function ProtocolSetupInstruments({
mount="extension"
speccedName={'gripperV1' as GripperModel}
attachedInstrument={attachedGripperMatch}
attachedCalibrationData={
attachedGripperMatch?.data.calibratedOffset ?? null
}
/>
) : null}
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
SPACING,
TYPOGRAPHY,
} from '@opentrons/components'
import { INCONSISTENT_PIPETTE_OFFSET } from '@opentrons/api-client'
import { useInstrumentsQuery } from '@opentrons/react-api-client'

import { StyledText } from '../../atoms/text'
Expand All @@ -16,10 +15,10 @@ import {
useIsOT3,
usePipetteOffsetCalibrations,
} from '../Devices/hooks'
import { getShowPipetteCalibrationWarning } from '../Devices/utils'
import { PipetteRecalibrationWarning } from '../Devices/PipetteCard/PipetteRecalibrationWarning'
import { PipetteOffsetCalibrationItems } from './CalibrationDetails/PipetteOffsetCalibrationItems'

import type { PipetteData } from '@opentrons/api-client'
import type { FormattedPipetteOffsetCalibration } from '.'

interface RobotSettingsPipetteOffsetCalibrationProps {
Expand Down Expand Up @@ -55,16 +54,6 @@ export function RobotSettingsPipetteOffsetCalibration({
ot3AttachedRightPipetteOffsetCal != null)
)
showPipetteOffsetCalItems = true
const pipetteCalibrationWarning =
instrumentsData?.data.some((i): i is PipetteData => {
const failuresList =
i.ok && i.data.calibratedOffset?.reasonability_check_failures != null
? i.data.calibratedOffset?.reasonability_check_failures
: []
if (failuresList.length > 0) {
return failuresList[0]?.kind === INCONSISTENT_PIPETTE_OFFSET
} else return false
}) ?? false

return (
<Flex
Expand All @@ -80,7 +69,9 @@ export function RobotSettingsPipetteOffsetCalibration({
{isOT3 ? (
<StyledText as="p">{t('pipette_calibrations_description')}</StyledText>
) : null}
{pipetteCalibrationWarning && <PipetteRecalibrationWarning />}
{getShowPipetteCalibrationWarning(instrumentsData) && (
<PipetteRecalibrationWarning />
)}
{showPipetteOffsetCalItems ? (
<PipetteOffsetCalibrationItems
robotName={robotName}
Expand Down
7 changes: 7 additions & 0 deletions app/src/pages/OnDeviceDisplay/InstrumentsDashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { onDeviceDisplayRoutes } from '../../App/OnDeviceDisplayApp'
import { Navigation } from '../../organisms/Navigation'
import { AttachedInstrumentMountItem } from '../../organisms/InstrumentMountItem'
import { GripperWizardFlows } from '../../organisms/GripperWizardFlows'
import { getShowPipetteCalibrationWarning } from '../../organisms/Devices/utils'
import { PipetteRecalibrationODDWarning } from './PipetteRealibrationODDWarning'
import type { GripperData, PipetteData } from '@opentrons/api-client'

const FETCH_PIPETTE_CAL_POLL = 10000
Expand Down Expand Up @@ -34,6 +36,11 @@ export const InstrumentsDashboard = (): JSX.Element => {
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing8}
>
{getShowPipetteCalibrationWarning(attachedInstruments) && (
<Flex paddingBottom={SPACING.spacing16}>
<PipetteRecalibrationODDWarning />
</Flex>
)}
{isNinetySixChannel ? (
<AttachedInstrumentMountItem
mount="left"
Expand Down
56 changes: 56 additions & 0 deletions app/src/pages/OnDeviceDisplay/PipetteRealibrationODDWarning.tsx
Copy link
Contributor

@koji koji Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import * as React from 'react'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file should probably be PipetteRecalibrationODDWarning

import { useTranslation, Trans } from 'react-i18next'
import {
Flex,
Icon,
Btn,
SPACING,
COLORS,
JUSTIFY_SPACE_BETWEEN,
ALIGN_CENTER,
BORDERS,
JUSTIFY_FLEX_START,
} from '@opentrons/components'
import { StyledText } from '../../atoms/text'

export const PipetteRecalibrationODDWarning = (): JSX.Element | null => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const PipetteRecalibrationODDWarning = (): JSX.Element | null => {
export function PipetteRecalibrationODDWarning(): JSX.Element | null {

const { t } = useTranslation('instruments_dashboard')
const [showBanner, setShowBanner] = React.useState<boolean>(true)
if (!showBanner) return null

return (
<Flex
justifyContent={JUSTIFY_SPACE_BETWEEN}
alignItems={ALIGN_CENTER}
borderRadius={BORDERS.borderRadiusSize3}
backgroundColor={COLORS.yellow3}
padding={`${SPACING.spacing12} ${SPACING.spacing16}`}
height="5.76rem"
width="60rem"
>
<Flex justifyContent={JUSTIFY_FLEX_START}>
<Icon
name="alert-circle"
color={COLORS.yellow2}
width="45px"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
width="45px"
size="1.75rem"

Screenshot 2023-09-21 at 6 31 14 PM

marginRight={SPACING.spacing12}
/>
<StyledText as="p">
<Trans
t={t}
i18nKey="pipette_calibrations_differ"
components={{ bold: <strong /> }}
/>
</StyledText>
</Flex>
<Btn onClick={() => setShowBanner(false)}>
<Icon
width={SPACING.spacing32}
height={SPACING.spacing32}
Comment on lines +48 to +49
Copy link
Contributor

@koji koji Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We avoid using SPACING for width, height, and size.

Suggested change
width={SPACING.spacing32}
height={SPACING.spacing32}
size="3rem"

Screenshot 2023-09-21 at 6 32 58 PM

name="close"
aria-label="close_icon"
/>
</Btn>
</Flex>
)
}