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(api): support offset in json protocol touch-tip command #2566

Merged
merged 4 commits into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
22 changes: 21 additions & 1 deletion api/src/opentrons/protocols/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ def _get_location(loaded_labware, command_type, params, default_values):

if offset_from_bottom is None:
# not all commands use offsets

# touch-tip uses offset from top, not bottom, as default
# when offsetFromBottomMm command-specific value is unset
if command_type == 'touch-tip':
# TODO: Ian 2018-10-29 remove this `-1` when
# touch-tip-mm-from-top is a required field
return labware.wells(well).top(
z=default_values.get('touch-tip-mm-from-top', -1))

return labware.wells(well)

return labware.wells(well).bottom(offset_from_bottom)
Expand Down Expand Up @@ -178,7 +187,18 @@ def dispatch_commands(protocol_data, loaded_pipettes, loaded_labware): # noqa:
pipette.dispense(volume, location)

elif command_type == 'touch-tip':
pipette.touch_tip(location)
# NOTE: if touch_tip can take a location tuple,
# this can be much simpler
(wellObject, locTuple) = location
Copy link
Member

Choose a reason for hiding this comment

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

[minor] I'd like to keep the api code using snake_case (aside from property names that get accessed directly from javascript) since that's what the majority of code already is.


# Use the offset baked into the wellObject.
# Do not allow API to apply its v_offset kwarg default value,
# and do not apply the JSON protocol's default offset.
z_from_bottom = locTuple[2]
offset_from_top = (
wellObject.properties['depth'] - z_from_bottom) * -1

pipette.touch_tip(wellObject, v_offset=offset_from_top)


def execute_protocol(protocol):
Expand Down
4 changes: 4 additions & 0 deletions protocol-designer/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export const END_TERMINAL_TITLE = 'FINAL DECK STATE'
export const DEFAULT_CHANGE_TIP_OPTION: 'always' = 'always'
export const DEFAULT_MM_FROM_BOTTOM_ASPIRATE = 1
export const DEFAULT_MM_FROM_BOTTOM_DISPENSE = 0.5

// NOTE: in the negative Z direction, to go down from top
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'

Expand Down
2 changes: 2 additions & 0 deletions protocol-designer/src/file-data/selectors/fileCreator.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {selectors as steplistSelectors} from '../../steplist'
import {
DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
DEFAULT_MM_FROM_BOTTOM_DISPENSE,
DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
} from '../../constants'
import type {BaseState} from '../../types'
import type {ProtocolFile, FilePipette, FileLabware} from '../../file-types'
Expand All @@ -29,6 +30,7 @@ const executionDefaults = {
'dispense-flow-rate': getPropertyAllPipettes('dispenseFlowRate'),
'aspirate-mm-from-bottom': DEFAULT_MM_FROM_BOTTOM_ASPIRATE,
'dispense-mm-from-bottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE,
'touch-tip-mm-from-top': DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
}

export const createFile: BaseState => ProtocolFile = createSelector(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,27 @@ describe('touchTip', () => {
expect(result.robotState).toEqual(robotStateWithTip)
})

test('touchTip with tip, specifying offsetFromBottomMm', () => {
const result = touchTip({
pipette: 'p300SingleId',
labware: 'sourcePlateId',
well: 'A1',
offsetFromBottomMm: 10,
})(robotStateWithTip)

expect(result.commands).toEqual([{
command: 'touch-tip',
params: {
pipette: 'p300SingleId',
labware: 'sourcePlateId',
well: 'A1',
offsetFromBottomMm: 10,
},
}])

expect(result.robotState).toEqual(robotStateWithTip)
})

test('touchTip with invalid pipette ID should throw error', () => {
const result = touchTipWithErrors({
pipette: 'badPipette',
Expand Down
9 changes: 6 additions & 3 deletions protocol-designer/src/step-generation/touchTip.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// @flow
// import cloneDeep from 'lodash/cloneDeep'
import {noTipOnPipette, pipetteDoesNotExist} from './errorCreators'
import type {RobotState, CommandCreator, CommandCreatorError, PipetteLabwareFields} from './'
import type {RobotState, CommandCreator, CommandCreatorError, TouchTipArgs} from './'

const touchTip = (args: PipetteLabwareFields): CommandCreator => (prevRobotState: RobotState) => {
const touchTip = (args: TouchTipArgs): CommandCreator => (prevRobotState: RobotState) => {
/** touchTip with given args. Requires tip. */
const actionName = 'touchTip'
const {pipette, labware, well} = args
const {pipette, labware, well, offsetFromBottomMm} = args

const pipetteData = prevRobotState.instruments[pipette]

Expand All @@ -30,6 +30,9 @@ const touchTip = (args: PipetteLabwareFields): CommandCreator => (prevRobotState
pipette,
labware,
well,
offsetFromBottomMm: offsetFromBottomMm == null
? undefined
: offsetFromBottomMm,
},
}]

Expand Down
9 changes: 7 additions & 2 deletions protocol-designer/src/step-generation/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ export type PipetteLabwareFields = {|
/* TODO optional uL/sec (or uL/minute???) speed here */
|}

export type TouchTipArgs = {|
...PipetteLabwareFields,
offsetFromBottomMm?: ?number,
|}

export type AspirateDispenseArgs = {|
...PipetteLabwareFields,
volume: number,
Expand All @@ -217,10 +222,10 @@ export type Command = {|
command: 'aspirate' | 'dispense',
params: AspirateDispenseArgs,
|} | {|
command: 'pick-up-tip' | 'drop-tip' | 'touch-tip',
command: 'pick-up-tip' | 'drop-tip' | 'blowout',
params: PipetteLabwareFields,
|} | {|
command: 'blowout',
command: 'touch-tip',
params: {|
...PipetteLabwareFields,
offsetFromBottomMm?: ?number,
Expand Down
8 changes: 4 additions & 4 deletions protocol-designer/src/steplist/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,10 @@ export const currentFormCanBeSaved: Selector<boolean | null> = createSelector(
getSelectedStepId,
allSteps,
labwareIngredSelectors.getLabware,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this should be uncommented and the true deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oof, thanks for catching that!

(formData, selectedStepId, allSteps, labware) =>
((typeof selectedStepId === 'number') && allSteps[selectedStepId] && formData)
? Object.values(stepFormToArgs(formData, {labware}).errors).length === 0
: null
(formData, selectedStepId, allSteps, labware) => true
// ((typeof selectedStepId === 'number') && allSteps[selectedStepId] && formData)
// ? Object.values(stepFormToArgs(formData, {labware}).errors).length === 0
// : null
)

export default {
Expand Down
27 changes: 24 additions & 3 deletions shared-data/protocol-json-schema/protocol-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
"default-values": {
"description": "Default values required for protocol execution",
"type": "object",
"$note": "TODO: Ian 2018-10-29 make touch-tip-mm-from-top required (breaking change)",
"required": [
"aspirate-flow-rate",
"dispense-flow-rate",
Expand All @@ -153,7 +154,8 @@
"aspirate-flow-rate": {"$ref": "#/definitions/flow-rate-for-pipettes"},
"dispense-flow-rate": {"$ref": "#/definitions/flow-rate-for-pipettes"},
"aspirate-mm-from-bottom": {"$ref": "#/definitions/mm-offset"},
"dispense-mm-from-bottom": {"$ref": "#/definitions/mm-offset"}
"dispense-mm-from-bottom": {"$ref": "#/definitions/mm-offset"},
"touch-tip-mm-from-top": {"$ref": "#/definitions/mm-offset"}
}
},

Expand Down Expand Up @@ -282,13 +284,32 @@
},

{
"description": "Pick up tip / drop tip / touch tip / blowout commands",
"description": "Touch tip commands",
"type": "object",
"required": ["command", "params"],
"additionalProperties": false,
"properties": {
"command": {
"enum": ["pick-up-tip", "drop-tip", "touch-tip", "blowout"]
"enum": ["touch-tip"]
},
"params": {
"allOf": [
{"$ref": "#/definitions/pipette-access-params"},
{"$ref": "#/definitions/offsetFromBottomMm"}
]
}
}
},


{
"description": "Pick up tip / drop tip / blowout commands",
"type": "object",
"required": ["command", "params"],
"additionalProperties": false,
"properties": {
"command": {
"enum": ["pick-up-tip", "drop-tip", "blowout"]
},
"params": {
"allOf": [
Expand Down