Skip to content
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

Merged
merged 16 commits into from
Mar 1, 2024

Conversation

jbleon95
Copy link
Contributor

@jbleon95 jbleon95 commented Feb 28, 2024

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 standard x, y, and z float parameters. There is no corresponding bottom 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.

metadata = {
    'protocolName': 'Trash offset test',
}

requirements = {
	"robotType": "Flex",
	"apiLevel": "2.17"
}

def run(context):
	trash_bin = context.load_trash_bin('A1')
	tip_rack = context.load_labware('opentrons_flex_96_tiprack_50ul', 'D2')

	pipette = context.load_instrument('flex_1channel_1000', mount="left", tip_racks=[tip_rack])

	pipette.pick_up_tip()
	# This will automatically cause a tip drop alternation
	pipette.drop_tip()

	pipette.pick_up_tip()
	# This will cause no tip drop alternation
	pipette.drop_tip(trash_bin)

	pipette.pick_up_tip()
	# This will cause no alternation and use the offset
	pipette.drop_tip(trash_bin.top(x=25, z=20))

	pipette.pick_up_tip()
	# This will resume tip drop alternation
	pipette.drop_tip()
metadata = {
    'protocolName': 'Trash and waste chute offset test',
}

requirements = {
	"robotType": "Flex",
	"apiLevel": "2.17"
}

def run(context):
	trash_bin = context.load_trash_bin('A3')
	waste_chute = context.load_waste_chute()
	tip_rack = context.load_labware('opentrons_flex_96_tiprack_50ul', 'D1')
	plate = context.load_labware('opentrons_96_wellplate_200ul_pcr_full_skirt', 'C1')

	pipette = context.load_instrument('flex_1channel_1000', mount="left", tip_racks=[tip_rack])

	pipette.pick_up_tip()

	pipette.move_to(trash_bin.top(z=30))
	pipette.aspirate(20, plate['A1'].top())

	pipette.dispense(20, waste_chute.top(x=-10, y=10))
	pipette.blow_out(waste_chute.top(z=-5))

	pipette.drop_tip()

Changelog

  • added top method for deck configured TrashBin and WasteChute
  • refactored TrashBin and WasteChute to be created in cores and have access to the engine client
  • added an internal height method using the engine client to remove hard coding in deck conflict

Review 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.

@jbleon95 jbleon95 added the DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available label Feb 28, 2024
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.80%. Comparing base (c2cce41) to head (cbb7fb8).
Report is 2 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@           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           
Flag Coverage Δ
g-code-testing 92.43% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
api/src/opentrons/commands/commands.py 95.45% <ø> (ø)
api/src/opentrons/commands/helpers.py 85.00% <ø> (ø)
api/src/opentrons/commands/types.py 100.00% <ø> (ø)
api/src/opentrons/protocol_api/__init__.py 100.00% <ø> (ø)
...i/src/opentrons/protocol_api/instrument_context.py 89.09% <ø> (ø)
api/src/opentrons/protocol_api/protocol_context.py 92.74% <ø> (ø)

@jbleon95 jbleon95 removed the DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available label Feb 29, 2024
@jbleon95 jbleon95 marked this pull request as ready for review February 29, 2024 14:59
@jbleon95 jbleon95 requested a review from a team as a code owner February 29, 2024 14:59
Copy link
Contributor

@CaseyBatten CaseyBatten left a 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."""
Copy link
Contributor

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?

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a 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!

Copy link
Member

@sanni-t sanni-t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good!

@jbleon95 jbleon95 merged commit 050cf4d into edge Mar 1, 2024
22 checks passed
@jbleon95 jbleon95 deleted the disposal_location_user_offsets branch March 1, 2024 15:09
ecormany added a commit that referenced this pull request Mar 8, 2024
# 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.
ecormany added a commit that referenced this pull request Mar 8, 2024
# 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.
ecormany added a commit that referenced this pull request Mar 29, 2024
# 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.
ecormany added a commit that referenced this pull request Apr 19, 2024
# 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.
jbleon95 added a commit that referenced this pull request Apr 30, 2024
# 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.
ecormany added a commit that referenced this pull request May 2, 2024
# 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.
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
…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]>
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
# 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.
Carlos-fernandez pushed a commit that referenced this pull request Jun 3, 2024
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants