-
Notifications
You must be signed in to change notification settings - Fork 178
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(shared-data): add liquid class definition for water #16671
base: edge
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #16671 +/- ##
===========================================
- Coverage 92.43% 79.36% -13.08%
===========================================
Files 77 120 +43
Lines 1283 4493 +3210
===========================================
+ Hits 1186 3566 +2380
- Misses 97 927 +830
Flags with carried forward coverage won't be shown. Click here to find out more.
|
"namespace": "opentrons", | ||
"byPipette": [ | ||
{ | ||
"pipetteModel": "p10_single", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipetteModel
is not appropriate for the value we are using here- which is the API load name of a pipette.
We should do one of these things-
- Call this field
pipetteLoadName
so that it is more appropriate. Keeping the pipette identity be pipette load name makes it straightforward for if & when we allow custom liquid class definition files. Users know the pipettes by their API load names and hence will find it intuitive to use the load name. But, this will make it a bit convoluted for PD because PD uses engine-relevant pipette naming. - Keep
pipetteModel
and use true model names, i.e.,p10
,p20
,p300
, etc. This will only work (and in fact would be preferred) if the liquid class values are identical for all generations of the given pipette model - Change to
pipetteName
which will be the name used in all layers except the PAPI layer. This name is the same as API load name for the OT2 pipettes. It diverges from the API load names of Flex pipettes.
Pros: More straightforward when using PD, and consistent with the name that most of our backend uses
Cons: Will be confusing if/when we allow users to create their own custom definitions files. But maybe that won't be a problem if the primary way a user will create custom liquid class definitions is by using some Opentrons software (custom liquid definitions creator?). Also this won't pose much of a problem when defining custom liquid classes in the API because we can always do the PAPI-load-name <-> PE-load-name conversion.
WIP
Addresses AUTH-833
Overview
Adds definition for water.
I have added placeholder values for all OT2 and Flex pipettes with their respective compatible, regular Opentrons tipracks (non-filter). @andySigler will be adding the actual values.
Test Plan and Hands on Testing
Review requests
Risk assessment
None