Skip to content

feat(api): allow stacker overlap offset override and fix labware height calcuations for stacker movement #18570

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

Merged
merged 16 commits into from
Jun 10, 2025

Conversation

ahiuchingau
Copy link
Contributor

Overview

The original objective of this PR is to allow user to supply an overlap offset of the labware stack during Flex Stacker configuration. This offset, currently calculated from labware definition, affects:

  1. the total number the labware can be stored inside the hopper,
  2. how far the shuttle would have to travel in the Z-axis during labware storage and retrieval.

While building this feature, I realized our shuttle movements were using incorrect labware heights because we hadn’t factored in the overlap value. We’re fixing that here too—apologies for the sizable diff!

Changelog

  1. Added stacking_offset_z to SetStoredLabware protocol engine command to allow overriding the calculated stacking offset with a user-defined value
  2. Updated docstrings in module_context.py to clarify the purpose of the stacking offset and provide detailed explanations of stacking configurations
  3. Added validation for the provided stacking offset override in module_context.get_max_storable_labware_from_list() if the stacker already has configuration
  4. Modified module_context.get_current_storable_labware_from_list() to behave similarly as get_current_storable_labware(), so that it checks the module substate to determine how many more can fit inside the hopper and should raise an error if the stacker hasn't been configured
  5. Added LabwareView.stacker_labware_pool_to_ordered_list() to make use of the raise_if_stacker_labware_pool_is_not_valid function to create the proper list of labware in the correct, top-first order
  6. Refactored overlap and height calculations to use stacker_labware_pool_to_ordered_list and get_stacker_labware_overlap_offset to ensure accuracy and consistency across the engine calculations and module context-level prediction
  7. Corrected the height value to be passed down to the flex stacker controller in retrieve.py and store.py using the calls to FlexStackerSubstate.get_pool_height_minus_overlap()

@ahiuchingau ahiuchingau requested review from a team as code owners June 9, 2025 20:52
Copy link

codecov bot commented Jun 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 23.54%. Comparing base (bd51931) to head (b2a0f10).
Report is 2 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #18570      +/-   ##
==========================================
- Coverage   23.54%   23.54%   -0.01%     
==========================================
  Files        3233     3233              
  Lines      278597   278677      +80     
  Branches    26980    26981       +1     
==========================================
  Hits        65604    65604              
- Misses     212975   213055      +80     
  Partials       18       18              
Flag Coverage Δ
step-generation 4.43% <ø> (-0.01%) ⬇️

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.

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.

Looks great to me, I think this is an awesome step forward for allowing override specs.

@@ -435,6 +435,7 @@ def empty(self, message: str | None) -> None:
def set_stored_labware_items(
self,
labware: Sequence[LabwareCoreType],
stacking_offset_z: float | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

[nit] imo in the core we can drop the = None, this is only called programmatically and the papi code can just always provide the None

@ahiuchingau ahiuchingau merged commit 89cb052 into edge Jun 10, 2025
49 checks passed
@ahiuchingau ahiuchingau deleted the EXEC-1613 branch June 10, 2025 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants