Add NFC+ UID registration page for plain NFC tags#15
Merged
Conversation
New page at /register/uid lets users register plain NFC tags (NTAG215 etc.) in Spoolman using the tag's UID as identifier. No data is written to the tag — it works as a simple ID badge for a spool. - UIDRegistrationHTML.h: full form matching existing writer page patterns - Read Tag UID button reads UID from scanner via /api/status - Register in Spoolman button creates filament + spool with nfc_id extra - Form fields: material, brand, color, weight, temps, density - NFC+ nav link added to all pages after OpenTag3D - NFC+ Registration card added to landing page Closes #4
|
Caution Review failedPull request was closed or merged during review Note
|
| Cohort / File(s) | Summary |
|---|---|
Navigation Link Updates src/ConfigHTML.h, src/OpenPrintTagWriterHTML.h, src/OpenTag3DWriterHTML.h, src/ReaderHTML.h, src/TigerTagWriterHTML.h, src/TroubleshootingHTML.h, src/UpdateHTML.h |
Inserted an NFC+ navigation link pointing to /register/uid in page header/nav markup. |
Landing Page src/LandingHTML.h |
Added NFC+ nav link and a new tool-card for "NFC+ Registration" describing UID-only registration. |
UID Registration Page src/UIDRegistrationHTML.h |
New HTML/JS page payload UID_REGISTRATION_HTML providing UID read flow (polls /api/status), form fields (manufacturer, material, weights, color, advanced fields), validation, UI states, and client POST to /api/register-uid. |
Server: Web Routes & API src/WebServerManager.cpp, src/WebServerManager.h |
Added route to serve the UID registration page (GET /register/uid) and a POST API handler (/api/register-uid) that validates input, ensures vendor exists (GET/POST), creates filament (POST), creates spool (POST) with extra.nfc_id containing the UID, and returns JSON success/error responses. |
Sequence Diagram
sequenceDiagram
actor User
participant Browser as Client (Browser)
participant Scanner as Scanner API (/api/status)
participant Server as Device WebServer
participant Spoolman as Spoolman API
User->>Browser: Open /register/uid
Browser->>Server: GET /register/uid
Server-->>Browser: Serve UID registration page
Browser->>Scanner: GET /api/status (pre-populate)
Scanner-->>Browser: Returns currentUid
User->>Browser: Click "Read Tag UID"
Browser->>Scanner: Poll /api/status until UID present
Scanner-->>Browser: Returns UID
User->>Browser: Fill form and submit
Browser->>Server: POST /api/register-uid (JSON payload with uid, manufacturer, material...)
Server->>Spoolman: GET /api/v1/vendor?name=...
alt vendor not found
Server->>Spoolman: POST /api/v1/vendor (create vendor)
Spoolman-->>Server: vendorId
end
Server->>Spoolman: POST /api/v1/filament (create filament)
Spoolman-->>Server: filamentId
Server->>Spoolman: POST /api/v1/spool (create spool with extra.nfc_id=UID)
Spoolman-->>Server: spoolId
Server-->>Browser: 200 OK { success, spool_id, filament_id, vendor_id, uid }
Browser-->>User: Display success (spool id, uid)
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'Add NFC+ UID registration page for plain NFC tags' clearly and specifically describes the main change: introducing a new page for registering plain NFC tags using their UID. |
| Description check | ✅ Passed | The description covers the Summary, Changes, User Flow, and Test plan sections, but lacks How to Test and full Checklist compliance with hardware testing and compilation verification. |
| Linked Issues check | ✅ Passed | The PR successfully implements all coding requirements from issue #4: new /register/uid page, Read Tag UID functionality, Spoolman registration with UID as nfc_id, and NFC+ navigation links across all pages. |
| Out of Scope Changes check | ✅ Passed | All changes are directly aligned with issue #4 requirements: new registration page, form fields, API endpoint, and navigation links. No unrelated modifications detected. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/uid-registration
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/ConfigHTML.h`:
- Line 52: The nav link to "/register/uid" in ConfigHTML.h points to a page
served by UIDRegistrationHTML.h but the route is not registered; add a route
registration in WebServerManager.cpp by calling _server.on("/register/uid",
HTTP_GET, [this]() { handleUIDRegistration(); }) and implement the handler
method handleUIDRegistration() in the WebServerManager class to send the
UIDRegistrationHTML content (reuse existing response helpers used by other
handlers).
In `@src/UIDRegistrationHTML.h`:
- Around line 279-280: The calls to readRequiredNumber for initialWeight and
remainingWeight (variables initialWeight and remainingWeight) are not validated
before use; update the form submission logic that consumes these values to
explicitly check for undefined or NaN and handle invalid input by showing a
user-facing error and aborting submission (or returning early) to avoid sending
malformed data to the Spoolman API. Specifically, after calling
readRequiredNumber('initial_weight_g') and readRequiredNumber('remaining_g'),
validate with isNaN/strict undefined checks, report which field is invalid
(initial_weight_g or remaining_g) via the existing UI/error display function,
and stop the API call until valid numeric values are provided.
- Around line 316-327: The browser fetches to Spoolman (seen in the vendor
lookup block where fetch(spoolmanUrl + '/api/v1/vendor?name=' + ... ) is used)
will be blocked by CORS when Spoolman is on a different origin; either route
these requests through the ESP32 (create a proxy endpoint on the ESP32 API and
replace direct fetch calls with calls to that endpoint, e.g., centralize into a
helper like proxyFetchSpoolman used by the three fetch sites), or explicitly
document that Spoolman must enable CORS (set SPOOLMAN_CORS_ORIGIN or
SPOOLMAN_DEBUG_MODE) and update the README and/or UI warnings to require that
configuration. Ensure you update all three fetch occurrences (vendor lookup and
the two other Spoolman fetches) to use the chosen approach.
- Around line 3-7: The GET /register/uid route is never registered and the
/api/status JSON omits spoolman_url; to fix, register the route in your web
server setup (add an entry for "GET /register/uid" that serves the UID
registration HTML handler or static content) in the WebServerManager route
registration code so the page stops 404'ing, and modify handleApiStatus() to
populate the JSON key "spoolman_url" from your configuration/service (e.g., read
the configured Spoolman URL or service instance and add it to the status
response) so the client-side JS can find spoolman_url and proceed with
registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c43da80c-d327-4f6a-ab6d-29b5675940ae
📒 Files selected for processing (9)
src/ConfigHTML.hsrc/LandingHTML.hsrc/OpenPrintTagWriterHTML.hsrc/OpenTag3DWriterHTML.hsrc/ReaderHTML.hsrc/TigerTagWriterHTML.hsrc/TroubleshootingHTML.hsrc/UIDRegistrationHTML.hsrc/UpdateHTML.h
Comment on lines
+3
to
+7
| // UID Registration page served at GET /register/uid | ||
| // | ||
| // API endpoints available to the page: | ||
| // GET /api/status — current tag state JSON (includes uid, spoolman_url) | ||
| // (Spoolman API calls are made directly from the browser to the configured Spoolman instance) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if /register/uid route is registered
echo "=== Checking for /register/uid route registration ==="
rg -n '/register/uid' --type cpp
echo ""
echo "=== Checking handleApiStatus for spoolman_url ==="
ast-grep --pattern 'void WebServerManager::handleApiStatus() {
$$$
}'
echo ""
echo "=== Checking if spoolman_url is set anywhere in status response ==="
rg -n 'spoolman_url' --type cppRepository: SpoolSense/spoolsense_scanner
Length of output: 2616
🏁 Script executed:
#!/bin/bash
# Verify route registration mechanism and handleApiStatus implementation
echo "=== Searching for handleApiStatus implementation ==="
rg -A 30 'handleApiStatus' src/WebServerManager.cpp | head -50
echo ""
echo "=== Checking for route registrations with server.on pattern ==="
rg -n 'server\.on\(' src/WebServerManager.cpp | head -20
echo ""
echo "=== Checking if /register/uid route is explicitly registered ==="
rg -n 'register.*uid' src/WebServerManager.cppRepository: SpoolSense/spoolsense_scanner
Length of output: 4001
🏁 Script executed:
#!/bin/bash
# Verify line 290 is within handleApiStatus and get broader context
echo "=== Context around line 290 in WebServerManager.cpp ==="
sed -n '280,300p' src/WebServerManager.cppRepository: SpoolSense/spoolsense_scanner
Length of output: 842
🏁 Script executed:
#!/bin/bash
# Find the complete handleApiStatus implementation
echo "=== Complete handleApiStatus implementation ==="
rg -A 100 'void WebServerManager::handleApiStatus\(' src/WebServerManager.cpp | head -120Repository: SpoolSense/spoolsense_scanner
Length of output: 5591
🏁 Script executed:
#!/bin/bash
# Find the end of handleApiStatus() to confirm spoolman_url is truly not included
echo "=== End of handleApiStatus function ==="
rg -A 150 'void WebServerManager::handleApiStatus\(' src/WebServerManager.cpp | tail -30
echo ""
echo "=== Check JavaScript at lines 291-302 in UIDRegistrationHTML.h ==="
sed -n '291,302p' src/UIDRegistrationHTML.hRepository: SpoolSense/spoolsense_scanner
Length of output: 1968
Route registration and API response mismatch prevent UID registration.
Two critical integration issues block the UID registration feature:
-
Route not registered: The
GET /register/uidroute is not registered inWebServerManager.cpp. The page will return 404 for all users. -
Missing
spoolman_urlin API response: The comment at line 6 claims/api/statusincludesspoolman_url, buthandleApiStatus()never populates this field. The JavaScript at lines 291-302 depends on this field to configure Spoolman integration—without it, registration always fails with "Spoolman URL is not configured."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/UIDRegistrationHTML.h` around lines 3 - 7, The GET /register/uid route is
never registered and the /api/status JSON omits spoolman_url; to fix, register
the route in your web server setup (add an entry for "GET /register/uid" that
serves the UID registration HTML handler or static content) in the
WebServerManager route registration code so the page stops 404'ing, and modify
handleApiStatus() to populate the JSON key "spoolman_url" from your
configuration/service (e.g., read the configured Spoolman URL or service
instance and add it to the status response) so the client-side JS can find
spoolman_url and proceed with registration.
Comment on lines
+279
to
+280
| var initialWeight = readRequiredNumber('initial_weight_g'); | ||
| var remainingWeight = readRequiredNumber('remaining_g'); |
There was a problem hiding this comment.
Missing validation for required numeric fields.
readRequiredNumber is called but the return values are not checked before use. If these return undefined or NaN for invalid input, the Spoolman API may receive malformed data.
Proposed fix
var initialWeight = readRequiredNumber('initial_weight_g');
+ if (initialWeight === undefined || isNaN(initialWeight)) {
+ setResult('Full Weight is required and must be a valid number.', 'error');
+ return;
+ }
var remainingWeight = readRequiredNumber('remaining_g');
+ if (remainingWeight === undefined || isNaN(remainingWeight)) {
+ setResult('Remaining Weight is required and must be a valid number.', 'error');
+ return;
+ }📝 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
| var initialWeight = readRequiredNumber('initial_weight_g'); | |
| var remainingWeight = readRequiredNumber('remaining_g'); | |
| var initialWeight = readRequiredNumber('initial_weight_g'); | |
| if (initialWeight === undefined || isNaN(initialWeight)) { | |
| setResult('Full Weight is required and must be a valid number.', 'error'); | |
| return; | |
| } | |
| var remainingWeight = readRequiredNumber('remaining_g'); | |
| if (remainingWeight === undefined || isNaN(remainingWeight)) { | |
| setResult('Remaining Weight is required and must be a valid number.', 'error'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/UIDRegistrationHTML.h` around lines 279 - 280, The calls to
readRequiredNumber for initialWeight and remainingWeight (variables
initialWeight and remainingWeight) are not validated before use; update the form
submission logic that consumes these values to explicitly check for undefined or
NaN and handle invalid input by showing a user-facing error and aborting
submission (or returning early) to avoid sending malformed data to the Spoolman
API. Specifically, after calling readRequiredNumber('initial_weight_g') and
readRequiredNumber('remaining_g'), validate with isNaN/strict undefined checks,
report which field is invalid (initial_weight_g or remaining_g) via the existing
UI/error display function, and stop the API call until valid numeric values are
provided.
- Add handleApiRegisterUid in WebServerManager — scanner proxies all Spoolman API calls (vendor lookup, filament creation, spool creation) to avoid browser CORS issues - Fix Read Tag UID to poll /api/status until tag is detected (500ms interval) - Fix Spoolman URL lookup: read from /api/config not /api/status - Fix filament creation: density and diameter are required by Spoolman, default to 1.24/1.75 when not specified - Fix spool extras: color_hex belongs on filament, not spool extras - Fix nfc_id extra field: Spoolman requires JSON-quoted string values - Add serial debug logging for API payloads
7 tasks
This was referenced Mar 27, 2026
This was referenced Apr 7, 2026
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
/register/uidfor registering plain NFC tags in SpoolmanUser Flow
nfc_idTest plan
Closes #4
Summary by CodeRabbit