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(shared-data, protocol-designer): return latest pipette model def f… #14945

Merged
merged 6 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
324 changes: 162 additions & 162 deletions protocol-designer/fixtures/protocol/8/doItAllV7MigratedToV8.json

Large diffs are not rendered by default.

176 changes: 88 additions & 88 deletions protocol-designer/fixtures/protocol/8/doItAllV8.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const FlowRateInput = (props: FlowRateInputProps): JSX.Element => {
name,
pipetteDisplayName,
} = props
const { t } = useTranslation(['form', 'application'])
const { t } = useTranslation(['form', 'application', 'shared'])
const DEFAULT_LABEL = t('step_edit_form.field.flow_rate.label')

const initialState: State = {
Expand Down Expand Up @@ -112,7 +112,10 @@ export const FlowRateInput = (props: FlowRateInputProps): JSX.Element => {

// show 0.1 not 0 as minimum, since bottom of range is non-inclusive
const displayMinFlowRate = minFlowRate || Math.pow(10, -DECIMALS_ALLOWED)
const rangeDescription = `between ${displayMinFlowRate} and ${maxFlowRate}`
const rangeDescription = t('step_edit_form.field.flow_rate.range', {
min: displayMinFlowRate,
max: maxFlowRate,
})
const outOfBounds =
modalFlowRateNum === 0 ||
minFlowRate > modalFlowRateNum ||
Expand All @@ -126,11 +129,14 @@ export const FlowRateInput = (props: FlowRateInputProps): JSX.Element => {
// and pristinity only masks the outOfBounds error, not the correctDecimals error
if (!modalUseDefault) {
if (!Number.isNaN(modalFlowRateNum) && !correctDecimals) {
errorMessage = `a max of ${DECIMALS_ALLOWED} decimal place${
DECIMALS_ALLOWED > 1 ? 's' : ''
} is allowed`
errorMessage = t('step_edit_form.field.flow_rate.error_decimals', {
decimals: `${DECIMALS_ALLOWED}`,
})
} else if (!isPristine && outOfBounds) {
errorMessage = `accepted range is ${displayMinFlowRate} to ${maxFlowRate}`
errorMessage = t('step_edit_form.field.flow_rate.error_out_of_bounds', {
min: displayMinFlowRate,
max: maxFlowRate,
})
}
}

Expand All @@ -155,21 +161,22 @@ export const FlowRateInput = (props: FlowRateInputProps): JSX.Element => {
className={modalStyles.modal}
buttons={[
{
children: 'Cancel',
children: t('shared:cancel'),
onClick: cancelModal,
},
{
children: 'Done',
children: t('shared:done'),
onClick: makeSaveModal(allowSave),
disabled: isPristine ? false : !allowSave,
},
]}
>
<h3 className={styles.header}>Flow Rate</h3>
<h3 className={styles.header}>{DEFAULT_LABEL}</h3>

<div className={styles.description}>
{`Our default aspirate speed is optimal for a ${pipetteDisplayName}
aspirating liquids with a viscosity similar to water`}
{t('step_edit_form.field.flow_rate.default_text', {
displayName: pipetteDisplayName,
})}
</div>

<div className={styles.flow_rate_type_label}>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import * as React from 'react'
import { describe, it, vi, beforeEach } from 'vitest'
import { fireEvent, screen } from '@testing-library/react'
import { fixtureP100096V2Specs } from '@opentrons/shared-data'
import { renderWithProviders } from '../../../../../__testing-utils__'
import { i18n } from '../../../../../localization'
import { getPipetteEntities } from '../../../../../step-forms/selectors'
import { FlowRateField } from '../index'

vi.mock('../../../../../step-forms/selectors')
const render = (props: React.ComponentProps<typeof FlowRateField>) => {
return renderWithProviders(<FlowRateField {...props} />, {
i18nInstance: i18n,
})[0]
}
const mockMockId = 'mockId'
describe('FlowRateField', () => {
let props: React.ComponentProps<typeof FlowRateField>

beforeEach(() => {
props = {
disabled: false,
flowRateType: 'aspirate',
volume: 100,
value: null,
name: 'flowRate',
tiprack: 'tipRack:opentrons_flex_96_tiprack_1000ul',
updateValue: vi.fn(),
onFieldBlur: vi.fn(),
onFieldFocus: vi.fn(),
pipetteId: mockMockId,
}
vi.mocked(getPipetteEntities).mockReturnValue({
[mockMockId]: {
name: 'p50_single_flex',
spec: {
liquids: fixtureP100096V2Specs.liquids,
displayName: 'mockPipDisplayName',
} as any,
id: mockMockId,
tiprackLabwareDef: [
{
parameters: {
loadName: 'opentrons_flex_96_tiprack_1000ul',
tipLength: 1000,
},
metadata: { displayName: 'mockDisplayName' },
} as any,
],
tiprackDefURI: ['mockDefURI1', 'mockDefURI2'],
},
})
})
it('renders the flowRateInput and clicking on it opens the modal with all the text', () => {
render(props)
screen.getByText('Flow Rate')
fireEvent.click(screen.getByRole('textbox'))
screen.getByText(
'The default mockPipDisplayName flow rate is optimal for handling aqueous liquids'
)
screen.getByText('aspirate speed')
screen.getByText('160 μL/s (default)')
screen.getByText('Custom')
screen.getByText('between 0.1 and Infinity')
screen.getByText('Cancel')
screen.getByText('Done')
})
})
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import * as React from 'react'
import { FlowRateInput, FlowRateInputProps } from './FlowRateInput'
import { FlowRateInput } from './FlowRateInput'
import { useSelector } from 'react-redux'
import { selectors as stepFormSelectors } from '../../../../step-forms'
import { FieldProps } from '../../types'
import { getMatchingTipLiquidSpecs } from '../../../../utils'
import type { FieldProps } from '../../types'
import type { FlowRateInputProps } from './FlowRateInput'

interface OP extends FieldProps {
interface FlowRateFieldProps extends FieldProps {
flowRateType: FlowRateInputProps['flowRateType']
volume: unknown
tiprack: unknown
Expand All @@ -14,14 +15,14 @@ interface OP extends FieldProps {
label?: FlowRateInputProps['label']
}

// Add a key to force re-constructing component when values change
export function FlowRateField(props: OP): JSX.Element {
export function FlowRateField(props: FlowRateFieldProps): JSX.Element {
const {
pipetteId,
flowRateType,
value,
volume,
tiprack,
name,
...passThruProps
} = props
const pipetteEntities = useSelector(stepFormSelectors.getPipetteEntities)
Expand All @@ -43,18 +44,18 @@ export function FlowRateField(props: OP): JSX.Element {
matchingTipLiquidSpecs?.defaultDispenseFlowRate.default ?? 0
}
}

return (
<FlowRateInput
{...passThruProps}
name={name}
value={value}
flowRateType={flowRateType}
pipetteDisplayName={pipetteDisplayName}
key={innerKey}
defaultFlowRate={defaultFlowRate}
minFlowRate={0}
// TODO(jr, 3/21/24): update max flow rate to real value instead of volume
maxFlowRate={pipette ? pipette.spec.liquids.default.maxVolume : Infinity}
// if uiMaxFlowRate does not exist then there is no maxFlowRate
maxFlowRate={matchingTipLiquidSpecs?.uiMaxFlowRate ?? Infinity}
/>
)
}
8 changes: 7 additions & 1 deletion protocol-designer/src/localization/en/form.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,13 @@
"label": "delay"
},
"tip_position": { "label": "tip position" },
"flow_rate": { "label": "Flow Rate" },
"flow_rate": {
"default_text": "The default {{displayName}} flow rate is optimal for handling aqueous liquids",
"error_decimals": "A max of {{decimals}} decimal places is allowed",
"error_out_of_bounds": "accepted range is {{min}} to {{max}}",
"label": "Flow Rate",
"range": "between {{min}} and {{max}}"
},
"volume": { "label": "volume per well" },
"well_order": {
"label": "Well order",
Expand Down
2 changes: 2 additions & 0 deletions protocol-designer/src/localization/en/shared.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{
"add": "add",
"amount": "Amount:",
"cancel": "Cancel",
"confirm_reorder": "Are you sure you want to reorder these steps, it may cause errors?",
"done": "Done",
"edit": "edit",
"exit": "exit",
"go_back": "go back",
Expand Down
8 changes: 4 additions & 4 deletions shared-data/js/__tests__/pipettes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('pipette data accessors', () => {
})

describe('getPipetteSpecsV2', () => {
it('returns the correct info for p1000_single_flex', () => {
it('returns the correct info for p1000_single_flex which should be the latest model version 3.6', () => {
const mockP1000Specs = {
$otSharedSchema: '#/pipette/schemas/2/pipetteGeometrySchema.json',
availableSensors: {
Expand All @@ -77,7 +77,7 @@ describe('pipette data accessors', () => {
channels: 1,
displayCategory: 'FLEX',
displayName: 'Flex 1-Channel 1000 μL',
dropTipConfigurations: { plungerEject: { current: 1, speed: 10 } },
dropTipConfigurations: { plungerEject: { current: 1, speed: 15 } },
liquids: {
default: {
$otSharedSchema:
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('pipette data accessors', () => {
plungerHomingConfigurations: { current: 1, speed: 30 },
plungerMotorConfigurations: { idle: 0.3, run: 1 },
plungerPositionsConfigurations: {
default: { blowout: 76.5, bottom: 71.5, drop: 90.5, top: 0.5 },
default: { blowout: 76.5, bottom: 71.5, drop: 90.5, top: 0 },
},
quirks: [],
shaftDiameter: 4.5,
Expand All @@ -142,7 +142,7 @@ describe('pipette data accessors', () => {
)
})
})
it('returns the correct liquid info for a p50 pipette with default and lowVolume', () => {
it('returns the correct liquid info for a p50 pipette model version with default and lowVolume', () => {
const tiprack50uL = 'opentrons/opentrons_flex_96_tiprack_50ul/1'
const tiprackFilter50uL = 'opentrons/opentrons_flex_96_filtertiprack_50ul/1'

Expand Down
77 changes: 60 additions & 17 deletions shared-data/js/pipettes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,24 +139,51 @@ const getChannelsFromString = (
}
}
}
const getVersionFromGen = (gen: Gen): string | null => {
const getVersionFromGen = (gen: Gen): number => {
switch (gen) {
case 'gen1': {
return '1_0'
return 1
}
case 'gen2': {
return '2_0'
return 2
}
case 'gen3':
case 'flex': {
return '3_0'
return 3
}
default: {
return null
return 0
}
}
}

const getHighestVersion = (
wholeVersion: string,
path: string,
pipetteModel: string,
channels: Channels | null,
majorVersion: number,
highestVersion: string
): string => {
const versionComponents = wholeVersion.split('_')
const majorPathVersion = parseInt(versionComponents[0])
jerader marked this conversation as resolved.
Show resolved Hide resolved
const minorPathVersion = parseInt(versionComponents[1])
const highestVersionComponents = highestVersion.split('_')
const minorHighestVersion = parseInt(highestVersionComponents[1])
if (majorPathVersion === majorVersion) {
// Compare the version number with the current highest version
// and make sure the given model, channels, and major/minor versions
// are found in the path
if (
minorPathVersion > minorHighestVersion &&
path.includes(`${majorPathVersion}_${minorPathVersion}`) &&
path.includes(pipetteModel) &&
path.includes(channels ?? '')
) {
highestVersion = `${majorPathVersion}_${minorPathVersion}`
}
}
return highestVersion
}
const V2_DEFINITION_TYPES = ['general', 'geometry']

/* takes in pipetteName such as 'p300_single' or 'p300_single_gen1'
Expand All @@ -173,14 +200,19 @@ export const getPipetteSpecsV2 = (
const nameSplit = name.split('_')
const pipetteModel = nameSplit[0] // ex: p300
const channels = getChannelsFromString(nameSplit[1] as PipChannelString) // ex: single -> single_channel
const gen = getVersionFromGen(nameSplit[2] as Gen)

let version: string
const pipetteGen = getVersionFromGen(nameSplit[2] as Gen)
let version: string = ''
let majorVersion: number
// the first 2 conditions are to accommodate version from the pipetteName
if (nameSplit.length === 2) {
version = '1_0'
} else if (gen != null) {
version = gen // ex: gen1 -> 1_0
// special-casing 96-channel
if (channels === 'ninety_six_channel') {
majorVersion = 3
} else {
majorVersion = 1
}
} else if (pipetteGen !== 0) {
majorVersion = pipetteGen // ex: gen1 -> 1
// the 'else' is to accommodate the exact version if PipetteModel was added
} else {
const versionNumber = nameSplit[2].split('v')[1]
Expand All @@ -190,13 +222,23 @@ export const getPipetteSpecsV2 = (
version = `${versionNumber}_0` // ex: 1 -> 1_0
}
}

let highestVersion: string = '0_0'
const generalGeometricMatchingJsons = Object.entries(generalGeometric).reduce(
(genericGeometricModules: GeneralGeometricModules[], [path, module]) => {
const wholeVersion = path.split('/')[7]
highestVersion = getHighestVersion(
wholeVersion,
path,
pipetteModel,
channels,
majorVersion,
highestVersion
)
V2_DEFINITION_TYPES.forEach(type => {
if (
`../pipette/definitions/2/${type}/${channels}/${pipetteModel}/${version}.json` ===
path
`../pipette/definitions/2/${type}/${channels}/${pipetteModel}/${
version === '' ? highestVersion : version
}.json` === path
) {
genericGeometricModules.push(module.default)
}
Expand All @@ -219,8 +261,9 @@ export const getPipetteSpecsV2 = (
liquidTypes.push(type)
}
if (
`../pipette/definitions/2/liquid/${channels}/${pipetteModel}/${type}/${version}.json` ===
path
`../pipette/definitions/2/liquid/${channels}/${pipetteModel}/${type}/${
jerader marked this conversation as resolved.
Show resolved Hide resolved
version === '' ? highestVersion : version
}.json` === path
) {
const index = liquidTypes.indexOf(type)
const newKeyName = index !== -1 ? liquidTypes[index] : path
Expand Down
Loading