feat(api): store labware objects in the stacker #18010

sfoster1 · 2025-04-08T16:58:45Z

We had this beautiful dream: the stacker wouldn't store labware. Instead, the stacker would know what kind of labware it should store, and how many of those labware it is storing, and objects would be created when you retrieved stuff. The problem with that is in the physical world there are labware objects in the stacker, and if we're not modeling them you can't know the state of those physical objects. So we have to reverse this decision and store the identity of the labware in the stacker.

This PR is the set of changes to accomplish that in the engine and account for it in the python API; the ts bindings; and at least somewhat in the app interface that already exists. It won't add any new API or app stuff.

The way that this works is:

The stacker contains a list of labware IDs that it contains
While a labware is in the stacker, its position in a stack (i.e. lid on labware, labware on adapter) is maintained but the stack as a whole is InStackerHopper(moduleId)
Labware must exist to go in to the stacker. That means that setStoredLabware and fill now handle making sure all the labware exists. They take lists of ids for PD compliance and also so that you can predefine labware if you want. They also still have a count argument. You can
- Say a number of labware with count, and the command will create that many labware with arbitrary IDs
- Not set count, and pass a list of IDs that don't currently exist, and the command will create labware with those IDs
- Not set count, and pass a list of IDs that do currently exist, and the command will move those labware in
  - Those labware must be in OFF_DECK or there's an error
Labware is moved out of the stacker, so retrieve doesn't create labware anymore
When you remove labware with empty, it's moved to OFF_DECK

fill, setStoredLabware, and empty all now specify lots of labware location sequences and ids in their results because they create or move labware where they previously do not. When fill and setStoredLabware create labware, they gloss those labware as having an original position descending to SYSTEM. Otherwise, they're the real original locations.

to come out of draft

Fix the tests that already exist
Write new tests to cover extra verification of labware state for the whole new-by-count/new-by-id/existing-id thing
Write new tests to make sure we do locations appropriately
Test on hardware
~~Make commit names that aren't so silly~~ that's what the squash is for

to merge

make a pr that implements this in the python api so it's testable

review requests

in addition to the standard stuff,

Do you feel like too much/too little/just enough is in stacker common
Is this an API the app can work with

codecov · 2025-04-10T21:28:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.01%. Comparing base (14ee9f8) to head (d116172).
Report is 1 commits behind head on edge.

Additional details and impacted files

@@           Coverage Diff           @@
##             edge   #18010   +/-   ##
=======================================
  Coverage   62.01%   62.01%           
=======================================
  Files        2989     2989           
  Lines      232004   232004           
  Branches    20013    20013           
=======================================
  Hits       143885   143885           
  Misses      87942    87942           
  Partials      177      177

Flag	Coverage Δ
protocol-designer	`18.87% <ø> (ø)`

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

🚀 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.

TamarZanzouri · 2025-04-11T14:39:41Z

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

+@dataclass
+class _LabwareDefPair:
+    definition: LabwareDefinition
+    id: str


this is labware id correct?

TamarZanzouri · 2025-04-11T15:17:41Z

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

+    labware: LoadedLabware,
+) -> None:
+    definitions_by_id[labware.id] = definition
+    display_names_by_id[labware.id] = None


should this be None? we can get int from the definition?

CaseyBatten

Refactor and new features look really great overall, thanks Seth! Stacker common is a bit massive now, but I think it makes sense for the logic to live there as opposed to split between retrieve/unsafe retrieve and store as it was. Logistical question, do we want to wait on this until the PAPI PR is up and merge them into eachother first before merging this to edge?

The structure for Labware group and location sequencing logic, as well as the changes to Store and Retrieve to track the contained labware groups make sense. Some notes below on some things I wanted to verify and also where we might need to change the app end logic since retrieve isn't the "loader" for labware anymore.

CaseyBatten · 2025-04-11T15:02:44Z

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

+    return group.lid.locationSequence
+
+
+def _check_one_preloaded_labware_known(


This all seems like solid validation, should be zero cases of mixed state labware groups anyways so nice!

CaseyBatten · 2025-04-11T15:14:27Z

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

+        return False
+
+
+def _check_one_preloaded_labware(  # noqa: C901


Looks good, but just wanted to verify one thing regarding lids. We are validating that lids must be on a labware location, but not that solo lids must be off deck. Are we fully planning to not distribute lids/lid stacks out of the stacker? We can always make that change later, not blocking.

CaseyBatten · 2025-04-11T15:39:03Z

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

+        .set_batch_loaded_labware(
+            definitions_by_id=definitions_by_id,
+            offset_ids_by_id=offset_ids_by_id,
+            display_names_by_id=display_names_by_id,
+            new_locations_by_id=new_locations_by_id,


This is fine as is, but last time I was modifying how we split out some of the batch loaded labware behavior I was wondering if it would be better to just send a set of LoadedLabware objects right down the line through state update all the way to the labware store. No need to gut it now, but these various dicts just do the "breaking up" work here instead of down at the handler in the end, so we could in theory save all this for that end step. I think the only reason I didn't at the time is that the new_location_by_id dict had to exist for location sequences so I left it all be.

CaseyBatten · 2025-04-11T16:00:59Z

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

+                    self._state_view,
+                    [
+                        NotOnDeckLocationSequenceComponent(
+                            logicalLocationName=SYSTEM_LOCATION


Excellent, this aligns with the way in which we spawn new lid stack objects onto the deck during a move lid to empty deck slot command.

CaseyBatten · 2025-04-11T16:11:46Z

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

-                lid_def,
-            )
-            .update_flex_stacker_labware_pool_count(params.moduleId, count)
+        state_update, new_contained_labware = await build_or_assign_labware_to_hopper(


We will need to update the client and our starting deck state logic to be aware that this is our new "source" of a load labware type of action instead of retrieve.

TamarZanzouri · 2025-04-11T20:35:45Z

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

+    ]
+
+
+def build_retrieve_labware_move_updates(


can you explain the method name?

TamarZanzouri · 2025-04-11T20:43:10Z

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

-        ..., description="The number of labware loaded into the stacker labware pool."
+        ..., description="The number of labware now stored in the hopper."
+    )
+    originalPrimaryLabwareLocationSequences: (


I might be missing something but why do we need this prop in setStoredLabware command?

sfoster1 force-pushed the exec-1394-restore-labware-ids-to-stacker-state branch 9 times, most recently from 5b923a8 to 3bac787 Compare April 10, 2025 20:53

sfoster1 added 2 commits April 10, 2025 16:59

update stacker stuff

fd648bc

a couple slight changes

d116172

sfoster1 force-pushed the exec-1394-restore-labware-ids-to-stacker-state branch from 3bac787 to d116172 Compare April 10, 2025 21:21

sfoster1 marked this pull request as ready for review April 10, 2025 21:22

sfoster1 requested a review from a team as a code owner April 10, 2025 21:22

sfoster1 requested review from vegano1, ahiuchingau and CaseyBatten April 10, 2025 21:22

TamarZanzouri reviewed Apr 11, 2025

View reviewed changes

CaseyBatten reviewed Apr 11, 2025

View reviewed changes

TamarZanzouri reviewed Apr 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): store labware objects in the stacker #18010

feat(api): store labware objects in the stacker #18010

sfoster1 commented Apr 8, 2025 •

edited

Loading

codecov bot commented Apr 10, 2025

TamarZanzouri Apr 11, 2025

TamarZanzouri Apr 11, 2025

CaseyBatten left a comment

CaseyBatten Apr 11, 2025

CaseyBatten Apr 11, 2025

CaseyBatten Apr 11, 2025

CaseyBatten Apr 11, 2025

CaseyBatten Apr 11, 2025

TamarZanzouri Apr 11, 2025

TamarZanzouri Apr 11, 2025

		return group.lid.locationSequence


		def _check_one_preloaded_labware_known(

feat(api): store labware objects in the stacker #18010

Are you sure you want to change the base?

feat(api): store labware objects in the stacker #18010

Conversation

sfoster1 commented Apr 8, 2025 • edited Loading

to come out of draft

to merge

review requests

codecov bot commented Apr 10, 2025

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfoster1 commented Apr 8, 2025 •

edited

Loading