Skip to content

feat(app): Implement Stacker Location Identify #18721

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 8 commits into from
Jun 25, 2025

Conversation

CaseyBatten
Copy link
Contributor

@CaseyBatten CaseyBatten commented Jun 24, 2025

Overview

Covers EXEC-1417

Now when adding a Stacker to the Deck Configuration an "Identify" button will appear next to the "Add" button. When pressing it will blink the Stacker LED for 10 seconds. If the stacker is added the stacker will cease blinking immediately.

Screencap:
Screenshot 2025-06-24 at 11 52 08 AM

Test Plan and Hands on Testing

  • Test on Live Stacker, ensure that when a stacker is identified it blinks for 10 seconds
  • Test on Live Stacker, ensure that when a stack is identified it begins blinking, and if the user then presses "Add" the stacker ceases blinking

Changelog

Added new Fixture Add modal case for showing multiple buttons (in this case the identify button).

Review requests

Any other protection cases we need for this? For now this only appears on the stacker modules, but I've planned it to be scalable enough for other identifiable modules in the future. The hook useSendIdentifyStacker will need to be replaced if we ever have other modules with toggleable LEDs.

Risk assessment

Medium/Low - Modified existing modal but only for new module behavior.

@CaseyBatten CaseyBatten requested a review from smb2268 June 24, 2025 15:49
@CaseyBatten CaseyBatten requested review from a team as code owners June 24, 2025 15:49
Copy link

codecov bot commented Jun 24, 2025

Codecov Report

Attention: Patch coverage is 20.27972% with 114 lines in your changes missing coverage. Please review.

Project coverage is 25.72%. Comparing base (144df01) to head (9648368).
Report is 13 commits behind head on edge.

Files with missing lines Patch % Lines
...DeviceDetailsDeckConfiguration/AddFixtureModal.tsx 0.00% 84 Missing ⚠️
components/src/organisms/FixtureOption/index.tsx 49.15% 30 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #18721      +/-   ##
==========================================
+ Coverage   24.75%   25.72%   +0.96%     
==========================================
  Files        3284     3284              
  Lines      285651   286042     +391     
  Branches    28648    29115     +467     
==========================================
+ Hits        70718    73576    +2858     
+ Misses     214912   212440    -2472     
- Partials       21       26       +5     
Flag Coverage Δ
app 3.15% <0.00%> (+1.11%) ⬆️
protocol-designer 19.06% <20.27%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
components/src/organisms/FixtureOption/index.tsx 50.00% <49.15%> (-14.16%) ⬇️
...DeviceDetailsDeckConfiguration/AddFixtureModal.tsx 0.00% <0.00%> (ø)

... and 120 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.

const module =
unconfiguredMods?.find(m => m.serialNumber === fixtureSerialNumber) ??
null
if (module !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a useState variable that gets set when we identify the module and blocks when identifying, the variable is then cleared once the timer expires. This prevents multiple timers from going off simultaneously, which causes the stacker to identify incorrectly.

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.

Looking good,
Left a comment on what we spoke about IRL, I think this is good to go once we add that!

</TertiaryButton>
</ListItem>
)
<StyledText desktopStyle="bodyDefaultSemiBold">{optionName}</StyledText>
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll also need odd style for this text - it should be oddStyle="bodyTextSemiBold"

@CaseyBatten CaseyBatten merged commit 76bbde0 into edge Jun 25, 2025
59 of 60 checks passed
@CaseyBatten CaseyBatten deleted the EXEC_1417_stacker_location_identify branch June 25, 2025 21:28
Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

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

Looks good! I made this ticket as a follow up to address the FixtureOption CTA issue: https://opentrons.atlassian.net/browse/EXEC-1634

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.

3 participants