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.

Sign up for GitHub

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): refactor Flex pipette card to reference /instruments #14702

Merged
merged 4 commits into from
Mar 26, 2024
Merged

fix(app): refactor Flex pipette card to reference /instruments #14702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
680d7f8
fix(app): refactor Flex pipette card to reference /instruments not /p…
smb2268 Mar 20, 2024
829804d
Fix linting
smb2268 Mar 20, 2024
5292830
Code review cleanup comments
smb2268 Mar 26, 2024
84825a0
fix ts error
smb2268 Mar 26, 2024
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
21 changes: 14 additions & 7 deletions app/src/molecules/InstrumentCard/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react'

import {
ALIGN_FLEX_START,
ALIGN_CENTER,
BORDERS,
Box,
COLORS,
Expand Down Expand Up @@ -80,13 +81,19 @@ export function InstrumentCard(props: InstrumentCardProps): JSX.Element {
</Flex>
) : null}
{instrumentDiagramProps?.pipetteSpecs != null ? (
<InstrumentDiagram
pipetteSpecs={instrumentDiagramProps.pipetteSpecs}
mount={instrumentDiagramProps.mount}
transform="scale(0.3)"
size="3.125rem"
transformOrigin="20% -10%"
/>
<Flex
alignItems={ALIGN_CENTER}
Copy link
Member

Choose a reason for hiding this comment

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

This is a separate bug, right? Like it's logically equivalent with the new transformOrigin at the standard viewpoint size, but it reflows better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this code wasn't hit previously because we weren't using InstrumentCard for pipettes - so I pulled the logic from what was actually being hit which lived here https://github.com/Opentrons/opentrons/blame/edge/app/src/organisms/Devices/PipetteCard/index.tsx#L261 and was recently updated by Nick to fix a styling bug

width="3.75rem"
height="3.375rem"
paddingRight={SPACING.spacing8}
>
<InstrumentDiagram
pipetteSpecs={instrumentDiagramProps.pipetteSpecs}
mount={instrumentDiagramProps.mount}
transform="scale(0.3)"
transformOrigin={'-5% 52%'}
/>
</Flex>
) : null}
<Flex
alignItems={ALIGN_FLEX_START}
Expand Down
118 changes: 57 additions & 61 deletions app/src/organisms/Devices/InstrumentsAndModules.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react'
import { useTranslation } from 'react-i18next'
import { getPipetteModelSpecs, LEFT, RIGHT } from '@opentrons/shared-data'
import {
useAllPipetteOffsetCalibrationsQuery,
useModulesQuery,
usePipettesQuery,
useInstrumentsQuery,
Expand All @@ -28,10 +27,10 @@ import { ModuleCard } from '../ModuleCard'
import { useIsFlex, useIsRobotViewable, useRunStatuses } from './hooks'
import {
getIs96ChannelPipetteAttached,
getOffsetCalibrationForMount,
getShowPipetteCalibrationWarning,
} from './utils'
import { PipetteCard } from './PipetteCard'
import { FlexPipetteCard } from './PipetteCard/FlexPipetteCard'
import { GripperCard } from '../GripperCard'
import { useIsEstopNotDisengaged } from '../../resources/devices/hooks/useIsEstopNotDisengaged'

Expand All @@ -43,7 +42,6 @@ import type {
} from '@opentrons/api-client'

const EQUIPMENT_POLL_MS = 5000
const FETCH_PIPETTE_CAL_POLL = 30000
interface InstrumentsAndModulesProps {
robotName: string
}
Expand All @@ -52,20 +50,22 @@ export function InstrumentsAndModules({
robotName,
}: InstrumentsAndModulesProps): JSX.Element | null {
const { t } = useTranslation(['device_details', 'shared'])
const isFlex = useIsFlex(robotName)
const attachedPipettes = usePipettesQuery(
{},
{
refetchInterval: EQUIPMENT_POLL_MS,
enabled: !isFlex,
}
)?.data ?? { left: undefined, right: undefined }
const isRobotViewable = useIsRobotViewable(robotName)
const currentRunId = useCurrentRunId()
const { isRunTerminal, isRunRunning } = useRunStatuses()
const isFlex = useIsFlex(robotName)
const isEstopNotDisengaged = useIsEstopNotDisengaged(robotName)

const { data: attachedInstruments } = useInstrumentsQuery({
refetchInterval: EQUIPMENT_POLL_MS,
enabled: isFlex,
})

const attachedGripper =
Expand Down Expand Up @@ -120,26 +120,6 @@ export function InstrumentsAndModules({
const leftColumnModules = attachedModules?.slice(0, halfAttachedModulesSize)
const rightColumnModules = attachedModules?.slice(halfAttachedModulesSize)

// The following pipetteOffset related code has been lifted out of the PipetteCard component
// to eliminate duplicated useInterval calls to `calibration/pipette_offset` coming from each card.
// Instead we now capture all offset calibration data here, and pass the appropriate calibration
// data to the associated card via props
const pipetteOffsetCalibrations =
useAllPipetteOffsetCalibrationsQuery({
refetchInterval: FETCH_PIPETTE_CAL_POLL,
enabled: !isFlex,
})?.data?.data ?? []
const leftMountOffsetCalibration = getOffsetCalibrationForMount(
pipetteOffsetCalibrations,
attachedPipettes,
LEFT
)
const rightMountOffsetCalibration = getOffsetCalibrationForMount(
pipetteOffsetCalibrations,
attachedPipettes,
RIGHT
)

return (
<Flex
alignItems={ALIGN_FLEX_START}
Expand Down Expand Up @@ -186,37 +166,46 @@ export function InstrumentsAndModules({
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing8}
>
<PipetteCard
pipetteId={attachedPipettes.left?.id}
pipetteModelSpecs={
attachedPipettes.left?.model != null
? getPipetteModelSpecs(attachedPipettes.left?.model) ?? null
: null
}
isPipetteCalibrated={
isFlex && attachedLeftPipette?.ok
? attachedLeftPipette?.data?.calibratedOffset
?.last_modified != null
: leftMountOffsetCalibration != null
}
mount={LEFT}
robotName={robotName}
pipetteIs96Channel={is96ChannelAttached}
pipetteIsBad={badLeftPipette != null}
isRunActive={currentRunId != null && isRunRunning}
isEstopNotDisengaged={isEstopNotDisengaged}
/>
{isFlex && (
<GripperCard
attachedGripper={attachedGripper}
isCalibrated={
attachedGripper?.ok === true &&
attachedGripper?.data?.calibratedOffset?.last_modified !=
null
{!isFlex ? (
<PipetteCard
pipetteId={attachedPipettes.left?.id}
pipetteModelSpecs={
attachedPipettes.left?.model != null
? getPipetteModelSpecs(attachedPipettes.left?.model) ??
null
: null
}
mount={LEFT}
robotName={robotName}
isRunActive={currentRunId != null && isRunRunning}
isEstopNotDisengaged={isEstopNotDisengaged}
/>
) : (
<>
<FlexPipetteCard
attachedPipette={attachedLeftPipette}
pipetteModelSpecs={
attachedLeftPipette?.instrumentModel != null
? getPipetteModelSpecs(
attachedLeftPipette.instrumentModel
) ?? null
: null
}
mount={LEFT}
isRunActive={currentRunId != null && isRunRunning}
isEstopNotDisengaged={isEstopNotDisengaged}
/>
<GripperCard
attachedGripper={attachedGripper}
isCalibrated={
attachedGripper?.ok === true &&
attachedGripper?.data?.calibratedOffset?.last_modified !=
null
}
isRunActive={currentRunId != null && isRunRunning}
isEstopNotDisengaged={isEstopNotDisengaged}
/>
</>
)}
{leftColumnModules.map((module, index) => (
<ModuleCard
Expand All @@ -237,7 +226,7 @@ export function InstrumentsAndModules({
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing8}
>
{!Boolean(is96ChannelAttached) && (
{!isFlex ? (
<PipetteCard
pipetteId={attachedPipettes.right?.id}
pipetteModelSpecs={
Expand All @@ -246,20 +235,27 @@ export function InstrumentsAndModules({
null
: null
}
isPipetteCalibrated={
isFlex && attachedRightPipette?.ok
? attachedRightPipette?.data?.calibratedOffset
?.last_modified != null
: rightMountOffsetCalibration != null
}
mount={RIGHT}
robotName={robotName}
pipetteIs96Channel={false}
pipetteIsBad={badRightPipette != null}
isRunActive={currentRunId != null && isRunRunning}
isEstopNotDisengaged={isEstopNotDisengaged}
/>
)}
) : null}
{isFlex && !is96ChannelAttached ? (
<FlexPipetteCard
attachedPipette={attachedRightPipette}
pipetteModelSpecs={
attachedRightPipette?.instrumentModel != null
? getPipetteModelSpecs(
attachedRightPipette.instrumentModel
) ?? null
: null
}
mount={RIGHT}
isRunActive={currentRunId != null && isRunRunning}
isEstopNotDisengaged={isEstopNotDisengaged}
/>
) : null}
{rightColumnModules.map((module, index) => (
<ModuleCard
key={`moduleCard_${String(module.moduleType)}_${String(
Expand Down
24 changes: 11 additions & 13 deletions app/src/organisms/Devices/PipetteCard/AboutPipetteSlideout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,30 @@ import {
StyledText,
TYPOGRAPHY,
} from '@opentrons/components'
import { useInstrumentsQuery } from '@opentrons/react-api-client'
import { Slideout } from '../../../atoms/Slideout'

import type { AttachedPipette } from '../../../redux/pipettes/types'
import type { PipetteData } from '@opentrons/api-client'
import type { PipetteModelSpecs, PipetteMount } from '@opentrons/shared-data'
import type { PipetteModelSpecs } from '@opentrons/shared-data'

interface AboutPipetteSlideoutProps {
pipetteId: AttachedPipette['id']
pipetteName: PipetteModelSpecs['displayName']
mount: PipetteMount
firmwareVersion?: string
onCloseClick: () => unknown
isExpanded: boolean
}

export const AboutPipetteSlideout = (
props: AboutPipetteSlideoutProps
): JSX.Element | null => {
const { pipetteId, pipetteName, isExpanded, mount, onCloseClick } = props
const {
pipetteId,
pipetteName,
isExpanded,
firmwareVersion,
onCloseClick,
} = props
const { i18n, t } = useTranslation(['device_details', 'shared'])
const { data: attachedInstruments } = useInstrumentsQuery()
const instrumentInfo =
attachedInstruments?.data?.find(
(i): i is PipetteData =>
i.instrumentType === 'pipette' && i.ok && i.mount === mount
) ?? null

return (
<Slideout
Expand All @@ -52,7 +50,7 @@ export const AboutPipetteSlideout = (
}
>
<Flex flexDirection={DIRECTION_COLUMN}>
{instrumentInfo?.firmwareVersion != null && (
{firmwareVersion != null && (
<>
<StyledText
as="h6"
Expand All @@ -66,7 +64,7 @@ export const AboutPipetteSlideout = (
paddingTop={SPACING.spacing4}
paddingBottom={SPACING.spacing16}
>
{instrumentInfo.firmwareVersion}
{firmwareVersion}
</StyledText>
</>
)}
Expand Down
Loading
Loading