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

refactor(step-generation, protocol-designer): create HydratedFormData & update atomic command args to match v10 params #17308

Merged
merged 10 commits into from
Jan 21, 2025
Merged

refactor(step-generation, protocol-designer): create HydratedFormData & update atomic command args to match v10 params #17308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
901bf47
clean up stepArgs and make hydratedFormData
jerader Jan 18, 2025
9e6f37e
fix whitescreens in disabledFields
jerader Jan 18, 2025
6fe1f94
cleanup
jerader Jan 18, 2025
5ea3bb6
fix prettier
jerader Jan 18, 2025
cdfb51e
fix lint
jerader Jan 21, 2025
ca58f2a
remove fields from HydratedMoveLiquidFormData
jerader Jan 21, 2025
54ad873
rename HydratedMixFormDataLegacy to HydratedMixFormData
jerader Jan 21, 2025
84d33de
update inPlace atomic command args to use command params
jerader Jan 21, 2025
9ded968
fix failed checks and clean up rebase
jerader Jan 21, 2025
a08219b
address feedback
jerader Jan 21, 2025
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
212 changes: 104 additions & 108 deletions protocol-designer/src/form-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,19 +175,7 @@ export interface BlowoutFields {
export interface ChangeTipFields {
changeTip?: ChangeTipOptions
}
export type MixForm = AnnotationFields &
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: MixForm wasn't in use anywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

So what's the deal with some of the form classes that are named "legacy," like HydratedMixFormDataLegacy?

Copy link
Collaborator Author

@jerader jerader Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the HydratedMoveLiquidFormDataLegacy is not in use. HydratedMixFormDataLegacy was in use, so i just removed Legacy because it matches the pattern we are using for the other form data types. Not sure why Legacy was added initially...

BlowoutFields &
ChangeTipFields & {
id: StepIdType
stepType: 'mix'
labware?: string
pipette?: string
times?: string
touchTip?: boolean
volume?: string
wells?: string[]
}
export type PauseForm = AnnotationFields & {
export type HydratedPauseFormData = AnnotationFields & {
stepType: 'pause'
id: StepIdType
pauseAction?:
Expand All @@ -197,11 +185,11 @@ export type PauseForm = AnnotationFields & {
pauseMessage?: string
pauseTemperature?: string
pauseTime?: string
moduleId?: string
}
// TODO: separate field values from from metadata
export interface FormData {
stepType: StepType
id: StepIdType // TODO: form value processing to ensure type
id: StepIdType
[key: string]: any
}
export const PROFILE_CYCLE: 'profileCycle' = 'profileCycle'
Expand Down Expand Up @@ -229,92 +217,80 @@ export type BlankForm = AnnotationFields & {
id: StepIdType
}

export interface HydratedMoveLiquidFormData {
export interface HydratedMoveLiquidFormData extends AnnotationFields {
id: string
stepType: 'moveLiquid'
stepName: string
fields: {
aspirate_airGap_checkbox: boolean
aspirate_delay_checkbox: boolean
aspirate_labware: LabwareEntity
aspirate_mix_checkbox: boolean
aspirate_touchTip_checkbox: boolean
aspirate_wellOrder_first: WellOrderOption
aspirate_wellOrder_second: WellOrderOption
aspirate_wells: string[]
blowout_checkbox: boolean
changeTip: ChangeTipOptions
dispense_airGap_checkbox: boolean
dispense_delay_checkbox: boolean
dispense_labware: LabwareEntity | AdditionalEquipmentEntity
dispense_mix_checkbox: boolean
dispense_touchTip_checkbox: boolean
dispense_wellOrder_first: WellOrderOption
dispense_wellOrder_second: WellOrderOption
dispense_wells: string[]
disposalVolume_checkbox: boolean
dropTip_location: string
nozzles: NozzleConfigurationStyle | null
path: PathOption
pipette: PipetteEntity
tipRack: string
volume: number
aspirate_airGap_volume?: number | null
aspirate_delay_mmFromBottom?: number | null
aspirate_delay_seconds?: number | null
aspirate_flowRate?: number | null
aspirate_mix_times?: number | null
aspirate_mix_volume?: number | null
aspirate_mmFromBottom?: number | null
aspirate_touchTip_mmFromBottom?: number | null
aspirate_wells_grouped?: boolean | null
aspirate_x_position?: number | null
aspirate_y_position?: number | null
blowout_flowRate?: number | null
blowout_location?: string | null
blowout_z_offset?: number | null
dispense_airGap_volume?: number | null
dispense_delay_mmFromBottom?: number | null
dispense_delay_seconds?: number | null
dispense_flowRate?: number | null
dispense_mix_times?: number | null
dispense_mix_volume?: number | null
dispense_mmFromBottom?: number | null
dispense_touchTip_mmFromBottom?: number | null
dispense_x_position?: number | null
dispense_y_position?: number | null
disposalVolume_volume?: number | null
dropTip_wellNames?: string[] | null
pickUpTip_location?: string | null
pickUpTip_wellNames?: string[] | null
preWetTip?: boolean | null
}
description?: string | null
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this PR makes a lot of sense for consistency. But how does this not break all the existing code that used fields like description? (and all the old saved JSON protocols)

Copy link
Collaborator Author

@jerader jerader Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good question, i updated the HydratedFormData types which are only used for ...inArgs fns and various form utils such as getDisabledFields. the steps in a JSON file uses FormData, before it was hydrated. Here is the util that hydrates the certain fields, if a field is not hydrated, it is left as it was in form data.

An example of a hydrated field is labware in mix and move liquid. When a user selects it in a form, it is just the labwareId. When hydrated, that labware id is mapped against the labware entities for its match and is hydrated as a LabwareEntity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So a JSON file shouldn't break when reimported because the fields are just the form data, not hydrated yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah! I wasn't aware that the Hydrated*Data was a separate thing from the data in the imported form.

Copy link
Collaborator Author

@jerader jerader Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In terms of description, it was never wired up in the form or FormData. so a JSON file shouldn't have that field. But also, i only edited the HydratedFormData for that, i think. 😄

aspirate_airGap_checkbox: boolean
aspirate_delay_checkbox: boolean
aspirate_labware: LabwareEntity
aspirate_mix_checkbox: boolean
aspirate_touchTip_checkbox: boolean
aspirate_wellOrder_first: WellOrderOption
aspirate_wellOrder_second: WellOrderOption
aspirate_wells: string[]
blowout_checkbox: boolean
changeTip: ChangeTipOptions
dispense_airGap_checkbox: boolean
dispense_delay_checkbox: boolean
dispense_labware: LabwareEntity | AdditionalEquipmentEntity
dispense_mix_checkbox: boolean
dispense_touchTip_checkbox: boolean
dispense_wellOrder_first: WellOrderOption
dispense_wellOrder_second: WellOrderOption
dispense_wells: string[]
disposalVolume_checkbox: boolean
dropTip_location: string
nozzles: NozzleConfigurationStyle | null
path: PathOption
pipette: PipetteEntity
tipRack: string
volume: number
aspirate_airGap_volume?: number | null
aspirate_delay_mmFromBottom?: number | null
aspirate_delay_seconds?: number | null
aspirate_flowRate?: number | null
aspirate_mix_times?: number | null
aspirate_mix_volume?: number | null
aspirate_mmFromBottom?: number | null
aspirate_touchTip_mmFromBottom?: number | null
aspirate_wells_grouped?: boolean | null
aspirate_x_position?: number | null
aspirate_y_position?: number | null
blowout_flowRate?: number | null
blowout_location?: string | null
blowout_z_offset?: number | null
dispense_airGap_volume?: number | null
dispense_delay_mmFromBottom?: number | null
dispense_delay_seconds?: number | null
dispense_flowRate?: number | null
dispense_mix_times?: number | null
dispense_mix_volume?: number | null
dispense_mmFromBottom?: number | null
dispense_touchTip_mmFromBottom?: number | null
dispense_x_position?: number | null
dispense_y_position?: number | null
disposalVolume_volume?: number | null
dropTip_wellNames?: string[] | null
pickUpTip_location?: string | null
pickUpTip_wellNames?: string[] | null
preWetTip?: boolean | null
}

export interface HydratedMoveLabwareFormData {
export interface HydratedMoveLabwareFormData extends AnnotationFields {
id: string
stepType: 'moveLabware'
stepName: string
fields: {
labware: LabwareEntity
newLocation: LabwareLocation
useGripper: boolean
}
description?: string | null
labware: LabwareEntity
newLocation: LabwareLocation
useGripper: boolean
}

export interface HydratedCommentFormData {
export interface HydratedCommentFormData extends AnnotationFields {
id: string
stepType: 'comment'
stepName: string
fields: {
message: string
}
stepDetails?: string | null
message: string
}

export interface HydratedMixFormDataLegacy {
export interface HydratedMixFormData extends AnnotationFields {
aspirate_delay_checkbox: boolean
blowout_checkbox: boolean
changeTip: ChangeTipOptions
Expand All @@ -327,7 +303,6 @@ export interface HydratedMixFormDataLegacy {
mix_wellOrder_second: WellOrderOption
nozzles: NozzleConfigurationStyle | null
pipette: PipetteEntity
stepName: string
stepType: 'mix'
tipRack: string
volume: number
Expand All @@ -346,46 +321,66 @@ export interface HydratedMixFormDataLegacy {
mix_y_position?: number | null
pickUpTip_location?: string | null
pickUpTip_wellNames?: string[] | null
stepDetails?: string | null
times?: number | null
}
export type MagnetAction = 'engage' | 'disengage'
export type HydratedMagnetFormData = AnnotationFields & {
engageHeight: string | null
id: string
magnetAction: MagnetAction
moduleId: string | null
moduleId: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I don't understand how this doesn't break old files? Like, what was it possible to export a file that had the moduleId missing? What happens if you try to import it after this change?

stepDetails: string | null
stepType: 'magnet'
}
export interface HydratedTemperatureFormData {
export interface HydratedTemperatureFormData extends AnnotationFields {
id: string
moduleId: string | null
setTemperature: 'true' | 'false'
stepDetails: string | null
stepType: 'temperature'
targetTemperature: string | null
}
export interface HydratedHeaterShakerFormData {
export interface HydratedHeaterShakerFormData extends AnnotationFields {
heaterShakerSetTimer: boolean | null
heaterShakerTimer: string | null
id: string
latchOpen: boolean
moduleId: string
setHeaterShakerTemperature: boolean
setShake: boolean
stepDetails: string | null
stepType: 'heaterShaker'
targetHeaterShakerTemperature: string | null
targetSpeed: string | null
}

export interface HydratedThermocyclerFormData extends AnnotationFields {
id: string
stepType: 'thermocycler'
blockIsActive: boolean
blockIsActiveHold: boolean
blockTargetTemp: string | null
blockTargetTempHold: string | null
lidIsActive: boolean
lidIsActiveHold: boolean
lidOpen: boolean
lidOpenHold: boolean
lidTargetTemp: string | null
lidTargetTempHold: string | null
moduleId: string
orderedProfileItems: string[]
profileItemsById: Record<string, ProfileItem>
profileTargetLidTemp: string | null
profileVolume: string | null
thermocyclerFormType: 'thermocyclerState' | 'thermocyclerProfile'
}

export type AbsorbanceReaderFormType =
| typeof ABSORBANCE_READER_INITIALIZE
| typeof ABSORBANCE_READER_READ
| typeof ABSORBANCE_READER_LID

export interface HydratedAbsorbanceReaderFormData {
export interface HydratedAbsorbanceReaderFormData extends AnnotationFields {
stepType: 'absorbanceReader'
id: string
absorbanceReaderFormType: AbsorbanceReaderFormType | null
fileName: string | null
lidOpen: boolean | null
Expand All @@ -397,13 +392,7 @@ export interface HydratedAbsorbanceReaderFormData {
referenceWavelengthActive: boolean
wavelengths: string[]
}
// TODO: Ian 2019-01-17 Moving away from this and towards nesting all form fields
// inside `fields` key, but deprecating transfer/consolidate/distribute is a pre-req
export type HydratedMoveLiquidFormDataLegacy = AnnotationFields &
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was also not in use anywhere, plus it was a legacy type

HydratedMoveLiquidFormData['fields'] & {
id: string
stepType: 'moveLiquid'
}

// fields used in TipPositionInput
export type TipZOffsetFields =
| 'aspirate_mmFromBottom'
Expand Down Expand Up @@ -451,7 +440,14 @@ export function getIsDelayPositionField(fieldName: string): boolean {
}
export type CountPerStepType = Partial<Record<StepType, number>>

// TODO: get real HydratedFormData type
export interface HydratedFormdata {
[key: string]: any
}
export type HydratedFormData =
| HydratedAbsorbanceReaderFormData
| HydratedCommentFormData
| HydratedHeaterShakerFormData
| HydratedMagnetFormData
| HydratedMixFormData
| HydratedMoveLabwareFormData
| HydratedMoveLiquidFormData
| HydratedPauseFormData
| HydratedTemperatureFormData
| HydratedThermocyclerFormData
4 changes: 2 additions & 2 deletions protocol-designer/src/load-file/migration/8_2_0.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { OT2_ROBOT_TYPE } from '@opentrons/shared-data'
import { PAUSE_UNTIL_TIME } from '../../constants'

import type { ProtocolFile } from '@opentrons/shared-data'
import type { PauseForm } from '../../form-types'
import type { HydratedPauseFormData } from '../../form-types'
import type { DesignerApplicationData } from './utils/getLoadLiquidCommands'

const getTimeFromIndividualUnits = (
Expand Down Expand Up @@ -42,7 +42,7 @@ export const migrateFile = (
pauseAction,
} = form
const pauseFormIndividualTimeUnitsRemoved = Object.keys(
form as PauseForm
form as HydratedPauseFormData
).reduce(
(accInner, key) =>
!['pauseSecond', 'pauseMinute', 'pauseHour'].includes(key)
Expand Down
4 changes: 3 additions & 1 deletion protocol-designer/src/organisms/Alerts/FormAlerts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ function FormAlertsComponent(props: FormAlertsProps): JSX.Element | null {
})

const profileItemsById: Record<string, ProfileItem> | null | undefined =
unsavedForm?.profileItemsById
unsavedForm?.stepType === 'thermocycler'
? unsavedForm?.profileItemsById
: null

let visibleDynamicFieldFormErrors: ProfileFormError[] = []

Expand Down
3 changes: 2 additions & 1 deletion protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
StepFieldName,
StepType,
PathOption,
HydratedFormData,
} from '../../../../form-types'
import type { FormError } from '../../../../steplist/formLevel'
import type { NozzleType } from '../../../../types'
Expand Down Expand Up @@ -253,7 +254,7 @@ export const makeSingleEditFieldProps = (
focusHandlers: FocusHandlers,
formData: FormData,
handleChangeFormInput: (name: string, value: unknown) => void,
hydratedForm: { [key: string]: any }, // TODO: create real HydratedFormData type
hydratedForm: HydratedFormData,
t: any
): FieldPropsByName => {
const { dirtyFields, blur, focusedField, focus } = focusHandlers
Expand Down
Loading
Loading