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(engine): fix labware gripperOffsets not allowing only a default offset #13472

Merged

Conversation

jbleon95
Copy link
Contributor

@jbleon95 jbleon95 commented Sep 6, 2023

Overview

As introduced in #13186, labware definitions can have a gripperOffsets property for providing an offset when picking up and dropping labware to and from that labware. It is required to have a default field, used for anything that does not have a specific slot name (e.g. SLOT_C1).

It was noticed in testing though that that default key, if there was no corresponding slot name key, was producing a KeyError because it was always attempting a direct access of the slot name key, regardless if it existed or not. This PR fixes that by first checking for the presence of the key before going for a direct access. This is not currently affecting any existing definition since the only one (opentrons_universal_flat_adapter) has all possible slot names it can be in in its gripperOffsets

Test Plan

Tested to make sure that a definition with a default only entry worked and did not crash, that gripping to and from a flat adapter still worked as expected, and that gripping to and from a labware with no offset continued to work.

Used this protocol with and without adding a default gripper offset to the aluminum block:

metadata = {
    'protocolName': 'Labware GripperOffsets test',
}

requirements = {
    "robotType": "OT-3",
    "apiLevel": "2.15"
}


def run(context):
    heater_shaker = context.load_module("heaterShakerModuleV1", location="D1")
    temp_module = context.load_module("temperatureModuleV2", location="D3")

    flat_adapter = heater_shaker.load_adapter("opentrons_universal_flat_adapter")
    aluminum_block = temp_module.load_adapter("opentrons_96_well_aluminum_block")

    corning_plate = context.load_labware("corning_384_wellplate_112ul_flat", location="C1")
    nest_plate = context.load_labware("nest_96_wellplate_100ul_pcr_full_skirt", location="C3")

    heater_shaker.open_labware_latch()

    context.move_labware(corning_plate, new_location=flat_adapter, use_gripper=True)
    context.home()
    heater_shaker.close_labware_latch()
    heater_shaker.open_labware_latch()
    context.move_labware(corning_plate, new_location="C1", use_gripper=True)


    context.move_labware(nest_plate, new_location=aluminum_block, use_gripper=True)
    context.home()
    context.move_labware(nest_plate, new_location="C3", use_gripper=True)
  "gripperOffsets": {
    "default": {
      "pickUpOffset": {
        "x": 0,
        "y": 0,
        "z": 0
      },
      "dropOffset": {
        "x": 0,
        "y": 0,
        "z": 10
      }
    }

Changelog

  • Fixed get_labware_gripper_offsets to check for presence of key before accessing it from parsed_offsets

Review requests

Risk assessment

Low, this fixes an exception that was being raised but otherwise does not change the function's behavior.

@jbleon95 jbleon95 requested a review from a team September 6, 2023 15:53
@jbleon95 jbleon95 requested a review from a team as a code owner September 6, 2023 15:53
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Fix makes sense to me. Thanks! One unrelated question about this area of the code.

@jbleon95 jbleon95 merged commit bd7f541 into chore_release-7.0.0 Sep 7, 2023
19 of 22 checks passed
@SyntaxColoring SyntaxColoring deleted the fix_labware_gripper_offset_default_only branch September 8, 2023 18:00
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.

2 participants