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(step-generation): introduce airGapInPlace command #17357

Merged
merged 2 commits into from
Jan 28, 2025
Merged

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Jan 27, 2025

closes AUTH-1365

Overview

introduce airGapInPlace command to be used whenever there is an aspirate command that is meant to be an air gap - meaning an aspirate air gap is now: moveToWell followed by airGapInPlace

Dispense air gap (in the form of meta: isAirGap in a dispense command) was removed because

the engine internally tracks whether a dispense is dispensing air or liquid (based on whether it was an aspirate or air gap command that added the liquid to the pipette). The engine doesn't look at meta information like that

Test Plan and Hands on Testing

create a protocol with a transfer step utilizing the advanced settings air gap for aspirate. export it and see that moveToWell followed by airGapInPlace is emitted in the json file. then upload it to the app and make sure that it passes analysis.

Changelog

  • introduce new airGapInPlace atomic command and wire it into transfer, consolidate, and distribute
  • add moveToWell to emit before the airgap in place

Risk assessment

medium-ish

@jerader jerader requested a review from a team as a code owner January 27, 2025 19:41
@jerader jerader requested a review from ddcc4 January 27, 2025 19:41
@@ -217,27 +197,12 @@ export const consolidate: CommandCreator<ConsolidateArgs> = (
const aspirateCommands = flatMap(
sourceWellChunk,
(sourceWell: string, wellIndex: number): CurriedCommandCreator[] => {
const airGapOffsetSourceWell =
getWellDepth(sourceLabwareDef, sourceWell) + AIR_GAP_OFFSET_FROM_TOP
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the AIR_GAP_OFFSET_FROM_TOP? And does it not matter anymore when you're switching to airGapInPlace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

spoke with @sanni-t, this was my bad but i should've added a moveToWell before the airGapInPlace so the tip is outside of the liquid. Thanks for flagging this!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Modified the logic so now it is: moveToWell to move to the specific position in the well, followed by airGapInPlace

step-generation/src/__tests__/consolidate.test.ts Outdated Show resolved Hide resolved
@jerader jerader requested review from ddcc4 and ncdiehl11 January 28, 2025 14:21
@ddcc4
Copy link
Contributor

ddcc4 commented Jan 28, 2025

Remind me: before this change, did we emit 1 or 2 JSON commands to do the airgap?
After this change, it'll be 2 commands, right?

@jerader
Copy link
Collaborator Author

jerader commented Jan 28, 2025

Remind me: before this change, did we emit 1 or 2 JSON commands to do the airgap? After this change, it'll be 2 commands, right?

@ddcc4 just 1! it was just an aspirate pretending to be an air gap by moving to the top of the well.

flowRate,
pipetteId,
volume,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

With your new command, you don't need to set meta: isAirGap anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nope! airGapInPlace replaces meta: isAirGap

@@ -26,7 +26,6 @@ import type { CommandCreator, CommandCreatorError } from '../../types'
export interface DispenseAtomicCommandParams extends DispenseParams {
nozzles: NozzleConfigurationStyle | null
tipRack: string
isAirGap?: boolean
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 was isAirGap doing in dispense? Previously, did we need the meta: isAirGap in dispense to indicate that we were removing an airgap (instead of liquid)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good question, according to what Seth said on slack:

the engine internally tracks whether a dispense is dispensing air or liquid (based on whether it was an aspirate or air gap command that added the liquid to the pipette). The engine doesn't look at meta information like that

@jerader jerader merged commit 6277a89 into edge Jan 28, 2025
16 checks passed
@jerader jerader deleted the pd_airGapInPlace branch January 28, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants