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

fix(protocol-designer): fix missing disposal volume in new distribute forms #2733

Merged
merged 2 commits into from
Dec 3, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class TipPositionModal extends React.Component<Props, State> {
}
applyChanges = () => {
const {value} = this.state
console.log('applying changes', value)
this.props.updateValue(value == null ? null : roundValue(value))
this.props.closeModal()
}
Expand Down
20 changes: 3 additions & 17 deletions protocol-designer/src/components/StepEditForm/formFields.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ import {
import i18n from '../../localization'
import {selectors as pipetteSelectors} from '../../pipettes'
import {selectors as labwareIngredSelectors} from '../../labware-ingred/reducers'
import {actions} from '../../steplist'
import {hydrateField} from '../../steplist/fieldLevel'
import type {StepFieldName} from '../../steplist/fieldLevel'
import {DISPOSAL_PERCENTAGE} from '../../steplist/formLevel/warnings'
import {SOURCE_WELL_DISPOSAL_DESTINATION} from '../../steplist/formLevel/stepFormToArgs/transferLikeFormToArgs'
import type {ChangeTipOptions} from '../../step-generation/types'
import type {BaseState, ThunkDispatch} from '../../types'
import type {BaseState} from '../../types'
import type {StepType} from '../../form-types'
import styles from './StepEditForm.css'
import StepField from './StepFormField'
Expand Down Expand Up @@ -118,18 +116,12 @@ export function DispenseDelayFields (props: DispenseDelayFieldsProps) {

type PipetteFieldOP = {name: StepFieldName, stepType?: StepType} & FocusHandlers
type PipetteFieldSP = {pipetteOptions: Options, getHydratedPipette: (string) => any} // TODO: real hydrated pipette type
type PipetteFieldDP = {updateDisposalVolume: (?mixed) => void}
type PipetteFieldProps = PipetteFieldOP & PipetteFieldSP & PipetteFieldDP
type PipetteFieldProps = PipetteFieldOP & PipetteFieldSP
const PipetteFieldSTP = (state: BaseState, ownProps: PipetteFieldOP): PipetteFieldSP => ({
pipetteOptions: pipetteSelectors.getEquippedPipetteOptions(state),
getHydratedPipette: (value) => hydrateField(state, ownProps.name, value),
})
const PipetteFieldDTP = (dispatch: ThunkDispatch<*>): PipetteFieldDP => ({
Copy link
Contributor Author

@IanLondon IanLondon Nov 28, 2018

Choose a reason for hiding this comment

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

this updateDisposalVolume logic should all happen in handleFormChange now

updateDisposalVolume: (disposalVolume: ?mixed) => {
dispatch(actions.changeFormInput({update: {aspirate_disposalVol_volume: disposalVolume}}))
},
})
export const PipetteField = connect(PipetteFieldSTP, PipetteFieldDTP)((props: PipetteFieldProps) => (
export const PipetteField = connect(PipetteFieldSTP)((props: PipetteFieldProps) => (
<StepField
name={props.name}
focusedField={props.focusedField}
Expand All @@ -143,12 +135,6 @@ export const PipetteField = connect(PipetteFieldSTP, PipetteFieldDTP)((props: Pi
onFocus={() => { props.onFieldFocus(props.name) }}
onChange={(e: SyntheticEvent<HTMLSelectElement>) => {
updateValue(e.currentTarget.value)
if (props.stepType === 'distribute') {
const hydratedPipette = props.getHydratedPipette(e.currentTarget.value)
if (hydratedPipette) {
props.updateDisposalVolume(hydratedPipette.maxVolume * DISPOSAL_PERCENTAGE)
}
}
}} />
</FormGroup>
)} />
Expand Down
2 changes: 2 additions & 0 deletions protocol-designer/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,6 @@ export const DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP = -1
export const DEFAULT_WELL_ORDER_FIRST_OPTION: 't2b' = 't2b'
export const DEFAULT_WELL_ORDER_SECOND_OPTION: 'l2r' = 'l2r'

export const DISPOSAL_VOLUME_PERCENTAGE = 0.2 // 20% percent of pipette capacity

export const WELL_LABEL_OFFSET: number = 8
3 changes: 2 additions & 1 deletion protocol-designer/src/steplist/actions/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ export type ChangeFormInputAction = {

export const changeFormInput = (payload: ChangeFormPayload) =>
(dispatch: ThunkDispatch<ChangeFormInputAction>, getState: GetState) => {
const unsavedForm = selectors.getUnsavedForm(getState())
dispatch({
type: 'CHANGE_FORM_INPUT',
payload: handleFormChange(payload, getState),
payload: handleFormChange(payload, unsavedForm, getState),
})
}

Expand Down
34 changes: 28 additions & 6 deletions protocol-designer/src/steplist/actions/handleFormChange.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// @flow
import uniq from 'lodash/uniq'
import {getPipetteNameSpecs} from '@opentrons/shared-data'
import {getWellSetForMultichannel} from '../../well-selection/utils'
import {selectors} from '../index'
import {selectors as pipetteSelectors} from '../../pipettes'
import {selectors as labwareIngredSelectors} from '../../labware-ingred/reducers'
import {DISPOSAL_VOLUME_PERCENTAGE} from '../../constants'

import type {PipetteChannels} from '@opentrons/shared-data'
import type {BaseState, GetState} from '../../types'
import type {FormData} from '../../form-types'
Expand Down Expand Up @@ -47,17 +49,20 @@ const getChannels = (pipetteId: string, state: BaseState): PipetteChannels => {
// Eventually we gotta allow arbitrary actions like DELETE_LABWARE
// (or more speculatively, CHANGE_PIPETTE etc), which affect saved forms from
// 'outside', to cause changes that run thru all the logic in this block
function handleFormChange (payload: ChangeFormPayload, getState: GetState): ChangeFormPayload {
function handleFormChange (
payload: ChangeFormPayload,
baseForm: ?FormData,
getState: GetState,
): ChangeFormPayload {
// Use state to handle form changes
const baseState = getState()
const unsavedForm = selectors.getUnsavedForm(baseState)

// pass thru, unchanged
if (unsavedForm == null) { return payload }
if (baseForm == null) { return payload }

let updateOverrides = getChangeLabwareEffects(payload.update)

if (unsavedForm.pipette && payload.update.pipette) {
if (baseForm.pipette && payload.update.pipette) {
if (typeof payload.update.pipette !== 'string') {
// this should not happen!
console.error('no next pipette, could not handleFormChange')
Expand All @@ -66,7 +71,23 @@ function handleFormChange (payload: ChangeFormPayload, getState: GetState): Chan
const nextChannels = getChannels(payload.update.pipette, baseState)
updateOverrides = {
...updateOverrides,
...reconcileFormPipette(unsavedForm, baseState, payload.update.pipette, nextChannels),
...reconcileFormPipette(baseForm, baseState, payload.update.pipette, nextChannels),
}
}

if (baseForm.stepType === 'distribute') {
if (typeof payload.update.pipette === 'string') {
const pipetteData = pipetteSelectors.getPipettesById(baseState)[payload.update.pipette]
const pipetteSpecs = getPipetteNameSpecs(pipetteData.model)
const disposalVol = pipetteSpecs
? pipetteSpecs.maxVolume * DISPOSAL_VOLUME_PERCENTAGE
: null

updateOverrides = {
...updateOverrides,
'aspirate_disposalVol_checkbox': true,
'aspirate_disposalVol_volume': disposalVol,
}
}
}

Expand Down Expand Up @@ -183,6 +204,7 @@ export const reconcileFormPipette = (formData: FormData, baseState: BaseState, n
}
}
}

return updateOverrides
}

Expand Down
46 changes: 34 additions & 12 deletions protocol-designer/src/steplist/actions/thunks.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// @flow
import {selectors} from '../index'
import handleFormChange from './handleFormChange'
import {uuid} from '../../utils'
import {selectors as labwareIngredsSelectors} from '../../labware-ingred/reducers'
import * as pipetteSelectors from '../../pipettes/selectors'
import {selectors as steplistSelectors} from '../../steplist'
import {actions as tutorialActions} from '../../tutorial'
import {getNextDefaultPipetteId, generateNewForm} from '../formLevel'

Expand All @@ -16,23 +17,17 @@ export type SelectStepAction = {

// get new or existing step for given stepId
function getStepFormData (state: BaseState, stepId: StepIdType, newStepType?: StepType): ?FormData {
const existingStep = selectors.getSavedForms(state)[stepId]
const existingStep = steplistSelectors.getSavedForms(state)[stepId]

if (existingStep) {
return existingStep
}

const defaultNextPipette = getNextDefaultPipetteId(
selectors.getSavedForms(state),
selectors.getOrderedSteps(state),
pipetteSelectors.getPipetteIdsByMount(state)
)

// TODO: Ian 2018-09-19 sunset 'steps' reducer. Right now, it's needed here to get stepType
// for any step that was created but never saved (never clicked Save button).
// Instead, new steps should have their stepType immediately added
// to 'savedStepForms' upon creation.
const steps = selectors.getSteps(state)
const steps = steplistSelectors.getSteps(state)
const stepTypeFromStepReducer = steps[stepId] && steps[stepId].stepType
const stepType = newStepType || stepTypeFromStepReducer

Expand All @@ -44,7 +39,6 @@ function getStepFormData (state: BaseState, stepId: StepIdType, newStepType?: St
return generateNewForm({
stepId,
stepType: stepType,
defaultNextPipette,
})
}

Expand All @@ -58,7 +52,35 @@ export const selectStep = (stepId: StepIdType, newStepType?: StepType): ThunkAct

dispatch(selectStepAction)

const formData = getStepFormData(getState(), stepId, newStepType)
const state = getState()
let formData = getStepFormData(state, stepId, newStepType)

const defaultPipetteId = getNextDefaultPipetteId(
steplistSelectors.getSavedForms(state),
steplistSelectors.getOrderedSteps(state),
pipetteSelectors.getEquippedPipettes(state),
)

// For a pristine step, if there is a `pipette` field in the form
// (added by upstream `getDefaultsForStepType` fn),
// then set `pipette` field of new steps to the next default pipette id.
//
// In order to trigger dependent field changes (eg default disposal volume),
// update the form thru handleFormChange.
const formHasPipetteField = formData && 'pipette' in formData
if (newStepType && formHasPipetteField && defaultPipetteId) {
const updatePayload = {update: {pipette: defaultPipetteId}}
const updatedFields = handleFormChange(
updatePayload,
formData,
getState).update

formData = {
...formData,
...updatedFields,
}
}

dispatch({
type: 'POPULATE_FORM',
payload: formData,
Expand Down Expand Up @@ -96,7 +118,7 @@ export type ReorderSelectedStepAction = {

export const reorderSelectedStep = (delta: number) =>
(dispatch: ThunkDispatch<ReorderSelectedStepAction>, getState: GetState) => {
const stepId = selectors.getSelectedStepId(getState())
const stepId = steplistSelectors.getSelectedStepId(getState())

if (stepId != null) {
dispatch({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@ import type {
FormData,
} from '../../form-types'

const FORMS_WITH_PIPETTE = ['transfer', 'mix', 'distribute', 'consolidate']

type NewFormArgs = {
stepId: StepIdType,
stepType: StepType,
defaultNextPipette: string,
}

// Add default values to a new step form
export default function generateNewForm (args: NewFormArgs): FormData {
const {stepId, stepType, defaultNextPipette} = args
const {stepId, stepType} = args
const baseForm: BlankForm = {
id: stepId,
stepType: stepType,
Expand All @@ -28,10 +25,6 @@ export default function generateNewForm (args: NewFormArgs): FormData {

let additionalFields = {}

if (FORMS_WITH_PIPETTE.includes(stepType)) {
additionalFields.pipette = defaultNextPipette
}

return {
...baseForm,
...getDefaultsForStepType(stepType),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default function getDefaultsForStepType (stepType: StepType) {
'dispense_wellOrder_second': DEFAULT_WELL_ORDER_SECOND_OPTION,
'dispense_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE,
'dispense_wells': [],
'pipette': null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of using a hard-coded array somewhere like const stepHasPipetteField = (stepType) => ['mix', 'transfer', 'consolidate', 'distribute'].includes(stepType) to figure out if the form should get a pipette (as was happening before in FORMS_WITH_PIPETTE array now deleted from generateNewForm.js), I'm hoping it will be easier to answer queries like "does this step have a 'foo' field?" by making sure to include it in the initial default values here, so we can always do 'foo' in formData anywhere downsteam

'volume': undefined,
}
case 'consolidate':
Expand All @@ -37,6 +38,7 @@ export default function getDefaultsForStepType (stepType: StepType) {
'dispense_labware': null,
'dispense_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE,
'dispense_wells': [],
'pipette': null,
'volume': undefined,
}
case 'mix':
Expand All @@ -45,9 +47,10 @@ export default function getDefaultsForStepType (stepType: StepType) {
'labware': null,
'aspirate_wellOrder_first': DEFAULT_WELL_ORDER_FIRST_OPTION,
'aspirate_wellOrder_second': DEFAULT_WELL_ORDER_SECOND_OPTION,
'wells': [],
'mix_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE, // NOTE: mix uses dispense for both asp + disp, for now
'pipette': null,
'volume': undefined,
'wells': [],
}
case 'distribute':
return {
Expand All @@ -62,6 +65,7 @@ export default function getDefaultsForStepType (stepType: StepType) {
'dispense_wellOrder_second': DEFAULT_WELL_ORDER_SECOND_OPTION,
'dispense_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE,
'dispense_wells': [],
'pipette': null,
'volume': undefined,
}
default:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// @flow
import findKey from 'lodash/findKey'
import last from 'lodash/last'
import type {PipetteData} from '../../../step-generation'
import type {StepIdType, FormData} from '../../../form-types'

// TODO: Ian 2018-09-18 once we support switching pipettes mid-protocol,
Expand All @@ -11,18 +13,19 @@ import type {StepIdType, FormData} from '../../../form-types'
export default function getNextDefaultPipetteId (
savedForms: {[StepIdType]: FormData},
orderedSteps: Array<StepIdType>,
pipetteIdsByMount: {left?: ?string, right?: ?string}
equippedPipettesById: {[string]: PipetteData}
): string {
const prevPipetteSteps = orderedSteps
.map(stepId => savedForms[stepId])
.filter(form => form && form.pipette)

const lastPipetteStep = last(prevPipetteSteps)

const nextDefaultPipette = (
// NOTE: order of findKey not guaranteed, expecting at most one pipette on each mount
const nextDefaultPipette: ?string = (
(lastPipetteStep && lastPipetteStep.pipette) ||
pipetteIdsByMount['left'] ||
pipetteIdsByMount['right']
findKey(equippedPipettesById, p => p.mount === 'left') ||
findKey(equippedPipettesById, p => p.mount === 'right')
)

if (!nextDefaultPipette) {
Expand Down
Loading