From 9c19fa2ebb9be882fad90336eb07f9819cdf2af9 Mon Sep 17 00:00:00 2001
From: Matthew Newton <matthew-git@newtoncomputing.co.uk>
Date: Sat, 9 Mar 2024 23:43:15 +0000
Subject: [PATCH 1/2] rework if tests in genericRead to make the logic simpler
 to read

---
 src/rfid.esp | 54 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/src/rfid.esp b/src/rfid.esp
index c6b40456..91957726 100644
--- a/src/rfid.esp
+++ b/src/rfid.esp
@@ -163,8 +163,16 @@ void pn532Read()
 	}
 }
 
+
+/*
+ *  Try first to read from RDM6300 hardware. If that received
+ *  nothing, check the other configured reader.
+ */
 void genericRead()
 {
+	/*
+	 *  Test RDM6300 125khz reader
+	 */
 	while (Serial.available() > 0)
 	{
 		RFIDr.rfidSerial(Serial.read());
@@ -181,42 +189,66 @@ void genericRead()
 #endif
 	}
 
-	if (config.readertype == READER_MFRC522_RDM6300 && uid.length() == 0)
-	{
-		mfrc522Read();
-	}
+	/*
+	 *  If nothing read from the RDM6300, check the other hardware
+	 */
+	if (uid.length() == 0) {
+		if (config.readertype == READER_MFRC522_RDM6300)
+		{
+			mfrc522Read();
+		}
 
-	else if (config.readertype == READER_WIEGAND_RDM6300 && uid.length() == 0)
-	{
-		wiegandRead();
-	}
+		else if (config.readertype == READER_WIEGAND_RDM6300)
+		{
+			wiegandRead();
+		}
 
-	else if (config.readertype == READER_PN532_RDM6300 && uid.length() == 0)
-	{
-		pn532Read();
+		else if (config.readertype == READER_PN532_RDM6300)
+		{
+			pn532Read();
+		}
 	}
 }
 
+
+/*
+ *  Main function to read RFID cards. This function will call the
+ *  correct reader function depending on the configured hardware,
+ *  or otherwise call genericRead to read both RDM6300 and another
+ *  configured reader.
+ */
 void rfidRead()
 {
+	/*
+	 *  Do not try and read if we are already processing a card
+	 */
 	if (rfidState == cardSwiped)
 	{
 		return;
 	}
+
+	/*
+	 *  Call the appropriate function based on the configured
+	 *  hardware
+	 */
 	if (config.readertype == READER_MFRC522)
 	{
 		mfrc522Read();
 	}
+
 	else if (config.readertype == READER_WIEGAND)
 	{
 		wiegandRead();
 	}
+
 	else if (config.readertype == READER_PN532)
 	{
 		pn532Read();
 	}
+
 	else if (config.readertype > READER_PN532)
 	{
+		// This is a combination of RDM6300 and one of the above
 		genericRead();
 	}
 }

From 195d29dfef9d61d99fa645f171adf38a4ab3fa32 Mon Sep 17 00:00:00 2001
From: Matthew Newton <matthew-git@newtoncomputing.co.uk>
Date: Sat, 9 Mar 2024 23:45:31 +0000
Subject: [PATCH 2/2] add some comments

helps us mere mortals can understand what's happening :)

also quote username in debug output for clarity
---
 src/rfid.esp | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/rfid.esp b/src/rfid.esp
index 91957726..e6851865 100644
--- a/src/rfid.esp
+++ b/src/rfid.esp
@@ -253,6 +253,10 @@ void rfidRead()
 	}
 }
 
+
+/*
+ *  Try and read a PIN code from Wiegand hardware
+ */
 void pinCodeRead()
 {
 	if (config.readertype != READER_WIEGAND ||
@@ -320,6 +324,11 @@ int weekdayFromMonday(int weekdayFromSunday) {
 	return ( weekdayFromSunday + 5 ) % 7;
 }
 
+
+/*
+ *  If we have successfully read an RFID card, check if access
+ *  should be granted
+ */
 void rfidProcess()
 {
 	if (rfidState == waitingRfid ||
@@ -328,12 +337,14 @@ void rfidProcess()
 		return;
 	}
 
+	/* Each user has a file named after the RFID UID */
 	File f = SPIFFS.open("/P/" + uid, "r");
 
 	/*
 	 *  If the file was not found then this is an unknown user, so no more
-	 *  processing to be done. However, we do a secondary check here to see
-	 *  if an old esp-rfid v1 uid exists and if so use that.
+	 *  processing to be done. However, for backwards compatibility we do a
+	 *  secondary check here to see if an old esp-rfid v1 uid exists and if
+	 *  so use that.
 	 */
 	if (!f)
 	{
@@ -352,6 +363,9 @@ void rfidProcess()
 #endif
 	}
 
+	/*
+	 *  Read the user's settings
+	 */
 	size_t size = f.size();
 	std::unique_ptr<char[]> buf(new char[size]);
 	f.readBytes(buf.get(), size);
@@ -359,6 +373,9 @@ void rfidProcess()
 	DynamicJsonDocument json(512);
 	auto error = deserializeJson(json, buf.get(), size);
 
+	/*
+	 *  Corrupt user data file, so return invalid user
+	 */
 	if (error)
 	{
 		processingState = notValid;
@@ -380,24 +397,32 @@ void rfidProcess()
 		return;
 	}
 
+	/*
+	 *  Get account type (for FIRST relay only) and username from user's data
+	 */
 	accountType = json["acctype"];
 	username = json["user"].as<String>();
 
 #ifdef DEBUG
 	Serial.println(" = known PICC");
-	Serial.print("[ INFO ] User Name: ");
+	Serial.print("[ INFO ] User Name: '");
 	if (username == "undefined")
 		Serial.print(uid);
 	else
 		Serial.print(username);
+	Serial.print("'");
 #endif
 
 	if (accountType == ACCESS_GRANTED)
 	{
+		/*
+		 *  Normal user - relay but no admin access
+		 */
 		unsigned long validSinceL = json["validsince"];
 		unsigned long validUntilL = json["validuntil"];
 		unsigned long nowL = epoch;
 		int hourTz = timeinfo.tm_hour;
+
 		if (validUntilL < nowL || validSinceL > nowL)
 		{
 			processingState = expired;
@@ -410,9 +435,15 @@ void rfidProcess()
 		}
 	} else if (accountType == ACCESS_ADMIN)
 	{
+		/*
+		 *  Admin user - enable relay (with no time limits) and wifi
+		 */
 		doEnableWifi = true;
 		processingState = validAdmin;
 	} else {
+		/*
+		 *  User exists but does not have access
+		 */
 		processingState = notValid;
 	}