Skip to content

Commit

Permalink
refactor(protocol-designer, step-generation): remove redundant offset…
Browse files Browse the repository at this point in the history
… parameters for mix

closes AUTH-1367
  • Loading branch information
jerader committed Jan 24, 2025
1 parent 10c4fab commit 22879fe
Show file tree
Hide file tree
Showing 20 changed files with 149 additions and 215 deletions.
43 changes: 13 additions & 30 deletions protocol-designer/src/analytics/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ import {
getFileMetadata,
getRobotStateTimeline,
} from '../file-data/selectors'
import {
DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
FIXED_TRASH_ID,
} from '../constants'
import { DEFAULT_MM_OFFSET_FROM_BOTTOM, FIXED_TRASH_ID } from '../constants'
import { trackEvent } from './mixpanel'
import { getHasOptedIn } from './selectors'
import { flattenNestedProperties } from './utils/flattenNestedProperties'
Expand Down Expand Up @@ -169,12 +165,12 @@ export const reduxActionToAnalyticsEvent = (
blowoutFlowRate: stepArgModified.blowoutFlowRateUlSec,
aspirateOffsetFromBottomMm:
stepArgModified.aspirateOffsetFromBottomMm ===
DEFAULT_MM_FROM_BOTTOM_ASPIRATE
DEFAULT_MM_OFFSET_FROM_BOTTOM
? DEFAULT_VALUE
: stepArgModified.aspirateOffsetFromBottomMm,
dispenseOffsetFromBottomMm:
stepArgModified.dispenseOffsetFromBottomMm ===
DEFAULT_MM_FROM_BOTTOM_DISPENSE
DEFAULT_MM_OFFSET_FROM_BOTTOM
? DEFAULT_VALUE
: stepArgModified.dispenseOffsetFromBottomMm,
aspirateXOffset:
Expand Down Expand Up @@ -212,32 +208,19 @@ export const reduxActionToAnalyticsEvent = (
dispenseFlowRate:
stepArgModified.dispenseFlowRateUlSec ?? DEFAULT_VALUE,
blowoutFlowRate: stepArgModified.blowoutFlowRateUlSec,
aspirateOffsetFromBottomMm:
stepArgModified.aspirateOffsetFromBottomMm ===
DEFAULT_MM_FROM_BOTTOM_ASPIRATE
? DEFAULT_VALUE
: stepArgModified.aspirateOffsetFromBottomMm,
dispenseOffsetFromBottomMm:
stepArgModified.dispenseOffsetFromBottomMm ===
DEFAULT_MM_FROM_BOTTOM_DISPENSE
? DEFAULT_VALUE
: stepArgModified.dispenseOffsetFromBottomMm,
aspirateXOffset:
stepArgModified.aspirateXOffset === 0
offsetFromBottomMm:
stepArgModified.offsetFromBottomMm ===
DEFAULT_MM_OFFSET_FROM_BOTTOM
? DEFAULT_VALUE
: stepArgModified.aspirateXOffset,
aspirateYOffset:
stepArgModified.aspirateYOffset === 0
: stepArgModified.offsetFromBottomMm,
xOffset:
stepArgModified.xOffset === 0
? DEFAULT_VALUE
: stepArgModified.aspirateYOffset,
dispenseXOffset:
stepArgModified.dispenseXOffset === 0
: stepArgModified.xOffset,
yOffset:
stepArgModified.yOffset === 0
? DEFAULT_VALUE
: stepArgModified.dispenseXOffset,
dispenseYOffset:
stepArgModified.dispenseYOffset === 0
? DEFAULT_VALUE
: stepArgModified.dispenseYOffset,
: stepArgModified.yOffset,
...additionalProperties,
},
}
Expand Down
4 changes: 1 addition & 3 deletions protocol-designer/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ export const END_TERMINAL_TITLE = 'FINAL DECK STATE'
// special ID for invisible deck setup step-form
export const INITIAL_DECK_SETUP_STEP_ID = '__INITIAL_DECK_SETUP_STEP__'
export const DEFAULT_CHANGE_TIP_OPTION: 'always' = 'always'
// TODO: Ian 2019-06-13 don't keep these as hard-coded static values (see #3587)
export const DEFAULT_MM_FROM_BOTTOM_ASPIRATE = 1
export const DEFAULT_MM_FROM_BOTTOM_DISPENSE = 1
export const DEFAULT_MM_OFFSET_FROM_BOTTOM = 1
// NOTE: in the negative Z direction, to go down from top
export const DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP = -1
export const DEFAULT_MM_BLOWOUT_OFFSET_FROM_TOP = 0
Expand Down
7 changes: 3 additions & 4 deletions protocol-designer/src/file-data/selectors/fileCreator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ import { selectors as uiLabwareSelectors } from '../../ui/labware'
import { swatchColors } from '../../organisms/DefineLiquidsModal/swatchColors'
import { getLoadLiquidCommands } from '../../load-file/migration/utils/getLoadLiquidCommands'
import {
DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
DEFAULT_MM_BLOWOUT_OFFSET_FROM_TOP,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
} from '../../constants'
import { getStepGroups } from '../../step-forms/selectors'
import { getFileMetadata, getRobotType } from './fileFields'
Expand Down Expand Up @@ -150,8 +149,8 @@ export const createFile: Selector<ProtocolFile> = createSelector(
// TODO: Ian 2019-06-13 load these into redux and always get them from redux, not constants.js
// This `defaultValues` key is not yet read by anything, but is populated here for auditability
// and so that later we can do #3587 without a PD migration
aspirate_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
dispense_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_DISPENSE,
aspirate_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
dispense_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
touchTip_mmFromTop: DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
blowout_mmFromTop: DEFAULT_MM_BLOWOUT_OFFSET_FROM_TOP,
},
Expand Down
14 changes: 6 additions & 8 deletions protocol-designer/src/organisms/TipPositionModal/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import floor from 'lodash/floor'
import round from 'lodash/round'
import { getIsTouchTipField } from '../../form-types'
import {
DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
} from '../../constants'
import { DECIMALS_ALLOWED, TOO_MANY_DECIMALS } from './constants'
Expand All @@ -17,20 +16,19 @@ export function getDefaultMmFromBottom(args: {

switch (name) {
case 'aspirate_mmFromBottom':
return DEFAULT_MM_FROM_BOTTOM_ASPIRATE
return DEFAULT_MM_OFFSET_FROM_BOTTOM

case 'aspirate_delay_mmFromBottom':
return DEFAULT_MM_FROM_BOTTOM_ASPIRATE
return DEFAULT_MM_OFFSET_FROM_BOTTOM

case 'dispense_mmFromBottom':
return DEFAULT_MM_FROM_BOTTOM_DISPENSE
return DEFAULT_MM_OFFSET_FROM_BOTTOM

case 'dispense_delay_mmFromBottom':
return DEFAULT_MM_FROM_BOTTOM_DISPENSE
return DEFAULT_MM_OFFSET_FROM_BOTTOM

case 'mix_mmFromBottom':
// TODO: Ian 2018-11-131 figure out what offset makes most sense for mix
return DEFAULT_MM_FROM_BOTTOM_DISPENSE
return DEFAULT_MM_OFFSET_FROM_BOTTOM

default:
// touch tip fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { fixture_tiprack_10_ul } from '@opentrons/shared-data/labware/fixtures/2
import { getStateAndContextTempTCModules } from '@opentrons/step-generation'
import {
DEFAULT_DELAY_SECONDS,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
} from '../../constants'
import { createPresavedStepForm } from '../utils/createPresavedStepForm'
import type { CreatePresavedStepFormArgs } from '../utils/createPresavedStepForm'
Expand Down Expand Up @@ -216,7 +216,7 @@ describe('createPresavedStepForm', () => {
aspirate_delay_seconds: `${DEFAULT_DELAY_SECONDS}`,
dispense_delay_checkbox: false,
dispense_delay_seconds: `${DEFAULT_DELAY_SECONDS}`,
mix_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_DISPENSE,
mix_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
mix_touchTip_mmFromBottom: null,
mix_wellOrder_first: 't2b',
mix_wellOrder_second: 'l2r',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
DEFAULT_CHANGE_TIP_OPTION,
DEFAULT_DELAY_SECONDS,
DEFAULT_MM_BLOWOUT_OFFSET_FROM_TOP,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
DEFAULT_WELL_ORDER_FIRST_OPTION,
DEFAULT_WELL_ORDER_SECOND_OPTION,
} from '../../constants'
Expand Down Expand Up @@ -31,7 +31,7 @@ export function getDefaultsForStepType(
dropTip_location: null,
dropTip_wellNames: undefined,
labware: null,
mix_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_DISPENSE,
mix_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
mix_touchTip_checkbox: false,
mix_touchTip_mmFromBottom: null,
mix_wellOrder_first: DEFAULT_WELL_ORDER_FIRST_OPTION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
fixture_tiprack_10_ul,
fixture_tiprack_300_ul,
} from '@opentrons/shared-data/labware/fixtures/2'
import { DEFAULT_MM_FROM_BOTTOM_DISPENSE } from '../../../../constants'
import { DEFAULT_MM_OFFSET_FROM_BOTTOM } from '../../../../constants'
import { dependentFieldsUpdateMix } from '../dependentFieldsUpdateMix'
import type { LabwareDefinition2 } from '@opentrons/shared-data'
import type {
Expand Down Expand Up @@ -132,7 +132,7 @@ describe('well selection should update', () => {
expect(handleFormHelper(patch, form)).toEqual({
...patch,
wells: ['A1'],
mix_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_DISPENSE,
mix_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
mix_touchTip_mmFromBottom: null,
mix_touchTip_checkbox: false,
})
Expand All @@ -145,7 +145,7 @@ describe('well selection should update', () => {
expect(handleFormHelper(patch, trashLabwareForm)).toEqual({
...patch,
wells: [],
mix_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_DISPENSE,
mix_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
mix_touchTip_mmFromBottom: null,
mix_touchTip_checkbox: false,
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { getWellsDepth } from '@opentrons/shared-data'
import {
DEFAULT_CHANGE_TIP_OPTION,
DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_BLOWOUT_OFFSET_FROM_TOP,
DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
} from '../../../constants'
import { getOrderedWells } from '../../utils'
import { getMixDelayData } from './getDelayData'
Expand Down Expand Up @@ -52,12 +51,8 @@ export const mixFormToArgs = (
hydratedFormData.dispense_flowRate ||
matchingTipLiquidSpecs?.defaultDispenseFlowRate.default

// NOTE: for mix, there is only one tip offset field,
// and it applies to both aspirate and dispense
const aspirateOffsetFromBottomMm =
hydratedFormData.mix_mmFromBottom || DEFAULT_MM_FROM_BOTTOM_ASPIRATE
const dispenseOffsetFromBottomMm =
hydratedFormData.mix_mmFromBottom || DEFAULT_MM_FROM_BOTTOM_DISPENSE
const offsetFromBottomMm =
hydratedFormData.mix_mmFromBottom || DEFAULT_MM_OFFSET_FROM_BOTTOM
// It's radiobutton, so one should always be selected.
// One changeTip option should always be selected.
console.assert(
Expand Down Expand Up @@ -103,18 +98,15 @@ export const mixFormToArgs = (
aspirateFlowRateUlSec: aspirateFlowRateUlSec ?? 0,
dispenseFlowRateUlSec: dispenseFlowRateUlSec ?? 0,
blowoutFlowRateUlSec: blowoutFlowRateUlSec ?? 0,
aspirateOffsetFromBottomMm,
dispenseOffsetFromBottomMm,
offsetFromBottomMm,
blowoutOffsetFromTopMm,
aspirateDelaySeconds,
tipRack: hydratedFormData.tipRack,
dispenseDelaySeconds,
// TODO(jr, 7/26/24): wire up wellNames
dropTipLocation: dropTip_location,
nozzles,
aspirateXOffset: mix_x_position ?? 0,
dispenseXOffset: mix_x_position ?? 0,
aspirateYOffset: mix_y_position ?? 0,
dispenseYOffset: mix_y_position ?? 0,
xOffset: mix_x_position ?? 0,
yOffset: mix_y_position ?? 0,
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { getWellsDepth } from '@opentrons/shared-data'
import { DEST_WELL_BLOWOUT_DESTINATION } from '@opentrons/step-generation'
import {
DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_BLOWOUT_OFFSET_FROM_TOP,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
} from '../../../constants'
import { getOrderedWells } from '../../utils'
Expand Down Expand Up @@ -207,9 +206,9 @@ export const moveLiquidFormToArgs = (
hydratedFormData.dispense_flowRate ||
matchingTipLiquidSpecs.defaultDispenseFlowRate.default,
aspirateOffsetFromBottomMm:
hydratedFormData.aspirate_mmFromBottom || DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
hydratedFormData.aspirate_mmFromBottom || DEFAULT_MM_OFFSET_FROM_BOTTOM,
dispenseOffsetFromBottomMm:
hydratedFormData.dispense_mmFromBottom || DEFAULT_MM_FROM_BOTTOM_DISPENSE,
hydratedFormData.dispense_mmFromBottom || DEFAULT_MM_OFFSET_FROM_BOTTOM,
blowoutFlowRateUlSec:
hydratedFormData.blowout_flowRate ||
matchingTipLiquidSpecs.defaultBlowOutFlowRate.default,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,15 @@ describe('mix step form -> command creator args', () => {
aspirateFlowRateUlSec: 5, // make sure flow rates are numbers instead of strings
dispenseFlowRateUlSec: 4,
blowoutFlowRateUlSec: 1000,
aspirateOffsetFromBottomMm: 0.5,
dispenseOffsetFromBottomMm: 0.5,
offsetFromBottomMm: 0.5,
blowoutOffsetFromTopMm: 0,
aspirateDelaySeconds: null,
tipRack: 'mockTiprack',
dispenseDelaySeconds: null,
dropTipLocation: undefined,
nozzles: undefined,
aspirateXOffset: 0,
dispenseXOffset: 0,
aspirateYOffset: 0,
dispenseYOffset: 0,
xOffset: 0,
yOffset: 0,
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
getMixData,
} from '../moveLiquidFormToArgs'
import { getOrderedWells } from '../../../utils'
import { DEFAULT_MM_FROM_BOTTOM_ASPIRATE } from '../../../../constants'
import { DEFAULT_MM_OFFSET_FROM_BOTTOM } from '../../../../constants'
import type { LabwareDefinition2 } from '@opentrons/shared-data'
import type {
HydratedMoveLiquidFormData,
Expand Down Expand Up @@ -229,7 +229,7 @@ describe('move liquid step form -> command creator args', () => {
expectedArgsChecked: {
aspirateDelay: {
seconds: 11,
mmFromBottom: DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { vi, it, describe, expect, afterEach } from 'vitest'
import {
DEFAULT_CHANGE_TIP_OPTION,
DEFAULT_DELAY_SECONDS,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_OFFSET_FROM_BOTTOM,
DEFAULT_WELL_ORDER_FIRST_OPTION,
DEFAULT_WELL_ORDER_SECOND_OPTION,
} from '../../../constants'
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('getDefaultsForStepType', () => {
blowout_checkbox: false,
blowout_location: null,
blowout_flowRate: null,
mix_mmFromBottom: DEFAULT_MM_FROM_BOTTOM_DISPENSE,
mix_mmFromBottom: DEFAULT_MM_OFFSET_FROM_BOTTOM,
mix_touchTip_mmFromBottom: null,
mix_touchTip_checkbox: false,
pipette: null,
Expand Down
3 changes: 1 addition & 2 deletions protocol-designer/src/steplist/test/generateSubsteps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ describe('generateSubstepItem', () => {
blowoutLocation: null,
blowoutFlowRateUlSec: 3,
blowoutOffsetFromTopMm: 3,
aspirateOffsetFromBottomMm: 4,
dispenseOffsetFromBottomMm: 10,
offsetFromBottomMm: 4,
aspirateFlowRateUlSec: 5,
dispenseFlowRateUlSec: 5,
dropTipLocation: FIXED_TRASH_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,14 @@ describe('generateRobotStateTimeline', () => {
aspirateFlowRateUlSec: 3.78,
dispenseFlowRateUlSec: 3.78,
blowoutFlowRateUlSec: 3.78,
aspirateOffsetFromBottomMm: 0.5,
dispenseOffsetFromBottomMm: 0.5,
offsetFromBottomMm: 0.5,
blowoutOffsetFromTopMm: 0,
aspirateDelaySeconds: null,
dispenseDelaySeconds: null,
nozzles: null,
tipRack: getLabwareDefURI(fixtureTiprack300ul as LabwareDefinition2),
aspirateXOffset: 0,
aspirateYOffset: 0,
dispenseXOffset: 0,
dispenseYOffset: 0,
xOffset: 0,
yOffset: 0,
},
},
}
Expand Down
Loading

0 comments on commit 22879fe

Please sign in to comment.