refactor(step-generation, protocol-designer): atomic command args to … #17324
Conversation
…match command params
| export interface ExtendedAspirateParams extends AspDispAirgapParams { | ||
| tipRack: string | ||
| nozzles: NozzleConfigurationStyle | null | ||
| isAirGap?: boolean |
There was a problem hiding this comment.
tipRack and nozzles are needed for some timeline errors. isAirGap is to signify if the command is an aspirate air gap or not. Gonna investigate that more though because i don't see that meta key in the latest aspirate command schema args...
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #17324 +/- ##
=======================================
Coverage 73.84% 73.84%
=======================================
Files 43 43
Lines 3304 3304
=======================================
Hits 2440 2440
Misses 864 864
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -95,6 +95,7 @@ export const generateRobotStateTimeline = ( | |||
| curryCommandCreator(moveToAddressableArea, { | |||
| pipetteId, | |||
| addressableAreaName, | |||
| offset: { x: 0, y: 0, z: 0 }, | |||
| @@ -105,6 +106,8 @@ interface ReplaceTipArgs { | |||
| } | |||
|
|
|||
| /** | |||
| TODO: need to move this out of atomic command since it breaks the rules of atomic commands. I don't | |||
| think it falls into the pattern of compount commands either though | |||
There was a problem hiding this comment.
Why doesn't it fit into the pattern of compound commands?
Also, compount -> compound.
There was a problem hiding this comment.
you're right, it can just be moved to compound. I think i was overthinking it. I'll move it
| @@ -254,7 +257,10 @@ export const replaceTip: CommandCreator<ReplaceTipArgs> = ( | |||
| channels === 96 && args.nozzles != null && args.nozzles !== stateNozzles | |||
| ? [ | |||
| curryCommandCreator(configureNozzleLayout, { | |||
| nozzles: args.nozzles, | |||
| configurationParams: { | |||
| primaryNozzle: args.nozzles === COLUMN ? 'A12' : undefined, | |||
There was a problem hiding this comment.
good question, it is the "default" primary nozzle for column pick up for a 96-channel. I'll use the constant we have for it instead of using the string.
…match command params
AUTH-1361
Overview
This is Pt2 of refactoring the atomic command args to match command params. all the module atomic commands were updated in pt1 so this refactors the remaining atomic commands
Test Plan and Hands on Testing
smoke test PD, this is mainly step-generation though so if the cypress migration tests pass i think this is good to go.
Changelog
migrate all atomic commands args to match command params and clean up affected tests
Review requests
returnTipisn't an atomic command but i'm not sure if it's a compound command either since it is only called within a compound command. Makes me think we need a new category for it or that we need to better define the differences between atomic vs compound commands. I'll think about this more.Risk assessment
med-ish but shouldn't affect any JSON files and commands emitted since its just refactoring the atomic command args