Skip to content

fix: register-uid deduplicates filaments and spools #136

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.

Sign up for GitHub

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
sjordan0228 merged 3 commits into from
Apr 10, 2026
Merged

fix: register-uid deduplicates filaments and spools #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e7fff8
fix: register-uid now deduplicates filaments and spools (#134, #135)
sjordan0228 Apr 9, 2026
1801457
fix: writeISO14443Pages skips reactivation when tag session is active
sjordan0228 Apr 9, 2026
38a9238
ux: writer pages remind user to keep tag on scanner during write+verify
sjordan0228 Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/HardwareNFCConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,17 @@ bool HardwareNFCConnection::writeISO14443Pages(uint8_t startPage, uint8_t pageCo
uint16_t totalBytes = pageCount * 4;
if (dataLen < totalBytes) return false;

// Reactivate tag before write sequence
iso14443a_->setupRF();
uint8_t response[10] = {0};
uint8_t uidLen = iso14443a_->activateTypeA(response, 1);
if (uidLen < 4) {
Serial.println("HardwareNFC: writeISO14443Pages - tag reactivation failed");
return false;
uint8_t uidLen = 0;

if (!tagSessionActive_) {
// Tag was halted — must reactivate before writing
iso14443a_->setupRF();
uidLen = iso14443a_->activateTypeA(response, 1);
if (uidLen < 4) {
Serial.println("HardwareNFC: writeISO14443Pages - tag reactivation failed");
return false;
}
Comment on lines +377 to +384
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep tagSessionActive_ in sync with HALT in write flow.

This new fast-path depends on tagSessionActive_, but writeISO14443Pages() halts at Line 414/Line 421 without clearing the flag. That can make the next call skip activation incorrectly and fall into avoidable retries/failures.

🔧 Proposed fix 
     if (!tagSessionActive_) {
         // Tag was halted — must reactivate before writing
         iso14443a_->setupRF();
         uidLen = iso14443a_->activateTypeA(response, 1);
         if (uidLen < 4) {
             Serial.println("HardwareNFC: writeISO14443Pages - tag reactivation failed");
             return false;
         }
+        tagSessionActive_ = true;
     }
@@
                 uidLen = iso14443a_->activateTypeA(response, 1);
                 if (uidLen < 4) {
                     Serial.printf("HardwareNFC: writeISO14443Pages - re-activation failed on retry %d\n", retry + 1);
                     continue;
                 }
+                tagSessionActive_ = true;
                 pageOk = iso14443a_->mifareBlockWrite4(page, data + (i * 4));
             }
@@
                 Serial.printf("HardwareNFC: writeISO14443Pages - FAILED at page %d after retries (%lums, %d retries total)\n",
                               page, elapsed, retryCount);
                 iso14443a_->mifareHalt();
+                tagSessionActive_ = false;
                 return false;  // total write failure; clean exit for caller to retry entire sequence
             }
@@
     unsigned long elapsed = millis() - writeStart;
     iso14443a_->mifareHalt();
+    tagSessionActive_ = false;
     Serial.printf("HardwareNFC: writeISO14443Pages - wrote %d pages starting at page %d (%lums, %d retries)\n",
                   pageCount, startPage, elapsed, retryCount);

Also applies to: 414-422

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

In `@src/HardwareNFCConnection.cpp` around lines 377 - 384, The write flow in
writeISO14443Pages() leaves tagSessionActive_ set even when the code halts the
tag, causing subsequent calls to skip reactivation; update writeISO14443Pages()
(and any helper that issues HALT) to clear tagSessionActive_ (set it false)
immediately after performing iso14443a_->halt()/halting logic so the fast-path
reactivation code (which checks tagSessionActive_) behaves correctly on the next
call; ensure any early-return paths after halting also clear tagSessionActive_
before returning.

}

// Write per-page with retry: ISO14443A tags lose activation after errors; retry with re-activate
Expand Down
4 changes: 2 additions & 2 deletions src/SharedJS.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ async function sharedWriteFlow(config) {

setStepState('step-write', 'active');
setBanner('statusBanner', 'Writing ' + config.formatName + ' data\u2026');
setResult('resultBox', 'Sending ' + config.formatName + ' payload to scanner.', '');
setResult('resultBox', 'Keep the tag on the scanner \u2014 writing and verifying takes a few seconds.', '');
await api(config.endpoint, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
Expand All @@ -591,7 +591,7 @@ async function sharedWriteFlow(config) {

setStepState('step-verify', 'active');
setBanner('statusBanner', 'Verifying write\u2026');
setResult('resultBox', 'Reading tag back to confirm data matches.', '');
setResult('resultBox', 'Reading tag back to confirm \u2014 keep the tag on the scanner.', '');

var verifyDeadline = Date.now() + 15000;
while (Date.now() < verifyDeadline) {
Expand Down
112 changes: 46 additions & 66 deletions src/WebServerManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,84 +363,64 @@ void WebServerManager::handleApiRegisterUid() {
http.end();
}

// --- Step 2: Create filament ---
StaticJsonDocument<512> filBody;
filBody["name"] = strlen(materialName) > 0 ? materialName : material;
filBody["material"] = material;
if (vendorId > 0) filBody["vendor_id"] = vendorId;
filBody["density"] = density > 0 ? density : 1.24f;
filBody["diameter"] = diameter > 0 ? diameter : 1.75f;
if (strlen(color) > 0) filBody["color_hex"] = color;
if (extruderTemp > 0) filBody["settings_extruder_temp"] = extruderTemp;
if (bedTemp > 0) filBody["settings_bed_temp"] = bedTemp;

String filJson;
serializeJson(filBody, filJson);
Serial.printf("register-uid: filament payload: %s\n", filJson.c_str());

snprintf(url, sizeof(url), "%s/api/v1/filament", baseUrl);
http.begin(client, url);
http.addHeader("Content-Type", "application/json");
code = http.POST(filJson);
if (code != 200 && code != 201) {
response = http.getString();
http.end();
char errMsg[128];
snprintf(errMsg, sizeof(errMsg), "Failed to create filament (HTTP %d)", code);
xSemaphoreGive(g_httpMutex);
sendError(500, errMsg);
return;
}
response = http.getString();
http.end();

int filamentId = -1;
{
StaticJsonDocument<1024> filDoc;
if (!deserializeJson(filDoc, response)) {
filamentId = filDoc["id"] | -1;
}
}
// --- Step 2: Find or create filament (#134) ---
int filamentId = enrichFindOrCreateFilament(client, http, baseUrl, material, color,
vendorId, density > 0 ? density : 1.24f,
diameter > 0 ? diameter : 1.75f,
bedTemp, extruderTemp, -1);
if (filamentId < 0) {
xSemaphoreGive(g_httpMutex);
sendError(500, "Failed to parse filament ID from Spoolman response");
sendError(500, "Failed to find or create filament");
return;
}
Serial.printf("register-uid: filament id=%d\n", filamentId);

// --- Step 3: Create spool with nfc_id ---
StaticJsonDocument<512> spoolBody;
spoolBody["filament_id"] = filamentId;
if (initialWeight > 0) spoolBody["initial_weight"] = initialWeight;
if (remainingWeight > 0) spoolBody["remaining_weight"] = remainingWeight;

JsonObject extra = spoolBody.createNestedObject("extra");
// Spoolman extra fields require JSON-encoded string values (double-quoted)
// --- Step 3: Find existing spool or create new (#135) ---
char quotedUid[128];
snprintf(quotedUid, sizeof(quotedUid), "\"%s\"", uid);
extra["nfc_id"] = quotedUid;

String spoolJson;
serializeJson(spoolBody, spoolJson);
Serial.printf("register-uid: spool payload: %s\n", spoolJson.c_str());
float existingInitialWeight = 0.0f;
int spoolId = enrichFindSpoolByUid(client, http, baseUrl, quotedUid, existingInitialWeight);

snprintf(url, sizeof(url), "%s/api/v1/spool", baseUrl);
http.begin(client, url);
http.addHeader("Content-Type", "application/json");
code = http.POST(spoolJson);
if (code != 200 && code != 201) {
if (spoolId > 0) {
// Spool exists — update it instead of creating a duplicate
Serial.printf("register-uid: found existing spool %d, updating\n", spoolId);
float effInitial = initialWeight > 0 ? initialWeight : existingInitialWeight;
if (!enrichUpdateSpool(client, http, baseUrl, spoolId, filamentId, remainingWeight, effInitial)) {
xSemaphoreGive(g_httpMutex);
sendError(500, "Failed to update existing spool");
return;
}
} else {
// No existing spool — create new
StaticJsonDocument<512> spoolBody;
spoolBody["filament_id"] = filamentId;
if (initialWeight > 0) spoolBody["initial_weight"] = initialWeight;
if (remainingWeight > 0) spoolBody["remaining_weight"] = remainingWeight;

JsonObject extra = spoolBody.createNestedObject("extra");
extra["nfc_id"] = quotedUid;

String spoolJson;
serializeJson(spoolBody, spoolJson);
Serial.printf("register-uid: creating spool: %s\n", spoolJson.c_str());

snprintf(url, sizeof(url), "%s/api/v1/spool", baseUrl);
http.begin(client, url);
http.addHeader("Content-Type", "application/json");
code = http.POST(spoolJson);
if (code != 200 && code != 201) {
response = http.getString();
http.end();
Serial.printf("register-uid: spool creation failed HTTP %d: %s\n", code, response.c_str());
String errMsg = "Failed to create spool (HTTP " + String(code) + "): " + response;
xSemaphoreGive(g_httpMutex);
_server.send(500, "application/json", "{\"error\":\"" + errMsg + "\"}");
Comment on lines +416 to +418
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unescaped error message can produce malformed JSON.

The response from Spoolman may contain quotes or special characters. Concatenating it directly into the JSON string bypasses the escaping logic in sendError(), potentially producing invalid JSON responses.

Proposed fix using sendError() 
         if (code != 200 && code != 201) {
             response = http.getString();
             http.end();
             Serial.printf("register-uid: spool creation failed HTTP %d: %s\n", code, response.c_str());
-            String errMsg = "Failed to create spool (HTTP " + String(code) + "): " + response;
             xSemaphoreGive(g_httpMutex);
-            _server.send(500, "application/json", "{\"error\":\"" + errMsg + "\"}");
+            char errMsg[128];
+            snprintf(errMsg, sizeof(errMsg), "Failed to create spool (HTTP %d)", code);
+            sendError(500, errMsg);
             return;
         }

This truncates the Spoolman response detail but ensures valid JSON. Alternatively, use sendError() with just the HTTP code and log the full response for debugging (which is already done on line 415).

📝 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
String errMsg = "Failed to create spool (HTTP " + String(code) + "): " + response;
xSemaphoreGive(g_httpMutex);
_server.send(500, "application/json", "{\"error\":\"" + errMsg + "\"}");
xSemaphoreGive(g_httpMutex);
char errMsg[128];
snprintf(errMsg, sizeof(errMsg), "Failed to create spool (HTTP %d)", code);
sendError(500, errMsg);
🧰 Tools
🪛 Clang (14.0.6)

[warning] 416-416: variable 'errMsg' 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/WebServerManager.cpp` around lines 416 - 418, The current code builds a
JSON error body by concatenating response into errMsg and calling _server.send
which can produce invalid JSON if response contains quotes/special chars;
replace that custom JSON construction with the existing sendError() helper (or
call _server.send with a properly escaped/JSON-encoded response) so the HTTP
error is returned safely and consistently—locate the block using errMsg,
xSemaphoreGive, and _server.send and change it to use sendError() (or a
JSON-encoding routine) while keeping the existing log of the full Spoolman
response for debugging.

return;
}
Comment on lines +394 to +420
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistent weight handling between inline creation and helper function.

The inline spool creation path sets remaining_weight directly (line 399), while enrichCreateSpool() (lines 1954-1959) and enrichUpdateSpool() (lines 1931-1937) convert remaining weight to used_weight with an explicit initial_weight. Spoolman computes remaining_weight = initial_weight - used_weight, so setting remaining_weight directly may behave differently or be less reliable.

Consider reusing enrichCreateSpool() here for consistency:

Proposed refactor to use enrichCreateSpool() 
     } else {
         // No existing spool — create new
-        StaticJsonDocument<512> spoolBody;
-        spoolBody["filament_id"] = filamentId;
-        if (initialWeight > 0) spoolBody["initial_weight"] = initialWeight;
-        if (remainingWeight > 0) spoolBody["remaining_weight"] = remainingWeight;
-
-        JsonObject extra = spoolBody.createNestedObject("extra");
-        extra["nfc_id"] = quotedUid;
-
-        String spoolJson;
-        serializeJson(spoolBody, spoolJson);
-        Serial.printf("register-uid: creating spool: %s\n", spoolJson.c_str());
-
-        snprintf(url, sizeof(url), "%s/api/v1/spool", baseUrl);
-        http.begin(client, url);
-        http.addHeader("Content-Type", "application/json");
-        code = http.POST(spoolJson);
-        if (code != 200 && code != 201) {
-            response = http.getString();
-            http.end();
-            Serial.printf("register-uid: spool creation failed HTTP %d: %s\n", code, response.c_str());
-            String errMsg = "Failed to create spool (HTTP " + String(code) + "): " + response;
-            xSemaphoreGive(g_httpMutex);
-            _server.send(500, "application/json", "{\"error\":\"" + errMsg + "\"}");
-            return;
-        }
-        response = http.getString();
-        http.end();
-
-        StaticJsonDocument<1024> spoolDoc;
-        if (!deserializeJson(spoolDoc, response)) {
-            spoolId = spoolDoc["id"] | -1;
-        }
+        Serial.printf("register-uid: creating spool for uid=%s\n", uid);
+        spoolId = enrichCreateSpool(client, http, baseUrl, filamentId, remainingWeight, quotedUid);
+        if (spoolId < 0) {
+            xSemaphoreGive(g_httpMutex);
+            sendError(500, "Failed to create spool");
+            return;
+        }
     }

Note: enrichCreateSpool() uses a hardcoded 1000g initial weight when remainingG > 0. If preserving the user-provided initialWeight is important, you may need to extend enrichCreateSpool() to accept an optional initial weight parameter.

🧰 Tools
🪛 Clang (14.0.6)

[note] 394-394: +1, nesting level increased to 1

(clang)

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

(clang)

[note] 412-412: +1

(clang)

[warning] 396-396: variable 'spoolBody' is not initialized

(cppcoreguidelines-init-variables)

[warning] 401-401: variable 'extra' is not initialized

(cppcoreguidelines-init-variables)

[warning] 404-404: variable 'spoolJson' is not initialized

(cppcoreguidelines-init-variables)

[warning] 416-416: variable 'errMsg' 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/WebServerManager.cpp` around lines 394 - 420, The inline spool creation
in the register-uid path sets remaining_weight directly which is inconsistent
with enrichCreateSpool()/enrichUpdateSpool() that derive used_weight and set an
initial_weight (avoiding direct remaining_weight); update the register-uid flow
to call enrichCreateSpool() (or extend enrichCreateSpool() to accept an optional
initialWeight and pass the user-provided initialWeight) so the created JSON uses
initial_weight and used_weight semantics (matching spoolman expectations)
instead of setting remaining_weight directly; ensure you remove the direct
remaining_weight write in the inline block and preserve quotedUid in the
extra.nfc_id field when delegating to enrichCreateSpool().

response = http.getString();
http.end();
Serial.printf("register-uid: spool creation failed HTTP %d: %s\n", code, response.c_str());
String errMsg = "Failed to create spool (HTTP " + String(code) + "): " + response;
xSemaphoreGive(g_httpMutex);
_server.send(500, "application/json", "{\"error\":\"" + errMsg + "\"}");
return;
}
response = http.getString();
http.end();

int spoolId = -1;
{
StaticJsonDocument<1024> spoolDoc;
if (!deserializeJson(spoolDoc, response)) {
spoolId = spoolDoc["id"] | -1;
Expand Down