Unreliable ID reads with RC522 RFID reader

I’m working on a project that reads in the UID from a Mifare type tag, using a reader labeled RFID-RC522 and an Elegoo Uno R3. It works for the most part but I’m seeing some inconsistencies in reading the tags. Sometimes they register and sometimes don’t, at various distances and different startup procedures - haven’t found a pattern so far. I’m doing a few things in the code that I’m unsure about so hoping to get some advice on the simplified code below.

  1. I’m using PCD_SetAntennaGain(rfid.RxGain_max); in setup. It seems to work a little better with this, but it could be my imagination. Am I using this correctly by simply adding this line with nothing else?

  2. I’ve changed around pins from the documentation to:
    #define SS_PIN 8
    #define RST_PIN 10
    I’m using an uno and needed fourth pwm pin for leds. I’ve not read anything that says this should be bad, and when I change them back to the documentation pin numbers 9 and 10 it seems to have the same inconsistencies.

  3. I’m making an array of byte arrays out of ints (I believe the way I’m defining them are ints, I could be wrong) to compare the values to the tag ID. This works, but I’m not confident if this is the best way to do this.

  4. I’ve commented out the following lines from the documentation, because they were stopping the rfid from reading over and over again. I’d like to read the tag every time I return to the start of the loop. Commenting these lines out made this possible, but I’m worried this could be causing a problem.
    // Halt PICC
    // rfid.PICC_HaltA();

// Stop encryption on PCD
// rfid.PCD_StopCrypto1();

This is the tutorial I found with the mfrc library: MFRC522 RFID Reader with Arduino Tutorial | Random Nerd Tutorials
This code is built off of one of the examples in that library, not the one on the page. Mainly the write new UID example, with much of it gutted, as I was only interested in the ID comparing stuff.

#include <SPI.h>
#include <MFRC522.h>

#define SS_PIN 8
#define RST_PIN 10

#define SYMBOL1 5
#define SYMBOL2 9
#define SYMBOL3 6
#define SYMBOL4 3
 
MFRC522 rfid(SS_PIN, RST_PIN); // Instance of the class

MFRC522::MIFARE_Key key; 

byte checkArray[9][4]={
  {117,18,113,28},
  {101,90,64,28},
  {101,42,213,28},
  {101,200,207,28},
  {101,222,168,28},
  {117,18,28,28},
  {117,9,121,28},
  {117,15,56,28},
  {117,15,135,28}
 };

void setup() 
{ 
  pinMode(SYMBOL1, OUTPUT);
  pinMode(SYMBOL2, OUTPUT);
  pinMode(SYMBOL3, OUTPUT);
  pinMode(SYMBOL4, OUTPUT);
  
  Serial.begin(9600);
  SPI.begin(); // Init SPI bus
  rfid.PCD_Init(); // Init MFRC522 

  for (byte i = 0; i < 6; i++) {
    key.keyByte[i] = 0xFF;
  }

  rfid.PCD_SetAntennaGain(rfid.RxGain_max);
}
 
void loop() 
{

  // Reset the loop if no new card present on the sensor/reader. This saves the entire process when idle.
  if ( ! rfid.PICC_IsNewCardPresent())
    return;

  if ( ! rfid.PICC_ReadCardSerial())
    return;

  Serial.print(F("PICC type: "));
  MFRC522::PICC_Type piccType = rfid.PICC_GetType(rfid.uid.sak);
  Serial.println(rfid.PICC_GetTypeName(piccType));

  // Check is the PICC of Classic MIFARE type
  if (piccType != MFRC522::PICC_TYPE_MIFARE_MINI &&  
    piccType != MFRC522::PICC_TYPE_MIFARE_1K &&
    piccType != MFRC522::PICC_TYPE_MIFARE_4K) {
    Serial.println(F("Your tag is not of type MIFARE Classic."));
    return;
  }

  Serial.print(F("In hex: "));
  for (int i = 0; i < 4; i++)
  {
    Serial.print(rfid.uid.uidByte[i]);
    Serial.print(" ");
  }
  Serial.println();

  for (int i = 0; i < 9; i++)
  {

    if (rfid.uid.uidByte[0] == checkArray[i][0] &&
      rfid.uid.uidByte[1] == checkArray[i][1] &&
      rfid.uid.uidByte[2] == checkArray[i][2] &&
      rfid.uid.uidByte[3] == checkArray[i][3])
      {
        Serial.println("Success!");
        flashLights();
        break;
      }
    
  }

  // Halt PICC
  //rfid.PICC_HaltA();

  // Stop encryption on PCD
  //rfid.PCD_StopCrypto1();
  return;

}

void flashLights()
{
  // Some loops for leds, removed to simplify forum post
  delay(20000); // Represents about how long the leds do stuff

  return;
}
  for (int i = 0; i < 4; i++)

Are you sure you don’t need a long to count to 4?

  1. I’m making an array of byte arrays out of ints (I believe the way I’m defining them are ints, I could be wrong)

Values aren’t ints. Your byte array is fine.

The code you started from sucks. Properly written code would allow loop() to do other things when no tag is present, or when the tag is the wrong type, or when the tag has been erased. Things like toggling the LED as illustrated in the blink without delay example, instead of blocking code.

Hi Paul, thanks so much for the quick reply!

Are you sure you don't need a long to count to 4?

I can try a long, but this seems to work well enough. I think a long may take up more memory than needed. I'm pretty novice to programming - only really touch it on and off over the years, so forgive me if I'm mistaken.

The code you started from sucks. Properly written code would allow loop() to do other things when no tag is present, or when the tag is the wrong type, or when the tag has been erased. Things like toggling the LED as illustrated in the blink without delay example, instead of blocking code.

Very sorry. Trying to learn from the examples in the library. I'll look into how to do this.

I think a long may take up more memory than needed.

So does an int, when a byte will do. Here's your leg back.

Very sorry.

You don't need to apologize for someone else publishing crappy code.