v1.6.1: NFC+ improvements, HA optimization, tag writer auto-populate#58
Merged
Conversation
…ce (#55) Discovery was re-published on every tag state update (10+ MQTT messages per scan). Now tracks lastDiscoveryUid_ and only re-publishes when the UID actually changes. Legacy entity cleanup (openprinttag_ prefixed) runs once per boot instead of every discovery call. Estimated MQTT traffic reduction: ~80% on repeated scans.
Replace per-call WiFiClient + HTTPClient stack objects with persistent file-scope statics. setReuse(true) keeps the TCP connection alive across requests. All calls serialized by httpMutex_ so no concurrency issues.
Add settings_extruder_temp and settings_bed_temp to SpoolDetails, SpoolmanSyncedPayload, and GenericTagSpoolInfo. Parse from Spoolman filament response in getSpoolDetails(). Display on reader page when present.
- Bump HA task stack 7168 → 8192 (discovery payloads overflowed) - Remove legacy openprinttag_ entity cleanup (no users on old naming) - NFC+ reader: show temps from Spoolman, keep polling until data arrives - Dynamic JSON buffer for UID lookup (sized to response, was fixed 16KB) - Reader page shows "Looking up in Spoolman... keep tag on reader"
Add spool picker to reader page for generic UID tags. Search and link unlinked tags or re-assign already linked tags to different spools. Two new proxy endpoints avoid CORS: - /api/spoolman/spools — streams spool list (512B buffer, no heap) - /api/spoolman/link — PATCHes nfc_id on new spool, clears old Also: dynamic JSON buffer for UID lookup (sized to response), reader keeps polling until Spoolman data arrives for NFC+ tags.
Brand + material was shown twice (line 1 and name line were identical). Drop the name line, keep single brand + material line above weight bar.
- Add 5s HTTP timeout on proxy requests to Spoolman - Add 10s safety timeout on streaming loop to prevent web server hang - Validate nfc_id is hex-only, max 16 chars before PATCH - Fail on old spool clear error instead of silently continuing - Remove redundant spoolmanUrl JS guard (server validates) - Hide spool picker on Scan Again - Show spool ID in picker rows for disambiguation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds extruder/bed temperature fields across Spoolman, NFC/Spool structs, managers, APIs, and UIs; introduces a reader spool-picker with link API; adds writer-page tag prefill utilities; refactors Spoolman HTTP client reuse and reduces Home Assistant discovery republish frequency. Changes
Sequence Diagram(s)sequenceDiagram
participant SM as SpoolmanManager
participant AM as ApplicationManager
participant NFC as NFCManager
participant HA as HomeAssistantManager
participant WS as WebServerManager
participant Reader as ReaderUI
SM->>AM: emit SPOOLMAN_SYNCED (includes extruder_temp, bed_temp)
AM->>NFC: setGenericTagSpoolInfo(...) with temps
NFC->>Reader: status update (includes temps)
AM->>HA: publish MQTT state
HA->>HA: compare UID vs lastDiscoveryUid_ → publishDiscovery() if changed
Reader->>WS: POST /api/spoolman/link (link request)
WS->>SM: proxy PATCH to Spoolman (set/clear extra.nfc_id) and respond
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/HomeAssistantManager.cpp`:
- Around line 401-410: The discovery dedupe (lastDiscoveryUid_) can remain set
when reconnect/bootstrap calls publishCurrentTagState() which publishes a blank
UID, preventing a subsequent real-UID discovery via publishDiscovery(); to fix,
clear/reset lastDiscoveryUid_ when publishCurrentTagState() is invoked as part
of the reconnect/bootstrap flow that publishes an empty tag (i.e., when
NFCManager::getInstance().getCurrentSpoolState(...) yields !state.present or
state.spool_id[0] == '\0'), so that the later strcmp(currentUid,
lastDiscoveryUid_) check will detect the new UID and call publishDiscovery();
update the reconnect/bootstrap path or publishCurrentTagState() to set
lastDiscoveryUid_[0] = '\0' in that blank-tag case.
In `@src/OpenPrintTagWriterHTML.h`:
- Around line 285-312: prefillFromTag sets material_search programmatically so
the derived material state (material_type/ material_name) isn't updated; after
the prefillFromTag .then handler finishes (inside the same callback that sets
search.value and hidden.value) call syncMaterialNameFromSelection() (and/or
trigger the same input handlers you use for user changes) to re-sync the hidden
material_type from the selected material, and ensure this resync runs after
loadMaterialDb() has completed so cross-format scans don't submit
material_type=0 while the UI shows a different material.
In `@src/OpenTag3DWriterHTML.h`:
- Around line 339-359: The prefillFromTag callback currently ignores the fields
buildPayload reads (diameter_um, print_temp_c, bed_temp_c); update the callback
to populate those DOM inputs from the scanned tag. In the then(function(d))
block, after syncing modifiers, set the diameter_um input (ID 'diameter_um' or
the appropriate field) from d.diameter_mm by converting mm→µm (multiply by 1000,
handle parseFloat/NaN), set print_temp_c from nozzle_min (fallback to nozzle_max
if nozzle_min missing), and set bed_temp_c from bed_min (fallback to bed_max if
missing); use the same element lookups pattern as the modifiers code and guard
null/undefined values.
In `@src/ReaderHTML.h`:
- Around line 183-194: In renderSpoolRow, vendor and filament fields
(fil.material, fil.name) are injected into an HTML string unescaped which allows
XSS; sanitize these values before building the returned HTML by HTML-encoding
vendor, fil.material and fil.name (and ensure spool.id is coerced/validated as a
number before embedding in the onclick) so the string concatenation uses escaped
text; update renderSpoolRow (and the same pattern at the other occurrence around
lines 221-224) to encode those values or else build DOM nodes and set
textContent instead of returning raw HTML.
In `@src/SharedJS.h`:
- Around line 258-323: normalizeTagData currently uses expressions like "value
|| 0" (and a later fill() path) which treat numeric 0 as falsy and overwrite
explicit zeros with defaults; change all such assignments in normalizeTagData to
preserve zero by testing for null/undefined (e.g., use value == null ? 0 : value
or Number.isFinite(value) ? value : 0) instead of "|| 0", and adjust the code
that uses fill() so it only fills missing/undefined values (check for value ==
null) rather than treating 0 as missing; update all occurrences in
normalizeTagData (and the analogous block referenced by the review) to follow
this null/undefined check pattern.
In `@src/SpoolmanManager.cpp`:
- Around line 225-265: The HTTP helper functions httpGet, httpPost and httpPatch
must call spoolmanHttp.end() after each request to clear request state; update
each function (httpGet, httpPost, httpPatch) to call spoolmanHttp.end() after
getting the response (or on all return paths, including error paths) while
preserving spoolmanHttp.setReuse(true) and the current begin()/GET|POST|PATCH
flow so the connection is properly reset between requests.
In `@src/TigerTagWriterHTML.h`:
- Around line 409-434: The manufacturer field mapping in prefillFromTag is
pointing to the wrong DOM id ('brand_search') — update the mapping in the
prefillFromTag call (the manufacturer property) to use the actual brand input id
'brand_name' so prefillFromTag populates the correct element; keep the existing
logic that sets document.getElementById('brand_id').value from
d.tigertag_brand_id and the fallback syncMaterialId()/brand handling unchanged.
In `@src/WebServerManager.cpp`:
- Around line 509-545: The current flow clears oldSpoolId before ensuring the
newSpoolId PATCH succeeds, risking detaching the tag if the second PATCH fails;
change the order so you first perform the PATCH to set nfc_id on newSpoolId (the
snprintf(body,...), http.begin(...), http.PATCH(body) block using newSpoolId and
nfcId) and only if that PATCH returns success (code == 200) then perform the
CLEAR PATCH for oldSpoolId (the earlier
http.PATCH("{\"extra\":{\"nfc_id\":\"\\\"\\\"\"}}") block); on new PATCH failure
return sendError(502, ...) without touching oldSpoolId so the original link
remains intact, and preserve existing response/error handling and timeouts.
- Around line 434-456: In the code path handling HTTP 200 (the block that
creates WiFiClient* stream = http.getStreamPtr() and int len = http.getSize()),
remove the premature end-of-body check that breaks when (!stream->available()) —
specifically delete or disable the final "if (len > 0 && !stream->available())
break;" so a temporary available()==0 does not terminate the transfer; instead
rely on content length (len) and stream connection state (stream->connected()) /
readBytes returning 0/timeouts to determine end-of-stream, ensuring the while
loop continues until the real end is reached.
🪄 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: 83e6a21f-416a-4190-a269-50b491cc6d78
📒 Files selected for processing (16)
.gitignoresrc/ApplicationManager.cppsrc/ApplicationManager.hsrc/HomeAssistantManager.cppsrc/HomeAssistantManager.hsrc/NFCManager.hsrc/OpenPrintTagWriterHTML.hsrc/OpenTag3DWriterHTML.hsrc/ReaderHTML.hsrc/SharedJS.hsrc/SpoolmanManager.cppsrc/SpoolmanManager.hsrc/TFTManager.cppsrc/TigerTagWriterHTML.hsrc/WebServerManager.cppsrc/WebServerManager.h
Comment on lines
+258
to
+323
| function normalizeTagData(s) { | ||
| if (!s || !s.present) return null; | ||
| var d = {}; | ||
| var kind = s.tag_kind || ''; | ||
|
|
||
| if (kind === 'TigerTag' && s.tigertag) { | ||
| var t = s.tigertag; | ||
| d.material = t.material_name || ''; | ||
| d.color = t.color_hex || ''; | ||
| d.manufacturer = t.brand_name || ''; | ||
| d.weight = t.weight_g || 0; | ||
| d.diameter = t.diameter_mm || 0; | ||
| d.nozzle_min = t.nozzle_temp_min || 0; | ||
| d.nozzle_max = t.nozzle_temp_max || 0; | ||
| d.bed_min = t.bed_temp_min || 0; | ||
| d.bed_max = t.bed_temp_max || 0; | ||
| d.dry_temp = t.dry_temp || 0; | ||
| d.dry_time = t.dry_time_hours || 0; | ||
| d.tigertag_material_id = t.material_id; | ||
| d.tigertag_brand_id = t.brand_id; | ||
| } else if (kind === 'OpenTag3D' && s.opentag3d) { | ||
| var o = s.opentag3d; | ||
| d.material = o.base_material || ''; | ||
| d.modifiers = o.modifiers || ''; | ||
| d.color = o.color_hex || ''; | ||
| d.manufacturer = o.manufacturer || ''; | ||
| d.weight = o.target_weight_g || 0; | ||
| d.diameter = o.diameter_mm || 0; | ||
| d.density = o.density || 0; | ||
| d.nozzle_min = o.min_print_temp || o.print_temp || 0; | ||
| d.nozzle_max = o.max_print_temp || o.print_temp || 0; | ||
| d.bed_min = o.min_bed_temp || o.bed_temp || 0; | ||
| d.bed_max = o.max_bed_temp || o.bed_temp || 0; | ||
| d.dry_temp = o.dry_temp || 0; | ||
| d.dry_time = o.dry_time_hours || 0; | ||
| } else if (s.tag_data_valid) { | ||
| // OpenPrintTag | ||
| d.material = s.material_name || ''; | ||
| d.material_type = s.material_type; | ||
| d.color = s.color || ''; | ||
| d.manufacturer = s.manufacturer || ''; | ||
| d.weight = s.initial_weight_g || 0; | ||
| d.remaining = s.remaining_g || 0; | ||
| d.density = s.density || 0; | ||
| d.diameter = s.diameter_mm || 0; | ||
| d.nozzle_min = s.min_print_temp || 0; | ||
| d.nozzle_max = s.max_print_temp || 0; | ||
| d.bed_min = s.min_bed_temp || 0; | ||
| d.bed_max = s.max_bed_temp || 0; | ||
| d.preheat = s.preheat_temp || 0; | ||
| d.spoolman_id = s.spoolman_id || 0; | ||
| } else if (s.material_name) { | ||
| // NFC+ with Spoolman data | ||
| d.material = s.material_name || ''; | ||
| d.color = s.color || ''; | ||
| d.manufacturer = s.manufacturer || ''; | ||
| d.remaining = s.remaining_g || 0; | ||
| d.nozzle_min = s.extruder_temp || 0; | ||
| d.nozzle_max = s.extruder_temp || 0; | ||
| d.bed_min = s.bed_temp || 0; | ||
| d.bed_max = s.bed_temp || 0; | ||
| } else { | ||
| return null; | ||
| } | ||
|
|
||
| return d; |
There was a problem hiding this comment.
Preserve numeric zeroes during normalization.
normalizeTagData() uses || 0 and fill() drops 0, so 0 can never be distinguished from “missing”. Empty spools or explicit zero-temp fields therefore leave stale form values behind.
Also applies to: 331-338
🧰 Tools
🪛 Clang (14.0.6)
[warning] 263-263: repeated branch in conditional chain
(bugprone-branch-clone)
[note] 278-278: end of the original
(clang)
[note] 278-278: clone 1 starts here
(clang)
[note] 293-293: clone 2 starts here
(clang)
[note] 309-309: clone 3 starts here
(clang)
[note] 319-319: clone 4 starts here
(clang)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SharedJS.h` around lines 258 - 323, normalizeTagData currently uses
expressions like "value || 0" (and a later fill() path) which treat numeric 0 as
falsy and overwrite explicit zeros with defaults; change all such assignments in
normalizeTagData to preserve zero by testing for null/undefined (e.g., use value
== null ? 0 : value or Number.isFinite(value) ? value : 0) instead of "|| 0",
and adjust the code that uses fill() so it only fills missing/undefined values
(check for value == null) rather than treating 0 as missing; update all
occurrences in normalizeTagData (and the analogous block referenced by the
review) to follow this null/undefined check pattern.
Comment on lines
225
to
+265
| // --- File-local HTTP helpers --- | ||
| // Persistent client + http objects — reuse TCP connection across requests. | ||
| // All Spoolman calls are serialized by httpMutex_ so no concurrent access. | ||
| static WiFiClient spoolmanClient; | ||
| static HTTPClient spoolmanHttp; | ||
|
|
||
| static int httpGet(const char* path, String& response) { | ||
| const char* baseUrl = ConfigurationManager::getInstance().getSpoolmanURL(); | ||
| WiFiClient client; | ||
| HTTPClient http; | ||
|
|
||
| char url[256]; | ||
| snprintf(url, sizeof(url), "%s%s", baseUrl, path); | ||
| http.begin(client, url); | ||
| int code = http.GET(); | ||
| spoolmanHttp.begin(spoolmanClient, url); | ||
| spoolmanHttp.setReuse(true); | ||
| int code = spoolmanHttp.GET(); | ||
| if (code > 0) { | ||
| response = http.getString(); | ||
| response = spoolmanHttp.getString(); | ||
| } | ||
| http.end(); | ||
| return code; | ||
| } | ||
|
|
||
| static int httpPost(const char* path, const char* body, String& response) { | ||
| const char* baseUrl = ConfigurationManager::getInstance().getSpoolmanURL(); | ||
| WiFiClient client; | ||
| HTTPClient http; | ||
|
|
||
| char url[256]; | ||
| snprintf(url, sizeof(url), "%s%s", baseUrl, path); | ||
| http.begin(client, url); | ||
| http.addHeader("Content-Type", "application/json"); | ||
| int code = http.POST(body); | ||
| spoolmanHttp.begin(spoolmanClient, url); | ||
| spoolmanHttp.setReuse(true); | ||
| spoolmanHttp.addHeader("Content-Type", "application/json"); | ||
| int code = spoolmanHttp.POST(body); | ||
| if (code > 0) { | ||
| response = http.getString(); | ||
| response = spoolmanHttp.getString(); | ||
| } | ||
| http.end(); | ||
| return code; | ||
| } | ||
|
|
||
| static int httpPatch(const char* path, const char* body, String& response) { | ||
| const char* baseUrl = ConfigurationManager::getInstance().getSpoolmanURL(); | ||
| WiFiClient client; | ||
| HTTPClient http; | ||
|
|
||
| char url[256]; | ||
| snprintf(url, sizeof(url), "%s%s", baseUrl, path); | ||
| http.begin(client, url); | ||
| http.addHeader("Content-Type", "application/json"); | ||
| int code = http.PATCH(body); | ||
| spoolmanHttp.begin(spoolmanClient, url); | ||
| spoolmanHttp.setReuse(true); | ||
| spoolmanHttp.addHeader("Content-Type", "application/json"); | ||
| int code = spoolmanHttp.PATCH(body); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Arduino-ESP32's HTTPClient, when using setReuse(true)to keep the TCP connection alive, shouldHTTPClient.end()still be called after each request and before the nextbegin()? Also, does addHeader()state persist across requests ifend() is omitted?
💡 Result:
-
Yes—still call
HTTPClient.end()after each request (and before the nextbegin()). WithsetReuse(true),end()does not necessarily close the socket:end()callsdisconnect(false), anddisconnect()keeps the TCP connection open when_reuse && _canReuseis true (“tcp keep open for reuse”). (git.liberatedsystems.co.uk)
This is also exactly how the official ReuseConnection example is written:setReuse(true)→begin()→GET()→end()in every loop. (github.com) -
No—
addHeader()does not persist across requests. Headers added viaaddHeader()are stored in_headers, andclear()resets_headersto an empty string.end()callsclear(), and even if you omitend(), the request flow callsclear()while handling the response headers—so your previously added headers won’t carry over to the next request. (git.liberatedsystems.co.uk)
Citations:
- 1: https://git.liberatedsystems.co.uk/jacob.eva/arduino-esp32/src/commit/c7bdb234bf6ccd2a96866b3344c766a6f1c174cb/libraries/HTTPClient/src/HTTPClient.cpp
- 2: https://github.com/espressif/arduino-esp32/blob/6cfa93f4/libraries/HTTPClient/examples/ReuseConnection/ReuseConnection.ino
- 3: https://git.liberatedsystems.co.uk/jacob.eva/arduino-esp32/src/commit/c7bdb234bf6ccd2a96866b3344c766a6f1c174cb/libraries/HTTPClient/src/HTTPClient.cpp
Add spoolmanHttp.end() after each request in these helpers.
These helpers must call spoolmanHttp.end() after retrieving the response in every iteration, even with setReuse(true). The official Arduino-ESP32 ReuseConnection example explicitly shows the pattern: begin() → GET()/POST()/PATCH() → end() per loop. end() clears request state and is required to properly reset between requests; omitting it will cause state to leak across requests.
Example fix for httpGet:
static int httpGet(const char* path, String& response) {
const char* baseUrl = ConfigurationManager::getInstance().getSpoolmanURL();
char url[256];
snprintf(url, sizeof(url), "%s%s", baseUrl, path);
spoolmanHttp.begin(spoolmanClient, url);
spoolmanHttp.setReuse(true);
int code = spoolmanHttp.GET();
if (code > 0) {
response = spoolmanHttp.getString();
}
spoolmanHttp.end(); // <-- Add this
return code;
}
🧰 Tools
🪛 Clang (14.0.6)
[warning] 228-228: variable 'spoolmanClient' is non-const and globally accessible, consider making it const
(cppcoreguidelines-avoid-non-const-global-variables)
[warning] 229-229: variable 'spoolmanHttp' is non-const and globally accessible, consider making it const
(cppcoreguidelines-avoid-non-const-global-variables)
[warning] 231-231: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 233-233: do not declare C-style arrays, use std::array<> instead
(modernize-avoid-c-arrays)
[warning] 237-237: variable 'code' is not initialized
(cppcoreguidelines-init-variables)
[warning] 244-244: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 244-244: 2 adjacent parameters of 'httpPost' of similar type ('const char *') are easily swapped by mistake
(bugprone-easily-swappable-parameters)
[note] 244-244: the first parameter in the range is 'path'
(clang)
[note] 244-244: the last parameter in the range is 'body'
(clang)
[warning] 246-246: do not declare C-style arrays, use std::array<> instead
(modernize-avoid-c-arrays)
[warning] 251-251: variable 'code' is not initialized
(cppcoreguidelines-init-variables)
[warning] 258-258: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 258-258: 2 adjacent parameters of 'httpPatch' of similar type ('const char *') are easily swapped by mistake
(bugprone-easily-swappable-parameters)
[note] 258-258: the first parameter in the range is 'path'
(clang)
[note] 258-258: the last parameter in the range is 'body'
(clang)
[warning] 260-260: do not declare C-style arrays, use std::array<> instead
(modernize-avoid-c-arrays)
[warning] 265-265: variable 'code' 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/SpoolmanManager.cpp` around lines 225 - 265, The HTTP helper functions
httpGet, httpPost and httpPatch must call spoolmanHttp.end() after each request
to clear request state; update each function (httpGet, httpPost, httpPatch) to
call spoolmanHttp.end() after getting the response (or on all return paths,
including error paths) while preserving spoolmanHttp.setReuse(true) and the
current begin()/GET|POST|PATCH flow so the connection is properly reset between
requests.
Comment on lines
+409
to
+434
| // Pre-fill from scanned tag if present | ||
| prefillFromTag({ | ||
| material: 'material_search', | ||
| color: 'colorHex', | ||
| colorPicker: 'colorPicker', | ||
| manufacturer: 'brand_search', | ||
| weight: 'weight_g', | ||
| nozzle_min: 'nozzle_min', | ||
| nozzle_max: 'nozzle_max', | ||
| bed_min: 'bed_min', | ||
| bed_max: 'bed_max', | ||
| dry_temp: 'dry_temp', | ||
| dry_time: 'dry_time' | ||
| }).then(function(d) { | ||
| if (!d) return; | ||
| // Sync hidden material_id from material name | ||
| if (d.tigertag_material_id !== undefined) { | ||
| document.getElementById('material_id').value = d.tigertag_material_id; | ||
| } else { | ||
| syncMaterialId(); | ||
| } | ||
| // Sync hidden brand_id from brand name | ||
| if (d.tigertag_brand_id !== undefined) { | ||
| document.getElementById('brand_id').value = d.tigertag_brand_id; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Bug: Incorrect field ID for manufacturer prefill.
The mapping manufacturer: 'brand_search' references a non-existent input field. The actual brand input field has id="brand_name" (line 82).
🐛 Proposed fix
prefillFromTag({
material: 'material_search',
color: 'colorHex',
colorPicker: 'colorPicker',
- manufacturer: 'brand_search',
+ manufacturer: 'brand_name',
weight: 'weight_g',
nozzle_min: 'nozzle_min',
nozzle_max: 'nozzle_max',
bed_min: 'bed_min',
bed_max: 'bed_max',
dry_temp: 'dry_temp',
dry_time: 'dry_time'
}).then(function(d) {📝 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
| // Pre-fill from scanned tag if present | |
| prefillFromTag({ | |
| material: 'material_search', | |
| color: 'colorHex', | |
| colorPicker: 'colorPicker', | |
| manufacturer: 'brand_search', | |
| weight: 'weight_g', | |
| nozzle_min: 'nozzle_min', | |
| nozzle_max: 'nozzle_max', | |
| bed_min: 'bed_min', | |
| bed_max: 'bed_max', | |
| dry_temp: 'dry_temp', | |
| dry_time: 'dry_time' | |
| }).then(function(d) { | |
| if (!d) return; | |
| // Sync hidden material_id from material name | |
| if (d.tigertag_material_id !== undefined) { | |
| document.getElementById('material_id').value = d.tigertag_material_id; | |
| } else { | |
| syncMaterialId(); | |
| } | |
| // Sync hidden brand_id from brand name | |
| if (d.tigertag_brand_id !== undefined) { | |
| document.getElementById('brand_id').value = d.tigertag_brand_id; | |
| } | |
| }); | |
| // Pre-fill from scanned tag if present | |
| prefillFromTag({ | |
| material: 'material_search', | |
| color: 'colorHex', | |
| colorPicker: 'colorPicker', | |
| manufacturer: 'brand_name', | |
| weight: 'weight_g', | |
| nozzle_min: 'nozzle_min', | |
| nozzle_max: 'nozzle_max', | |
| bed_min: 'bed_min', | |
| bed_max: 'bed_max', | |
| dry_temp: 'dry_temp', | |
| dry_time: 'dry_time' | |
| }).then(function(d) { | |
| if (!d) return; | |
| // Sync hidden material_id from material name | |
| if (d.tigertag_material_id !== undefined) { | |
| document.getElementById('material_id').value = d.tigertag_material_id; | |
| } else { | |
| syncMaterialId(); | |
| } | |
| // Sync hidden brand_id from brand name | |
| if (d.tigertag_brand_id !== undefined) { | |
| document.getElementById('brand_id').value = d.tigertag_brand_id; | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/TigerTagWriterHTML.h` around lines 409 - 434, The manufacturer field
mapping in prefillFromTag is pointing to the wrong DOM id ('brand_search') —
update the mapping in the prefillFromTag call (the manufacturer property) to use
the actual brand input id 'brand_name' so prefillFromTag populates the correct
element; keep the existing logic that sets
document.getElementById('brand_id').value from d.tigertag_brand_id and the
fallback syncMaterialId()/brand handling unchanged.
Critical: - XSS: escape Spoolman vendor/material names in spool picker HTML Major: - Streaming proxy: remove premature !available() break that truncated JSON - Re-assign: set new spool first, clear old after (prevents detached tags) - OPT writer: dispatch input event after prefill to sync material state - OT3D writer: prefill basic print/bed temps and diameter dropdown - HA discovery: reset lastDiscoveryUid_ on MQTT reconnect - Spoolman HTTP: document begin()/setReuse(true) pattern
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ReaderHTML.h (1)
310-327: 🧹 Nitpick | 🔵 TrivialRemove redundant
render(s)call.Line 322 calls
render(s)again, butrender(s)was already called at line 313 with the same status object. This results in unnecessarily re-rendering the same content.Proposed fix
if (!tagFound && !isGenericPending) { tagFound = true; stopPolling(); scanBtn.classList.remove('hidden'); } else if (!tagFound) { // Still waiting for Spoolman lookup — keep polling - render(s); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ReaderHTML.h` around lines 310 - 327, In poll(), remove the redundant second call to render(s) inside the else if (!tagFound) branch: keep the initial render(s) at the top of the s.present block and delete the duplicate render(s) in the "Still waiting for Spoolman lookup" branch so the logic around tagFound, isGenericPending, stopPolling(), and scanBtn.classList.remove('hidden') remains unchanged but avoids double-rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/ReaderHTML.h`:
- Around line 310-327: In poll(), remove the redundant second call to render(s)
inside the else if (!tagFound) branch: keep the initial render(s) at the top of
the s.present block and delete the duplicate render(s) in the "Still waiting for
Spoolman lookup" branch so the logic around tagFound, isGenericPending,
stopPolling(), and scanBtn.classList.remove('hidden') remains unchanged but
avoids double-rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2ccfecfa-33bd-4032-b163-cead16fe798e
📒 Files selected for processing (6)
src/HomeAssistantManager.cppsrc/OpenPrintTagWriterHTML.hsrc/OpenTag3DWriterHTML.hsrc/ReaderHTML.hsrc/SpoolmanManager.cppsrc/WebServerManager.cpp
This was referenced Apr 4, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reader page and NFC+ improvements, HA MQTT traffic reduction, Spoolman performance, and tag writer auto-populate.
Changes
NFC+ Reader Improvements
HA Optimization
Spoolman Performance
Tag Writer Auto-Populate (#57)
Code Review Fixes
Other
Test plan
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation