Entry Code Issue - Always access granted

Hi Guys,

Could you help me out why I am always getting 'unlocked' output with the following code no matter what code i enter?

const int greenled = 8;
const int redled = 7;
const int b1 = 4;
const int b2 = 5;
const int b3 = 6;
const int unlockpin = 3;

int pwcount;
byte combination[] = "1332";
byte userInput[4];

int buttonstate1 = 0;
int buttonstate2 = 0;
int buttonstate3 = 0;

void setup() {
 pinMode(greenled, OUTPUT);
 pinMode(redled, OUTPUT);
 pinMode(b1, INPUT_PULLUP);
 pinMode(b2, INPUT_PULLUP);
 pinMode(b3, INPUT_PULLUP);
 Serial.begin(9600);
 pinMode(unlockpin, OUTPUT);
}

void loop(){
 buttonstate1 = digitalRead(b1);
 buttonstate2 = digitalRead(b2);
 buttonstate3 = digitalRead(b3);

 if (buttonstate1 == LOW){
   userInput[pwcount] = '1';
   pwcount++;
   delay(300);
   Serial.print('1');
 }
 if (buttonstate2 == LOW){
   userInput[pwcount] = '2';
   pwcount++;
   delay(300);
   Serial.print('2');
 }
 if (buttonstate3 == LOW){
   userInput[pwcount] = '3';
   pwcount++;
   delay(300);
   Serial.print('3');
 }
 for(byte n = 0; n <=4; n++){


   if (userInput[n] == combination[n] && pwcount >=4){
     digitalWrite(redled, LOW);
     digitalWrite(greenled, HIGH);
     delay(500);
     digitalWrite(greenled, LOW);
     delay(500);
     Serial.println("unlocked");
     pwcount = 0;
     digitalWrite(unlockpin, HIGH);
         }
   else {
     if(userInput[n] != combination[n] && pwcount >=4){
       digitalWrite(greenled, LOW);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       Serial.println("Denied");
       pwcount = 0;
       n = 0;
     }
   }
 }  }
for(byte n = 0; n <=4; n++){

Five iterations for a four digit code?

Please remember to use code tags when posting code.

It may simplify things for you to use memcmp() since your data is already in arrays. This way you don't need a counter for the compare.

C++ reference memcmp()

AVR memcmp()

Not familiar with memcmp(). I can't figure this out, I changed the [n] into [pwcount] but now it is always denied. What am I missing?

const int greenled = 8;
const int redled = 7;
const int b1 = 4;
const int b2 = 5;
const int b3 = 6;
const int unlockpin = 3;

int pwcount;
byte combination[] = "1332";
byte userInput[4];

int buttonstate1 = 0;
int buttonstate2 = 0;
int buttonstate3 = 0;

void setup() {
 pinMode(greenled, OUTPUT);
 pinMode(redled, OUTPUT);
 pinMode(b1, INPUT_PULLUP);
 pinMode(b2, INPUT_PULLUP);
 pinMode(b3, INPUT_PULLUP);
 Serial.begin(9600);
 pinMode(unlockpin, OUTPUT);
}

void loop(){
 buttonstate1 = digitalRead(b1);
 buttonstate2 = digitalRead(b2);
 buttonstate3 = digitalRead(b3);

 if (buttonstate1 == LOW){
   userInput[pwcount] = '1';
   pwcount++;
   delay(300);
   Serial.print('1');
 }
 if (buttonstate2 == LOW){
   userInput[pwcount] = '2';
   pwcount++;
   delay(300);
   Serial.print('2');
 }
 if (buttonstate3 == LOW){
   userInput[pwcount] = '3';
   pwcount++;
   delay(300);
   Serial.print('3');
 }
 for(byte n = 0; n <=4; n++){


   if (userInput[pwcount] == combination[n] && pwcount >=4){   //CHANGED N INTO pwcount on userinput here
     digitalWrite(redled, LOW);
     digitalWrite(greenled, HIGH);
     delay(500);
     digitalWrite(greenled, LOW);
     delay(500);
     Serial.println("unlocked");
     pwcount = 0;
     digitalWrite(unlockpin, HIGH);
         }
   else {
     if(userInput[n] != combination[n] && pwcount >=4){
       digitalWrite(greenled, LOW);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       digitalWrite(redled, HIGH);
       delay(500);
       digitalWrite(redled, LOW);
       delay(500);
       Serial.println("Denied");
       pwcount = 0;
       n = 0;
     }
   }
 }  }

for(byte n = 0; n <=4; n++) Deja vu

zorion:
Not familiar with memcmp(). I can't figure this out, I changed the [n] into [pwcount] but now it is always denied. What am I missing?

Run this.

void setup() {
  Serial.begin(9600);
 for (byte n = 0; n <= 4; n++) {
    Serial.print(n);
    Serial.print(" ");
  }
}

void loop() {
}

[/quote]

hmm, when I run your code I can see that I am looking at 5 digits but my serial is outputting 4 and then it says denied or unlocked. I think i will have a look at the memcpc function instead...

zorion:
hmm, when I run your code I can see that I am looking at 5 digits but my serial is outputting 4 and then it says denied or unlocked. I think i will have a look at the memcpc function instead...

see replies 2 and 5

AWOL:
see replies 2 and 5

hm when I use for(byte n = 0; n <=3; n++) it only looks for 3 digits and then decides if access is denied or granted

I am having a real issue with this code. How can I get it to count the next value of the array and not only look at the first value? I have looked at examples on google but a lot of them come up with the same FOR loop.

Post your code as it looks now and your observations of how it behaves.

const int greenled = 8;
const int redled = 7;
const int b1 = 4;
const int b2 = 5;
const int b3 = 6;
const int unlockpin = 3;


byte combination[] = "2331";
byte userInput[4];
int pwcount;

int buttonstate1 = 0;
int buttonstate2 = 0;
int buttonstate3 = 0;

void setup() {
pinMode(greenled, OUTPUT);
pinMode(redled, OUTPUT);
pinMode(b1, INPUT_PULLUP);
pinMode(b2, INPUT_PULLUP);
pinMode(b3, INPUT_PULLUP);
Serial.begin(9600);
pinMode(unlockpin, OUTPUT);
}

void loop(){
buttonstate1 = digitalRead(b1);
buttonstate2 = digitalRead(b2);
buttonstate3 = digitalRead(b3);

if (buttonstate1 == LOW){
 userInput[pwcount] = '1';
 pwcount++;
 delay(300);
 Serial.print('1');
}
if (buttonstate2 == LOW){
 userInput[pwcount] = '2';
 pwcount++;
 delay(300);
 Serial.print('2');
}
if (buttonstate3 == LOW){
 userInput[pwcount] = '3';
 pwcount++;
 delay(300);
 Serial.print('3');
}
for(byte n = 0; n < 4; n++)

{


 if (userInput[n] != combination[n] && pwcount ==4){ 
  
   Serial.println("denied");
   pwcount = 0;
   n = 0;
       }
 else {
   if(userInput[n] == combination[n] && pwcount ==4){ // I understand that when pwcount matches 4 it will only check the first element of the array as n starts out as 0 in this FOR loop. As soon as the first element of 'userInput' matches the first element of 'combination' it will continue to access granted. What do I need here to check all 4 elements of the arrays? Is this even possible with a FOR loop?
    
     Serial.println("granted");
     pwcount = 0;
     n = 0;
   }
 }
}  }

Looking at the pieces of paper analogy: What I have are 2 arrays of 4 papers (indexed 0-3). One array is empty and the other has 1 number on each paper. I first need to put a number on each of the empty papers and when all papers have a number I need to compare the 2 complete arrays against each other.

Try this: Create a boolean and give it a nice name, like codeMatched. Right before entering the loop set codeMatched true. Now, as you iterate through the loop if any pair does *not * match set codeMatched to false. Once you exit the loop the status of codeMatched will tell you if a good code was entered.

Delta_G:
Compare the complete arrays is memcmp.

I was looking more for an algorithm to loop through the papers. Like look at the first one if it matches then move on and look at the second one etc til you get to the last one. If you find any that don't match then call it a fail and stop right there.

What you're doing right now is the opposite. You're comparing the first and if it is not a match then you move on to the next. And if you find any one that is a match then you call it success and unlock the door.

If I run and test the code I have now, it only compares the first one. I can enter anything I want on 2-4 and it will accept the code as long as the first is a 2 (Code is 2331). How do I get it to go through all the values? Is this possible in a loop?

Delta_G:
Yes it is possible. Read #16 it describes how.

I kind of get that but I still can't get it to check more then 1 value. This is what I have now. Access is still granted when the first entry matches. This is what I have now:

const int greenled = 8;
const int redled = 7;
const int b1 = 4;
const int b2 = 5;
const int b3 = 6;
const int unlockpin = 3;

int pwcount;
byte combination[4] = "2323";
byte userInput[4];

int buttonstate1 = 0;
int buttonstate2 = 0;
int buttonstate3 = 0;

bool CodeMatched;


void setup() {
 pinMode(greenled, OUTPUT);
 pinMode(redled, OUTPUT);
 pinMode(b1, INPUT_PULLUP);
 pinMode(b2, INPUT_PULLUP);
 pinMode(b3, INPUT_PULLUP);
 Serial.begin(9600);
 pinMode(unlockpin, OUTPUT);
}

void loop() {
 buttonstate1 = digitalRead(b1);
 buttonstate2 = digitalRead(b2);
 buttonstate3 = digitalRead(b3);

 if (buttonstate1 == LOW) {
   userInput[pwcount] = '1';
   pwcount++;
   delay(300);
   Serial.print('1');
 }
 if (buttonstate2 == LOW) {
   userInput[pwcount] = '2';
   pwcount++;
   delay(300);
   Serial.print('2');
 }
 if (buttonstate3 == LOW) {
   userInput[pwcount] = '3';
   pwcount++;
   delay(300);
   Serial.print('3');
 }
 CodeMatched = true;
 { for (byte n = 0; n <= 4; n++)

     if (userInput[n] != combination[n] && pwcount == 4)

     { Serial.println("Denied");
       Serial.println(CodeMatched);
       CodeMatched = false;
       Serial.println(CodeMatched);
       pwcount = 0;
       

     } else




       if (CodeMatched == true && pwcount == 4)
       {
         Serial.println("granted");
         digitalWrite(unlockpin, HIGH);
         pwcount = 0;
         n = 0;
         delay(1000);

       }
 }
}

How does this:

if (userInput[n] != combination[n] && pwcount == 4)

jibe with

dougp:
Now, as you iterate through the loop if any pair does *not * match set codeMatched to false.

Delta_G:
Since it's going off the end of the array, it doesn't really matter what else it does.

Yes, the looping must be fixed first. I missed that.

Delta_G:
Compare the complete arrays is memcmp.

I was looking more for an algorithm to loop through the papers. Like look at the first one if it matches then move on and look at the second one etc til you get to the last one. If you find any that don't match then call it a fail and stop right there.

What you're doing right now is the opposite. You're comparing the first and if it is not a match then you move on to the next. And if you find any one that is a match then you call it success and unlock the door.

Delta_G:

byte combination[4] = "2323";

byte userInput[4];






for (byte n = 0; n <= 4; n++)




Since it's going off the end of the array, it doesn't really matter what else it does. 

I think that is his way of trying to make the comparison only happen after 4 buttons have been pressed.

Should also check out the state change example. When this runs the right way, the loop will repeat a thousand thousand times between the time you press the button and the time your finger gets back off the button even if you're the fastest button pusher in your town. So you don't want to keep entering a number anytime the button IS pressed. Only once when it BECOMES pressed. Should check out the State Change Example.

What do you mean with going off the end of the array? Does this look better? Also incorporated the state change example:

const int greenled = 8;
const int redled = 7;
const int b1 = 4;
const int b2 = 5;
const int b3 = 6;
const int unlockpin = 3;

int pwcount;
byte combination[] = "1332";
byte userInput[4];

int buttonstate1 = 0;
int lastButtonState1 = 0;
int buttonstate2 = 0;
int lastButtonState2 = 0;
int buttonstate3 = 0;
int lastButtonState3 = 0;

int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button

bool CodeMatched;


void setup() {
 pinMode(greenled, OUTPUT);
 pinMode(redled, OUTPUT);
 pinMode(b1, INPUT_PULLUP);
 pinMode(b2, INPUT_PULLUP);
 pinMode(b3, INPUT_PULLUP);
 Serial.begin(9600);
 pinMode(unlockpin, OUTPUT);
}

void loop() {
 buttonstate1 = digitalRead(b1);
 buttonstate2 = digitalRead(b2);
 buttonstate3 = digitalRead(b3);

 if (buttonstate1 != lastButtonState1) {
   if (buttonstate1 == HIGH) {
     userInput[pwcount] = '1';
     pwcount++;
     delay(300);
     Serial.print('1');
     Serial.println("on");
   } else {
     Serial.println("off");
   }

   delay(50); //debounce
 }

 if (buttonstate2 != lastButtonState2) {
   if (buttonstate2 == HIGH) {
     userInput[pwcount] = '2';
     pwcount++;
     delay(300);
     Serial.print('2');
     Serial.println("on");
   } else {
     Serial.println("off");
   }
   delay(50); //debounce
 }

 if (buttonstate3 != lastButtonState3) {
   if (buttonstate3 == HIGH) {
     userInput[pwcount] = '3';
     pwcount++;
     delay(300);
     Serial.print('3');
     Serial.println("on");
   } else {
     Serial.println("off");
   }
   delay(50); //debounce
 }

 if (pwcount == 4); {

   for (byte n = 0; n <= 4; n++)
   {
     CodeMatched = true;
     if (userInput[n] != combination[n])

     {
       Serial.println("Denied");
       CodeMatched = false;
     }

     {
       if (CodeMatched) 
       {
         Serial.println("Granted");
         digitalWrite(unlockpin, HIGH);
         delay(10000);
         pwcount = 0;
       }




     }
   }
 }
}

zorion:
What do you mean with going off the end of the array?

Run this:

for (byte n = 0; n <= 4; n++){
  Serial.println(n);
}

What numbers are printed?