RC522 RFID reader trouble

system · October 6, 2014, 11:45pm

Okay so I was getting help from another extremely helpful guy on here (that’s why I’m as far as I am) but I need some help on the last step of the rfid part of my project. I have three readers connected, and they work. I have two partial uids set to light a led while the rest should do nothing. The problem is after I read a tag that lights the led, the next tag, regardless of uid, will light the led also. It only does this once. So I’m trying to figure out how to stop this from happening. any ideas?

/* MOSI: Pin 11 / ICSP-4
 * MISO: Pin 12 / ICSP-1
 * SCK : Pin 13 / ICSP-3
 * SS : Pin 10 (Configurable)
 * RST : Pin 9 (Configurable)
 */

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

#define SS_PIN 10
#define SS2_PIN 4
#define SS3_PIN 8
#define RST_PIN 9
#define redLed 7

MFRC522 readerOne(SS_PIN, RST_PIN);
MFRC522 readerTwo(SS2_PIN, RST_PIN);
MFRC522 readerThree(SS3_PIN, RST_PIN);

void setup() {
  pinMode(redLed, OUTPUT);
  SPI.begin();
  readerOne.PCD_Init();
  readerTwo.PCD_Init();
  readerThree.PCD_Init();
}
       
  int myCat = 0x03;  
  int yourCat = 0xF3;
  int readTag;
  int readTagTwo;
  int readTagThree; 
  
void loop() { 

  readTag = (readerOne.uid.uidByte[0]);
  readTagTwo = (readerTwo.uid.uidByte[0]);
  readTagThree = (readerThree.uid.uidByte[0]);
  
  if (readerOne.PICC_IsNewCardPresent()){
     (readerOne.PICC_ReadCardSerial()); { 
        if(readTag == myCat){  
        digitalWrite(redLed, HIGH); 
        delay(1800);
        digitalWrite(redLed, LOW);}
     else{   
        if(readTag == yourCat){  
        digitalWrite(redLed, HIGH); 
        delay(1800);
        digitalWrite(redLed, LOW);
      }
      }
      }
      }  
      
  if (readerTwo.PICC_IsNewCardPresent()){
     (readerTwo.PICC_ReadCardSerial()); { 
        if(readTagTwo == myCat){  
        digitalWrite(redLed, HIGH); 
        delay(1800);
        digitalWrite(redLed, LOW);}
     else{   
        if(readTagTwo == yourCat){  
        digitalWrite(redLed, HIGH); 
        delay(1800);
        digitalWrite(redLed, LOW);
      }
      }
      }
      } 
      
   if (readerThree.PICC_IsNewCardPresent()){
     (readerThree.PICC_ReadCardSerial()); { 
        if(readTagThree == myCat){  
        digitalWrite(redLed, HIGH); 
        delay(1800);
        digitalWrite(redLed, LOW);}
     else{   
        if(readTagThree == yourCat){  
        digitalWrite(redLed, HIGH); 
        delay(1800);
        digitalWrite(redLed, LOW);
      }
      }
      }
      }
  }

system · October 7, 2014, 12:02am

here's a video displaying what's going on.

system · October 7, 2014, 11:36am

You should be setting the mode for pin 10 to OUTPUT for SPI to work properly.

#define SS_PIN 10
#define SS2_PIN 4
#define SS3_PIN 8

Nothing, two, three... Is that how you count?

  readTag = (readerOne.uid.uidByte[0]);
  readTagTwo = (readerTwo.uid.uidByte[0]);
  readTagThree = (readerThree.uid.uidByte[0]);

What is this doing? If you haven't scanned a tag, does the uid member even contain valid data?

Why do you assume that a name like uidByte will be an int? It might be a float, don't you think?

     (readerOne.PICC_ReadCardSerial()); {

Why is the call to the method in parentheses? Why is there a useless { on the end of the line?

        if(readTag == myCat){

Regardless of what the reader actually read, compare some other (possibly garbage) value to a known value. Why?

system · October 7, 2014, 9:17pm

Thanks for the reply PaulS. I'll set pin 10 to output, rename readTag. I get that. I'm super new to all this so I'll probably make tons of elementary mistakes but I'll try to answer your questions.

  readTag = (readerOne.uid.uidByte[0]);
  readTagTwo = (readerTwo.uid.uidByte[0]);
  readTagThree = (readerThree.uid.uidByte[0]);

this is defining the uid read by the reader. It's nothing until it is scanned, then it's the uid of whichever tag was scanned. I'm sure there is a better, more legitimate way of doing it.

Why do you assume that a name like uidByte will be an int? It might be a float, don't you think?

Maybe my problem is my lack of knowledge on float. If it helps, I'm only using the first or first two bytes of the uid because it's easier for me to understand the code that way. I'm honestly not sure if that is a valid thing to do.

Why is the call to the method in parentheses? Why is there a useless { on the end of the line?

I don't know. I thought it needed it, I guess. I'll remove it.

Regardless of what the reader actually read, compare some other (possibly garbage) value to a known value. Why?

Maybe I am completely confused, but this seems like the most necessary part of my code. The read uid has to be compared to something in order to know whether or not it is that something, right? If I only want certain tags to trigger an event, I have to define which tags do or which tags don't, right?

I don't understand how the read uid could be possibly garbage. It's the uid of the scanned tag. It works like it should, just not when a tag which lights the led was read just before one that shouldn't light it.

Like I said, I know nothing about c++ or anything like that. I appreciate the help.

nickgammon · October 7, 2014, 11:03pm

I can’t see where the tags are being read. Can you clarify that part?

nickgammon · October 7, 2014, 11:06pm

Oh, I see it now:

  readTag = (readerOne.uid.uidByte[0]);
  readTagTwo = (readerTwo.uid.uidByte[0]);
  readTagThree = (readerThree.uid.uidByte[0]);

  if (readerOne.PICC_IsNewCardPresent()){
    (readerOne.PICC_ReadCardSerial()); 
    {

First get the value. Then read the card. Do you see a flaw here?

system · October 8, 2014, 4:25am

Thanks for the reply. I'm sorry, but no, I don't really see. I thought reading the card was getting the value. Sorry. Could you explain it? Treat me like a toddler. My knowledge of these things is very disjointed and all over the place, so I think I miss fundamental problems in syntax and everything else really.

nickgammon · October 8, 2014, 4:44am

Do things in this order:

See if a card is present.
Read the card
Save the tag information

You are doing it the other way around. This isn't syntax. This is looking at the order in which you are doing things in the code.

nickgammon · October 8, 2014, 4:45am

Here:

  readTag = (readerOne.uid.uidByte[0]);   // <--- save the tag data

...


  if (readerOne.PICC_IsNewCardPresent()){   // <-- see if we have a card
    (readerOne.PICC_ReadCardSerial());    //  <-- read it

system · October 8, 2014, 5:53am

ok. so I should put the readtag after the readcard serial? I thought I was just defining readtag as what would we read from the tag.

like this?

  if (readerOne.PICC_IsNewCardPresent()){   // <-- see if we have a card
               (readerOne.PICC_ReadCardSerial());    //  <-- read it
              readTag = (readerOne.uid.uidByte[0]);   // <--- save the tag data

system · October 8, 2014, 6:02am

YES!

Thanks!

My woes are over!

system · October 8, 2014, 1:19pm

My woes are over!

Not hardly. Those cards have multi-byte tags. You are only caring about the first byte. Sooner or later, you are going to find two cards that have the same first byte, but where the other bytes are different, and you are going to accept both as equivalent.

system · May 5, 2021, 8:54pm