-
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(api): Tip tracking for all 96ch configurations #14488
feat(api): Tip tracking for all 96ch configurations #14488
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #14488 +/- ##
==========================================
- Coverage 67.50% 67.43% -0.08%
==========================================
Files 2514 2485 -29
Lines 72376 71474 -902
Branches 9317 9121 -196
==========================================
- Hits 48857 48195 -662
+ Misses 21314 21114 -200
+ Partials 2205 2165 -40
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -56,6 +59,7 @@ def __init__(self) -> None: | |||
channels_by_pipette_id={}, | |||
length_by_pipette_id={}, | |||
active_channels_by_pipette_id={}, | |||
nozzle_map=None, |
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 will need to be mapped by pipette id
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.
Sweet, thank you! Some preliminary comments. I haven't gotten to the new algorithmic stuff in tips.py
yet.
…starting tip only
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.
Sweet, thank you. Here are a few small suggestions, plus one more meaningful one about exposing the NozzleMap
type.
…unctions that take it
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 since my last review look good, thanks. This looks good to me if it looks good to you and @sanni-t.
As mentioned in calls, we think there's a lot of room for simplifying the logic in state/tips.py
. I'm happy for that to happen in other PRs if you think the tests you're introducing here will cover us enough in the meantime.
Adds tip tracking for all 96ch and 8ch configurations as long as no starting tip is specified
Adds tip tracking for all 96ch and 8ch configurations as long as no starting tip is specified
Overview
Addresses PLAT-53.
Previously tip tracking was gated to several pre-determined configuration types. Inside the TipView, we had each of these special cased and were handling them independently (such as a special case for single column pickup, for full 96 ch pickup, and for single tip pickup). This logic has been preserved, but can only be reached using starting tips.
In its place, the new logic instead utilizes information garnered from the nozzle configuration map to identify a critical tip within the tiprack based on the current configuration's ideal direction of entry. It then identifies a "cluster" of tips that would be picked up, and validates that none of them are Used. Positively confirmed clusters return their critical tip as the well to target for pickup, ensuring the entire desired cluster is picked up. Tips from that cluster are then set as Used, so on follow-up passes the next valid section of remaining tips may be used.
Test Plan
The following protocol should pass analysis and should have the following events occur:
NOTE: In order to execute this protocol on the Flex, you would first have to disable the raise case under _get_tip_presence() in tip_presence_manager.py. This is because currently the hardware controller does not support quadrants.
Changelog
Review requests
Does the logic at play in get_next_tip(...) make sense?
Does anything standout as possibly causing problems for us down the line?
Risk assessment
Medium/Low - After much testing and refactoring, I think we've arrived at a stable form of implementing this that frees up the ability to utilize partial tip configurations of all varieties alongside automatic tip tracking. The splits track for get_next_tip() is an appropriate solution due to the support needed for things like starting tip, which unfortunately would introduce unneeded complexity to partial tip pickup (primarily for multi/96 ch pipettes).