-
Notifications
You must be signed in to change notification settings - Fork 180
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): simulate liquid probe results #17582
Conversation
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.
Inline changes, but let's also do the following:
The idea for this is that eventually the simulated-value standin will be an instance of a class that can support math operations and return itself; and that new instances will be returned from every unique probe. To support that now, we should make this a class and an instance of that class instead of a string literal. We should define it in opentrons.types
, the way other data structures shared by the engine and protocol api, like SlotName
, are defined; then we can import it into the engine.
api/src/opentrons/protocol_engine/types/liquid_level_detection.py
Outdated
Show resolved
Hide resolved
f7af5d7
to
c57ecd8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #17582 +/- ##
=======================================
Coverage 25.68% 25.68%
=======================================
Files 2851 2851
Lines 219457 219457
Branches 17971 17971
=======================================
Hits 56363 56363
Misses 163079 163079
Partials 15 15
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.
Looks great! Let's maybe use the modern type syntax more though. I'm really excited for this one
api/src/opentrons/protocol_engine/types/liquid_level_detection.py
Outdated
Show resolved
Hide resolved
5ef5536
to
7be981c
Compare
A PR has been opened to address analyses snapshot changes. Please review the changes here: #17653 |
27b98e5
to
da6d913
Compare
A PR has been opened to address analyses snapshot changes. Please review the changes here: #17659 |
A PR has been opened to address analyses snapshot changes. Please review the changes here: #17659 |
Overview
In implementing liquid probing, we implemented a placeholder for simulating
liquid_probe_in_place
, where we just return the top position of a well unconditionally. This is fine for testingaspirate
, but causes a problem if you want to dispense, or really simulate a realistic chain of liquid handling actions.This code changes that simulated liquid probe function so that it creates and returns a
SimulatedProbeResult
object, and passes that into the protocol engine. This object will only be created bypipetting::liquid_probe_in_place
, and will keep track of the state of aspirates/dispenses in a given well until anotherliquid_probe
is called on the same well. I think we can eventually use this to do some cool error prevention/handling as it pertains to real liquid probe results from on-robot runs, but for now the running state will just be updated in the same place where well updates like_handle_liquid_probed
and_handle_liquid_operated
are made.Changelog
VirtualPipettingHandler::liquid_probe_in_place
create & return aSimulatedProbeResult
instead of well depthLiquidTrackingType
that is a Union offloat
andSimulatedProbeResult
geometry.py
functions and places where liquid height/volume values are analyzed to handleSimulatedProbeResult
sSimulatedType
inupdate_types
Test Plan
edge
edge