From a58110dfee34e91acfb3916efd589c6112b376fa Mon Sep 17 00:00:00 2001 From: IanLondon Date: Tue, 23 Oct 2018 19:08:42 -0400 Subject: [PATCH] fix(protocol-designer): do not add __air__ on blowout Closes #2498 --- .../src/step-generation/blowout.js | 2 +- .../dispenseUpdateLiquidState.js | 36 ++++++++++----- .../src/step-generation/dropTip.js | 2 +- .../test-with-flow/blowout.test.js | 2 +- .../dispenseUpdateLiquidState.test.js | 44 ++++++++++++++++++- .../test-with-flow/dropTip.test.js | 4 +- .../test-with-flow/transfer.test.js | 12 +++-- .../src/step-generation/types.js | 2 + .../src/step-generation/utils.js | 27 +++++++----- 9 files changed, 97 insertions(+), 34 deletions(-) diff --git a/protocol-designer/src/step-generation/blowout.js b/protocol-designer/src/step-generation/blowout.js index a5867a99689..7e535b309bf 100644 --- a/protocol-designer/src/step-generation/blowout.js +++ b/protocol-designer/src/step-generation/blowout.js @@ -50,7 +50,7 @@ const blowout = (args: PipetteLabwareFields): CommandCreator => (prevRobotState: pipetteData, labwareId: labware, labwareType: prevRobotState.labware[labware].type, - volume: pipetteData.maxVolume, // update liquid state as if it was a dispense, but with max volume of pipette + useFullVolume: true, well, }, prevRobotState.liquidState), }, diff --git a/protocol-designer/src/step-generation/dispenseUpdateLiquidState.js b/protocol-designer/src/step-generation/dispenseUpdateLiquidState.js index d4e5fc18b6f..a5384dc0cbe 100644 --- a/protocol-designer/src/step-generation/dispenseUpdateLiquidState.js +++ b/protocol-designer/src/step-generation/dispenseUpdateLiquidState.js @@ -1,9 +1,10 @@ // @flow +import assert from 'assert' import cloneDeep from 'lodash/cloneDeep' import mapValues from 'lodash/mapValues' import reduce from 'lodash/reduce' -import {splitLiquid, mergeLiquid, getWellsForTips} from './utils' -import type {RobotState, LocationLiquidState, PipetteData} from './' +import {splitLiquid, mergeLiquid, getWellsForTips, getLocationTotalVolume} from './utils' +import type {RobotState, LocationLiquidState, PipetteData, SourceAndDest} from './types' type LiquidState = $PropertyType @@ -11,7 +12,8 @@ export default function updateLiquidState ( args: { pipetteId: string, pipetteData: PipetteData, - volume: number, + volume?: number, + useFullVolume?: boolean, labwareId: string, labwareType: string, well: string, @@ -19,23 +21,33 @@ export default function updateLiquidState ( prevLiquidState: LiquidState ): LiquidState { // TODO: Ian 2018-06-14 return same shape as aspirateUpdateLiquidState fn: {liquidState, warnings}. - const {pipetteId, pipetteData, volume, labwareId, labwareType, well} = args - type SourceAndDest = {|source: LocationLiquidState, dest: LocationLiquidState|} + const {pipetteId, pipetteData, volume, useFullVolume, labwareId, labwareType, well} = args + + assert( + !(useFullVolume && typeof volume === 'number'), + 'dispenseUpdateLiquidState takes either `volume` or `useFullVolume`, but got both') + assert(typeof volume === 'number' || useFullVolume, + 'in dispenseUpdateLiquidState, either volume or useFullVolume are required') + + const {wellsForTips, allWellsShared} = getWellsForTips( + pipetteData.channels, labwareType, well) // remove liquid from pipette tips, // create intermediate object where sources are updated tip liquid states // and dests are "droplets" that need to be merged to dest well contents const splitLiquidStates: {[tipId: string]: SourceAndDest} = mapValues( prevLiquidState.pipettes[pipetteId], - (prevTipLiquidState: LocationLiquidState) => - splitLiquid( - volume, - prevTipLiquidState - ) + (prevTipLiquidState: LocationLiquidState): SourceAndDest => { + if (useFullVolume) { + const totalTipVolume = getLocationTotalVolume(prevTipLiquidState) + return totalTipVolume > 0 + ? splitLiquid(totalTipVolume, prevTipLiquidState) + : {source: {}, dest: {}} + } + return splitLiquid(volume || 0, prevTipLiquidState) + } ) - const {wellsForTips, allWellsShared} = getWellsForTips(pipetteData.channels, labwareType, well) - // add liquid to well(s) const labwareLiquidState = allWellsShared // merge all liquid into the single well diff --git a/protocol-designer/src/step-generation/dropTip.js b/protocol-designer/src/step-generation/dropTip.js index c5e0d2d4c5a..dfdd825171b 100644 --- a/protocol-designer/src/step-generation/dropTip.js +++ b/protocol-designer/src/step-generation/dropTip.js @@ -37,7 +37,7 @@ const dropTip = (pipetteId: string): CommandCreator => (prevRobotState: RobotSta pipetteData: prevRobotState.instruments[pipetteId], labwareId: FIXED_TRASH_ID, labwareType: 'fixed-trash', - volume: prevRobotState.instruments[pipetteId].maxVolume, // update liquid state as if it was a dispense, but with max volume of pipette + useFullVolume: true, well: 'A1', }, prevRobotState.liquidState), }, diff --git a/protocol-designer/src/step-generation/test-with-flow/blowout.test.js b/protocol-designer/src/step-generation/test-with-flow/blowout.test.js index 2a0dbe4d48a..8bc8958003d 100644 --- a/protocol-designer/src/step-generation/test-with-flow/blowout.test.js +++ b/protocol-designer/src/step-generation/test-with-flow/blowout.test.js @@ -114,7 +114,7 @@ describe('blowout', () => { expect(updateLiquidState).toHaveBeenCalledWith({ pipetteId: 'p300SingleId', labwareId: 'sourcePlateId', - volume: 300, // pipette's max vol + useFullVolume: true, well: 'A1', labwareType: 'trough-12row', pipetteData: robotStateWithTip.instruments.p300SingleId, diff --git a/protocol-designer/src/step-generation/test-with-flow/dispenseUpdateLiquidState.test.js b/protocol-designer/src/step-generation/test-with-flow/dispenseUpdateLiquidState.test.js index a72255d40b7..754ce6ae258 100644 --- a/protocol-designer/src/step-generation/test-with-flow/dispenseUpdateLiquidState.test.js +++ b/protocol-designer/src/step-generation/test-with-flow/dispenseUpdateLiquidState.test.js @@ -1,5 +1,6 @@ // @flow import merge from 'lodash/merge' +import omit from 'lodash/omit' import { createEmptyLiquidState, createTipLiquidState, @@ -35,7 +36,7 @@ beforeEach(() => { }) describe('...single-channel pipette', () => { - test('fully dispense single ingredient into empty well', () => { + test('fully dispense single ingredient into empty well, with explicit volume', () => { const initialLiquidState = merge( {}, getBlankLiquidState(), @@ -73,6 +74,47 @@ describe('...single-channel pipette', () => { }) }) + test('fully dispense single ingredient into empty well, with useFullVolume', () => { + const initialLiquidState = merge( + {}, + getBlankLiquidState(), + { + pipettes: { + p300SingleId: { + '0': { + ingred1: {volume: 150}, + }, + }, + }, + } + ) + + const result = _updateLiquidState( + { + ...omit(dispenseSingleCh150ToA1Args, 'volume'), + useFullVolume: true, + }, + initialLiquidState + ) + + expect(result).toMatchObject({ + pipettes: { + p300SingleId: { + '0': { + ingred1: {volume: 0}, + }, + }, + }, + labware: { + sourcePlateId: { + A1: {ingred1: {volume: 150}}, + A2: {}, + B1: {}, + }, + }, + }) + }) + test('dispense ingred 1 into well containing ingreds 1 & 2', () => { const initialLiquidState = merge( {}, diff --git a/protocol-designer/src/step-generation/test-with-flow/dropTip.test.js b/protocol-designer/src/step-generation/test-with-flow/dropTip.test.js index 3da8409aa4b..8d49dc14f12 100644 --- a/protocol-designer/src/step-generation/test-with-flow/dropTip.test.js +++ b/protocol-designer/src/step-generation/test-with-flow/dropTip.test.js @@ -117,7 +117,7 @@ describe('dropTip', () => { updateLiquidState.mockReturnValue(mockLiquidReturnValue) }) - test('dropTip calls dispenseUpdateLiquidState with the max volume of the pipette', () => { + test('dropTip calls dispenseUpdateLiquidState with useFullVolume: true', () => { const initialRobotState = makeRobotState({singleHasTips: true, multiHasTips: true}) const result = dropTip('p300MultiId')(initialRobotState) @@ -126,7 +126,7 @@ describe('dropTip', () => { { pipetteId: 'p300MultiId', labwareId: 'trashId', - volume: 300, // pipette's max vol + useFullVolume: true, well: 'A1', labwareType: 'fixed-trash', pipetteData: robotStateWithTip.instruments.p300MultiId, diff --git a/protocol-designer/src/step-generation/test-with-flow/transfer.test.js b/protocol-designer/src/step-generation/test-with-flow/transfer.test.js index cd5450287a6..f479e9aaf33 100644 --- a/protocol-designer/src/step-generation/test-with-flow/transfer.test.js +++ b/protocol-designer/src/step-generation/test-with-flow/transfer.test.js @@ -7,7 +7,6 @@ import { commandFixtures as cmd, } from './fixtures' import _transfer from '../transfer' -import {FIXED_TRASH_ID} from '../../constants' const transfer = compoundCommandCreatorNoErrors(_transfer) const transferWithErrors = compoundCommandCreatorHasErrors(_transfer) @@ -256,9 +255,14 @@ describe('single transfer exceeding pipette max', () => { cmd.dispense('B3', 50, {labware: 'destPlateId'}), ]) - // ignore trash contents because of "__air__" from dropped tips - // TODO: Ian 2018-09-17 fix "__air__" and remove this line $FlowFixMe - expectedFinalLiquidState.labware[FIXED_TRASH_ID] = result.robotState.liquidState.labware[FIXED_TRASH_ID] + // unlike the other test cases here, we have a new tip when aspirating from B1. + // so there's only ingred 1, and no ingred 0 + // $FlowFixMe flow doesn't like assigning to these objects + expectedFinalLiquidState.pipettes.p300SingleId['0'] = {'1': {volume: 0}} + + // likewise, there's no residue of ingred 0 in B3 from a dirty tip. + // $FlowFixMe flow doesn't like assigning to these objects + expectedFinalLiquidState.labware.destPlateId.B3 = {'1': {volume: 350}} expect(result.robotState.liquidState).toEqual(merge( {}, diff --git a/protocol-designer/src/step-generation/types.js b/protocol-designer/src/step-generation/types.js index eabf8b13983..a3f42dd5c24 100644 --- a/protocol-designer/src/step-generation/types.js +++ b/protocol-designer/src/step-generation/types.js @@ -160,6 +160,8 @@ export type SingleLabwareLiquidState = {[well: string]: LocationLiquidState} export type LabwareLiquidState = {[labwareId: string]: SingleLabwareLiquidState} +export type SourceAndDest = {|source: LocationLiquidState, dest: LocationLiquidState|} + // TODO Ian 2018-02-09 Rename this so it's less ambigious with what we call "robot state": RobotSimulationState? export type RobotState = {| instruments: { // TODO Ian 2018-05-23 rename this 'pipettes' to match tipState (& to disambiguate from future 'modules') diff --git a/protocol-designer/src/step-generation/utils.js b/protocol-designer/src/step-generation/utils.js index 33b23e47da8..1acac7fa3f6 100644 --- a/protocol-designer/src/step-generation/utils.js +++ b/protocol-designer/src/step-generation/utils.js @@ -8,9 +8,10 @@ import last from 'lodash/last' import {computeWellAccess} from '@opentrons/shared-data' import type { CommandCreator, + LocationLiquidState, RobotState, + SourceAndDest, Timeline, - LocationLiquidState, } from './types' import {AIR} from '@opentrons/components' @@ -91,13 +92,9 @@ export const commandCreatorsTimeline = (commandCreators: Array) type Vol = {volume: number} -/** Breaks a liquid volume state into 2 parts. Assumes all liquids are evenly mixed. */ -export function splitLiquid (volume: number, sourceLiquidState: LocationLiquidState): { - source: LocationLiquidState, - dest: LocationLiquidState, -} { - const totalSourceVolume = reduce( - sourceLiquidState, +export function getLocationTotalVolume (loc: LocationLiquidState): number { + return reduce( + loc, (acc: number, ingredState: Vol, ingredId: string) => { // air is not included in the total volume return (ingredId === AIR) @@ -106,13 +103,18 @@ export function splitLiquid (volume: number, sourceLiquidState: LocationLiquidSt }, 0 ) +} + +/** Breaks a liquid volume state into 2 parts. Assumes all liquids are evenly mixed. */ +export function splitLiquid (volume: number, sourceLiquidState: LocationLiquidState): SourceAndDest { + const totalSourceVolume = getLocationTotalVolume(sourceLiquidState) - // TODO Ian 2018-03-19 figure out what to do with air warning reporting - // if (AIR in sourceLiquidState) { - // console.warn('Splitting liquid with air present', sourceLiquidState) - // } + if (AIR in sourceLiquidState) { + console.warn('Splitting liquid with air present', sourceLiquidState) + } if (totalSourceVolume === 0) { + console.warn('splitting with zero source volume') // Splitting from empty source return { source: sourceLiquidState, @@ -121,6 +123,7 @@ export function splitLiquid (volume: number, sourceLiquidState: LocationLiquidSt } if (volume > totalSourceVolume) { + console.warn('volume to split exceeds total source volume, adding air', sourceLiquidState, volume, totalSourceVolume) // Take all of source, plus air return { source: mapValues(sourceLiquidState, () => ({volume: 0})),