Password invalid even after correct keypad entry (somtimes)

Projects Programming
1

I'm try to write code for my Arduino UNO R3 that will Lock or Unlock a door.

For the purpose of testing, I'm currently just using Pin 13 to turn 'On' and 'Off' the "L" LED (representing Locked or Unlocked).

I'm using a 3x4 keypad to enter a 4 digit password. After the password is pressed, I then press the "#" key to check if the password is correct or not.

Below is the code I have written:

#include "Keypad.h"

#define Password_Lenght 12 // Give enough room for 7 chars + NULL char
int lockLED = 13;
bool lockState = false;
const byte ROWS = 4, COLS = 3;
char Data[Password_Lenght], Master[Password_Lenght] = "1234", keyData;
char keys[ROWS][COLS] = {
  {'1','2','3'},
  {'4','5','6'},
  {'7','8','9'},
  {'*','0','#'}
};
byte rowPins[ROWS] = {5, 6, 7, 8}, colPins[COLS] = {9, 10, 11}, data_count = 0;
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

void setup(){
  pinMode(lockLED, OUTPUT); digitalWrite(lockLED, LOW);
  Serial.begin(9600);
}

void loop(){
  getKeypadEntry();
  if (lockState == false){
    digitalWrite(lockLED, LOW);
  }
  if (lockState == true){
    digitalWrite(lockLED, HIGH);
  }
}

void getKeypadEntry(){
  char keyData = keypad.getKey();
  switch (keyData){
    case  NO_KEY:
      break;
    case '0': case '1': case '2': case '3': case '4':
    case '5': case '6': case '7': case '8': case '9':
      Data[data_count] = keyData;
      data_count++;
      Serial.println(keyData);
      break;
    case '#':
      checkPassword();
      break;
    case '*':
      data_count = 0;
      Data [0] = (char)0;
      break;
  }
}

void checkPassword(){
  if(!strcmp(Data, Master)){
    Serial.println(Data);
    lockState = !lockState;
    if (lockState == true){
      Serial.println("Door is Locked.");
      data_count = 0;
      Data[0] = (char)0;
    }
    else{
      Serial.println("Door is Unlocked.");
      data_count = 0;
      Data[0] = (char)0;
    }
  }else{
    Serial.println("Invalid Code");
    data_count = 0;
    Data[0] = (char)0;
  }
}

After uploading, it works as intended when I enter the correct paswword:

Enter "1234" + "#": LED turns On

Enter "1234" + "#" again: LED turns Off

And so on...

The problem I'm having is that if I test for a wrong password, it somtimes gives me an "Invalid Code", eventhough the correct code was entered. Once it does this it will never accept the correct password again until I reset the Arduino board.

I've looked at this code and serached Google for way too many hours trying to understand to problem I'm having. Therfore I'm throwing my hands up and asking for help.

Any thoughts why an incorrect password would cause the code to no longer recognize the correct password?

Thanks

2 
     Data[data_count] = keyData;
      data_count++;
      Serial.println(keyData);
      break;

You are not inserting the termination character into the next position after saving a digit

3

It is my understanding that the "#" key is my termination character. If I add the "#" key to the 'Data' string then Data = "1234#" which would not match the 'Master' string which is "1234".

I guess I'm not understanding what you are suggesting.

What would you expect the termination charater to be?

4

What would you expect the termination charater to be?

'\0' is the termination character for a C style string. It is there to indicate the end of the string.

5

So should I add the null "\0" to the 'Data' string before checking if the password is correct?

I little more insite to explain what is happening.

When I enter the correct code "1234" it works great. But after typing the correct code I can enter just "123" and it still works (Lock/Unlock) because 'Data' still shows to be "1234".

I thought I was clearing out the 'Data' string with the following code:

data_count = 0;
Data [0] = (char)0;

But it seems this is not clearing out the 'Data' string.

Where the code seems to get stuck showing "Invalid Key" for ever, even if the password is correct, is if I enter more than 4 characters on the keypad (not including the '#' key).

If I type "12345" the program will show "Invalid Key" on the Serial Monitor. But If I follow that entry with "1234" is still shows "Invalid Key".

6

As said, because you don't have the terminating '\0'.

And as you already suspect, add it when the # is pressed.

7

I thought I was clearing out the 'Data' string with the following code:

data_count = 0;

Data [0] = (char)0;

Puts a '\0' in the first position of the Data array. That is all

8

sterretje:
As said, because you don't have the terminating '\0'.

And as you already suspect, add it when the # is pressed.

I think the \0 should be added in place of putting the # there.

Paul

9

Okay, here is my updated code:

#include "Keypad.h"

#define Password_Lenght 12 // Give enough room for 12 chars + NULL char
int lockLED = 13;
bool lockState = false;
const byte ROWS = 4, COLS = 3;
char Data[Password_Lenght], Master[Password_Lenght] = "1234", keyData;
char keys[ROWS][COLS] = {
  {'1','2','3'},
  {'4','5','6'},
  {'7','8','9'},
  {'*','0','#'}
};
byte rowPins[ROWS] = {5, 6, 7, 8}, colPins[COLS] = {9, 10, 11}, data_count = 0;
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

void setup(){
  pinMode(lockLED, OUTPUT); digitalWrite(lockLED, LOW);
  Serial.begin(9600);
}

void loop(){
  getKeypadEntry();
  if (lockState == false){
    digitalWrite(lockLED, LOW);
  }
  if (lockState == true){
    digitalWrite(lockLED, HIGH);
  }
}

void getKeypadEntry(){
  char keyData = keypad.getKey();
  switch (keyData){
    case  NO_KEY:
      break;
    case '0': case '1': case '2': case '3': case '4':
    case '5': case '6': case '7': case '8': case '9':
      Data[data_count] = keyData;
      data_count++;
      Serial.println(keyData);
      break;
    case '#':
      data_count++;
      Data[data_count] = '\0';
      checkPassword();
      break;
    case '*':
      data_count = 0;
      Data [0] = '\0';
      break;
  }
}

void checkPassword(){
  if(!strcmp(Data, Master)){
    Serial.println(Data);
    lockState = !lockState;
    if (lockState == true){
      Serial.println("Door is Locked.");
      Data [0] = '\0';
    }
    else{
      Serial.println("Door is Unlocked.");
      Data [0] = '\0';
    }
  }else{
    Serial.println("Invalid Code");
    Data [0] = '\0';
  }
}

I added a null to the end of my 'Data' array/string after pressing "#":

case '#':
      data_count++;
      Data[data_count] = '\0';
      checkPassword();
      break;

And I changed the way I'm clearing out/emtying the 'Data' array with the following:

case '*':
      data_count = 0;
      Data [0] = '\0';
      break;

But it's still not working correctly.

Now it will only work if I press the "*" key before entering the password again.
Still if I enter the incorrect password it will always show "Invalid Code".

10

This won't work, because you already incremented data_count after the last digit was stored, resulting in unpredictable data after the last digit.

case '#':
      data_count++;
      Data[data_count] = '\0';

Fix that by simply terminating the string.

case '#':
      Data[data_count] = '\0';
11

UKHeliBob:
Puts a '\0' in the first position of the Data array. That is all

So how would I emty the 'Data' array if it has more than one character?

12

jremington:
This won't work, because you already incremented data_count after the last digit was stored, resulting in unpredictable data after the last digit.

case '#':

data_count++;
     Data[data_count] = '\0';




Fix that by simply terminating the string.

case '#':
     Data[data_count] = '\0';

Yep, you are correct. I fixed that part of it.
But I was still having to press the "*" key before it would work (Lock/Unlock) again. I didn't understand why because all it is doing is:

data_count = 0;
Data [0] = '\0';

And I thought I was doing that same thing after each time the password was entered correctly:

void checkPassword(){
  if(!strcmp(Data, Master)){
    Serial.println(Data);
    lockState = !lockState;
    if (lockState == true){
      Serial.println("Door is Locked.");
      Data [0] = '\0';
    }
    else{
      Serial.println("Door is Unlocked.");
      Data [0] = '\0';
    }
  }else{
    Serial.println("Invalid Code");
    Data [0] = '\0';
  }
}

Then I realized I forgot to add a line to set data_count to 0.
So I added that back in.

void checkPassword(){
  if(!strcmp(Data, Master)){
    Serial.println(Data);
    lockState = !lockState;
    if (lockState == true){
      Serial.println("Door is Locked.");
      data_count = 0;
      Data [0] = '\0';
    }
    else{
      Serial.println("Door is Unlocked.");
      data_count = 0;
      Data [0] = '\0';
    }
  }else{
    Serial.println("Invalid Code");
    data_count = 0;
    Data [0] = '\0';
  }
}

It seems to be working now. I'll try more "Invalid Code" variations to see if it will fail.

13

Well everything appears to be working properly now. Here is the final code:

#include "Keypad.h"

#define Password_Lenght 12 // Give enough room for 12 chars + NULL char
int lockLED = 13;
bool lockState = false;
const byte ROWS = 4, COLS = 3;
char Data[Password_Lenght], Master[Password_Lenght] = "1234", keyData;
char keys[ROWS][COLS] = {
  {'1','2','3'},
  {'4','5','6'},
  {'7','8','9'},
  {'*','0','#'}
};
byte rowPins[ROWS] = {5, 6, 7, 8}, colPins[COLS] = {9, 10, 11}, data_count = 0;
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

void setup(){
  pinMode(lockLED, OUTPUT); digitalWrite(lockLED, LOW);
  Serial.begin(9600);
}

void loop(){
  getKeypadEntry();
  if (lockState == false){
    digitalWrite(lockLED, LOW);
  }
  if (lockState == true){
    digitalWrite(lockLED, HIGH);
  }
}

void getKeypadEntry(){
  char keyData = keypad.getKey();
  switch (keyData){
    case  NO_KEY:
      break;
    case '0': case '1': case '2': case '3': case '4':
    case '5': case '6': case '7': case '8': case '9':
      Data[data_count] = keyData;
      data_count++;
      Serial.println(keyData);
      break;
    case '#':
      //data_count++;
      Data[data_count] = '\0';
      checkPassword();
      break;
    case '*':
      data_count = 0;
      Data [0] = '\0';
      break;
  }
}

void checkPassword(){
  if(!strcmp(Data, Master)){
    Serial.println(Data);
    lockState = !lockState;
    if (lockState == true){
      Serial.println("Door is Locked.");
      data_count = 0;
      Data [0] = '\0';
    }
    else{
      Serial.println("Door is Unlocked.");
      data_count = 0;
      Data [0] = '\0';
    }
  }else{
    Serial.println("Invalid Code");
    data_count = 0;
    Data [0] = '\0';
  }
}

I've worked with 'String' variables many times before without too many problems, but this is the first time I've used char arrays. Obviously I need to pratice with them more and know what the array contains and where the pointer is for that array at any given time.

Huge thanks to each of you for your help.

14

Glad you got it working!

My policy is to keep the C-string properly terminated at all times, so during data entry, I would do the following:

    case '0': case '1': case '2': case '3': case '4':
    case '5': case '6': case '7': case '8': case '9':
      Data[data_count] = keyData;
      data_count++;
      Data[data_count]=0; //make sure string is properly terminated
      Serial.println(keyData);
      break;

That way you can safely print the C-string while it is being entered, for debug purposes, e.g.

    case '0': case '1': case '2': case '3': case '4':
    case '5': case '6': case '7': case '8': case '9':
      Data[data_count] = keyData;
      data_count++;
      Data[data_count]=0; //make sure string is properly terminated
      Serial.println(Data);  //echo partial string for debugging
      break;

Finally, rather than using switch case, the above can be simplified by using character type functions as follows:

     if (isdigit(keyData)) {
      Data[data_count] = keyData;
      data_count++;
      Data[data_count]=0; //make sure string is properly terminated
      Serial.println(keyData);
      }
15

Thanks jremington, I'll implement your suggestions. I'm always in favor of minimizing code and saving memory.