-
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
fix(api): OT-2 trash bins in 2.16 get added to engine at the start of protocol #14475
fix(api): OT-2 trash bins in 2.16 get added to engine at the start of protocol #14475
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## chore_release-7.2.0 #14475 +/- ##
====================================================
Coverage 67.78% 67.78%
====================================================
Files 2519 2519
Lines 72011 72011
Branches 9263 9263
====================================================
Hits 48812 48812
Misses 20991 20991
Partials 2208 2208
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -158,3 +162,17 @@ def create_protocol_context( | |||
deck=deck, | |||
bundled_data=bundled_data, | |||
) | |||
# If we're loading an engine based core into the context, and we're on api level 2.16 or above, and on an OT-2 | |||
# we need to insert a fixed trash addressable area into the protocol engine for correctness in anything that relies |
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.
👍
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 makes sense to me!! thank you for the comments they helped! if tested its good to merge imo
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.
Thank you!
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.
Nice!!
# Overview Add protocols pulled from v7.2.0 pull requests for Analysis Snapshot testing. All work filed under [RQA-2434](https://opentrons.atlassian.net/browse/RQA-2434) # Test Plan - [x] Add partial tip pickup protocols from Sanitti - [x] Add ABR protocol from [RABR-23](https://opentrons.atlassian.net/browse/RABR-23) - [x] Run new protocols locally and verify results - [x] Run in workflow dispatch action and verify success [[link]](https://github.com/Opentrons/opentrons/actions/runs/8160792870/job/22308167177) # Changelog Here are the protocols added and where I found them: - [RQA-2098](https://opentrons.atlassian.net/browse/RQA-2098) - Flex_P1000_96_Gripper_TC_TM_HS_AnalysisError_GripperCollisionWithTips.json - #14491 - Flex_None_None_TC_2_14_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_15_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_16_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_17_verifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_14_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_15_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_16_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_17_VerifyThermocyclerLoadedSlots.py - #14475 - Flex_None_None_TC_2_16_AnalysisError_TrashBinAndThermocyclerConflict.py - OT2_None_None_2_16_verifyDoesNotDeadlock.py - OT2_None_None_HS_2_16_AnalysisError_HeaterShakerConflictWithTrashBin1.py - OT2_None_None_HS_2_16_AnalysisError_HeaterShakerConflictWithTrashBin2.py - #14547 - Flex_P1000_96_None_TC_2_16_AnalysisError_pipetteCollisionWithThermocyclerLid.py - #14522 - Flex_P1000_96_None_TC_2_16_AnalysisError_pipetteCollisionWithThermocyclerLidClips.py - Dispense functionality validation - OT2_P300M_P20S_TC_HS_TM_2_15_dispense_changes.py - OT2_P300M_P20S_TC_HS_TM_2_16_dispense_changes.py - OT2_P300M_P20S_TC_HS_TM_2_17_dispense_changes.py - #14253 - OT2_P300S_None_2_16_verifyNoFloatingPointErrorInPipetting.py # Review requests There are a few pull requests that I had questions about. I will @ the people that worked on them to answer the questions - #14437 - @sfoster1, Can I use the protocol found in [RABR-23](https://opentrons.atlassian.net/browse/RABR-23) to get this to happen? Is there another way? - #14509 - @SyntaxColoring, can analysis capture this change or is this only relavant during actual protocol runtime? - #14510 - @TamarZanzouri or @SyntaxColoring, can I raise a generic python exception inside the protocol to trigger this functionality? - #14512 - @Laura-Danielle or @SyntaxColoring, Iant to validate my logic. This not a good canidate for snapshot testing because you have to run LPC and Calibration which is not taken into account during analysis. Can you confirm? # Risk assessment None [RQA-2434]: https://opentrons.atlassian.net/browse/RQA-2434?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RQA-2098]: https://opentrons.atlassian.net/browse/RQA-2098?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RABR-23]: https://opentrons.atlassian.net/browse/RABR-23?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Add protocols pulled from v7.2.0 pull requests for Analysis Snapshot testing. All work filed under [RQA-2434](https://opentrons.atlassian.net/browse/RQA-2434) - [x] Add partial tip pickup protocols from Sanitti - [x] Add ABR protocol from [RABR-23](https://opentrons.atlassian.net/browse/RABR-23) - [x] Run new protocols locally and verify results - [x] Run in workflow dispatch action and verify success [[link]](https://github.com/Opentrons/opentrons/actions/runs/8160792870/job/22308167177) Here are the protocols added and where I found them: - [RQA-2098](https://opentrons.atlassian.net/browse/RQA-2098) - Flex_P1000_96_Gripper_TC_TM_HS_AnalysisError_GripperCollisionWithTips.json - #14491 - Flex_None_None_TC_2_14_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_15_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_16_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_17_verifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_14_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_15_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_16_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_17_VerifyThermocyclerLoadedSlots.py - #14475 - Flex_None_None_TC_2_16_AnalysisError_TrashBinAndThermocyclerConflict.py - OT2_None_None_2_16_verifyDoesNotDeadlock.py - OT2_None_None_HS_2_16_AnalysisError_HeaterShakerConflictWithTrashBin1.py - OT2_None_None_HS_2_16_AnalysisError_HeaterShakerConflictWithTrashBin2.py - #14547 - Flex_P1000_96_None_TC_2_16_AnalysisError_pipetteCollisionWithThermocyclerLid.py - #14522 - Flex_P1000_96_None_TC_2_16_AnalysisError_pipetteCollisionWithThermocyclerLidClips.py - Dispense functionality validation - OT2_P300M_P20S_TC_HS_TM_2_15_dispense_changes.py - OT2_P300M_P20S_TC_HS_TM_2_16_dispense_changes.py - OT2_P300M_P20S_TC_HS_TM_2_17_dispense_changes.py - #14253 - OT2_P300S_None_2_16_verifyNoFloatingPointErrorInPipetting.py There are a few pull requests that I had questions about. I will @ the people that worked on them to answer the questions - #14437 - @sfoster1, Can I use the protocol found in [RABR-23](https://opentrons.atlassian.net/browse/RABR-23) to get this to happen? Is there another way? - #14509 - @SyntaxColoring, can analysis capture this change or is this only relavant during actual protocol runtime? - #14510 - @TamarZanzouri or @SyntaxColoring, can I raise a generic python exception inside the protocol to trigger this functionality? - #14512 - @Laura-Danielle or @SyntaxColoring, Iant to validate my logic. This not a good canidate for snapshot testing because you have to run LPC and Calibration which is not taken into account during analysis. Can you confirm? None [RQA-2434]: https://opentrons.atlassian.net/browse/RQA-2434?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RQA-2098]: https://opentrons.atlassian.net/browse/RQA-2098?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RABR-23]: https://opentrons.atlassian.net/browse/RABR-23?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Add protocols pulled from v7.2.0 pull requests for Analysis Snapshot testing. All work filed under [RQA-2434](https://opentrons.atlassian.net/browse/RQA-2434) - [x] Add partial tip pickup protocols from Sanitti - [x] Add ABR protocol from [RABR-23](https://opentrons.atlassian.net/browse/RABR-23) - [x] Run new protocols locally and verify results - [x] Run in workflow dispatch action and verify success [[link]](https://github.com/Opentrons/opentrons/actions/runs/8160792870/job/22308167177) Here are the protocols added and where I found them: - [RQA-2098](https://opentrons.atlassian.net/browse/RQA-2098) - Flex_P1000_96_Gripper_TC_TM_HS_AnalysisError_GripperCollisionWithTips.json - #14491 - Flex_None_None_TC_2_14_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_15_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_16_verifyThermocyclerLoadedSlots.py - Flex_None_None_TC_2_17_verifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_14_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_15_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_16_VerifyThermocyclerLoadedSlots.py - OT2_None_None_TC_2_17_VerifyThermocyclerLoadedSlots.py - #14475 - Flex_None_None_TC_2_16_AnalysisError_TrashBinAndThermocyclerConflict.py - OT2_None_None_2_16_verifyDoesNotDeadlock.py - OT2_None_None_HS_2_16_AnalysisError_HeaterShakerConflictWithTrashBin1.py - OT2_None_None_HS_2_16_AnalysisError_HeaterShakerConflictWithTrashBin2.py - #14547 - Flex_P1000_96_None_TC_2_16_AnalysisError_pipetteCollisionWithThermocyclerLid.py - #14522 - Flex_P1000_96_None_TC_2_16_AnalysisError_pipetteCollisionWithThermocyclerLidClips.py - Dispense functionality validation - OT2_P300M_P20S_TC_HS_TM_2_15_dispense_changes.py - OT2_P300M_P20S_TC_HS_TM_2_16_dispense_changes.py - OT2_P300M_P20S_TC_HS_TM_2_17_dispense_changes.py - #14253 - OT2_P300S_None_2_16_verifyNoFloatingPointErrorInPipetting.py There are a few pull requests that I had questions about. I will @ the people that worked on them to answer the questions - #14437 - @sfoster1, Can I use the protocol found in [RABR-23](https://opentrons.atlassian.net/browse/RABR-23) to get this to happen? Is there another way? - #14509 - @SyntaxColoring, can analysis capture this change or is this only relavant during actual protocol runtime? - #14510 - @TamarZanzouri or @SyntaxColoring, can I raise a generic python exception inside the protocol to trigger this functionality? - #14512 - @Laura-Danielle or @SyntaxColoring, Iant to validate my logic. This not a good canidate for snapshot testing because you have to run LPC and Calibration which is not taken into account during analysis. Can you confirm? None [RQA-2434]: https://opentrons.atlassian.net/browse/RQA-2434?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RQA-2098]: https://opentrons.atlassian.net/browse/RQA-2098?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RABR-23]: https://opentrons.atlassian.net/browse/RABR-23?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Overview
This PR refactors #14433 to fix a known inconsistency it introduced.
When the new
TrashBin
andWasteChute
objects were introduced in api level 2.16 in version 7.1.0 of the robot, there was a peculiarity of how they were loaded in PAPI versus when they would actually appear in the engine during analysis. Because of the way the protocol engine commands were designed, they'd only be recognized when first used i.e. whenever the pipette would first move to the trash for a drop tip, dispense, or blow out. This caused analysis caught errors to display wrong line numbers and made relying on the addressable area store in engine during analysis more difficult.To fix all this, in #14325 a protocol action was added to allow the addressable areas representing the trash bin and waste chute to be immediately added to engine on a load. This worked for Flex protocols, but because of a threading issue caused OT-2 protocols to hang during analysis. To fix this issue, #14433 reverted OT-2s to load the fixed trash addressable area on first reference, not at the start of the protocol.
This PR fixes that by refactoring the protocol core methods that add trash bins and waste chutes to the engine and internal list of disposal locations. Now there are two methods, one that just adds a disposal location (a
Labware
,TrashBin
orWasteChute
to the engine core'sself._disposal_locations
list and another one that does that and adds and validates the area, only for the latter two objects. Now for OT-2s, in theProtocolContext.__init__
method the OT-2 fixed trashTrashBin
is only appended to the core list. In order to fix the inconsistency, increate_protocol_context
the protocol engine method is called directly to add that trash.Test Plan
Re-using test protocols from previous PRs, this protocol passes analysis/simulate without deadlocking
These protocols correctly fail analysis
Changelog
skip_add_to_engine
boolean flag forappend_disposal_location
and remove adding it to engine in that methodadd_disposal_location_to_engine
to add and verify against the engine/deck conflict for new trash objectsshould_load_fixed_trash_area_for_python_protocol
method to see if we should add the area increate_protocol_context
and to make sure it stays in sync withProtocolContext.__init__
Risk assessment
Low.