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(protocol-designer, step-generation): wire up trash bin commands #14052

Merged
merged 11 commits into from
Dec 1, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Nov 28, 2023

closes RAUT-866 RAUT-883, partially addresses RAUT-876

Overview

This PR wires up dispense, air_gap, blow_out into the trash bin as well as a bit of clean up followup work.

Test Plan

Create a flex protocol and make sure the trash bin is added. Add some Transfer steps where you dispense and drop tip into the trash bin. see that nothing errors.

then export the protocol and examine the commands, you should see the moveToAddressableArea commands followed by the correct inPlace command for the action. You can even import the protocol in the app and it should pass analysis!

Then reimport the protocol back to PD and see that it imports correctly. (note: a timeline error might appear saying that the drop tip entity is missing, a fix for that will be in a follow up)

Changelog

  • wiring up PD components to have trash bin option in the transfer form
  • default aspirate and mix trash steps to null in the migration
  • wire up in the compound commands for dispensing, air gapping , or blowing out over a waste chute
  • some follow up clean up turning the low volume pipettes into a constant
  • refactor wasteChuteCommandsUtil and movableTrashCommandsUtil to be of return type CurriedCommandCreator[] since before, they were treated like compound commands but they are actually more like utils. So it is refactored to be similar to blowoutUtil
  • fix the broken cypess tests (Migrations, MixSettings, TransferSettings) and fix the bugs found when fixing the cypress test

Review requests

see test plan

Risk assessment

low

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Merging #14052 (03ae556) into edge (4e19e98) will increase coverage by 0.07%.
Report is 12 commits behind head on edge.
The diff coverage is 22.44%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #14052      +/-   ##
==========================================
+ Coverage   70.50%   70.58%   +0.07%     
==========================================
  Files        2507     2503       -4     
  Lines       71147    71048      -99     
  Branches     8900     8906       +6     
==========================================
- Hits        50163    50150      -13     
+ Misses      18805    18721      -84     
+ Partials     2179     2177       -2     
Flag Coverage Δ
app 67.77% <ø> (+0.04%) ⬆️
protocol-designer 45.01% <17.50%> (-0.01%) ⬇️
shared-data 75.03% <44.44%> (+1.38%) ⬆️
step-generation 86.84% <ø> (+0.08%) ⬆️

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

Files Coverage Δ
...src/components/FileSidebar/utils/getUnusedTrash.ts 83.33% <ø> (ø)
protocol-designer/src/step-forms/reducers/index.ts 48.99% <ø> (+1.08%) ⬆️
protocol-designer/src/steplist/fieldLevel/index.ts 27.11% <ø> (ø)
protocol-designer/src/ui/labware/selectors.ts 61.70% <100.00%> (+2.08%) ⬆️
protocol-designer/src/utils/index.ts 38.70% <ø> (ø)
shared-data/js/constants.ts 100.00% <100.00%> (ø)
...eneration/src/commandCreators/atomic/replaceTip.ts 48.00% <ø> (ø)
...ration/src/commandCreators/compound/consolidate.ts 100.00% <ø> (ø)
...eration/src/commandCreators/compound/distribute.ts 91.83% <ø> (-0.17%) ⬇️
...tep-generation/src/commandCreators/compound/mix.ts 94.73% <ø> (-0.72%) ⬇️
... and 20 more

... and 15 files with indirect coverage changes

@jerader jerader marked this pull request as ready for review November 28, 2023 21:43
@jerader jerader requested a review from a team as a code owner November 28, 2023 21:43
@jerader jerader requested a review from a team November 28, 2023 21:43
@jerader jerader requested a review from a team as a code owner November 28, 2023 21:43
@jerader jerader requested review from TamarZanzouri and removed request for a team and TamarZanzouri November 28, 2023 21:43
@jerader jerader force-pushed the pd_wireup-trash-bin branch from bb6f4bb to 44f43a9 Compare November 28, 2023 22:05
@jerader jerader force-pushed the pd_wireup-trash-bin branch from a246fbe to 682da25 Compare November 29, 2023 19:52
Comment on lines +107 to +109
!args.destLabware ||
(!invariantContext.labwareEntities[args.destLabware] &&
!invariantContext.additionalEquipmentEntities[args.destLabware])
Copy link
Collaborator Author

@jerader jerader Nov 29, 2023

Choose a reason for hiding this comment

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

this should work right? For some reason it only works if you import a protocol that is missing the dest labware (rather than deleting the dest labware mid protocol). I also noticed labwareDoesNotExist up above on line 99 has the same behavior. Maybe that can be investigated later.

Copy link
Member

@shlokamin shlokamin Dec 1, 2023

Choose a reason for hiding this comment

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

it depends where — step generation shorts circuits as soon as it sees an error so if there was another error before this in another form we wouldnt see anything

@jerader jerader requested a review from shlokamin December 1, 2023 14:08
const isWasteChute =
additionalEquipmentEntities[wasteChuteOrLabwareId]?.name === 'wasteChute'
const trashOrLabwareId = formData[addFieldNamePrefix('labware')]
const isTrash =
Copy link
Member

Choose a reason for hiding this comment

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

nit: could we call this disposalLocation or something? cc @ncdiehl11 and @SyntaxColoring

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

disposalLocation makes sense to me. I wasn't sure the best word to encompass both trash bin and waste chute.

@jerader jerader requested a review from shlokamin December 1, 2023 18:13
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.

lgtm!

@jerader jerader merged commit aa6312f into edge Dec 1, 2023
41 of 42 checks passed
@jerader jerader deleted the pd_wireup-trash-bin branch December 1, 2023 20:40
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