RFID to Control LED

Hi, I'm trying to make the RFID be able to turn on an LED upon tapping the key, and then turning it off by tapping the same key again. In the event that the 2nd tap never happens, I want the LED to turn off by itself after 2 minutes. However, I'm having issue with the 2nd part.

Right now, I'm only able to get the LED to turn off after tapping it a 2nd time. The LED does not turn off no matter how long I wait.

Could someone provide some guidance? I'm pretty new to Arduino as well so there might be some redundant code around somewhere.

#include <Wire.h>
#include <SPI.h>
#include <Adafruit_PN532.h>


#define PN532_SCK  (2)
#define PN532_MOSI (3)
#define PN532_SS   (4)
#define PN532_MISO (5)


#define LED 13  // LED connected to pin 13


// Defining pins for I2C for shield
#define PN532_IRQ   (2)
#define PN532_RESET (3)  // Not connected by default on the NFC Shield


// For breakout/shield with I2C connection:
Adafruit_PN532 nfc(PN532_IRQ, PN532_RESET);


// Declaring last recorded time (previousMillis) and latest recorded time (currentMillis)
unsigned long previousMillis = 0;
unsigned long currentMillis = millis(); // millis() is a timer function
unsigned long LEDMillis = 0;

void setup(void) {

  pinMode(LED, OUTPUT); // Declare the LED as an output
  
  Serial.begin(115200); // Baud rate
  while (!Serial) delay(10); 

  Serial.println("Hello!");

  nfc.begin();

  // Shield detection
  uint32_t versiondata = nfc.getFirmwareVersion(); 
  if (! versiondata) {  // If board not connected or wrong board
    Serial.print("Didn't find PN53x board");  
    while (1); // halt
  }
  // If board connected
  Serial.print("Found chip PN5"); Serial.println((versiondata>>24) & 0xFF, HEX); 
  Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC); 
  Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);
  
  // Configure board to read RFID tags
  nfc.SAMConfig();

  // Standby mode
  Serial.println("Waiting for an ISO14443A Card ...");
}



void loop(void) {
  uint8_t success;
  uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 };  // Buffer to store the returned UID
  uint8_t uidLength;                        // Length of the UID (4 or 7 bytes depending on ISO14443A card type)
    
  // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
  // 'uid' will be populated with the UID, and uidLength will indicate
  // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);
  LEDMillis = millis();
  
  if (success) {
    nfc.PrintHex(uid, uidLength); // Displays ID of card/tag
    Serial.println("");
    
    if (uidLength == 4) {

      // Card ID
      uint8_t keya[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };


      currentMillis = millis(); // Defining current time. Timer starts here
            
      // Turn LED on for 1st RFID key
      if ((digitalRead(LED) == LOW) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {  // ID: 0x33 0x06 0x64 0x97
        previousMillis = currentMillis;
        digitalWrite(LED, HIGH); // Turn the LED on
      }
      else if ((digitalRead(LED) == HIGH) && (currentMillis - previousMillis >= 1000) && (currentMillis - previousMillis < 120000) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {
        digitalWrite(LED, LOW); // Tap again within 2 minutes to off
      }
      else if ((digitalRead(LED) == HIGH) && (currentMillis - previousMillis >= 120000)) {
        digitalWrite(LED, LOW); // OR Off after 2 minutes

      }


    // Start with block 4 (the first block of sector 1) cos sector 0 contains manufacturer data
      success = nfc.mifareclassic_AuthenticateBlock(uid, uidLength, 4, 0, keya);
    
      if (success)  {
        uint8_t data[16];
    
        // Reading the contents of block 4
        success = nfc.mifareclassic_ReadDataBlock(4, data);
    
        if (success)
        {
          delay(1000);
        }
      }
    }
  }
  
  }
}

Your code for this is inside the test of the success variable. Shouldn't it be in loop() and not dependant on the value of success ?

Do you mean like this? The LED still doesn't turn off by itself after 2 minutes

#include <Wire.h>
#include <SPI.h>
#include <Adafruit_PN532.h>


#define PN532_SCK  (2)
#define PN532_MOSI (3)
#define PN532_SS   (4)
#define PN532_MISO (5)


#define LED 13  // LED connected to pin 13


// Defining pins for I2C for shield
#define PN532_IRQ   (2)
#define PN532_RESET (3)  // Not connected by default on the NFC Shield


// For breakout/shield with I2C connection:
Adafruit_PN532 nfc(PN532_IRQ, PN532_RESET);


// Declaring last recorded time (previousMillis) and latest recorded time (currentMillis)
unsigned long previousMillis = 0;
unsigned long currentMillis = millis(); // millis() is a timer function


void setup(void) {

  pinMode(LED, OUTPUT); // Declare the LED as an output
  
  Serial.begin(115200); // Baud rate
  while (!Serial) delay(10); 

  Serial.println("Hello!");

  nfc.begin();

  // Shield detection
  uint32_t versiondata = nfc.getFirmwareVersion(); 
  if (! versiondata) {  // If board not connected or wrong board
    Serial.print("Didn't find PN53x board");  
    while (1); // halt
  }
  // If board connected
  Serial.print("Found chip PN5"); Serial.println((versiondata>>24) & 0xFF, HEX); 
  Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC); 
  Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);
  
  // Configure board to read RFID tags
  nfc.SAMConfig();

  // Standby mode
  Serial.println("Waiting for an ISO14443A Card ...");
}



void loop(void) {
  uint8_t success;
  uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 };  // Buffer to store the returned UID
  uint8_t uidLength;                        // Length of the UID (4 or 7 bytes depending on ISO14443A card type)


  if ((digitalRead(LED) == 1) && (currentMillis - previousMillis >= 120000)) {
    digitalWrite(LED, 0); // OR Off after 2 minutes
  }

  // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
  // 'uid' will be populated with the UID, and uidLength will indicate
  // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);



  
  if (success) {
    nfc.PrintHex(uid, uidLength); // Displays ID of card/tag
    Serial.println("");
    
    if (uidLength == 4) {

      // Card ID
      uint8_t keya[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };


      currentMillis = millis(); // Defining current time. Timer starts here
            
      // Turn LED on for 1st RFID key
      if ((digitalRead(LED) == 0) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {  // ID: 0x33 0x06 0x64 0x97
        previousMillis = currentMillis;
        digitalWrite(LED, 1); // Turn the LED on
      }
      else if ((digitalRead(LED) == 1) && (currentMillis - previousMillis >= 1000) && (currentMillis - previousMillis < 3000) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {
        digitalWrite(LED, 0); // Tap again within 2 minutes to off
      }




      
    // Start with block 4 (the first block of sector 1) cos sector 0 contains manufacturer data
      success = nfc.mifareclassic_AuthenticateBlock(uid, uidLength, 4, 0, keya);
    
      if (success)  {
        uint8_t data[16];
    
        // Reading the contents of block 4
        success = nfc.mifareclassic_ReadDataBlock(4, data);
    
        if (success)
        {
          delay(1000);
        }
      }
    }
  }
}

As an experiment try printing a message when you turn the LED on and off. Is it happening when you expect it to ?

I've placed 4 print commands, printing the numbers 1 through 4, at every point where the LED changes state.
The digitalWrite to turn on the LED upon scanning, where I placed the command to print "2", works as intended.

However, for the 2nd tap, depending on the duration between the 1st and 2nd taps, the numbers "1" and "3" are printed. "1" appears when the duration between both taps is more than 3 seconds. "3" appears when the duration is less than 3 seconds.

#include <Wire.h>
#include <SPI.h>
#include <Adafruit_PN532.h>


#define PN532_SCK  (2)
#define PN532_MOSI (3)
#define PN532_SS   (4)
#define PN532_MISO (5)


#define LED 13  // LED connected to pin 13


// Defining pins for I2C for shield
#define PN532_IRQ   (2)
#define PN532_RESET (3)  // Not connected by default on the NFC Shield


// For breakout/shield with I2C connection:
Adafruit_PN532 nfc(PN532_IRQ, PN532_RESET);


// Declaring last recorded time (previousMillis) and latest recorded time (currentMillis)
unsigned long previousMillis = 0;
unsigned long currentMillis = millis(); // millis() is a timer function


void setup(void) {

  pinMode(LED, OUTPUT); // Declare the LED as an output
  
  Serial.begin(115200); // Baud rate
  while (!Serial) delay(10); 


  nfc.begin();

  // Shield detection
  uint32_t versiondata = nfc.getFirmwareVersion(); 
  if (! versiondata) {  // If board not connected or wrong board
    Serial.print("Didn't find PN53x board");  
    while (1); // halt
  }
  // If board connected
  Serial.print("Found chip PN5"); Serial.println((versiondata>>24) & 0xFF, HEX); 
  Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC); 
  Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);
  
  // Configure board to read RFID tags
  nfc.setPassiveActivationRetries(0x9A);
  nfc.SAMConfig();

  // Standby mode
  Serial.println("Waiting for an ISO14443A Card ...");
}



void loop(void) {
  uint8_t success;
  uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 };  // Buffer to store the returned UID
  uint8_t uidLength;                        // Length of the UID (4 or 7 bytes depending on ISO14443A card type)



  if ((digitalRead(LED) == 1) && (currentMillis - previousMillis >= 3000)) {
    digitalWrite(LED, 0); // OR Off after 2 minutes
    Serial.print("1");
  }

  // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
  // 'uid' will be populated with the UID, and uidLength will indicate
  // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);



  
  if (success) {
    nfc.PrintHex(uid, uidLength); // Displays ID of card/tag
    Serial.println("");
    
    if (uidLength == 4) {

      // Card ID
      uint8_t keya[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };


      currentMillis = millis(); // Defining current time. Timer starts here
            
      // Turn LED on for 1st RFID key
      if ((digitalRead(LED) == 0) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {  // ID: 0x33 0x06 0x64 0x97
        previousMillis = currentMillis;
        digitalWrite(LED, 1); // Turn the LED on
        Serial.print("2");
      }
      else if ((digitalRead(LED) == 1) && (currentMillis - previousMillis >= 1000) && (currentMillis - previousMillis < 3000) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {
        digitalWrite(LED, 0); // Tap again within 2 minutes to off
        Serial.print("3");
      }


      
    // Start with block 4 (the first block of sector 1) cos sector 0 contains manufacturer data
      success = nfc.mifareclassic_AuthenticateBlock(uid, uidLength, 4, 0, keya);
    
      if (success)  {
        uint8_t data[16];
    
        // Reading the contents of block 4
        success = nfc.mifareclassic_ReadDataBlock(4, data);
    
        if (success)
        {
          delay(1000);
        }
      }
    }
  }
}

I've placed print commands at every digitalWrite command to print numbers from 1 to 4.
I've also changed the timer to 3000ms for testing purposes.

The output is as follows:
The number 2 is printed whenever I scan the key for the 1st time. This part is working as intended.
However, for the 2nd scan to off the LED, the output prints the numbers 1 and 3, depending on the duration between the 1st and 2nd scan. The number 1 appears when the duration between both scans are over 3 seconds, while the number 3 appears when the duration is less than 3 seconds.
The LED does not off by itself at all, only when I've scanned it a 2nd time.

#include <Wire.h>
#include <SPI.h>
#include <Adafruit_PN532.h>


#define PN532_SCK  (2)
#define PN532_MOSI (3)
#define PN532_SS   (4)
#define PN532_MISO (5)


#define LED 13  // LED connected to pin 13


// Defining pins for I2C for shield
#define PN532_IRQ   (2)
#define PN532_RESET (3)  // Not connected by default on the NFC Shield


// For breakout/shield with I2C connection:
Adafruit_PN532 nfc(PN532_IRQ, PN532_RESET);


// Declaring last recorded time (previousMillis) and latest recorded time (currentMillis)
unsigned long previousMillis = 0;
unsigned long currentMillis = millis(); // millis() is a timer function


void setup(void) {

  pinMode(LED, OUTPUT); // Declare the LED as an output
  
  Serial.begin(115200); // Baud rate
  while (!Serial) delay(10); 


  nfc.begin();

  // Shield detection
  uint32_t versiondata = nfc.getFirmwareVersion(); 
  if (! versiondata) {  // If board not connected or wrong board
    Serial.print("Didn't find PN53x board");  
    while (1); // halt
  }
  // If board connected
  Serial.print("Found chip PN5"); Serial.println((versiondata>>24) & 0xFF, HEX); 
  Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC); 
  Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);
  
  // Configure board to read RFID tags
  nfc.setPassiveActivationRetries(0x9A);
  nfc.SAMConfig();

  // Standby mode
  Serial.println("Waiting for an ISO14443A Card ...");
}



void loop(void) {
  uint8_t success;
  uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 };  // Buffer to store the returned UID
  uint8_t uidLength;                        // Length of the UID (4 or 7 bytes depending on ISO14443A card type)



  if ((digitalRead(LED) == 1) && (currentMillis - previousMillis >= 3000)) {
    digitalWrite(LED, 0); // OR Off after 2 minutes
    Serial.print("1");
  }

  // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
  // 'uid' will be populated with the UID, and uidLength will indicate
  // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);



  
  if (success) {
    nfc.PrintHex(uid, uidLength); // Displays ID of card/tag
    Serial.println("");
    
    if (uidLength == 4) {

      // Card ID
      uint8_t keya[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };


      currentMillis = millis(); // Defining current time. Timer starts here
            
      // Turn LED on for 1st RFID key
      if ((digitalRead(LED) == 0) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {  // ID: 0x33 0x06 0x64 0x97
        previousMillis = currentMillis;
        digitalWrite(LED, 1); // Turn the LED on
        Serial.print("2");
      }
      else if ((digitalRead(LED) == 1) && (currentMillis - previousMillis >= 1000) && (currentMillis - previousMillis < 3000) && (uid[0] == 0x33) && (uid[1] == 0x06) && (uid[2] == 0x64) && (uid[3] == 0x97)) {
        digitalWrite(LED, 0); // Tap again within 2 minutes to off
        Serial.print("3");
      }


      
    // Start with block 4 (the first block of sector 1) cos sector 0 contains manufacturer data
      success = nfc.mifareclassic_AuthenticateBlock(uid, uidLength, 4, 0, keya);
    
      if (success)  {
        uint8_t data[16];
    
        // Reading the contents of block 4
        success = nfc.mifareclassic_ReadDataBlock(4, data);
    
        if (success)
        {
          delay(1000);
        }
      }
    }
  }
}

Your code is overly complicated. I've tried to simplify. Note the turn off delay is 15 seconds to make it easier to test.

#include <Wire.h>
#include <SPI.h>
#include <Adafruit_PN532.h>


#define PN532_SCK  (2)
#define PN532_MOSI (3)
#define PN532_SS   (4)
#define PN532_MISO (5)


#define LED 13  // LED connected to pin 13


// Defining pins for I2C for shield
#define PN532_IRQ   (2)
#define PN532_RESET (3)  // Not connected by default on the NFC Shield


// For breakout/shield with I2C connection:
Adafruit_PN532 nfc(PN532_IRQ, PN532_RESET);


// Declaring last recorded time (previousMillis) and latest recorded time (currentMillis)
unsigned long previousMillis = 0;
unsigned long currentMillis = millis(); // millis() is a timer function


void setup(void) 
{
  pinMode(LED, OUTPUT); // Declare the LED as an output
  
  Serial.begin(115200); // Baud rate
  while (!Serial) delay(10); 


  nfc.begin();

  // Shield detection
  uint32_t versiondata = nfc.getFirmwareVersion(); 
  if (! versiondata) {  // If board not connected or wrong board
    Serial.print("Didn't find PN53x board");  
    while (1); // halt
  }
  // If board connected
  Serial.print("Found chip PN5"); Serial.println((versiondata>>24) & 0xFF, HEX); 
  Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC); 
  Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);
  
  // Configure board to read RFID tags
  nfc.setPassiveActivationRetries(0x9A);
  nfc.SAMConfig();

  // Standby mode
  Serial.println("Waiting for an ISO14443A Card ...");
}

boolean onLED = false;
unsigned long onTime = 0;
uint32_t MAX_ON_TIME = 15000;

void loop(void) 
{
  uint8_t success;
  uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 };  // Buffer to store the returned UID
  uint8_t uidLength;                        // Length of the UID (4 or 7 bytes depending on ISO14443A card type)

  // Turn off LED if it has been on for more the maximum time.
  if (onLED && millis() > onTime + MAX_ON_TIME)  
  {
    onLED = false;
    onTime = 0;
    digitalWrite(LED, LOW);
  }


  // Wait for an ISO14443A type cards (Mifare, etc.).  When one is found
  // 'uid' will be populated with the UID, and uidLength will indicate
  // if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
  
  success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);

  if (success) 
  {
    nfc.PrintHex(uid, uidLength); // Displays ID of card/tag
    Serial.println("");
    
    if (uidLength == 4) 
    {
      // Card ID
      uint8_t keya[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
           
      
      // Is this the correct card?
      if (uid[0] == 0x33 && uid[1] == 0x06 && uid[2] == 0x64 && uid[3] == 0x97)
        {
          if (onLED) // Already on, so turn off.
          {
            onLED = false;
            onTime = 0;
            digitalWrite(LED, LOW);
          }
          else // Already off, so turn on.
          {
            onLED = true;
            onTime = millis();
            digitalWrite(LED, HIGH);
          }
         }

      delay(500); // Just in case the card keeps getting read again & again.
      
//    // Start with block 4 (the first block of sector 1) cos sector 0 contains manufacturer data
//      success = nfc.mifareclassic_AuthenticateBlock(uid, uidLength, 4, 0, keya);
//    
//      if (success)  
//      {
//        uint8_t data[16];
//    
//        // Reading the contents of block 4
//        success = nfc.mifareclassic_ReadDataBlock(4, data);
//    
//        if (success)
//        {
//          delay(1000);
//        }
//      }
    }
  }
}

in place of onLED, just read the pin -- digitalRead (LED) (this is fine unless performance becomes critical

what happens when you want to check for a different RFID

suggest having a comparison routine that compares 2 UIDs, return whether they match

all of above amounts to just toggling the led

digitalWrite (LED, ! digitalRead (LED));

onTIme can always be set to millis() if digitalRead (LED) is used to check whether it needs to be turned off

Yes all valid... I was trying to get his sketch doing what he wanted... I realise there are more optimal ways to code this but I thought by being overly verbose he would better understand where it was going wrong before. As for the single key... that was not his problem so didn't want to change that code... he can fix that.

you made the code much easier to read

I see! I'm still relatively new to this.
I've found a solution to my problem. Just had to define currentMillis outside of the if loops. But I'll be sure to try yours too!
Thanks for your help!

You've missed some understanding though, if you think if loops.