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): raise an error if pipette nozzles might collide with thermocycler lid clips #14522

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

sanni-t
Copy link
Member

@sanni-t sanni-t commented Feb 16, 2024

Addresses RQA-2311

Overview

This is the second part of the 3-part fix for RQA-2311.

Test Plan

The following protocol attempts to move the 96-ch to A2 in a way that it would collide with the TC pins. Make sure error is raised during analysis as well as during the actual run on the robot.

Change the destination well and verify that it does NOT raise for target positions that don't hit the thermocycler.

from opentrons import protocol_api
from opentrons.protocol_api import COLUMN, ALL
from opentrons import types

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

def run(protocol: protocol_api.ProtocolContext):
	thermocycler  = protocol.load_module('thermocycler module gen2')
	tiprack       = protocol.load_labware('opentrons_flex_96_tiprack_200ul', 'A2')
	p1000 		  = protocol.load_instrument("flex_96channel_1000", "left")
	thermocycler.open_lid()
	p1000.configure_nozzle_layout(style=COLUMN, start="A12", tip_racks=[tiprack])
	p1000.pick_up_tip(tiprack.wells()[0].center().move(types.Point(x=-10, y=10, z=-10)))

Changelog

  • added absolute measurements of the TC lid's clips
  • added collision check for the no-go zone around the lid clips
  • minor refactors

Review requests

  • any questions/ concerns about the approach?

Risk assessment

Low. Only adds a new no-go zone for the 96-channel.

@sanni-t sanni-t requested a review from a team as a code owner February 16, 2024 21:26
@sanni-t sanni-t requested a review from a team February 16, 2024 21:26
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f2978c7) 67.76% compared to head (57ca946) 67.80%.
Report is 10 commits behind head on chore_release-7.2.0.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.2.0   #14522      +/-   ##
=======================================================
+ Coverage                67.76%   67.80%   +0.03%     
=======================================================
  Files                     2517     2517              
  Lines                    72054    72209     +155     
  Branches                  9276     9367      +91     
=======================================================
+ Hits                     48828    48960     +132     
- Misses                   21008    21022      +14     
- Partials                  2218     2227       +9     
Flag Coverage Δ
app 64.79% <ø> (+0.16%) ⬆️
g-code-testing 92.43% <ø> (ø)
protocol-designer 37.93% <ø> (ø)
shared-data 75.27% <100.00%> (+<0.01%) ⬆️
step-generation 86.90% <ø> (ø)

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

Files Coverage Δ
api/src/opentrons/protocol_engine/state/modules.py 91.30% <ø> (ø)
...ta/python/opentrons_shared_data/module/__init__.py 93.93% <100.00%> (+0.18%) ⬆️

... and 8 files with indirect coverage changes

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.

Looks like this will help to satisfy the ticket that it seeks to solve, so good on that front. I have a question down below regarding cases we may not be catching with this logic however.

and engine_state.modules.is_flex_deck_with_thermocycler()
):
tc_right_clip_pos = FLEX_TC_LID_CLIP_POSITIONS_IN_DECK_COORDINATES["right_clip"]
for bound_vertex in pipette_bounds:
Copy link
Contributor

Choose a reason for hiding this comment

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

So currently we're only doing this check on the outside edge of the right clip. What happens in the following case:
If a single channel pipette were to move inside the thermocycler then manually offsets to a position in which it would collide with the inside edge (so left edge) of the right clip, would we not catch it?

Same question applies to anything that would collide with the left clip.

Copy link
Member Author

Choose a reason for hiding this comment

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

This check looks for anything left of the right edge of the right clip so it covers your case. That said, these checks are currently only enabled for partially configured pipettes so it won't raise for single channels.

But it just made me realize that this might actually raise an unnecessary error if an 8-channel is to the left of the left clip. Not sure if it's actually possible for an 8-channel to be in that nook safely. But I can update this check regardless.

@sanni-t sanni-t merged commit 7b456ed into chore_release-7.2.0 Feb 20, 2024
54 checks passed
@sanni-t sanni-t deleted the api-check_collision_with_tc_lid_fangs branch February 20, 2024 22:42
DerekMaggio added a commit that referenced this pull request Mar 7, 2024
# 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
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
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
Carlos-fernandez pushed a commit that referenced this pull request Jun 3, 2024
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
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.

3 participants