Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
# Changelog

## [Unreleased]

### Changed

- **Atomic OpenPrintTag write** — /api/write-tag now builds a fresh CBOR tag and writes all fields in a single NFC pass (~5s vs ~25s). Eliminates sequential write drops, scan loop race conditions, and CBOR re-encoding overflow. Existing field values are preserved when not specified.

### Fixed

- Buffer overflow in mifareBlockWrite16 — cmd[1] wrote past 1-byte array (undefined behavior)
- Mutex-less tag_data reads — sendSpoolUpdatedMessage and processWriteQueue now hold tagMutex when reading currentSpool
- ESP.restart() during NFC write — scan task is now paused before restart to prevent tag corruption
- NFCScanTask stack bumped 6144→8192 bytes
Comment thread
coderabbitai[bot] marked this conversation as resolved.

---

## [1.5.5] - 2026-03-26

### Added
Expand Down
2 changes: 1 addition & 1 deletion lib/PN5180/PN5180ISO14443.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ bool PN5180ISO14443::mifareBlockRead(uint8_t blockno, uint8_t *buffer) {


uint8_t PN5180ISO14443::mifareBlockWrite16(uint8_t blockno, uint8_t *buffer) {
uint8_t cmd[1];
uint8_t cmd[2];
// Clear RX CRC
writeRegisterWithAndMask(CRC_RX_CONFIG, 0xFFFFFFFE);

Expand Down
157 changes: 150 additions & 7 deletions src/NFCManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void NFCManager::startScanTask() {
xTaskCreatePinnedToCore(
scanTaskFunc,
"NFCScanTask",
6144,
8192,
this,
1,
&scanTaskHandle,
Expand Down Expand Up @@ -1315,9 +1315,11 @@ void NFCManager::processWriteQueue() {
suppressReDetectionUid_[0] = '\0';
batchHadSuppressSync_ = false;

// Send SpoolDetected to notify listeners (tests, MQTT, LCD) of final state.
// The suppress_spoolman_sync flag prevents Spoolman sync for write_spoolman_spool batches.
sendSpoolDetectedMessage(hadSuppressSync);
// Send SpoolDetected under mutex — sendSpoolDetectedMessage reads currentSpool.tag_data
if (xSemaphoreTake(tagMutex, pdMS_TO_TICKS(100)) == pdTRUE) {
sendSpoolDetectedMessage(hadSuppressSync);
xSemaphoreGive(tagMutex);
}
}
}
}
Expand Down Expand Up @@ -1630,6 +1632,144 @@ bool NFCManager::executeWrite(const NFCWriteRequest& request) {
return ok;
}

// Handle WRITE_ATOMIC — build complete CBOR map from sidecar fields, write once
if (request.type == NFCWriteType::WRITE_ATOMIC) {
if (!atomicWriteFields_.pending) {
Serial.println("NFCManager: WRITE_ATOMIC - no atomic fields pending");
return false;
}

// Copy sidecar by value and clear pending immediately — prevents a
// concurrent HTTP call from overwriting fields mid-write
AtomicWriteFields f = atomicWriteFields_;
atomicWriteFields_.pending = false;

// Validate and snapshot under mutex
if (xSemaphoreTake(tagMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
Serial.println("NFCManager: WRITE_ATOMIC - could not acquire tagMutex");
return false;
}
if (!currentSpool.tag_data_valid) {
xSemaphoreGive(tagMutex);
return false;
}
if (request.expected_spool_id[0] != '\0' &&
strcmp(currentSpool.spool_id, request.expected_spool_id) != 0) {
Serial.printf("NFCManager: WRITE_ATOMIC rejected: expected %s but found %s\n",
request.expected_spool_id, currentSpool.spool_id);
xSemaphoreGive(tagMutex);
return false;
}
// Read defaults from the existing tag for any fields not provided
uint8_t cur_mat_type = 0; opt_get_material_type(&currentSpool.tag_data, &cur_mat_type);
uint8_t cur_color[4] = {255,255,255,255}; opt_get_primary_color(&currentSpool.tag_data, cur_color);
float cur_full_wt = 1000.0f; opt_get_actual_full_weight(&currentSpool.tag_data, &cur_full_wt);
float cur_consumed = 0.0f; opt_get_consumed_weight(&currentSpool.tag_data, &cur_consumed);
char cur_brand[33] = {0}; opt_get_brand_name(&currentSpool.tag_data, cur_brand, sizeof(cur_brand));
float cur_density = 0.0f; opt_get_density(&currentSpool.tag_data, &cur_density);
float cur_diameter = 0.0f; opt_get_filament_diameter(&currentSpool.tag_data, &cur_diameter);
char cur_mat_name[33] = {0}; opt_get_material_name(&currentSpool.tag_data, cur_mat_name, sizeof(cur_mat_name));
int32_t cur_sm_id = -1; opt_get_gp_spoolman_id(&currentSpool.tag_data, &cur_sm_id);
int16_t cur_min_pt = 0, cur_max_pt = 0, cur_pre_t = 0, cur_min_bt = 0, cur_max_bt = 0;
opt_get_min_print_temp(&currentSpool.tag_data, &cur_min_pt);
opt_get_max_print_temp(&currentSpool.tag_data, &cur_max_pt);
opt_get_preheat_temp(&currentSpool.tag_data, &cur_pre_t);
opt_get_min_bed_temp(&currentSpool.tag_data, &cur_min_bt);
opt_get_max_bed_temp(&currentSpool.tag_data, &cur_max_bt);
xSemaphoreGive(tagMutex);

// Build a FRESH tag from scratch — avoids CBOR re-encoding overflow
// that happens when updating fields in an existing map
opt_init(&writeScratchTag_);
opt_error_t err = opt_format_empty_tag(&writeScratchTag_, 312, 32);
if (err != OPT_OK) {
Serial.printf("NFCManager: WRITE_ATOMIC format failed: %s\n", opt_error_str(err));
atomicWriteFields_.pending = false;
return false;
}

// Set material class (required by spec)
opt_set_material_class(&writeScratchTag_, OPT_MATERIAL_CLASS_FFF);

// Apply each field: use provided value if has_*, else use existing tag default
#define ATOMIC_SET(name, call) do { \
err = (call); \
if (err != OPT_OK) { \
Serial.printf("NFCManager: WRITE_ATOMIC " name " failed: %s\n", opt_error_str(err)); \
} \
} while(0)

ATOMIC_SET("material_type", opt_set_material_type(&writeScratchTag_, f.has_material_type ? f.material_type : cur_mat_type));
ATOMIC_SET("color", opt_set_primary_color(&writeScratchTag_, f.has_color ? f.color : cur_color));
ATOMIC_SET("initial_weight", opt_set_actual_full_weight(&writeScratchTag_, f.has_initial_weight ? f.initial_weight_g : cur_full_wt));
ATOMIC_SET("consumed_weight",opt_set_consumed_weight(&writeScratchTag_, f.has_consumed_weight ? f.consumed_weight : cur_consumed));
if (f.has_brand_name || cur_brand[0] != '\0')
ATOMIC_SET("brand_name", opt_set_brand_name(&writeScratchTag_, f.has_brand_name ? f.brand_name : cur_brand));
if (f.has_density || cur_density > 0.0f)
ATOMIC_SET("density", opt_set_density(&writeScratchTag_, f.has_density ? f.density : cur_density));
if (f.has_diameter || cur_diameter > 0.0f)
ATOMIC_SET("diameter", opt_set_filament_diameter(&writeScratchTag_, f.has_diameter ? f.diameter_mm : cur_diameter));
if (f.has_material_name || cur_mat_name[0] != '\0')
ATOMIC_SET("material_name", opt_set_material_name(&writeScratchTag_, f.has_material_name ? f.material_name : cur_mat_name));
if (f.has_min_print_temp || cur_min_pt != 0)
ATOMIC_SET("min_print_temp", opt_set_min_print_temp(&writeScratchTag_, f.has_min_print_temp ? f.min_print_temp : cur_min_pt));
if (f.has_max_print_temp || cur_max_pt != 0)
ATOMIC_SET("max_print_temp", opt_set_max_print_temp(&writeScratchTag_, f.has_max_print_temp ? f.max_print_temp : cur_max_pt));
if (f.has_preheat_temp || cur_pre_t != 0)
ATOMIC_SET("preheat_temp", opt_set_preheat_temp(&writeScratchTag_, f.has_preheat_temp ? f.preheat_temp : cur_pre_t));
if (f.has_min_bed_temp || cur_min_bt != 0)
ATOMIC_SET("min_bed_temp", opt_set_min_bed_temp(&writeScratchTag_, f.has_min_bed_temp ? f.min_bed_temp : cur_min_bt));
if (f.has_max_bed_temp || cur_max_bt != 0)
ATOMIC_SET("max_bed_temp", opt_set_max_bed_temp(&writeScratchTag_, f.has_max_bed_temp ? f.max_bed_temp : cur_max_bt));
// Spoolman ID goes in aux region
int32_t sm_id = f.has_spoolman_id ? f.spoolman_id : cur_sm_id;
if (sm_id > 0)
ATOMIC_SET("spoolman_id", opt_set_gp_spoolman_id(&writeScratchTag_, sm_id));
#undef ATOMIC_SET

// Single-pass NFC write — no sequential drops, no scan loop race
Serial.println("NFCManager: WRITE_ATOMIC writing to NFC...");
opt_nfc_hal_t* hal = connection_->getHal();
err = opt_write_to_nfc(&writeScratchTag_, hal);
if (err != OPT_OK) {
Serial.printf("NFCManager: WRITE_ATOMIC NFC write failed: %s\n", opt_error_str(err));
return false;
}

// Verify with retries — re-read tag to confirm write succeeded
bool verified = false;
const int maxRetries = 3;
for (int retry = 0; retry < maxRetries; retry++) {
vTaskDelay(pdMS_TO_TICKS(50 * (retry + 1)));
err = opt_read_from_nfc(&writeScratchTag_, hal, 0, 78);
if (err == OPT_OK) {
err = opt_parse_ndef(&writeScratchTag_);
if (err == OPT_OK) { verified = true; break; }
}
Serial.printf("NFCManager: WRITE_ATOMIC verify retry %d: %s\n", retry + 1, opt_error_str(err));
}
if (!verified) {
Serial.println("NFCManager: WRITE_ATOMIC verification failed — not committing");
return false;
}

// Commit verified data to shared state under mutex
if (xSemaphoreTake(tagMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
Serial.println("NFCManager: WRITE_ATOMIC succeeded but commit lock failed");
return false;
}
currentSpool.tag_data = writeScratchTag_;
currentSpool.tag_data_valid = true;
currentSpool.blank_tag_present = false;
currentSpool.kind = TagKind::OpenPrintTag;
addToRecentSpools();
sendSpoolDetectedMessage();
xSemaphoreGive(tagMutex);

Serial.println("NFCManager: WRITE_ATOMIC complete");
return true;
}

// For non-FORMAT writes: take mutex to validate + modify in-memory data, then release for NFC I/O
if (xSemaphoreTake(tagMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
Serial.println("NFCManager: executeWrite - could not acquire tagMutex");
Expand Down Expand Up @@ -1795,10 +1935,13 @@ void NFCManager::sendSpoolUpdatedMessage(uint32_t request_id, NFCWriteType type,
msg.payload.spoolUpdated.success = success;
msg.payload.spoolUpdated.suppress_sync = 0; // Default: allow sync

// Get remaining weight from tag data
// Get remaining weight from tag data under mutex to prevent torn reads
float remaining_grams = 0;
if (currentSpool.tag_data_valid) {
opt_get_remaining_weight(&currentSpool.tag_data, &remaining_grams);
if (xSemaphoreTake(tagMutex, pdMS_TO_TICKS(50)) == pdTRUE) {
if (currentSpool.tag_data_valid) {
opt_get_remaining_weight(&currentSpool.tag_data, &remaining_grams);
}
xSemaphoreGive(tagMutex);
}
msg.payload.spoolUpdated.kg_remaining = remaining_grams / 1000.0f;

Expand Down
36 changes: 36 additions & 0 deletions src/NFCManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,48 @@
#include "TigerTagParser.h"
#include "opentag3d_lib.h"

// Sidecar for WRITE_ATOMIC: filled by HTTP handler, consumed by scan task.
// All fields are optional — only those with has_* = true are applied.
struct AtomicWriteFields {
bool has_material_type = false;
uint8_t material_type = 0;
bool has_color = false;
uint8_t color[4] = {0};
bool has_initial_weight = false;
float initial_weight_g = 0;
bool has_consumed_weight = false;
float consumed_weight = 0;
bool has_brand_name = false;
char brand_name[33] = {0};
bool has_spoolman_id = false;
int32_t spoolman_id = 0;
bool has_density = false;
float density = 0;
bool has_diameter = false;
float diameter_mm = 0;
bool has_material_name = false;
char material_name[33] = {0};
bool has_min_print_temp = false;
int16_t min_print_temp = 0;
bool has_max_print_temp = false;
int16_t max_print_temp = 0;
bool has_preheat_temp = false;
int16_t preheat_temp = 0;
bool has_min_bed_temp = false;
int16_t min_bed_temp = 0;
bool has_max_bed_temp = false;
int16_t max_bed_temp = 0;
volatile bool pending = false;
};

class NFCManager {
public:
static NFCManager& getInstance();
bool begin(); // Init hardware + queues
void startScanTask(); // Start FreeRTOS scan task
bool enqueueWrite(const NFCWriteRequest& req); // Queue a write request
bool enqueueRawWrite(const NFCWriteRequest& req, const uint8_t* data, size_t dataSize);
void setAtomicWriteFields(const AtomicWriteFields& fields) { atomicWriteFields_ = fields; }
bool writeSpoolmanDataToTag(int32_t spoolman_id, const char* expected_spool_id = nullptr);
bool isRequestCompleted(uint32_t request_id); // Check if request done
void requestCurrentSpool(); // Clear dedup to resend current spool
Expand Down Expand Up @@ -107,6 +142,7 @@ class NFCManager {
bool rawWritePending_ = false;
bool writeRawTag();
opt_tag_t writeScratchTag_; // Reused by scan task write path to avoid large stack frames
AtomicWriteFields atomicWriteFields_; // Sidecar for WRITE_ATOMIC (filled by HTTP, consumed by scan task)

// Write queue (FreeRTOS)
QueueHandle_t writeQueue = nullptr;
Expand Down
1 change: 1 addition & 0 deletions src/NFCWriteTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ enum class NFCWriteType : uint8_t {
SET_MAX_BED_TEMP, // Set maximum bed temperature (°C)
WRITE_TIGERTAG, // Write 40-byte TigerTag binary to NTAG pages 4-13
WRITE_OPENTAG3D, // Write NDEF-wrapped OpenTag3D payload to NTAG pages
WRITE_ATOMIC, // Atomic single-pass write: build complete CBOR map, write once
};

struct NFCWriteRequest {
Expand Down
Loading