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

fix(api): prevent moving a labware onto itself #16600

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

jbleon95
Copy link
Contributor

@jbleon95 jbleon95 commented Oct 24, 2024

Overview

Fixes PLAT-572.

This PR fixes a bug found where you were able to move a labware that could be stacked upon the same labware type (for example, the thermocycler lid) onto the same instance of the labware, ie.e stacking it on itself. Not only does this make no logical or physical sense, it would cause a RecursionError the next time any gantry movement happened, since during the course of finding the highest Z we would recursively find the height of any stacked labware. If a labware's parent happens to be itself (which should never happen), it will keep recursively trying to find a deck slot or module until the max recursion error occurs.

Now, when moving to an OnLabwareLocation, we check to make sure that the labware ID of the labware being moved does not match the labware ID of the new location. If it does, we will raise an error informing that the labware is trying to be moved on itself.

For more information on this recursive error, see PR #16600.

Test Plan and Hands on Testing

The following protocol now fails with a LabwareMovementNotAllowedError on the move_labware line, not with a RecursionError on the pick_up_tip line.

metadata = {
    'protocolName': 'Impossible Geometry Stacking Test',
}

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


def run(context):
    trash_bin = context.load_trash_bin('A3')
    tip_rack = context.load_labware('opentrons_flex_96_tiprack_50ul', 'D2')
    pipette = context.load_instrument('flex_1channel_50', mount="left", tip_racks=[tip_rack])

    lid1 = context.load_labware("opentrons_tough_pcr_auto_sealing_lid", "C3")
    lid2 = lid1.load_labware("opentrons_tough_pcr_auto_sealing_lid")

    context.move_labware(lid2, lid2, use_gripper=True)

    pipette.pick_up_tip()

Changelog

  • Prevent moving a labware onto the same instance of labware

Risk assessment

Low.

@jbleon95 jbleon95 requested a review from a team as a code owner October 24, 2024 18:24
Copy link
Contributor

@vegano1 vegano1 left a comment

Choose a reason for hiding this comment

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

This is perfect, thank you.

However, what about this case?

@@ -186,6 +186,10 @@ async def execute(self, params: MoveLabwareParams) -> _ExecuteReturn: # noqa: C
top_labware_definition=current_labware_definition,
bottom_labware_id=available_new_location.labwareId,
)
if params.labwareId == available_new_location.labwareId:
raise LabwareMovementNotAllowedError(
"Cannot move a labware onto itself."
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that's the error! :shipit:

@jbleon95 jbleon95 merged commit df01e77 into edge Oct 24, 2024
27 checks passed
@jbleon95 jbleon95 deleted the labware_movement_onto_self_fix branch October 24, 2024 20:12
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