Skip to content

feat: Bambu AMS dashboard blueprint + tray_assign command (#148)#150

Merged
sjordan0228 merged 9 commits into
devfrom
feature/bambu-dashboard-blueprint
Apr 11, 2026
Merged

feat: Bambu AMS dashboard blueprint + tray_assign command (#148)#150
sjordan0228 merged 9 commits into
devfrom
feature/bambu-dashboard-blueprint

Conversation

@sjordan0228
Copy link
Copy Markdown
Contributor

@sjordan0228 sjordan0228 commented Apr 11, 2026

Summary

  • Adds HA blueprint spoolsense_bambu_dashboard.yaml — watches Bambu AMS tray entities, publishes cmd/tray_update to scanner via MQTT
  • Updates loading blueprint spoolsense_bambu_ams.yaml — replaces broken input_text helper writes with cmd/tray_assign MQTT publish (no HA helpers needed)
  • Adds cmd/tray_assign firmware handler — stores UID-to-tray mapping in NVS, triggers Spoolman weight lookup
  • Wires Spoolman sync response to dashboard tray weight — weight auto-populates after tray assignment
  • Fixes cold boot dashboard display — moves cache render to post-boot sequence
  • Updates flow diagram for website

Architecture

  • Scanner is single source of truth for UID-to-tray mapping (no HA helpers)
  • Two blueprints: loading (scan→load→assign) + dashboard (tray change→MQTT update)
  • Deduction via cmd/deduct_tray is a planned follow-up (pending print_weight attribute verification — confirmed format: "AMS 1 Tray 3": 11.01)

New Files

File Purpose
homeassistant/blueprints/spoolsense_bambu_dashboard.yaml HA blueprint: tray state → MQTT
docs/bambu-dashboard-flow.html Visual data flow diagram

Test plan

  • Dashboard blueprint triggers on tray state change — verified on live HA
  • Loading blueprint sends cmd/tray_assign — verified scan→load flow
  • Dashboard renders correct trays from blueprint data
  • NVS cache survives cold boot (with RST pin wired)
  • Scan interruption → 5s revert to dashboard
  • Compile: esp32dev, esp32s3zero, esp32s3devkitc
  • Spoolman weight lookup after tray_assign (needs tagged spool in AMS)

Summary by CodeRabbit

  • Documentation

    • Added a Bambu AMS Dashboard data flow page illustrating spool scan, dashboard update, and weight enrichment phases.
  • New Features

    • New Home Assistant blueprint: SpoolSense → Bambu AMS Dashboard (ordered tray list → MQTT tray_update).
    • Tray assignment workflow via MQTT (tray_assign) and tray deduction command (deduct_tray) supported.
    • Dashboard display now initializes and falls back to a tray-dashboard or ready screen as appropriate.
  • Refactor

    • Blueprints updated to accept a configurable scanner device ID and publish tray-indexed MQTT payloads.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 11, 2026

Warning

Rate limit exceeded

@sjordan0228 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 19 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 19 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 27e605b7-0773-4cbb-bd65-ab04611b9566

📥 Commits

Reviewing files that changed from the base of the PR and between 5c6ca02 and b499cc7.

📒 Files selected for processing (4)
  • docs/bambu-dashboard-flow.html
  • homeassistant/blueprints/spoolsense_bambu_ams.yaml
  • homeassistant/blueprints/spoolsense_bambu_dashboard.yaml
  • src/ApplicationManager.cpp
📝 Walkthrough

Walkthrough

Adds Bambu AMS dashboard support: new Home Assistant blueprints for tray assign/update and deduction, MQTT command handlers (tray_assign, tray_update, deduct_tray), tray UID/spoolman_id in tray state, ApplicationManager TRAY_ASSIGN/assignment flow with NVS persistence and optional Spoolman weight enrichment, and TFT dashboard rendering changes.

Changes

Cohort / File(s) Summary
Documentation
docs/bambu-dashboard-flow.html
New static HTML doc showing end-to-end Bambu AMS dashboard data flow across spool scan, dashboard update, and weight enrichment phases with component/message layout and TFT dashboard mock.
Home Assistant Blueprints
homeassistant/blueprints/spoolsense_bambu_ams.yaml, homeassistant/blueprints/spoolsense_bambu_dashboard.yaml, homeassistant/blueprints/spoolsense_bambu_deduction.yaml
Added scanner_device_id input and replaced per-tray input_text writes with MQTT tray_assign/tray_update/deduct_tray payloads; new dashboard blueprint builds ordered tray JSON (normalized colors, computed tray_index/ams_id) and publishes to spoolsense/<id>/cmd/tray_update; deduction blueprint switched to tray-indexed deduct_tray payload.
Types
src/TrayDashboardTypes.h
Extended TrayData with char uid[17] and int32_t spoolman_id to persist NFC UID and Spoolman ID per tray.
Application Manager (API & Logic)
src/ApplicationManager.h, src/ApplicationManager.cpp
Added AppMessageType::TRAY_ASSIGN, new public trayDashboardState_ and pending-assignment fields, and handleTrayAssign() handler that validates tray_index, stores UID/spoolman_id, persists to NVS, and enqueues lookup-only Spoolman sync. Integrated Spoolman sync callback to enrich tray weight when spool_id matches a stored UID (NATIVE_TEST guard).
HomeAssistant MQTT handler
src/HomeAssistantManager.cpp
Added tray_assign and deduct_tray MQTT command branches: tray_assign deserializes JSON, stages pending assign fields in ApplicationManager and dispatches TRAY_ASSIGN; deduct_tray resolves tray UID by tray_index, stages/executes spoolman deduction, updates dashboard weight, persists and triggers TRAY_UPDATE.
Display init / TFT
src/main.cpp
Refined startup display logic: when Bambu dashboard enabled, fetch cached TrayDashboardState and call showTrayDashboard() if has_data, otherwise show fallback text; preserved original non-dashboard ready flow.

Sequence Diagram(s)

sequenceDiagram
    actor HA as Home Assistant
    participant MQTT as MQTT Broker
    participant App as SpoolSense App
    participant NVS as NVS Storage
    participant SM as Spoolman API
    participant TFT as TFT Display

    rect rgba(100,200,255,0.5)
    Note over HA,App: Tray Assignment via MQTT
    HA->>MQTT: publish spoolsense/{id}/cmd/tray_assign\n{tray_index, uid, spoolman_id}
    MQTT->>App: receive tray_assign
    App->>App: parse JSON\nset pendingAssignTrayIndex_/Uid_/SpoolmanId_
    App->>App: dispatch TRAY_ASSIGN
    App->>App: handleTrayAssign()\nupdate trayDashboardState_
    App->>NVS: persist TrayDashboardState
    alt uid not empty
      App->>SM: enqueue SpoolmanSyncRequest (lookup_only=true)
      SM-->>App: spoolman sync callback with spool_id/weight
      App->>App: match spool_id -> update tray weight\npersist to NVS
      App->>TFT: refresh tray dashboard
    end
    end

    rect rgba(100,255,150,0.5)
    Note over HA,App: Tray Dashboard Update
    HA->>MQTT: publish spoolsense/{id}/cmd/tray_update\n{trays:[...]}
    MQTT->>App: receive tray_update
    App->>App: update internal dashboard state\nrender via TFTManager
    App->>TFT: showTrayDashboard()
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description covers summary, architecture, new files, and test plan, but is missing the required 'Changes' and 'How to Test' sections from the template. Reorganize the description to explicitly include 'Changes' and 'How to Test' sections as specified in the repository template, and add a checklist confirming hardware testing and compilation verification.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: adding a Bambu AMS dashboard blueprint and implementing the tray_assign command handler.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/bambu-dashboard-blueprint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/L Large change (200-500 lines) label Apr 11, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/bambu-dashboard-flow.html`:
- Around line 1-2: Add a lang attribute to the root <html> element to improve
accessibility and internationalization: locate the opening <html> tag in the
document and set an appropriate language code (e.g., lang="en" or the project's
primary locale) so screen readers and user agents can correctly interpret the
page language.

In `@homeassistant/blueprints/spoolsense_bambu_ams.yaml`:
- Around line 125-131: The current payload builds tray_index from the position
in tray_entities (tray_entities.index(loaded_tray)), which breaks if entities
are selected out of order; change the logic to derive the actual AMS tray number
from the entity's attributes when available (e.g., check
loaded_tray.attributes.tray_number or loaded_tray.attributes.tray_index and use
that value as tray_index, falling back to the list index only if the attribute
is missing), and also add a short "Order matters" note to the ams_tray_entities
input description so users are warned when the attribute isn't present.

In `@homeassistant/blueprints/spoolsense_bambu_dashboard.yaml`:
- Around line 50-51: The code sets a sanitized color variable but uses a
redundant conditional expression; simplify the assignment of the Jinja variable
`color` by removing the unnecessary ternary and just truncate to six characters
(replace the line that defines `color` using the conditional with a single
`color[:6]` operation after `color_raw | replace('#', '')`) so `color` is always
the first six hex characters.
- Around line 51-52: The template builds the tray JSON via string concatenation
into the tray variable which can break or allow injection if material (a) or
color contain quotes/backslashes; change the tray construction to create a Jinja
dict/object (use keys tray_index, ams_id, material, color) and render it with
the tojson filter (e.g., {'tray_index': loop.index0, 'ams_id': (loop.index0 //
4), 'material': a, 'color': color} | tojson) so values are properly escaped and
valid JSON when assigned to tray; update the reference to the color slicing
(color = color[:6] if ...) to happen before serializing.

In `@src/ApplicationManager.cpp`:
- Line 1361: The log in tray_assign is misleading because assignments for
missing tray indices are not actually cached; implement real caching by adding a
member (e.g., pendingTrayAssignments: std::unordered_map<int, AssignmentData>)
to ApplicationManager, have tray_assign store the assignment into
pendingTrayAssignments when idx is not present, and update the tray_update
handler to check pendingTrayAssignments for the created index and apply+erase
any pending assignment; alternatively, if you prefer the simpler fix, change the
Serial.printf message in tray_assign to "index %d not in current dashboard,
assignment ignored" to reflect current behavior (choose one approach and apply
consistently in ApplicationManager::tray_assign and
ApplicationManager::tray_update).
- Around line 976-986: The nested `#ifndef` NATIVE_TEST around the block that
writes trayDashboardState_ with Preferences and conditionally calls
display_->showTrayDashboard(...) is redundant because the surrounding scope is
already guarded; remove the inner `#ifndef/`#endif pair and leave the code as-is
(retain Preferences prefs; prefs.begin("spoolsense", false);
prefs.putBytes("tray_dash", &trayDashboardState_, sizeof(TrayDashboardState));
prefs.end(); and the dashEnabled check using
ConfigurationManager::getInstance().isBambuDashboardEnabled() and
display_->showTrayDashboard(trayDashboardState_)) so the logic remains unchanged
but the duplicate preprocessor guard is eliminated.

In `@src/ApplicationManager.h`:
- Around line 188-192: The public staging fields pendingAssignTrayIndex_,
pendingAssignUid_, and pendingAssignSpoolmanId_ expose ApplicationManager state
and should be replaced by a payload-based message so HomeAssistantManager posts
a self-contained AppMessage instead of mutating state; add a TrayAssignPayload
struct in the AppMessage::payload union (fields tray_index, uid[17],
spoolman_id), change HomeAssistantManager to populate msg.payload.trayAssign
when sending HA_TRAY_ASSIGN, and update ApplicationManager::handleTrayAssign to
read from msg.payload.trayAssign (removing use of pendingAssign* members) to
eliminate races and ensure cleanup after processing.

In `@src/HomeAssistantManager.cpp`:
- Around line 948-950: The code silently defaults trayIndex to 0 when reading
assignDoc["tray_index"], which can misassign tray 0; change the read to use a
sentinel (e.g., uint8_t trayIndex = 255) or avoid defaulting and then validate
presence via assignDoc.containsKey("tray_index") or
assignDoc["tray_index"].isNull() before using it; if the field is missing or
equals the sentinel, bail out or return an error instead of proceeding with
trayIndex (update the logic around the assignDoc read and subsequent use of
trayIndex in the function to reject malformed payloads).
- Around line 940-964: The staging fields set when processing the "tray_assign"
command are left set and must be cleared after consumption; update
ApplicationManager::handleTrayAssign (the handler for
AppMessageType::TRAY_ASSIGN) to reset
ApplicationManager::pendingAssignTrayIndex_ to an invalid value (e.g. 0xFF or
-1), zero/empty out pendingAssignUid_ (memset or set first char to '\0'), and
set pendingAssignSpoolmanId_ back to -1 immediately after the handler has read
and applied them so stale data cannot be reused; alternatively replace the
shared staging fields with a payload carried inside the AppMessage union/struct
so each message owns its data.

In `@src/TrayDashboardTypes.h`:
- Around line 13-14: The spoolman_id field in TrayDashboardState is not
default-initialized to the sentinel -1, so zero-initializing the struct yields 0
(a valid spool ID); update the TrayDashboardState definition to ensure
spoolman_id defaults to -1 (either by adding a default member initializer
"int32_t spoolman_id = -1;" or adding a constructor that sets spoolman_id = -1),
and keep the uid comment; ensure any existing code that relied on zero-init is
adjusted or explicitly documents the new default.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f9170a0a-bac0-400e-b6d6-f64a11509381

📥 Commits

Reviewing files that changed from the base of the PR and between 783394a and 9bd5bfc.

📒 Files selected for processing (8)
  • docs/bambu-dashboard-flow.html
  • homeassistant/blueprints/spoolsense_bambu_ams.yaml
  • homeassistant/blueprints/spoolsense_bambu_dashboard.yaml
  • src/ApplicationManager.cpp
  • src/ApplicationManager.h
  • src/HomeAssistantManager.cpp
  • src/TrayDashboardTypes.h
  • src/main.cpp

Comment thread docs/bambu-dashboard-flow.html Outdated
Comment thread homeassistant/blueprints/spoolsense_bambu_ams.yaml
Comment thread homeassistant/blueprints/spoolsense_bambu_dashboard.yaml Outdated
Comment thread homeassistant/blueprints/spoolsense_bambu_dashboard.yaml Outdated
Comment on lines +976 to +986
#ifndef NATIVE_TEST
Preferences prefs;
prefs.begin("spoolsense", false);
prefs.putBytes("tray_dash", &trayDashboardState_, sizeof(TrayDashboardState));
prefs.end();

bool dashEnabled = ConfigurationManager::getInstance().isBambuDashboardEnabled();
if (dashEnabled && display_) {
display_->showTrayDashboard(trayDashboardState_);
}
#endif
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Redundant nested #ifndef NATIVE_TEST directive.

This block is already inside a #ifndef NATIVE_TEST guard starting at line 921. The nested directive is unnecessary and reduces readability.

♻️ Remove redundant guard
                 if (weightG != trayDashboardState_.trays[i].weight_g) {
                     trayDashboardState_.trays[i].weight_g = weightG;
                     Serial.printf("ApplicationManager: Dashboard tray %d weight updated to %dg\n",
                                   trayDashboardState_.trays[i].tray_index, weightG);
-#ifndef NATIVE_TEST
                     Preferences prefs;
                     prefs.begin("spoolsense", false);
                     prefs.putBytes("tray_dash", &trayDashboardState_, sizeof(TrayDashboardState));
                     prefs.end();

                     bool dashEnabled = ConfigurationManager::getInstance().isBambuDashboardEnabled();
                     if (dashEnabled && display_) {
                         display_->showTrayDashboard(trayDashboardState_);
                     }
-#endif
                 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#ifndef NATIVE_TEST
Preferences prefs;
prefs.begin("spoolsense", false);
prefs.putBytes("tray_dash", &trayDashboardState_, sizeof(TrayDashboardState));
prefs.end();
bool dashEnabled = ConfigurationManager::getInstance().isBambuDashboardEnabled();
if (dashEnabled && display_) {
display_->showTrayDashboard(trayDashboardState_);
}
#endif
if (weightG != trayDashboardState_.trays[i].weight_g) {
trayDashboardState_.trays[i].weight_g = weightG;
Serial.printf("ApplicationManager: Dashboard tray %d weight updated to %dg\n",
trayDashboardState_.trays[i].tray_index, weightG);
Preferences prefs;
prefs.begin("spoolsense", false);
prefs.putBytes("tray_dash", &trayDashboardState_, sizeof(TrayDashboardState));
prefs.end();
bool dashEnabled = ConfigurationManager::getInstance().isBambuDashboardEnabled();
if (dashEnabled && display_) {
display_->showTrayDashboard(trayDashboardState_);
}
}
🧰 Tools
🪛 Clang (14.0.6)

[note] 983-983: +5, including nesting penalty of 4, nesting level increased to 5

(clang)


[note] 983-983: +1

(clang)


[warning] 976-976: nested redundant #ifndef; consider removing it

(readability-redundant-preprocessor)


[warning] 977-977: variable 'prefs' is not initialized

(cppcoreguidelines-init-variables)


[warning] 983-983: implicit conversion 'DisplayI *' -> bool

(readability-implicit-bool-conversion)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ApplicationManager.cpp` around lines 976 - 986, The nested `#ifndef`
NATIVE_TEST around the block that writes trayDashboardState_ with Preferences
and conditionally calls display_->showTrayDashboard(...) is redundant because
the surrounding scope is already guarded; remove the inner `#ifndef/`#endif pair
and leave the code as-is (retain Preferences prefs; prefs.begin("spoolsense",
false); prefs.putBytes("tray_dash", &trayDashboardState_,
sizeof(TrayDashboardState)); prefs.end(); and the dashEnabled check using
ConfigurationManager::getInstance().isBambuDashboardEnabled() and
display_->showTrayDashboard(trayDashboardState_)) so the logic remains unchanged
but the duplicate preprocessor guard is eliminated.

Comment thread src/ApplicationManager.cpp Outdated
Comment thread src/ApplicationManager.h
Comment on lines +188 to +192

// Tray assign staging (written by HomeAssistantManager, consumed by handleTrayAssign)
uint8_t pendingAssignTrayIndex_ = 0;
char pendingAssignUid_[17] = {0};
int32_t pendingAssignSpoolmanId_ = -1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider using message payload instead of public staging fields.

These staging fields break encapsulation by allowing HomeAssistantManager to directly mutate ApplicationManager state. Other message types (e.g., HA_WRITE_TAG, SPOOLMAN_SYNCED) use the AppMessage::payload union to pass data cleanly through the message queue.

A payload-based approach would:

  • Eliminate the race condition risk if multiple tray_assign messages arrive rapidly
  • Make the data flow explicit and self-contained
  • Allow proper cleanup after message processing
♻️ Suggested payload struct approach
// In the union section of AppMessage (around line 141):
struct TrayAssignPayload {
    uint8_t tray_index;
    char uid[17];
    int32_t spoolman_id;
} trayAssign;

// Then HomeAssistantManager populates msg.payload.trayAssign instead of shared fields
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ApplicationManager.h` around lines 188 - 192, The public staging fields
pendingAssignTrayIndex_, pendingAssignUid_, and pendingAssignSpoolmanId_ expose
ApplicationManager state and should be replaced by a payload-based message so
HomeAssistantManager posts a self-contained AppMessage instead of mutating
state; add a TrayAssignPayload struct in the AppMessage::payload union (fields
tray_index, uid[17], spoolman_id), change HomeAssistantManager to populate
msg.payload.trayAssign when sending HA_TRAY_ASSIGN, and update
ApplicationManager::handleTrayAssign to read from msg.payload.trayAssign
(removing use of pendingAssign* members) to eliminate races and ensure cleanup
after processing.

Comment on lines +940 to +964
// tray_assign: loading blueprint assigns a scanned UID to a specific AMS tray
if (strcmp(command, "tray_assign") == 0) {
StaticJsonDocument<128> assignDoc;
if (deserializeJson(assignDoc, payload)) {
publishCommandResponse(command, false, "invalid_json");
return;
}

uint8_t trayIndex = assignDoc["tray_index"] | 0;
const char* uid = assignDoc["uid"] | "";
int32_t spoolmanId = assignDoc["spoolman_id"] | -1;

ApplicationManager& app = ApplicationManager::getInstance();
app.pendingAssignTrayIndex_ = trayIndex;
strncpy(app.pendingAssignUid_, uid, sizeof(app.pendingAssignUid_) - 1);
app.pendingAssignUid_[sizeof(app.pendingAssignUid_) - 1] = '\0';
app.pendingAssignSpoolmanId_ = spoolmanId;

AppMessage msg = {};
msg.type = AppMessageType::TRAY_ASSIGN;
app.sendMessage(msg);

publishCommandResponse(command, true, nullptr);
return;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Staging fields are not cleared after handleTrayAssign() processes them.

The staging fields (pendingAssignTrayIndex_, pendingAssignUid_, pendingAssignSpoolmanId_) are written here but never reset after handleTrayAssign() consumes them. If the tray index is not found in the current dashboard (see src/ApplicationManager.cpp:1361), the stale data persists and could be incorrectly applied when:

  1. A subsequent TRAY_UPDATE message populates a matching tray index
  2. Another unrelated message handler accidentally reads these fields

Either clear the staging fields in handleTrayAssign() after processing, or use a message payload union instead of shared state.

🐛 Proposed fix in ApplicationManager::handleTrayAssign()
 void ApplicationManager::handleTrayAssign() {
     uint8_t idx = pendingAssignTrayIndex_;
+    // Clear staging fields immediately to prevent stale data reuse
+    pendingAssignTrayIndex_ = 0;
+    char uid[17];
+    strncpy(uid, pendingAssignUid_, sizeof(uid) - 1);
+    uid[sizeof(uid) - 1] = '\0';
+    pendingAssignUid_[0] = '\0';
+    int32_t spoolmanId = pendingAssignSpoolmanId_;
+    pendingAssignSpoolmanId_ = -1;
+
     if (idx >= MAX_TRAYS) {
         Serial.printf("ApplicationManager: tray_assign rejected — index %d out of range\n", idx);
         return;
     }
 
     for (uint8_t i = 0; i < trayDashboardState_.tray_count; i++) {
         if (trayDashboardState_.trays[i].tray_index == idx) {
-            strncpy(trayDashboardState_.trays[i].uid, pendingAssignUid_, sizeof(trayDashboardState_.trays[i].uid) - 1);
+            strncpy(trayDashboardState_.trays[i].uid, uid, sizeof(trayDashboardState_.trays[i].uid) - 1);
             trayDashboardState_.trays[i].uid[sizeof(trayDashboardState_.trays[i].uid) - 1] = '\0';
-            trayDashboardState_.trays[i].spoolman_id = pendingAssignSpoolmanId_;
+            trayDashboardState_.trays[i].spoolman_id = spoolmanId;
 
             Serial.printf("ApplicationManager: Tray %d assigned UID=%s spoolman_id=%d\n",
-                          idx, pendingAssignUid_, pendingAssignSpoolmanId_);
+                          idx, uid, spoolmanId);
 // ... rest of function uses local copies
🧰 Tools
🪛 Clang (14.0.6)

[note] 941-941: +1, including nesting penalty of 0, nesting level increased to 1

(clang)


[note] 943-943: +2, including nesting penalty of 1, nesting level increased to 2

(clang)


[warning] 942-942: variable 'assignDoc' is not initialized

(cppcoreguidelines-init-variables)


[warning] 948-948: variable 'trayIndex' is not initialized

(cppcoreguidelines-init-variables)


[warning] 949-949: variable 'uid' is not initialized

(cppcoreguidelines-init-variables)


[warning] 950-950: variable 'spoolmanId' is not initialized

(cppcoreguidelines-init-variables)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/HomeAssistantManager.cpp` around lines 940 - 964, The staging fields set
when processing the "tray_assign" command are left set and must be cleared after
consumption; update ApplicationManager::handleTrayAssign (the handler for
AppMessageType::TRAY_ASSIGN) to reset
ApplicationManager::pendingAssignTrayIndex_ to an invalid value (e.g. 0xFF or
-1), zero/empty out pendingAssignUid_ (memset or set first char to '\0'), and
set pendingAssignSpoolmanId_ back to -1 immediately after the handler has read
and applied them so stale data cannot be reused; alternatively replace the
shared staging fields with a payload carried inside the AppMessage union/struct
so each message owns its data.

Comment on lines +948 to +950
uint8_t trayIndex = assignDoc["tray_index"] | 0;
const char* uid = assignDoc["uid"] | "";
int32_t spoolmanId = assignDoc["spoolman_id"] | -1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent default to tray index 0 when tray_index is missing from JSON.

If the JSON payload is malformed or missing tray_index, the code defaults to 0, which is a valid tray index. This could cause unintended assignments to tray 0.

Consider validating that tray_index is explicitly present in the payload before proceeding, or use a sentinel value like 255 as the default and reject if not overwritten.

🛡️ Proposed validation
-        uint8_t trayIndex = assignDoc["tray_index"] | 0;
+        if (!assignDoc.containsKey("tray_index")) {
+            publishCommandResponse(command, false, "missing_tray_index");
+            return;
+        }
+        uint8_t trayIndex = assignDoc["tray_index"];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uint8_t trayIndex = assignDoc["tray_index"] | 0;
const char* uid = assignDoc["uid"] | "";
int32_t spoolmanId = assignDoc["spoolman_id"] | -1;
if (!assignDoc.containsKey("tray_index")) {
publishCommandResponse(command, false, "missing_tray_index");
return;
}
uint8_t trayIndex = assignDoc["tray_index"];
const char* uid = assignDoc["uid"] | "";
int32_t spoolmanId = assignDoc["spoolman_id"] | -1;
🧰 Tools
🪛 Clang (14.0.6)

[warning] 948-948: variable 'trayIndex' is not initialized

(cppcoreguidelines-init-variables)


[warning] 949-949: variable 'uid' is not initialized

(cppcoreguidelines-init-variables)


[warning] 950-950: variable 'spoolmanId' is not initialized

(cppcoreguidelines-init-variables)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/HomeAssistantManager.cpp` around lines 948 - 950, The code silently
defaults trayIndex to 0 when reading assignDoc["tray_index"], which can
misassign tray 0; change the read to use a sentinel (e.g., uint8_t trayIndex =
255) or avoid defaulting and then validate presence via
assignDoc.containsKey("tray_index") or assignDoc["tray_index"].isNull() before
using it; if the field is missing or equals the sentinel, bail out or return an
error instead of proceeding with trayIndex (update the logic around the
assignDoc read and subsequent use of trayIndex in the function to reject
malformed payloads).

Comment thread src/TrayDashboardTypes.h
Comment on lines +13 to +14
char uid[17]; // SpoolSense NFC tag UID hex (up to 16 chars + null)
int32_t spoolman_id; // Spoolman spool ID (-1 if unknown)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing default initialization for spoolman_id field.

When TrayDashboardState is zero-initialized (e.g., TrayDashboardState state = {}), spoolman_id will be 0, not -1. Since 0 is a valid Spoolman spool ID and -1 indicates "unknown", this can lead to incorrect behavior when checking if a tray has an assigned spool.

Consider adding a comment or ensuring callers explicitly initialize to -1, or use a constructor/initializer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/TrayDashboardTypes.h` around lines 13 - 14, The spoolman_id field in
TrayDashboardState is not default-initialized to the sentinel -1, so
zero-initializing the struct yields 0 (a valid spool ID); update the
TrayDashboardState definition to ensure spoolman_id defaults to -1 (either by
adding a default member initializer "int32_t spoolman_id = -1;" or adding a
constructor that sets spoolman_id = -1), and keep the uid comment; ensure any
existing code that relied on zero-init is adjusted or explicitly documents the
new default.

@github-actions github-actions Bot added size/XL Extra large change (500+ lines) and removed size/L Large change (200-500 lines) labels Apr 11, 2026
@sjordan0228 sjordan0228 merged commit 6f723e7 into dev Apr 11, 2026
1 check passed
@sjordan0228 sjordan0228 deleted the feature/bambu-dashboard-blueprint branch April 21, 2026 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large change (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant