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

refactor(api, shared-data): Module load location refactor for module cutout load and compound fixtures #17499

Merged
merged 13 commits into from
Feb 18, 2025

Conversation

CaseyBatten
Copy link
Contributor

@CaseyBatten CaseyBatten commented Feb 11, 2025

Overview

Covers EXEC-1140, RABR-716, EXEC-1106

This PR introduces a engine refactor which will allow modules to "load" at the equipment and substate levels onto addressable areas for OT-2 (directly on the slot) or into cutout IDs for the Flex. It also enhances our location occupation validation logic to allow for more sophisticated checking of Fixtures and labware in regards to slot occupation. It removes custom hard coded logic that skipped the stacker (previously to enable functionality) and now instead supports compound fixtures in real validation (such as Waste chute with Flex Stacker or Magnetic Block with Flex Stacker). This also allows for fixtures, modules and labware to be loaded in any order so long as there would be no physical conflict with the locations referenced in a given load command.

To Do:

  1. Fix existing test cases
  2. Add new test cases which check each permutation for load order on compound spaces (Mag Block on D3, Labware on mag Block, Flex Stacker in D3)
  3. The modules.py function get_by_slot(...) probably needs to be updated to return a list of modules, as we can now have more than one module in a slot...

Test Plan and Hands on Testing

  • The following protocol should pass analysis, alongside permutations of the protocol which change load order:
from opentrons.protocol_api import ProtocolContext
requirements = {"robotType": "Flex", "apiLevel": "2.23"}

def run(protocol: ProtocolContext):
    stacker_b = protocol.load_module("flexStackerModuleV1", "B4")
    mag_block = protocol.load_module("magneticBlockV1", "B3")
    plate_on_mag = mag_block.load_labware("nest_96_wellplate_100ul_pcr_full_skirt")
    stacker_c = protocol.load_module("flexStackerModuleV1", "C4")
    
    stacker_d = protocol.load_module("flexStackerModuleV1", "D4")
    stacker_d.enter_static_mode()
    tiprack_d = stacker_d.load_labware("opentrons_flex_96_tiprack_1000ul")
    tiprack_c3 = protocol.load_labware("opentrons_flex_96_tiprack_1000ul", "C3")

    waste = protocol.load_waste_chute()
    protocol.move_labware(tiprack_d, waste, use_gripper=True)
    protocol.move_labware(tiprack_c3, waste, use_gripper=True)
    protocol.move_labware(plate_on_mag, waste, use_gripper=True)

    stacker_c.load_labware_to_hopper(
        "opentrons_flex_96_tiprack_1000ul", 2)
    stacker_b.load_labware_to_hopper(
        "opentrons_flex_96_tiprack_1000ul", 2)

Changelog

  • Addition of addressable area view as a subject of the module view (may change if possible)
  • In the module substate loaded modules are now in a dictionary of areas and cutout IDs rather than Deck Slot Names
  • The engine now exclusively converts the provided load location into an addressable area, and will now either "load" the module "onto" that area (OT2) or will find the root cutoutID which provides that area and "load" the module "into" that cutout.

Review requests

Risk assessment

Medium to low - This only touches backend code and not core or API functionality, but we should make sure that no special labware movement or object load rules have had their behavior changed due to this.

@CaseyBatten CaseyBatten requested review from a team as code owners February 11, 2025 21:33
@smb2268 smb2268 requested a review from a team as a code owner February 11, 2025 21:40
@smb2268 smb2268 requested review from shlokamin and removed request for a team February 11, 2025 21:40
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Structure of this looks great, thank you!

@@ -133,37 +134,39 @@ async def execute(self, params: LoadModuleParams) -> SuccessData[LoadModuleResul
# addressable areas and deck configurations the same way between OT-2 and Flex.
# Can this be simplified?
if self._state_view.config.robot_type == "OT-2 Standard":
Copy link
Member

Choose a reason for hiding this comment

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

nit: there's a deck configuration method that's something like get_deck_supports_cutout_fixtures() that might be a better call here

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.

🚀

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.25%. Comparing base (2f3849d) to head (84e6ab7).
Report is 2 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17499      +/-   ##
==========================================
+ Coverage   26.17%   26.25%   +0.08%     
==========================================
  Files        2838     2799      -39     
  Lines      217730   215886    -1844     
  Branches     9280     9268      -12     
==========================================
- Hits        56989    56687     -302     
+ Misses     160724   159182    -1542     
  Partials       17       17              
Flag Coverage Δ
protocol-designer 18.85% <ø> (-0.01%) ⬇️
step-generation 4.36% <ø> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...DeviceDetailsDeckConfiguration/AddFixtureModal.tsx 0.00% <ø> (ø)
...rdware-sim/DeckConfigurator/FlexStackerFixture.tsx 9.43% <ø> (+1.43%) ⬆️
shared-data/js/constants.ts 100.00% <ø> (ø)

... and 50 files with indirect coverage changes

@CaseyBatten CaseyBatten added the gen-analyses-snapshot-pr Generate a healing PR if the analyses snapshot test fails label Feb 18, 2025
Copy link
Contributor

A PR has been opened to address analyses snapshot changes. Please review the changes here: #17547

Copy link
Contributor

A PR has been opened to address analyses snapshot changes. Please review the changes here: #17547

@CaseyBatten CaseyBatten merged commit 6ce82af into edge Feb 18, 2025
63 checks passed
@CaseyBatten CaseyBatten deleted the module_location_resolution_refactor branch February 18, 2025 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-analyses-snapshot-pr Generate a healing PR if the analyses snapshot test fails
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants