-
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): allow custom user offsets for deck configured trash bins and waste chute #14560
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #14560 +/- ##
=======================================
Coverage 67.80% 67.80%
=======================================
Files 2509 2509
Lines 72156 72156
Branches 9287 9287
=======================================
Hits 48922 48922
Misses 21006 21006
Partials 2228 2228
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…sposal_location_user_offsets
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.
These changes look good, and don't interrupt checks we've added elsewhere for height conflicts. We may want to test that when doing manual offsets over a trashbin on a 96ch drop tip with something tall like a 96ch adapter infront of it, that the instrument doesn't collide with that adapter. I believe all of our conflict checks take the actual current position of the instrument into account when checking for heigh collisions with slot items, so we are likely safely raising an error in such a case.
"""Abstract class for disposal location.""" | ||
|
||
def top(self, x: float = 0, y: float = 0, z: float = 0) -> DisposalLocation: | ||
"""Returns a disposal location with a user set offset.""" |
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.
can we change this name? maybe disposale_location?
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.
great change! a few comments but overall looks great!
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!
# Overview Follow up to further explain behavior introduced in #14560. Addresses RTC-400. # Test Plan - [sandbox](http://sandbox.docs.opentrons.com/docs-trash-offsets/v2/robot_position.html#position-relative-to-trash-containers) - simulated snippets in 2.16 / `edge` # Changelog - new § on Labware and Deck Positions page - updated description and example for `move_to()` to cover new acceptable `location`s - version added statements for `TrashBin`, `WasteChute`, and their methods in API Reference # Review requests - double check code snippets. not only simulate, but do what i say they do? - confirm version added for the methods. it feels like we may need to gate this to 2.17, because otherwise there will be a big functionality gap between robot system 7.1 (which can't do any of this) and 7.2 (which can). the classes are unambiguously _New in version 2.16._ # Risk assessment low…but see API version gate caveat above.
# Overview Follow up to further explain behavior introduced in #14560. Addresses RTC-400. # Test Plan - [sandbox](http://sandbox.docs.opentrons.com/docs-trash-offsets/v2/robot_position.html#position-relative-to-trash-containers) - simulated snippets in 2.16 / `edge` # Changelog - new § on Labware and Deck Positions page - updated description and example for `move_to()` to cover new acceptable `location`s - version added statements for `TrashBin`, `WasteChute`, and their methods in API Reference # Review requests - double check code snippets. not only simulate, but do what i say they do? - confirm version added for the methods. it feels like we may need to gate this to 2.17, because otherwise there will be a big functionality gap between robot system 7.1 (which can't do any of this) and 7.2 (which can). the classes are unambiguously _New in version 2.16._ # Risk assessment low…but see API version gate caveat above.
# Overview Follow up to further explain behavior introduced in #14560. Addresses RTC-400. # Test Plan - [sandbox](http://sandbox.docs.opentrons.com/docs-trash-offsets/v2/robot_position.html#position-relative-to-trash-containers) - simulated snippets in 2.16 / `edge` # Changelog - new § on Labware and Deck Positions page - updated description and example for `move_to()` to cover new acceptable `location`s - version added statements for `TrashBin`, `WasteChute`, and their methods in API Reference # Review requests - double check code snippets. not only simulate, but do what i say they do? - confirm version added for the methods. it feels like we may need to gate this to 2.17, because otherwise there will be a big functionality gap between robot system 7.1 (which can't do any of this) and 7.2 (which can). the classes are unambiguously _New in version 2.16._ # Risk assessment low…but see API version gate caveat above.
# Overview Follow up to further explain behavior introduced in #14560. Addresses RTC-400. # Test Plan - [sandbox](http://sandbox.docs.opentrons.com/docs-trash-offsets/v2/robot_position.html#position-relative-to-trash-containers) - simulated snippets in 2.16 / `edge` # Changelog - new § on Labware and Deck Positions page - updated description and example for `move_to()` to cover new acceptable `location`s - version added statements for `TrashBin`, `WasteChute`, and their methods in API Reference # Review requests - double check code snippets. not only simulate, but do what i say they do? - confirm version added for the methods. it feels like we may need to gate this to 2.17, because otherwise there will be a big functionality gap between robot system 7.1 (which can't do any of this) and 7.2 (which can). the classes are unambiguously _New in version 2.16._ # Risk assessment low…but see API version gate caveat above.
# Overview Closes AUTH-42 In PR #14560, the ability to provide custom offsets for disposal location objects (trash bins and waste chutes loaded in API v2.16 and above) was added to be introduced in v2.18. With this addition came a slight change in behavior to our alternate tip drop behavior. For context, automatic tip drop alternation was something added in API v2.15, that when no explicit location was provided for `drop_tip`, the pipette's location it dropped the tip in would cycle between a left bias and right bias, to prevent tips from stacking up as quickly. When deck configured trash was introduced initially, there was no way to provide custom offsets, and so we'd always do the tip alternation regardless of whether the trash location was passed or not. With the addition of offsets, we've gone back to the pattern we used for labware based trash, where if you provide it even with no offset, we go to the center of the trash. However, the PR did not add a version gate for this change in behavior, which meant that 2.16 and 2.17 protocols would behave slightly differently in this new robot version. This PR fixes that and preserves parity with those API levels. # Test Plan Tested the following two protocols on robot and ensured that the 2.17 protocol and 2.18 worked as expected. ``` metadata = { 'protocolName': 'Tip Drop Alternation 2.17 test', } requirements = { "robotType": "Flex", "apiLevel": "2.17" } def run(context): trash = context.load_trash_bin('A3') tip_rack = context.load_labware('opentrons_flex_96_tiprack_200ul', 'C2') pipette = context.load_instrument("flex_1channel_1000", mount="left", tip_racks=[tip_rack]) # On 2.17 it should alternate for all four drop tip calls, regardless of trash being provided as location or not pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip() pipette.pick_up_tip() pipette.drop_tip() ``` ``` metadata = { 'protocolName': 'Tip Drop Alternation 2.18 test', } requirements = { "robotType": "Flex", "apiLevel": "2.18" } def run(context): trash = context.load_trash_bin('A3') tip_rack = context.load_labware('opentrons_flex_96_tiprack_200ul', 'C2') pipette = context.load_instrument("flex_1channel_1000", mount="left", tip_racks=[tip_rack]) # On 2.18 it should alternate only for the latter two calls, and go to the XY center for the first two pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip() pipette.pick_up_tip() pipette.drop_tip() ``` # Changelog - added a version gate for `alternate_tip_drop` to preserve 2.16 and 2.17 tip dropping behavior. # Review requests Is there anything that should be added to the docstring or release notes regarding this change? # Risk assessment Low.
# Overview Follow up to further explain behavior introduced in #14560. Addresses RTC-400. # Test Plan - [sandbox](http://sandbox.docs.opentrons.com/docs-trash-offsets/v2/robot_position.html#position-relative-to-trash-containers) - simulated snippets in 2.16 / `edge` # Changelog - new § on Labware and Deck Positions page - updated description and example for `move_to()` to cover new acceptable `location`s - version added statements for `TrashBin`, `WasteChute`, and their methods in API Reference # Review requests - double check code snippets. not only simulate, but do what i say they do? - confirm version added for the methods. it feels like we may need to gate this to 2.17, because otherwise there will be a big functionality gap between robot system 7.1 (which can't do any of this) and 7.2 (which can). the classes are unambiguously _New in version 2.16._ # Risk assessment low…but see API version gate caveat above.
…nd waste chute (#14560) Adds the ability to dispense, blow out, drop tip, and move to deck configured (api level 2.16 and above) trash bins and waste chutes --------- Co-authored-by: Edward Cormany <[email protected]>
# Overview Closes AUTH-42 In PR #14560, the ability to provide custom offsets for disposal location objects (trash bins and waste chutes loaded in API v2.16 and above) was added to be introduced in v2.18. With this addition came a slight change in behavior to our alternate tip drop behavior. For context, automatic tip drop alternation was something added in API v2.15, that when no explicit location was provided for `drop_tip`, the pipette's location it dropped the tip in would cycle between a left bias and right bias, to prevent tips from stacking up as quickly. When deck configured trash was introduced initially, there was no way to provide custom offsets, and so we'd always do the tip alternation regardless of whether the trash location was passed or not. With the addition of offsets, we've gone back to the pattern we used for labware based trash, where if you provide it even with no offset, we go to the center of the trash. However, the PR did not add a version gate for this change in behavior, which meant that 2.16 and 2.17 protocols would behave slightly differently in this new robot version. This PR fixes that and preserves parity with those API levels. # Test Plan Tested the following two protocols on robot and ensured that the 2.17 protocol and 2.18 worked as expected. ``` metadata = { 'protocolName': 'Tip Drop Alternation 2.17 test', } requirements = { "robotType": "Flex", "apiLevel": "2.17" } def run(context): trash = context.load_trash_bin('A3') tip_rack = context.load_labware('opentrons_flex_96_tiprack_200ul', 'C2') pipette = context.load_instrument("flex_1channel_1000", mount="left", tip_racks=[tip_rack]) # On 2.17 it should alternate for all four drop tip calls, regardless of trash being provided as location or not pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip() pipette.pick_up_tip() pipette.drop_tip() ``` ``` metadata = { 'protocolName': 'Tip Drop Alternation 2.18 test', } requirements = { "robotType": "Flex", "apiLevel": "2.18" } def run(context): trash = context.load_trash_bin('A3') tip_rack = context.load_labware('opentrons_flex_96_tiprack_200ul', 'C2') pipette = context.load_instrument("flex_1channel_1000", mount="left", tip_racks=[tip_rack]) # On 2.18 it should alternate only for the latter two calls, and go to the XY center for the first two pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip(trash) pipette.pick_up_tip() pipette.drop_tip() pipette.pick_up_tip() pipette.drop_tip() ``` # Changelog - added a version gate for `alternate_tip_drop` to preserve 2.16 and 2.17 tip dropping behavior. # Review requests Is there anything that should be added to the docstring or release notes regarding this change? # Risk assessment Low.
…nd waste chute (#14560) Adds the ability to dispense, blow out, drop tip, and move to deck configured (api level 2.16 and above) trash bins and waste chutes --------- Co-authored-by: Edward Cormany <[email protected]>
Overview
Closes RSS-391.
This PR adds the ability to dispense, blow out, drop tip, and move to deck configured (api level 2.16 and above) trash bins and waste chutes. In addition, a few refactors were done to make these objects more robust for future development. This includes giving them access to the engine client, moving the creation of them to the core, and adding unit test coverage for these.
The offset feature is done via a new
.top()
method, which returns an identical trash bin/waste chute with an offset that is set there via the standardx
,y
, andz
float parameters. There is no correspondingbottom
parameter added, since these do not make sense for either the trash bin or waste chute.One difference in behavior that will be seen in this future 2.18 version is that if you provide the trash bin directly as an argument for drop tip, i.e.
drop_tip(trash_bin)
, even without an offset, we will not automatically alternate tip drop location. That will now only happen when it is the user set/automatically set trash and no argument is provided, which matches the behavior for labware based trash bins.Currently these do no have API version gates. Once 7.2.0 is released and 2.17 is official, and we bump up our next version to 2.18, those will be added, which will also cover the change in behavior above so there is no difference in execution for lower api levels in the future 7.3.0.
Test Plan
Tested the functionality of the trash offsets with the following protocols. Some older version API protocols were ran to ensure nothing was broken in the refactors, in addition to unit test and integration test coverage for the trash.
Changelog
top
method for deck configuredTrashBin
andWasteChute
TrashBin
andWasteChute
to be created in cores and have access to the engine clientheight
method using the engine client to remove hard coding in deck conflictReview requests
I chose
top
as the name of the offset method to match what we use with wells, but I am open to other suggestions if we don't think that is a good API name.Risk assessment
Medium-low, I think our unit test coverage and the addition of protocol api integration tests have us well covered, and the implementation of the offset was done pretty straightforwardly.