Problem: RFID door lock accepts EVERYTHING!

Hello,

I am quite a newbie at programming. I have possessed my arduino for a long time and have tinkered with it here and there, learning bits of code and how this all works.

Recently, I decided to undergo the ever-so-famous RFID door lock project. Guess what, I DID IT ;D. But there's a problem. The code I'm using seems to accept every single RFID tag I use. It's horrible!

I've spent about the past 24 hours (excluding sleep of course) trying to figure this out. I want it to accept the tags I put in the code and to deny the rest. I've looked all over these forums and the internet and it seems that others had similar problems, I just don't quite understand the codes and the fixes. Don't get me wrong, I'm not someone looking for a handout - I did my best to learn how this all works and I think I am getting the hang of it.

This is just so fustrating >:( since I have to get this done ASAP because I leave for the army soon. I'll post my code and a link to the website I started this project from.

//BEGIN CODE

#define servoPin 4 // control pin for servo motor (White or yellow wire of servo)
#define minPulse 500 // minimum servo position (Open position)
#define maxPulse 2200 // maximum servo position (Closed position)
#define rxPin 8 // SOUT pin of RFID module
#define txPin 9
#define enable 2 // /ENABLE pin of RFID module
#define LED1 13 // LED output pin
#define LED2 12 // other LED output pin for two-way LED (yellow)
#define switchPin 7
#include <SoftwareSerial.h>

boolean open = true; // default start up is to assume the lock is open
int val = 0;
char code[10];
int bytesread = 0;
int pulse, switchVal;

char tag1[10] = {'0', '4', '1', '5', 'E', 'D', '4', '8', 'C', 'B'}; // why won't you work!?!?!
char tag2[10] = {'3', '6', '0', '0', '8', 'E', '5', 'E', '6', '9'};
char tag3[10] = {'2', '1', '0', '0', 'D', 'F', 'A', 'A', 'F', '8'};



void LEDControl(int state){

switch (state){
case 1:
digitalWrite(LED1,HIGH);
digitalWrite(LED2,LOW);
break;
case 2:
digitalWrite(LED2,HIGH);
digitalWrite(LED1,LOW);
break;
case 3:
for(int y=0;y<5;y++){
digitalWrite(LED1,HIGH);
digitalWrite(LED2,LOW);
delay(250);
digitalWrite(LED2,HIGH);
digitalWrite(LED1,LOW);
delay(250);
}
}

}

boolean checkTag(char *tag){

for (int x=0;x<10;x++){
if( tag[x] != code[x]){
return false;
}
}
return true;
}

boolean findGoodTag(){
if (checkTag(tag1)){ return true;}
else if (checkTag(tag2)){ return true;}
else if (checkTag(tag3)){ return true;}

// Add more lines right here like the one above if you have more tags

else{
Serial.print("Bad tag: ");
Serial.println(code);
LEDControl(3);
return false;
}

}
void moveServo(){

if (open){
pulse = minPulse;
open = false;
LEDControl(1);
}
else if (!open){
pulse = maxPulse;
open = true;
LEDControl(2);
}

for (int x =1;x<150;x++){
delay (10); // don't know why this works, but it does
digitalWrite(servoPin, HIGH); // start the pulse
delayMicroseconds(pulse); // pulse width
digitalWrite(servoPin, LOW); // stop the pulse
}

}



void setup() {

pinMode(servoPin, OUTPUT); // Set servo pin as an output pin
pinMode(LED1,OUTPUT); // Set LED pin as output
pinMode(LED2,OUTPUT); // Set LED pin as output
Serial.begin(9600);
Serial.println("Begin");
pinMode(enable,OUTPUT); // Set digital pin 2 as OUTPUT to connect it to the RFID /ENABLE pin
digitalWrite(enable, LOW); // Activate the RFID reader
pinMode(switchPin, INPUT);
}

void loop() {
SoftwareSerial RFID = SoftwareSerial(rxPin,txPin);
RFID.begin(2400);

switchVal = digitalRead(switchPin);



if((val = RFID.read()) == 10)
{ // check for header
if(switchVal == HIGH){
Serial.println("Button");
moveServo();
}
bytesread = 0;
while(bytesread<10)
{ // read 10 digit code
val = RFID.read();
if((val == 10)||(val == 13))
{ // if header or stop bytes before the 10 digit reading
break; // stop reading
}
code[bytesread] = val; // add the digit
bytesread++; // ready to read next digit
}



if((bytesread == 10) && (findGoodTag()))
{ // if 10 digit read is complete
digitalWrite(enable, HIGH); // dectivate the RFID reader
moveServo();
delay(500);
digitalWrite(enable, LOW); // Activate the RFID reader
}
}
}

and here is the source of the code, from Scott Zumwalt at Adventures in Technology: Arduino RFID Door lock

You should not be creating a new instance of the SoftwareSerial class on every pass though loop. Create a single, global instance, and use that.

SoftwareSerial is obsolete. You should be using NewSoftSerial (see the library page for a link. I'm tired of finding and posting the link).

A few Serial.print() and/or Serial.println() statements would go a long way towards determining what the problem is.

I suspect that the problem is with the way that you are reading the tags. The first byte read when the tag is scanned is thrown away. Why is that?

while(bytesread<10)
{ // read 10 digit code
val = RFID.read();
if((val == 10)||(val == 13))
{ // if header or stop bytes before the 10 digit reading
break; // stop reading
}
code[bytesread] = val; // add the digit
bytesread++; // ready to read next digit
}

Then, you (try to) read the next 10 bytes, even of there are not 10 bytes to read. You store all of the bytes you read, even if they were not valid.

I'm surprised that the door is ever unlocked.

Add some print statements to findGoodTag() and checkTag(). Print the tag being checked and the tag that it is to be compared to.

Finally, is there some reason to not use the Servo library? I see that you didn't write your own software serial class.

What are you doing with pin 7? It's not talked about in the messy code from that example you copied. However, if it's high, any code will toggle the servo. Not sure what it's doing there, looks like Scott meant to have a button to manually lock/unlock but didn't finish the logic to prevent the servo from oscillating every time through the loop.

You could do yourself (and us) a favour by finding the autoformat tool (Ctrl-T) on the Arduino's IDE and using it.
It isn't perfect, but it's a lot better than your manual formatting.
My eyes hurt.

Note: the variable "open" is a boolean - if it isn't true, it's must be false, there's no need to test both cases.