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

feat(api): add setStoredLabware command #17540

Merged
merged 2 commits into from
Feb 19, 2025
Merged

Conversation

sfoster1
Copy link
Member

Adds the new setStoredLabware command to constrain the flex stacker labware pool.

The idea here is that you configure each stacker with some concept of "this is the kind of labware you contain", and an amount of labware. Then, you update how much it contains and when you retrieve stuff retrieve creates a new labware object. Only labware that entered the stacker through store retains its individuality.

This PR creates the command and adds the new data to the stacker substate, but it's not yet used for anything; static mode remains, loading labware objects into the hopper remains. In followup prs, we'll remove that older loading capability, and switch things over to using the new way of doing things.

testing

  • automated tests, really; the command does nothing

reviews

  • structure and arguments and such

Closes EXEC-1212

@sfoster1 sfoster1 requested review from a team as code owners February 18, 2025 17:55
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.62%. Comparing base (70a4ac8) to head (3bcab0c).
Report is 1 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #17540   +/-   ##
=======================================
  Coverage   63.62%   63.62%           
=======================================
  Files        2838     2838           
  Lines      218019   218019           
  Branches    18107    18107           
=======================================
  Hits       138704   138704           
  Misses      79122    79122           
  Partials      193      193           
Flag Coverage Δ
protocol-designer 18.85% <ø> (ø)
step-generation 4.36% <ø> (ø)

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

self._state_view.labware.get_definition(lid_id),
labware_defs[0],
]
stack_height = self._state_view.geometry.get_height_of_labware_stack(
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

@@ -127,6 +127,39 @@ def __init__(
or pipette_data_provider.VirtualPipetteDataProvider()
)

async def load_definition_for_details(
Copy link
Contributor

Choose a reason for hiding this comment

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

typo? should be load_definition_FROM_details

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess. I read it as "load the definition for the labware with these details". happy to change it if you want me to

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.

The changes look good to me.

@sfoster1 sfoster1 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: #17541

Copy link
Contributor

@CaseyBatten CaseyBatten left a comment

Choose a reason for hiding this comment

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

Looks great, state update implementation was straightforward.

Comment on lines +148 to +154
state_update = (
update_types.StateUpdate()
.update_flex_stacker_labware_pool_definition(
params.moduleId, labware_def, adapter_def, lid_def
)
.update_flex_stacker_labware_pool_count(params.moduleId, count)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, makes sense.

Comment on lines +42 to +53
pool_primary_definition = self.pool_primary_definition
pool_adapter_definition = self.pool_adapter_definition
pool_lid_definition = self.pool_lid_definition
if update.pool_constraint != NO_CHANGE:
pool_primary_definition = update.pool_constraint.primary_definition
pool_adapter_definition = update.pool_constraint.adapter_definition
pool_lid_definition = update.pool_constraint.lid_definition

pool_count = self.pool_count
if update.pool_count != NO_CHANGE:
pool_count = update.pool_count

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks solid

@sfoster1 sfoster1 force-pushed the exec-1209-in-stacker-hopper-location branch from 9da89a9 to 47390f0 Compare February 19, 2025 15:11
@sfoster1 sfoster1 requested a review from a team as a code owner February 19, 2025 15:11
@sfoster1 sfoster1 requested review from TamarZanzouri and removed request for a team February 19, 2025 15:11
Base automatically changed from exec-1209-in-stacker-hopper-location to edge February 19, 2025 16:01
Adds the new setStoredLabware command to constrain the flex stacker
labware pool.

Closes EXEC-1212
@sfoster1 sfoster1 force-pushed the exec-1212-set-stored-labware-pe branch from 4f0c5c3 to 097d92f Compare February 19, 2025 16:02
Copy link
Contributor

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

…snapshots (#17552)

This PR was requested on the PR
#17540

Co-authored-by: sfoster1 <[email protected]>
@sfoster1 sfoster1 merged commit 829abc6 into edge Feb 19, 2025
54 checks passed
@sfoster1 sfoster1 deleted the exec-1212-set-stored-labware-pe branch February 19, 2025 17:20
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.

3 participants