-
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, labware-library): add deck riser definition and new Lid category to LL #16410
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #16410 +/- ##
==========================================
+ Coverage 73.77% 74.72% +0.95%
==========================================
Files 41 42 +1
Lines 2989 3118 +129
==========================================
+ Hits 2205 2330 +125
- Misses 784 788 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
This looks straight forward to me, great work!
@@ -95,6 +95,7 @@ export const SINGLE_CHANNEL_COMPATIBLE_LABWARE = [ | |||
'opentrons/opentrons_flex_96_tiprack_1000ul/1', | |||
'opentrons/opentrons_flex_96_tiprack_200ul/1', | |||
'opentrons/opentrons_flex_96_tiprack_50ul/1', | |||
'opentrons/opentrons_tough_pcr_auto_sealing_lid/1', |
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.
@syao1226
Let's verify that the auto-sealing lid does not appear as selectable labware in the Quick Transfer flow. If it does appear, then I think the generateCompatibleLabwareForPipette
utility needs to take into account lids or maybe this specific lid and possibly omit them from being added to the (SINGLE_CHANNEL_COMPATIBLE_LABWARE) array.
@smb2268
Is this accurate?
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.
Yeah, since this util is meant to return the labware that each pipette is able to aspirate/dispense into I think we should change the conditional on lines 16-17 of generateCompatibleLabwareForPipette
to be:
definition.allowedRoles != null &&
(definition.allowedRoles.includes('adapter') || definition.allowedRoles.includes('lid'))
However, we do have an additional layer in the quick transfer flow that only shows labware with display categories reservoir
, tubeRack
, and wellPlate
so the lid won't be displayed as an option regardless.
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.
okay i'll update generateCompatibleLabwareForPipette
to omit lids. thanks!
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.
changes look good to me!
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.
Thank you for making the quick transfer changes, this looks good ! Great work !
re AUTH-869, AUTH-892, PLAT-533, and AUTH-887
Overview
Adding a new deck riser definition with its image to shared data so it can be displayed in the Labware Library, and updating the TC lid definition to include a stacking offset for the deck riser. Also, adding a new 'Lid' category to the Labware Library for the TC auto-sealing lids.
Test Plan and Hands on Testing
For testing deck riser definition on a Flex:
deck_riser_with_tc_lid_test1.zip
Changelog
Review requests
Risk assessment