-
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
fix(api): associate thermocycler with all slots it occupies #14491
fix(api): associate thermocycler with all slots it occupies #14491
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## chore_release-7.2.0 #14491 +/- ##
=======================================================
- Coverage 67.74% 67.74% -0.01%
=======================================================
Files 2517 2517
Lines 72068 72067 -1
Branches 9278 9278
=======================================================
- Hits 48823 48822 -1
Misses 21027 21027
Partials 2218 2218
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.
LGTM, Let's do it!
# 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
Addresses RQA-2311
Overview
This PR fixes a long-standing issue that the thermocycler is associated only with the slot it was loaded, even though it occupies more than one slot on both OT-2 & Flex. This marks the first step in making our backend aware of multiple slots used by any module.
The observable change of this PR is that
protocol_context.deck
in API 2.14 & above now returns the thermocycler module for items of slot 8, 10, 11 on OT-2 & A1 on Flex, when there is a thermocycler loaded in slot 7/ B1Currently non-observable changes (due to practical use cases and non-public API features):
Test Plan
The changes in this PR can be tested with analysis. Change API levels to make sure analysis passes for all versions >= 2.14.
Protocol analysis snapshot tests would be very useful for this.
Other than that, we just need to make sure that protocols run fine and as usual with these changes.
Changelog
state.geometry.get_slot_item(slot)
now returns the module that has the given slot in its 'additional slots occupied' liststate.geometry.get_highest_z_in_slot
now returns height of module occupying the slot (regardless of if it was loaded in it)Review requests
Risk assessment
Low-medium.
For suggested use of our API, this changes actually fixes a long-standing bug and is an improvement over the current API at a low risk. But for any protocols that might have been using loop-holes in the deck class, this change would break the behavior.