From 73ceeb493c7eeeb4333990ad1bb22ae0d9d9ab1f Mon Sep 17 00:00:00 2001
From: Teo-CD <teocd.perso@outlook.fr>
Date: Sat, 6 Jan 2024 00:15:45 +0000
Subject: [PATCH] RFID: Clean up and generalize

Add a constant for the ID block used and replace it.

Clean up some comments and clarify expectations.

readBlock() did not properly halt tags after errors and the else
clause was superfluous. Properly handle error status.

Move currentActiveTags to be static and initialize it.
This allows for keeping track of max/current tags accross multiple instances.
---
 include/RFID.h |  7 +++++--
 src/RFID.cpp   | 18 ++++++++++--------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/RFID.h b/include/RFID.h
index ef5b02b..c134fa4 100644
--- a/include/RFID.h
+++ b/include/RFID.h
@@ -30,6 +30,8 @@ struct uidNode {
 
 class RFID {
 public:
+	static constexpr int maxTags = 2;
+
 	/***
 	 * Use the main SPI port and NFC chip select by default, but allow choosing
 	 * an alternate chip select and/or SPI bus if needed to allow multiple
@@ -49,10 +51,10 @@ public:
 	 */
 	int8_t checkTags();
 private:
+	static const byte tagIdBlock = 0x11;
 	/* Used for "encrypted" communication with the tags. */
 	static MFRC522Constants::MIFARE_Key defaultKey;
-	static constexpr int maxTags = 2;
-	int currentActiveTags = 0;
+	static int currentActiveTags;
 
 	/*
 	 * The linked list tracking active tags.
@@ -90,6 +92,7 @@ private:
 	/**
 	 * Read block at blockADdr from tagUid and return if the read succeeded or not.
 	 * The read block will be in comData.
+	 * The tag needs to be already woken up.
 	 * @param tagUid Uid of the tag to be read.
 	 * @param blockAddr Address of the block to be read.
 	 * @return True if read succeeded, false otherwise.
diff --git a/src/RFID.cpp b/src/RFID.cpp
index 22c69fc..fd40870 100644
--- a/src/RFID.cpp
+++ b/src/RFID.cpp
@@ -5,6 +5,7 @@
 #include "RFID.h"
 
 MFRC522Constants::MIFARE_Key RFID::defaultKey = {0xFF,0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
+int RFID::currentActiveTags = 0;
 
 void RFID::init() {
 	// Create and set up a static linked list to manage active tags.
@@ -58,11 +59,11 @@ int8_t RFID::checkTags() {
 		return 0;
 	}
 	// If the read fails, we cannot use this tag anyway so remove it from the list.
-	if (!readBlock(newTag->uid, 0x11) ) {
+	if (!readBlock(newTag->uid, tagIdBlock) ) {
 		removeActiveTag(newTag, nullptr);
 		return 0;
 	}
-	// TODO: ID check ?
+	// Don' t check the ID, otherwise we cannot program the tag.
 	newTag->tag_ID = comData[0];
 	return newTag->tag_ID;
 }
@@ -101,16 +102,17 @@ bool RFID::readBlock(MFRC522Constants::Uid &uidToRead, byte blockAddr) {
 	if (mfrc.PCD_Authenticate(
 			MFRC522Constants::PICC_Command::PICC_CMD_MF_AUTH_KEY_A,
 			blockAddr, (&defaultKey),&uidToRead)!= MFRC522Constants::STATUS_OK) {
-		Serial.println("Failed to authenticate");
+		Serial.println("readBlock: Failed to authenticate");
+		mfrc.PICC_HaltA();
 		return false;
-	} else {
-		byte size = sizeof(comData);
-		mfrc.MIFARE_Read(blockAddr, comData, &size);
-//		Serial.printf("Read block : 0x%lX\n", *(uint32_t *)comData);
 	}
+
+	byte size = sizeof(comData);
+	MFRC522::StatusCode status = mfrc.MIFARE_Read(blockAddr, comData, &size);
+//	Serial.printf("Read block : 0x%lX\n", *(uint32_t *)comData);
 	// Needed otherwise no new communications can happen.
 	mfrc.PCD_StopCrypto1();
 	// No need to keep the tag active.
 	mfrc.PICC_HaltA();
-	return true;
+	return status == MFRC522Constants::STATUS_OK;
 }