Skip to content
Closed
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
19 changes: 19 additions & 0 deletions src/HardwareNFCConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,22 @@ void HardwareNFCConnection::logDiagnostics() {
Serial.println("HardwareNFC DIAG: --- end test ---");
#endif
}

bool HardwareNFCConnection::ntagGetVersion(uint8_t* versionOut) {
if (!iso14443a_ || !versionOut) return false;

uint8_t cmd = 0x60;
if (!iso14443a_->sendData(&cmd, 1, 0x00)) return false;

uint32_t rxStatus;
uint16_t rxLen = 0;
for (int i = 0; i < 10; i++) {
delay(1);
iso14443a_->readRegister(RX_STATUS, &rxStatus);
rxLen = rxStatus & 0x000001ff;
if (rxLen >= 8) break;
}
if (rxLen < 8) return false;

return iso14443a_->readData(8, versionOut) != nullptr;
}
1 change: 1 addition & 0 deletions src/HardwareNFCConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class HardwareNFCConnection : public NFCConnectionI {
bool writeISO14443Pages(uint8_t startPage, uint8_t pageCount, const uint8_t* data, uint16_t dataLen) override;
uint8_t getLastSAK() const override { return lastSAK_; }
uint16_t getLastATQA() const override { return lastATQA_; }
bool ntagGetVersion(uint8_t* versionOut) override;
void getReaderInfo(char* buf, size_t len) const override;
// Diagnostics: log RF_STATUS, IRQ_STATUS, SYSTEM_STATUS registers
void logDiagnostics() override;
Expand Down
14 changes: 14 additions & 0 deletions src/HardwareNFCConnectionPN532.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,17 @@ void HardwareNFCConnectionPN532::logDiagnostics() {
Serial.println("PN532: No response during diagnostics");
}
}

bool HardwareNFCConnectionPN532::ntagGetVersion(uint8_t* versionOut) {
if (!pn532_ || !ready_ || !versionOut) return false;

uint8_t cmd = 0x60;
uint8_t response[8];
uint8_t responseLength = sizeof(response);

if (!pn532_->inDataExchange(&cmd, 1, response, &responseLength)) return false;
if (responseLength < 8) return false;

memcpy(versionOut, response, 8);
return true;
}
1 change: 1 addition & 0 deletions src/HardwareNFCConnectionPN532.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class HardwareNFCConnectionPN532 : public NFCConnectionI {
bool detectTag(uint8_t* uid, uint8_t* uidLength) override;
uint8_t getLastSAK() const override { return lastSAK_; }
uint16_t getLastATQA() const override { return lastATQA_; }
bool ntagGetVersion(uint8_t* versionOut) override;
void setCurrentUid(const uint8_t* uid, uint8_t length) override;
opt_nfc_hal_t* getHal() override;
uint16_t readISO14443Pages(uint8_t startPage, uint8_t pageCount, uint8_t* buffer, uint16_t bufferSize) override;
Expand Down
4 changes: 4 additions & 0 deletions src/NFCConnectionI.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ class NFCConnectionI {
virtual uint8_t getLastSAK() const { return 0; }
virtual uint16_t getLastATQA() const { return 0; }

// NTAG GET_VERSION (0x60) — returns 8-byte version info for NTAG/Ultralight EV1.
// versionOut must point to an 8-byte buffer. Returns true on success.
virtual bool ntagGetVersion(uint8_t* versionOut) { return false; }

// Set current UID for addressed read/write commands
virtual void setCurrentUid(const uint8_t* uid, uint8_t length) = 0;

Expand Down
37 changes: 35 additions & 2 deletions src/NFCManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ void NFCManager::scanLoop() {
currentSpool.present = true;
currentSpool.blank_tag_present = false;
currentSpool.kind = TagKind::BambuTag;
currentSpool.variant = NtagVariant::Unknown;
currentSpool.tag_data_valid = false;
lastTigerTagValid_ = false;
memcpy(lastSeenUid, uid, uidLength);
Expand Down Expand Up @@ -486,6 +487,7 @@ void NFCManager::scanLoop() {
currentSpool.uid_length = uidLength;
currentSpool.present = true;
currentSpool.blank_tag_present = false;
currentSpool.variant = scan.variant;
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 | 🟠 Major

Reset variant on all non-NTAG state transitions to avoid stale API data.

currentSpool.variant is assigned for Bambu and generic ISO14443A paths, but it is not consistently cleared in ISO15693/blank/remove flows. That can leave stale ntag_variant / ntag_pages in status after tag type changes.

Suggested fix
@@ bool NFCManager::readAndParseTag(uint8_t* uid, uint8_t uid_length) {
     currentSpool.present = true;
     currentSpool.tag_data_valid = true;
+    currentSpool.variant = NtagVariant::Unknown;
@@ if (!readOk) {
         currentSpool.blank_tag_present = true;
         currentSpool.kind = TagKind::BlankTag;
+        currentSpool.variant = NtagVariant::Unknown;
@@ else { // No tag detected - clear state
                 currentSpool.present = false;
                 currentSpool.blank_tag_present = false;
+                currentSpool.variant = NtagVariant::Unknown;
@@ bool NFCManager::scanOnce() {
     currentSpool.blank_tag_present = true;
     currentSpool.kind = TagKind::BlankTag;
+    currentSpool.variant = NtagVariant::Unknown;
@@ else { // scanOnce no tag
             currentSpool.present = false;
             currentSpool.blank_tag_present = false;
+            currentSpool.variant = NtagVariant::Unknown;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NFCManager.cpp` at line 490, currentSpool.variant is only set for Bambu
and ISO14443A paths but never cleared on ISO15693/blank/remove or other non-NTAG
transitions, leaving stale ntag_variant/ntag_pages in status; update the state
transition code for ISO15693 handling and the blank/remove flows (where
currentSpool.variant is not overwritten) to explicitly clear/reset
currentSpool.variant (and related NTAG fields if present) whenever the tag type
is not NTAG so the status can't retain stale values—look for assignments to
currentSpool.variant in the Bambu/ISO14443A code paths and add matching reset
logic in the ISO15693, blank, and remove handlers in NFCManager.cpp.

memcpy(lastSeenUid, uid, uidLength);
lastSeenUidLength = uidLength;
lastSeenValid = true;
Expand Down Expand Up @@ -1214,10 +1216,22 @@ void NFCManager::sendOpenSpoolMessage(const char* uid, const OpenSpoolData& os)
ApplicationManager::getInstance().sendMessage(msg);
}

static NtagVariant mapStorageByte(uint8_t storage) {
switch (storage) {
case 0x0F: return NtagVariant::NTAG213;
case 0x11: return NtagVariant::NTAG215;
case 0x13: return NtagVariant::NTAG216;
case 0x06: return NtagVariant::UltralightEV1_48;
case 0x09: return NtagVariant::UltralightEV1_128;
default: return NtagVariant::Unknown;
}
}

TagScanResult NFCManager::classifyTag(const uint8_t* uid, uint8_t uid_length) {
TagScanResult result;
result.present = true;
result.tag_data_valid = false;
result.variant = NtagVariant::Unknown;
// Protocol inferred from UID length: ISO15693 always 8 bytes, ISO14443A always 4 or 7.
if (uid_length == 8) {
result.protocol = TagProtocol::ISO15693;
Expand All @@ -1228,12 +1242,17 @@ TagScanResult NFCManager::classifyTag(const uint8_t* uid, uint8_t uid_length) {
// Check SAK to distinguish NTAG/Ultralight from MIFARE Classic
uint8_t sak = connection_->getLastSAK();
if (sak == 0x08 || sak == 0x18) {
// SAK 0x08 = MIFARE Classic 1K, 0x18 = MIFARE Classic 4K
// Likely a Bambu Lab spool tag
result.kind = TagKind::BambuTag;
Serial.printf("NFCManager: MIFARE Classic detected (SAK=0x%02X) — treating as Bambu tag\n", sak);
} else {
result.kind = TagKind::GenericUidTag;
// GET_VERSION to identify exact NTAG model
uint8_t version[8];
if (connection_->ntagGetVersion(version)) {
result.variant = mapStorageByte(version[6]);
Serial.printf("NFCManager: %s detected (%d pages)\n",
ntagVariantName(result.variant), ntagUsablePages(result.variant));
}
}
}
uint8_t len = uid_length < 8 ? uid_length : 8;
Expand Down Expand Up @@ -1589,6 +1608,17 @@ static opt_error_t applyWriteUpdate(opt_tag_t& tag, const NFCWriteRequest& reque
}
}

bool NFCManager::checkWriteCapacity(uint8_t startPage, uint8_t pageCount, const char* writeType) {
uint16_t maxPages = ntagUsablePages(currentSpool.variant);
if (maxPages == 0) return true; // Unknown variant — skip check
if (startPage + pageCount > maxPages) {
Serial.printf("NFCManager: %s rejected — needs %d pages (start=%d), tag has %d (%s)\n",
writeType, pageCount, startPage, maxPages, ntagVariantName(currentSpool.variant));
Comment on lines +1611 to +1616
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 | 🟠 Major

Capacity boundary check uses the wrong frame of reference.

startPage + pageCount > maxPages mixes absolute page index with usable-page count. If ntagUsablePages() returns count from page 4 (e.g., 45 on NTAG213), valid writes near boundary are incorrectly rejected.

Suggested fix
 bool NFCManager::checkWriteCapacity(uint8_t startPage, uint8_t pageCount, const char* writeType) {
-    uint16_t maxPages = ntagUsablePages(currentSpool.variant);
-    if (maxPages == 0) return true; // Unknown variant — skip check
-    if (startPage + pageCount > maxPages) {
+    const uint16_t usablePages = ntagUsablePages(currentSpool.variant);
+    if (usablePages == 0) {
+        return true; // Unknown variant — skip check
+    }
+
+    constexpr uint16_t kFirstUsablePage = 4;
+    const uint16_t writeEndExclusive = static_cast<uint16_t>(startPage) + pageCount;
+    const uint16_t usableEndExclusive = kFirstUsablePage + usablePages;
+    if (writeEndExclusive > usableEndExclusive) {
         Serial.printf("NFCManager: %s rejected — needs %d pages (start=%d), tag has %d (%s)\n",
-            writeType, pageCount, startPage, maxPages, ntagVariantName(currentSpool.variant));
+            writeType, pageCount, startPage, usablePages, ntagVariantName(currentSpool.variant));
         return false;
     }
     return true;
 }
📝 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
bool NFCManager::checkWriteCapacity(uint8_t startPage, uint8_t pageCount, const char* writeType) {
uint16_t maxPages = ntagUsablePages(currentSpool.variant);
if (maxPages == 0) return true; // Unknown variant — skip check
if (startPage + pageCount > maxPages) {
Serial.printf("NFCManager: %s rejected — needs %d pages (start=%d), tag has %d (%s)\n",
writeType, pageCount, startPage, maxPages, ntagVariantName(currentSpool.variant));
bool NFCManager::checkWriteCapacity(uint8_t startPage, uint8_t pageCount, const char* writeType) {
const uint16_t usablePages = ntagUsablePages(currentSpool.variant);
if (usablePages == 0) {
return true; // Unknown variant — skip check
}
constexpr uint16_t kFirstUsablePage = 4;
const uint16_t writeEndExclusive = static_cast<uint16_t>(startPage) + pageCount;
const uint16_t usableEndExclusive = kFirstUsablePage + usablePages;
if (writeEndExclusive > usableEndExclusive) {
Serial.printf("NFCManager: %s rejected — needs %d pages (start=%d), tag has %d (%s)\n",
writeType, pageCount, startPage, usablePages, ntagVariantName(currentSpool.variant));
return false;
}
return true;
}
🧰 Tools
🪛 Clang (14.0.6)

[warning] 1611-1611: use a trailing return type for this function

(modernize-use-trailing-return-type)


[warning] 1611-1611: method 'checkWriteCapacity' can be made static

(readability-convert-member-functions-to-static)


[warning] 1611-1611: 2 adjacent parameters of 'checkWriteCapacity' of similar type ('int') are easily swapped by mistake

(bugprone-easily-swappable-parameters)


[note] 1611-1611: the first parameter in the range is 'startPage'

(clang)


[note] 1611-1611: the last parameter in the range is 'pageCount'

(clang)


[warning] 1612-1612: variable 'maxPages' is not initialized

(cppcoreguidelines-init-variables)


[warning] 1613-1613: statement should be inside braces

(readability-braces-around-statements)

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

In `@src/NFCManager.cpp` around lines 1611 - 1616, The capacity check in
NFCManager::checkWriteCapacity incorrectly compares an absolute startPage to a
usable-page count from ntagUsablePages(currentSpool.variant); change the logic
to compute remaining usable pages from the absolute startPage and ensure
pageCount fits into that remainder. Specifically, in checkWriteCapacity use the
usable-pages value as a total and verify startPage is within bounds and then
assert pageCount <= (maxPages - startPage) (or equivalently startPage +
pageCount <= maxPages) so writes near the upper boundary aren’t wrongly
rejected; keep the same Serial.printf message using
ntagVariantName(currentSpool.variant) for context. Ensure checks still handle
maxPages == 0 and invalid startPage values.

return false;
}
return true;
}

bool NFCManager::executeWrite(const NFCWriteRequest& request) {
// Handle FORMAT_NEW — formatNewSpool() manages its own mutex
if (request.type == NFCWriteType::FORMAT_NEW) {
Expand Down Expand Up @@ -1648,6 +1678,7 @@ bool NFCManager::executeWrite(const NFCWriteRequest& request) {
xSemaphoreGive(tagMutex);

// Write 40 bytes = 10 pages starting at page 4
if (!checkWriteCapacity(4, 10, "WRITE_TIGERTAG")) return false;
bool ok = connection_->writeISO14443Pages(4, 10, request.data.tigertag_data, 40);
if (ok) {
Serial.println("NFCManager: WRITE_TIGERTAG succeeded");
Expand Down Expand Up @@ -1758,6 +1789,7 @@ bool NFCManager::executeWrite(const NFCWriteRequest& request) {
// Write to NTAG pages starting at page 4
uint8_t pagesNeeded = (uint8_t)(idx / 4);
Serial.printf("NFCManager: WRITE_OPENTAG3D - writing %u bytes (%u pages), payload=%d\n", idx, pagesNeeded, payloadLen);
if (!checkWriteCapacity(4, pagesNeeded, "WRITE_OPENTAG3D")) return false;
bool ok = connection_->writeISO14443Pages(4, pagesNeeded, ndefBuf, idx);
if (ok) {
Serial.printf("NFCManager: WRITE_OPENTAG3D succeeded (%u bytes, %u pages)\n", idx, pagesNeeded);
Expand Down Expand Up @@ -1844,6 +1876,7 @@ bool NFCManager::executeWrite(const NFCWriteRequest& request) {

uint8_t pagesNeeded = (uint8_t)(idx / 4);
Serial.printf("NFCManager: WRITE_OPENSPOOL - writing %u bytes (%u pages)\n", idx, pagesNeeded);
if (!checkWriteCapacity(4, pagesNeeded, "WRITE_OPENSPOOL")) return false;
bool ok = connection_->writeISO14443Pages(4, pagesNeeded, ndefBuf, idx);
if (ok) {
Serial.printf("NFCManager: WRITE_OPENSPOOL succeeded (%u bytes, %u pages)\n", idx, pagesNeeded);
Expand Down
1 change: 1 addition & 0 deletions src/NFCManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class NFCManager {
void sendTagRemovedMessage();
void processWriteQueue();
bool executeWrite(const NFCWriteRequest& request);
bool checkWriteCapacity(uint8_t startPage, uint8_t pageCount, const char* writeType);
void sendSpoolUpdatedMessage(uint32_t request_id, NFCWriteType type, bool success);

// Deduplication
Expand Down
33 changes: 33 additions & 0 deletions src/NFCTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,37 @@ enum class TagProtocol : uint8_t {
Unknown
};

enum class NtagVariant : uint8_t {
Unknown = 0,
NTAG213, // 45 pages, 144 usable bytes
NTAG215, // 135 pages, 504 usable bytes
NTAG216, // 231 pages, 888 usable bytes
UltralightEV1_48, // 20 pages, 48 usable bytes
UltralightEV1_128 // 41 pages, 128 usable bytes
};

inline uint16_t ntagUsablePages(NtagVariant v) {
switch (v) {
case NtagVariant::NTAG213: return 45;
case NtagVariant::NTAG215: return 135;
case NtagVariant::NTAG216: return 231;
case NtagVariant::UltralightEV1_48: return 20;
case NtagVariant::UltralightEV1_128:return 41;
default: return 0;
}
}

inline const char* ntagVariantName(NtagVariant v) {
switch (v) {
case NtagVariant::NTAG213: return "NTAG213";
case NtagVariant::NTAG215: return "NTAG215";
case NtagVariant::NTAG216: return "NTAG216";
case NtagVariant::UltralightEV1_48: return "Ultralight EV1 48B";
case NtagVariant::UltralightEV1_128:return "Ultralight EV1 128B";
default: return "Unknown";
}
}

enum class TagKind : uint8_t {
OpenPrintTag, // ordinal 0 — memset to zero produces safe default
GenericUidTag, // UID-only tag (e.g. NTAG215) — ISO14443A
Expand All @@ -26,6 +57,7 @@ enum class TagKind : uint8_t {
struct TagScanResult {
TagProtocol protocol;
TagKind kind;
NtagVariant variant;
char uid_hex[17]; // null-terminated UID hex string (up to 8 bytes = 16 hex chars)
bool present;
bool tag_data_valid;
Expand All @@ -35,6 +67,7 @@ struct CurrentSpoolState {
bool present;
bool blank_tag_present; // Deprecated: use kind == TagKind::BlankTag instead
TagKind kind;
NtagVariant variant;
char spool_id[17];
uint8_t uid[8]; // ISO15693 uses 8-byte UID
uint8_t uid_length;
Expand Down
4 changes: 4 additions & 0 deletions src/WebServerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,10 @@ void WebServerManager::handleApiStatus() {
doc["uid"] = state.spool_id;
doc["tag_data_valid"] = state.tag_data_valid;
doc["tag_kind"] = tagKindToString(state.kind);
if (state.variant != NtagVariant::Unknown) {
doc["ntag_variant"] = ntagVariantName(state.variant);
doc["ntag_pages"] = ntagUsablePages(state.variant);
}

if (state.kind == TagKind::TigerTag) {
// TigerTag — include parsed TigerTag data
Expand Down