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

refactor(step-generation): update nozzles in robotState not from initialRobotState #14155

Merged
merged 6 commits into from
Dec 8, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Dec 8, 2023

Overview

There was a PD bug where if you have multiple column pick up steps followed by a full tip pick up step, it'd error. This is for 96-channel! But this PR fixes it and cleans up the reset command

Test Plan

Test by adding 2 column pick up transfer steps followed by a Full tip pick up step. See that it doesn't error. Check tip state and see that it updates correctly. Download the protocol and see that there are 2 configureNozzleLayout commands for column and then for All

Changelog

  • remove setting nozzles from the initial robot state and set it through configureNozzleLayout state update.
  • remove prevNozzles since it isn't needed anymore
  • clean up the usage the configureNozzleLayout reset command, it isn't needed since the robot will reset the nozzles after each run

Review requests

see test plan

Risk assessment

low

@jerader jerader requested a review from a team December 8, 2023 20:49
@jerader jerader requested a review from a team as a code owner December 8, 2023 20:49
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Merging #14155 (12539d0) into edge (1ce77e5) will increase coverage by 0.00%.
Report is 2 commits behind head on edge.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #14155   +/-   ##
=======================================
  Coverage   70.36%   70.37%           
=======================================
  Files        2481     2481           
  Lines       70325    70332    +7     
  Branches     8780     8780           
=======================================
+ Hits        49486    49495    +9     
+ Misses      18687    18685    -2     
  Partials     2152     2152           
Flag Coverage Δ
protocol-designer 44.57% <0.00%> (+0.06%) ⬆️
step-generation 86.97% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ration/src/commandCreators/compound/consolidate.ts 100.00% <ø> (ø)
...eration/src/commandCreators/compound/distribute.ts 91.66% <ø> (ø)
...tep-generation/src/commandCreators/compound/mix.ts 94.44% <ø> (ø)
...eneration/src/commandCreators/compound/transfer.ts 98.38% <ø> (ø)
...neration/src/getNextRobotStateAndWarnings/index.ts 72.85% <ø> (ø)
step-generation/src/robotStateSelectors.ts 83.33% <ø> (+1.93%) ⬆️
step-generation/src/types.ts 100.00% <ø> (ø)
step-generation/src/utils/misc.ts 89.09% <ø> (ø)
...tocol-designer/src/file-data/selectors/commands.ts 31.03% <0.00%> (+3.76%) ⬆️

... and 2 files with indirect coverage changes

@jerader jerader requested review from shlokamin and removed request for a team December 8, 2023 20:52
Comment on lines -481 to -484
const configureNozzleLayoutCommandReset = getConfigureNozzleLayoutCommandReset(
args.pipette,
prevNozzles
)
Copy link
Collaborator Author

@jerader jerader Dec 8, 2023

Choose a reason for hiding this comment

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

this isn't actually needed since the robot will reset the nozzles to "All" at the end of each run. So i removed it to clean it up. Plus it should go in generateRobotStateTimeline instead since that's where the final drop tip commands get emitted

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

much cleaner now! 😄 🧹

Comment on lines 278 to 279
console.log('args.nozzles', args.nozzles)
console.log('state nozzles', stateNozzles)
Copy link
Member

Choose a reason for hiding this comment

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

leftover console logs

@jerader jerader merged commit 8a9330c into edge Dec 8, 2023
17 of 18 checks passed
@jerader jerader deleted the pd_fix-96-channel-bug branch December 8, 2023 21:20
ncdiehl11 pushed a commit that referenced this pull request Dec 19, 2023
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