feat(api,app): finish up new stacker API #17620

sfoster1 · 2025-02-28T21:41:04Z

This PR removes static mode and updates the protocol api to use the new way of using the stacker.

The rest of the PR fixes things we missed in previous PRs, such as

We actually have to build new LabwareCore entries for the labware returned by retrieve in the protocol core
When we load multiple labware in retrieve, we need to be able to tell the rest of the engine about the rest of the labware we're loading, since if we're loading labware on adapter all at once, the internals need to know the identity and details of the adapter to figure out what to do with the labware
While we're not adding handling for command text for these commands, we do need to add something so the app doesn't crash. There's initial support for reading the data from the command results, but mostly I made the LPC internals ignore labware that we don't know the definition for instead of hitting the error boundary. So now labware that starts its life in the stacker hopper just won't show up in the app at all.
The app couldn't properly handle the new combo fixtures, because it was eagerly loading all the addressable areas provided by a default module feature when it saw a loadModule, which means there would be a fundamental conflict if you first put a stacker in D3 (providing flexStackerModuleV1D4 and D3 from the flexStackerModuleV1 cutout fixture, even if you weren't using D3) and then used a waste chute, which would conflict with D3.

to come out of dev

fix the tests that are failing
add new tests

review requirements

what do you think of the new labware core stuff
what do you think of the new way we load labware

testing

I ran this protocol:

from opentrons.protocol_api import ProtocolContext, OFF_DECK

metadata = {
    "protocolName": "test new stacker api",
}

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


def run(protocol: ProtocolContext) -> None:
    instrument = protocol.load_instrument("flex_8channel_1000", "left")

    stacker_1 = protocol.load_module("flexStackerModuleV1", "D4")
    stacker_2 = protocol.load_module("flexStackerModuleV1", "C4")

    stacker_1.set_stored_labware(
        "opentrons_flex_96_tiprack_200ul", lid="opentrons_flex_tiprack_lid", count=6
    )
    stacker_2.set_stored_labware(
        "opentrons_flex_96_tiprack_200ul", lid="opentrons_flex_tiprack_lid", count=6
    )

    def dump_rack_from_each():
        tiprack_1 = stacker_1.retrieve()
        protocol.move_labware(tiprack_1, "D2", use_gripper=True)
        tiprack_2 = stacker_2.retrieve()
        protocol.move_labware(tiprack_2, "C2", use_gripper=True)
        protocol.move_labware(tiprack_2, OFF_DECK)
        protocol.move_labware(tiprack_1, OFF_DECK)

    def swap_racks():
        tiprack_1 = stacker_1.retrieve()
        tiprack_2 = stacker_2.retrieve()
        protocol.move_labware(tiprack_1, "D3", use_gripper=True)
        protocol.move_labware(tiprack_2, stacker_1, use_gripper=True)
        protocol.move_labware(tiprack_1, stacker_2, use_gripper=True)
        stacker_1.store()
        stacker_2.store()

    dump_rack_from_each()
    swap_racks()
    dump_rack_from_each()
    swap_racks()
    dump_rack_from_each()
    swap_racks()
    dump_rack_from_each()
    swap_racks()
    dump_rack_from_each()
    swap_racks()
    dump_rack_from_each()

and it worked and the app didn't crash!

codecov · 2025-02-28T21:45:53Z

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 24.29%. Comparing base (d097caf) to head (b4a6ca1).
Report is 2 commits behind head on edge.

Files with missing lines	Patch %	Lines
...d-data/js/helpers/getAddressableAreasInProtocol.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #17620      +/-   ##
==========================================
- Coverage   25.67%   24.29%   -1.38%     
==========================================
  Files        2851     2851              
  Lines      219301   219298       -3     
  Branches    18012    18012              
==========================================
- Hits        56314    53288    -3026     
- Misses     162972   165996    +3024     
+ Partials       15       14       -1

Flag	Coverage Δ
protocol-designer	`18.87% <0.00%> (ø)`
step-generation	`4.34% <0.00%> (ø)`

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

Files with missing lines	Coverage Δ
...d-data/js/helpers/getAddressableAreasInProtocol.ts	`1.94% <0.00%> (ø)`

... and 60 files with indirect coverage changes

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ahiuchingau

The python side of things (labware core) make sense to me. Just got a nit.

ahiuchingau · 2025-03-03T18:07:27Z

api/src/opentrons/protocol_api/core/engine/module_core.py

@@ -701,20 +701,6 @@ class FlexStackerCore(ModuleCore, AbstractFlexStackerCore):

    _sync_module_hardware: SynchronousAdapter[hw_modules.FlexStacker]

-    def set_static_mode(self, static: bool) -> None:


ahiuchingau · 2025-03-03T18:17:30Z

api/src/opentrons/protocol_engine/commands/flex_stacker/retrieve.py

+                location=adapter_location,
+                loadName=adapter_lw.definition.parameters.loadName,
+                definitionUri=adapter_uri,
+                offsetId=None,
            )
        # Always load the primary labware
        if stacker_state.pool_primary_definition is None:


Can we make sure the pool primary definition exists before constructing the adapter definition?

We could, but keep in mind that there will be no state changes until this command completes and sends its state updates - the only thing we're doing here is getting information and building objects, we haven't submitted them to state yet

CaseyBatten

Looking good so far, some notes below on an issue

CaseyBatten · 2025-03-03T18:10:45Z

api/src/opentrons/protocol_engine/commands/flex_stacker/store.py


-        if stacker_state.pool_count == 5:
+        if stacker_state.pool_count == 6:


This and other hardcoded references will need replacement with Alise's solution from the internal height calculation PR, whenever/whichever merges first will need to coordinate as discussed in person.

CaseyBatten · 2025-03-03T18:31:00Z

api/src/opentrons/protocol_api/core/engine/protocol.py

+            core = LabwareCore(labware_id, self._engine_client)
+            self._labware_cores_by_id[labware_id] = core
+            return core


This is good for the api core, but we aren't actually updating the LoadedCoreMap at the protocol_api end, the _contexts_by_core dict never gets updated. We'd need to do something like self._core_map.add(...) a layer above or parallel to this somewhere. What will happen right now is if we do the following:

def run(protocol: ProtocolContext): stacker_b = protocol.load_module("flexStackerModuleV1", "B4") stacker_b.set_stored_labware(load_name="opentrons_flex_96_tiprack_1000ul", adapter="opentrons_flex_96_tiprack_adapter", count=6) labware = stacker_b.retrieve() protocol.move_labware(labware, "D1", True) adapter = stacker_b.labware protocol.move_labware(adapter, OFF_DECK, False)

We'd get an error like this when trying to do module_context.labware on the adapter: KeyError [line 10]: <opentrons.protocol_api.core.engine.labware.LabwareCore object at 0x1517c2b30>

We could add all three to the core map in retrieve and that would do it, right?

Uhhh I think that should cover us yeah! However its probably important to note any future batch-loading commands that we make would need to follow that same pattern.

Actually I think only the primary labware and the adapter need to be added to the core map, lids don't go in the core map as per load_labware's implementation.

…17638) Add the app-side changes from #17620 that handle - Adding the retrieve command to some of the command parse utilities (but almost certainly not all of them) - Fixing an LPC whitescreen when there are any unknown definitions, which really makes the problems with forgetting the locations of some of the command parsing stuff much worse - Adding retrieve types ## Review requests - cohesive enough when separated from the other pr? ## testing - [x] we should no longer crash when looking at a protocol that loads labware in a way we don't know about. instead that labware just isn't shown.

Moves the new stacker api to the python protocol API. This doesn't require that many changes - in fact, we're just removing things. We don't have set_static_mode anymore (this concept no longer exists); we don't have load_stacker_hopper() anymore (this has been replaced with fill/empty). Closes EXEC-1215

because that's what protocols use. Also, do a bunch of integration.

Two big things: - The core actually needs to make new labware cores sometimes, which is pretty easy to accomplish. We're getting the labware by asking the engine "hey what's on this module" after the command completes (rather than by getting the results of the command) because if the command failed and went through error recovery, its results may not be reliable. - More seriously, retrieve() needs to simultaneously load multiple labware. To do this, it uses internal engine methods that inspect what the position of the loaded labware is and walks the parent list down to the deck. So if you load labware A on location L and labware B on labware A simultaneously, how can you do this? The answer is probably "have a mechanism for doing state updates ahead of time as a temporary and operate on the projected state", but that's a lot so the answer for now is "the affected methods get a projection of a slice of state".

We do deck configuration conflict checking by accumulating all used addressable areas; figuring out every set of cutout fixtures that could provide those addressable areas; and then figuring out if there are any cutout fixtures loaded currently that are not in any of those sets of cutout fixtures. We accumulate addressable areas differently in the engine and the app. In the engine, as a command decides what to do and which addressable areas it will touch, it marks those addressable areas as used. It can do that because, well, it's the thing touching them. And it can and will mark an addressable area as used if it's used, _even if it's not apparent to anything outside the engine that this addressable area is in fact used_. But in the app, we can't see inside the engine. We can only see the command parameters and the command results. So we emulate this, but we can't emulate it fully, because there are some commands that touch addressable areas that don't show up in their commands or results - for instance, the absorbance reader open and close lid commands. So the app, specifically for modules because that's where the problem is, was essentially doing deck configuration twice for modules: when it saw a module loaded, it would figure out the addressable area for that module (which isn't in the command) by loading the cutout fixture for that module into the cutout that module's location specified and then marking all the addressable areas that the module fixture provides when loaded into that addressable area as used. Meantime, the engine is still just marking the addressable areas that are actually used as used. This worked until we added these combo fixtures that fuse the stacker with other fixtures, like the magblock or the waste chute. Now, when the app proactively loads the two addressable areas provided by the default stacker fixture, there is no resolvable set of cutout fixtures that can also provide the addressable areas used by the combo fixtures: nothing can be provide D3, _and_ flexStackerModuleV1D4, _and_ gripperWasteChute, because D3 can't be there. This was causing the app to break. One fix is to only load the "primary" (here, first) AA provided by a module fixture, and rely on some combination of the projection back through deck config to make the other AAs load and on the use of those AAs in subsequent commands if threy matter. A better fix is to just have the engine list the possible cutout fixture configurations that will work for the protocol. But this is good for now.

…reas

…of presumed slot area

CaseyBatten

Changes integrate to allow coverage of movement between module addressable areas for labware.

sfoster1 requested review from a team as code owners February 28, 2025 21:41

sfoster1 requested review from smb2268 and removed request for a team February 28, 2025 21:41

sfoster1 marked this pull request as draft February 28, 2025 21:47

sfoster1 marked this pull request as ready for review March 3, 2025 16:00

sfoster1 requested a review from CaseyBatten March 3, 2025 16:00

ahiuchingau approved these changes Mar 3, 2025

View reviewed changes

CaseyBatten requested changes Mar 3, 2025

View reviewed changes

sfoster1 force-pushed the stacker-api-and-app branch from 3cffa38 to 2e5001c Compare March 3, 2025 22:00

sfoster1 mentioned this pull request Mar 3, 2025

chore(app): add stacker/retrieve command handling and fix lpc crash #17638

Merged

1 task

sfoster1 added 10 commits March 3, 2025 17:28

chore: test stacker pools at 6 max lw

5dfbf92

because that's what protocols use. Also, do a bunch of integration.

api tests: retrieve

89af5c0

api tests: fixup equipment

374e593

api test: geometry location sequence test

025358c

does nobody format anymore

6a99040

retrieve fixup

4f8ea6e

api: fail in analysis for bad deck configs with gripper

e8ac4e9

sfoster1 force-pushed the stacker-api-and-app branch from d55ad3a to e8ac4e9 Compare March 3, 2025 22:28

CaseyBatten added 5 commits March 3, 2025 17:32

Add adapter core from stacker to the core map

eafca9d

fix module location source on gripper move to adhere to addressable a…

2072915

…reas

geometry changes to account for module real addressable area instead …

65b7856

…of presumed slot area

location clarification and cleanup

6af12b4

Merge branch 'edge' into stacker-api-and-app

798ce39

CaseyBatten approved these changes Mar 5, 2025

View reviewed changes

CaseyBatten added 3 commits March 5, 2025 18:23

linting

19aaf2f

base module position calculation from cutout

bb95bf5

linting

b4a6ca1

sfoster1 merged commit 2842ebf into edge Mar 6, 2025
52 of 53 checks passed

sfoster1 deleted the stacker-api-and-app branch March 6, 2025 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api,app): finish up new stacker API #17620

feat(api,app): finish up new stacker API #17620

sfoster1 commented Feb 28, 2025 •

edited

Loading

codecov bot commented Feb 28, 2025 •

edited

Loading

ahiuchingau left a comment

ahiuchingau Mar 3, 2025

ahiuchingau Mar 3, 2025

sfoster1 Mar 3, 2025

CaseyBatten left a comment

CaseyBatten Mar 3, 2025

CaseyBatten Mar 3, 2025

sfoster1 Mar 3, 2025

CaseyBatten Mar 3, 2025

CaseyBatten Mar 3, 2025

CaseyBatten left a comment

		@@ -701,20 +701,6 @@ class FlexStackerCore(ModuleCore, AbstractFlexStackerCore):

		_sync_module_hardware: SynchronousAdapter[hw_modules.FlexStacker]

		def set_static_mode(self, static: bool) -> None:


		if stacker_state.pool_count == 5:
		if stacker_state.pool_count == 6:

feat(api,app): finish up new stacker API #17620

feat(api,app): finish up new stacker API #17620

Conversation

sfoster1 commented Feb 28, 2025 • edited Loading

to come out of dev

review requirements

testing

codecov bot commented Feb 28, 2025 • edited Loading

Codecov Report

ahiuchingau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CaseyBatten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CaseyBatten left a comment

Choose a reason for hiding this comment

sfoster1 commented Feb 28, 2025 •

edited

Loading

codecov bot commented Feb 28, 2025 •

edited

Loading