-
Notifications
You must be signed in to change notification settings - Fork 179
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): add new user-facing helper functions for static liquid tracking #17360
Conversation
c43dc45
to
b85cfa2
Compare
ac308ea
to
762756f
Compare
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.
The idea behind the changes looks good, but I think by changing where and how some of these methods are called we can reduce complexity some. I like the engine internal refactor.
Please add tests for all of this.
api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py
Outdated
Show resolved
Hide resolved
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.
I think we can pull one more argument from estimate_liquid_height_after_pipetting
and keep it to just operation_volume
.
94cf3be
to
ff38927
Compare
35a8e69
to
016cffc
Compare
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 good to me, but please update the description (and the resulting commit message when you merge) for the changes that you've made
Overview
This provides the protocol api with some new lld-related functions for users:
InstrumentContext.get_minimum_liquid_sense_height() -> float
provides the minimum allowed liquid-level detection height for a given pipette and tip combination
Well.estimate_liquid_height_after_pipetting(starting_liquid_height: float, operation_volume: float) -> float
provides the projected ending height of the liquid meniscus after an aspirate or dispense. Note that it will have to be provided with a
starting_liquid_height
in the protocol, but this can be grabbed fromInstrumentContext.measure_liquid_height
. This will not propagate anInvalidLiquidHeightFoundError
if the result it finds is an invalid well location.Well.current_liquid_height(self) -> float:
provides the current known liquid height. If no liquid is probed or loaded in before this is called, it will cause aLiquidHeightUnknownError
.Well.current_liquid_volume(self) -> float:
provides the current known liquid volume. If no liquid is probed or loaded in before this is called, it will cause aLiquidVolumeUnknownError
.Changelog
get_well_height_after_volume
toget_well_height_after_liquid_handling
get_well_height_after_liquid_handling_no_error
ingeometry.py
LiquidVolumeUnknownError
for when we try to callwell.current_liquid_volume
but no liquid has been loaded or probedfind_height_at_well_volume
to disbale throwing anInvalidLiquidHeightError
, to enable estimating liquid height without making an aspirate or dispense moveget_well_current_volume
ingeometry.py
to get the volume from the most recent well liquid infoReview Requests
get_well_height_after_liquid_handling_no_error
could just be implemented as the addition of araise_error_if_result_invalid
argument toget_well_height_after_liquid_handling
, but I made it a separate function so it would be harder to disable raising that error by accident and causing a collision. Thoughts on this?