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): add default values to optional fields of QUADRANT configuration #16002

Closed

Conversation

sanni-t
Copy link
Member

@sanni-t sanni-t commented Aug 14, 2024

Closes RQA-2992

Overview

The QuadrantNozzleLayoutConfiguration has two optional fields- frontRightNozzle and backLeftNozzle, but they didn't have a default value assigned. So when the app tries to fetch the run's data, if the RunStore reads the following valid command from the database-

{
  "id": "44c2ebda-4e81-42f4-bbd7-91710ca30242",
  "createdAt": "2024-08-13T20:00:19.662654+00:00",
  "commandType": "configureNozzleLayout",
  "key": "2a57aa0e5f1e286b479c11d95e2cce35",
  "status": "succeeded",
  "params": {
    "pipetteId": "984f1433-acfd-48e7-b213-3c6fdfd85321",
    "configurationParams": {
      "style": "QUADRANT",
      "primaryNozzle": "H1",
      "backLeftNozzle": "B1"
    }
  },
  "result": {},
  "startedAt": "2024-08-13T20:00:19.664360+00:00",
  "completedAt": "2024-08-13T20:00:19.676488+00:00",
  "notes": []
}

..then pydantic runs into a validation error since it doesn't know how to parse configurationParams with a missing frontRightNozzle.

This was resulting in the 'Download Run Log' option to give a 500 error.

Once default values are set for the fields, pydantic is able to parse the params correctly and run log downloads without any issues.

Test Plan and Hands on Testing

  • Run the below protocol on a robot
  • Close the run (this is important)
  • Download run log from app. This should succeed

Protocol for testing:

from opentrons.protocol_api import PARTIAL_COLUMN

metadata = {
    "protocolName": "8 Channel SINGLE Happy Path A1 or H1",
    "description": "Tip Rack South Clearance for the 8 Channel pipette and a SINGLE partial tip configuration.",
}

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

def run(protocol):
    trash = protocol.load_trash_bin("A3")  # must load trash bin
    thermocycler = protocol.load_module("thermocycler module gen2")
    partial_tip_rack = protocol.load_labware(
        load_name="opentrons_flex_96_tiprack_50ul",
        label="Partial Tip Rack",
        location="B3",
    )
    pipette = protocol.load_instrument(instrument_name="flex_8channel_50", mount="right")
    # mount on the right and you will get an error.
    pipette.configure_nozzle_layout(
        style=PARTIAL_COLUMN,
        start="H1",
        end="B1",  # 2 Tips
        tip_racks=[partial_tip_rack],
    )
    source_labware_B2 = protocol.load_labware(
        load_name="nest_96_wellplate_100ul_pcr_full_skirt",
        label="B2 Source Labware",
        location="B2",
    )
    destination_labware_C2 = protocol.load_labware(
        load_name="nest_96_wellplate_100ul_pcr_full_skirt",
        label="C2 Destination Labware",
        location="C2",
    )
    volume = 10  # Default volume for actions that require it
    pipette.transfer(volume, source_labware_B2["A6"], destination_labware_C2["A6"])

Review requests

It's been a while since I looked at the partial tip code so want to make sure that parts of the code that consume quadrant configuration know to verify the presence of at least one of frontRightNozzle or backLeftNozzle and to handle the case where neither is specified.

The alternative to this approach would be to make the two fields non-optional and make the python API extract the info of the missing field before calling the engine command.

Risk assessment

None. Bug fix.

@sanni-t sanni-t requested a review from a team as a code owner August 14, 2024 19:57
@sanni-t
Copy link
Member Author

sanni-t commented Aug 14, 2024

The alternative to this approach would be to make the two fields non-optional and make the python API extract the info of the missing field before calling the engine command.

Talked with @CaseyBatten and it's looking like this is the better approach. Going to check the work involved and make the changes so hold off on reviewing.

@sanni-t sanni-t force-pushed the fix-nozzle_config_optional_fields branch from b158be7 to ea89a57 Compare August 15, 2024 19:32
@sanni-t
Copy link
Member Author

sanni-t commented Aug 15, 2024

Opening a new PR since the fix approach changed

@sanni-t sanni-t closed this Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant