Need help figuring this out

Here is the code
i m making a keypad lock with keypad and servo
the problem is , when insert the code in the keypad it opens the servo ... 53A ... but if i go 5436A it still opens , i m new to arduino , can someone help me?

#include <Servo.h>
#include <Keypad.h>

Servo ServoMotor;
char* password = "53A";  // change the password here, just pick any 3 numbers
int position = 0;
const byte ROWS = 4;
const byte COLS = 4;
char keys[ROWS][COLS] = {
{'1','2','3','A'},
{'4','5','6','B'},
{'7','8','9','C'},
{'*','0','#','D'}
};

byte rowPins[ROWS] = { 8, 7, 6, 9 };
byte colPins[COLS] = { 5, 4, 3, 2 };
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
int RedpinLock = 12;
int GreenpinUnlock = 13;

void setup()
{
ServoMotor.attach(11);
LockedPosition(true);
}

void loop()
{
char key = keypad.getKey();
if (key == '*' || key == '#')
{
position = 0;
LockedPosition(true);
}
if (key == password[position])
{
position ++;
}
if (position == 3)
{
LockedPosition(false);
}
delay(100);
}
void LockedPosition(int locked)
{
if (locked)
{
digitalWrite(RedpinLock, HIGH);
digitalWrite(GreenpinUnlock, LOW);
ServoMotor.write(11);
}
else
{
digitalWrite(RedpinLock, LOW);
digitalWrite(GreenpinUnlock, HIGH);
ServoMotor.write(87);
}
}
  if (key == password[position])
  {
    position ++;
  }

If the character entered matches the password character at that position you increment the password character index but if the wrong character is entered you ignore it

So as long as the password characters are entered in the correct order, even if other characters are entered between them the lock will open. You need to think about what you want the program to do if a wrong character is entered

You don't seem to have any code to invalidate an incorrect password.

And, to make it easier to read your code please use the Auto Format tool

...R

For each keypress, you check if it matches the current Position in the Password. If it is, you move to the next Position of the Password.

What happens if the keypress DOES NOT MATCH? Nothing! Therefore, you ONLY CONSIDER KEYPRESSES THAT ARE CORRECT!

That's why it opens even if false keys are pressed.

So, Change your program. If a keypress is incorrect, reset Position to 0.

i already tryed doing IF key != password position=0 but then it would not even open with the correct password :confused:

jp1599:
i already tryed doing IF key != password position=0 but then it would not even open with the correct password :confused:

try something like this, untested:

#include <Keypad.h>

#define MAX_PASSWORD_LENGTH 8

char* storedPassword = "53A";

const byte ROWS = 4;
const byte COLS = 4;
char keys[ROWS][COLS] = {
  {'1', '2', '3', 'A'},
  {'4', '5', '6', 'B'},
  {'7', '8', '9', 'C'},
  {'*', '0', '#', 'D'}
};

byte rowPins[ROWS] = { 8, 7, 6, 9 };
byte colPins[COLS] = { 5, 4, 3, 2 };
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  if (char* enteredPassword = lookForPassword(keypad, '#'))
  {
    Serial.println(enteredPassword);
    if (strcmp(enteredPassword, storedPassword) == 0)
    {
      Serial.println(F("SUCCESS"));
    }
    else
    {
      Serial.println(F("EPIC FAIL"));
    }
  }
}

char* lookForPassword(Keypad& kpd, const char enterChar)
{
  static char password[MAX_PASSWORD_LENGTH];
  static int idx = 0;
  if (char key = kpd.getKey())
  {
    if (key == enterChar)
    {
      idx = 0;
      return password;
    }
    if (isDigit(key))
    {
      password[idx++] = key;
      password[idx] = '\0';
      if (idx > MAX_PASSWORD_LENGTH - 1)
      {
        idx = 0;
        return password;
      }
    }
  }
  return nullptr;
}

where the # indicates that you've finished entering the password...

jp1599:
i already tryed doing IF key != password position=0 but then it would not even open with the correct password :confused:

Please post the actual code that you used. The whole program. If you were to put some Serial.print()s in strategic places and printed relevant variables it would help you to figure out what is wrong.

Coming from a fairly new coder myself. Here's my suggestion. Go back and edit your 1st post. Change the Subject to read something like this:

Need help with keypad lock with keypad and servo.

You'll probably get a better response. If I was looking through the forum... a non-descriptive post such as this one would be the last to be looked at. This will better your chances to get the help you need

Edit; Now that I look at the responses. You got some great help. Just keep this in mind for your next request.

UKHeliBob:
Please post the actual code that you used. The whole program. If you were to put some Serial.print()s in strategic places and printed relevant variables it would help you to figure out what is wrong.

here, this was what i tryed :

#include <Servo.h>
#include <Keypad.h>

Servo ServoMotor;
char* password = "53A"; // change the password here, just pick any 3 numbers
int position = 0;
const byte ROWS = 4;
const byte COLS = 4;
char keys[ROWS][COLS] = {
{'1','2','3','A'},
{'4','5','6','B'},
{'7','8','9','C'},
{'*','0','#','D'}
};

byte rowPins[ROWS] = { 8, 7, 6, 9 };
byte colPins[COLS] = { 5, 4, 3, 2 };
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
int RedpinLock = 12;
int GreenpinUnlock = 13;

void setup()
{
ServoMotor.attach(11);
LockedPosition(true);
}

void loop()
{
char key = keypad.getKey();
if (key == '*' || key == '#')
{
position = 0;
LockedPosition(true);
}
if (key == password[position])
{
position ++;
}
if (key != password)
{
position=0;
}
if (position == 3)
{
LockedPosition(false);
}
delay(100);
}
void LockedPosition(int locked)
{
if (locked)
{
digitalWrite(RedpinLock, HIGH);
digitalWrite(GreenpinUnlock, LOW);
ServoMotor.write(11);
}
else
{
digitalWrite(RedpinLock, LOW);
digitalWrite(GreenpinUnlock, HIGH);
ServoMotor.write(87);
}
}

jp1599:
here, this was what i tryed :

did you try example in reply #5 to this post?

Look at what the IDE autoformatting does. Isn't that a ton easier to read?

#include <Servo.h>
#include <Keypad.h>

Servo ServoMotor;
char* password = "53A";  // change the password here, just pick any 3 numbers
int position = 0;
const byte ROWS = 4;
const byte COLS = 4;
char keys[ROWS][COLS] = {
  {'1', '2', '3', 'A'},
  {'4', '5', '6', 'B'},
  {'7', '8', '9', 'C'},
  {'*', '0', '#', 'D'}
};

byte rowPins[ROWS] = { 8, 7, 6, 9 };
byte colPins[COLS] = { 5, 4, 3, 2 };
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
int RedpinLock = 12;
int GreenpinUnlock = 13;

void setup()
{
  ServoMotor.attach(11);
  LockedPosition(true);
}

void loop()
{
  char key = keypad.getKey();
  if (key == '*' || key == '#')
  {
    position = 0;
    LockedPosition(true);
  }
  if (key == password[position])
  {
    position ++;
  }
  if (key != password)
  {
    position = 0;
  }
  if (position == 3)
  {
    LockedPosition(false);
  }
  delay(100);
}
void LockedPosition(int locked)
{
  if (locked)
  {
    digitalWrite(RedpinLock, HIGH);
    digitalWrite(GreenpinUnlock, LOW);
    ServoMotor.write(11);
  }
  else
  {
    digitalWrite(RedpinLock, LOW);
    digitalWrite(GreenpinUnlock, HIGH);
    ServoMotor.write(87);
  }
}
  if (key != password)

key is a single char. password is an array of chars (actually pointers to chars). Could they ever be equal ?

You test key against the character in password that you are expecting using

  if (key == password[position])
  {
    position ++;
  }

If there is no match then set position back to zero

  if (key == password[position])
  {
    position ++;
  }
else
  {
    position = 0;
  }

UKHeliBob:

  if (key != password)

key is a single char. password is an array of chars (actually pointers to chars). Could they ever be equal ?

You test key against the character in password that you are expecting using

  if (key == password[position])

{
    position ++;
  }



If there is no match then set position back to zero


if (key == password[position])
  {
    position ++;
  }
else
  {
    position = 0;
  }

i did that but it´s still accepting other passwords as long they have 53A in them EX: 5436A :confused:

Post your complete program as it is now

correction, it is not even opening with the correct one, its stuck on locked mode :o

[pre]#include <Servo.h>
#include <Keypad.h>

Servo ServoMotor;
char* password = "53A";  // change the password here, just pick any 3 numbers
int position = 0;
const byte ROWS = 4;
const byte COLS = 4;
char keys[ROWS][COLS] = {
{'1','2','3','A'},
{'4','5','6','B'},
{'7','8','9','C'},
{'*','0','#','D'}
};

byte rowPins[ROWS] = { 8, 7, 6, 9 };
byte colPins[COLS] = { 5, 4, 3, 2 };
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
int RedpinLock = 12;
int GreenpinUnlock = 13;

void setup()
{
ServoMotor.attach(11);
LockedPosition(true);
}

void loop()
{
char key = keypad.getKey();
if (key == '*' || key == '#')
{
position = 0;
LockedPosition(true);
}
if (key == password[position])
{
position ++;
}
else 
{
  position = 0;
  }



if (position == 3)
{
LockedPosition(false);
}
delay(100);
}
void LockedPosition(int locked)
{
if (locked)
{
digitalWrite(RedpinLock, HIGH);
digitalWrite(GreenpinUnlock, LOW);
ServoMotor.write(11);
}
else
{
digitalWrite(RedpinLock, LOW);
digitalWrite(GreenpinUnlock, HIGH);
ServoMotor.write(87);
}[/pre]
}

  char key = keypad.getKey();Does this function wait for a key to be pressed or does it return a value immediately ? If the latter then during the time that the key is pressed the loop() function will repeat many times and the user will have entered the same character several times, hence their entry will not match the password.

Put some Serial.print()s in the program to see the value of key and position to confirm what is going on.

The keypad library has a function named waitForKey() that will wait for a key to be pressed before returning a value. This, of course, blocks program execution until a key is pressed but that may not be a problem with what you are trying to do.

UKHeliBob:
  char key = keypad.getKey();Does this function wait for a key to be pressed or does it return a value immediately ? If the latter then during the time that the key is pressed the loop() function will repeat many times and the user will have entered the same character several times, hence their entry will not match the password.

Put some Serial.print()s in the program to see the value of key and position to confirm what is going on.

The keypad library has a function named waitForKey() that will wait for a key to be pressed before returning a value. This, of course, blocks program execution until a key is pressed but that may not be a problem with what you are trying to do.

Wow , thank you so much for all your help, Thanks to the others too, it´s working now with Waitforkey() , here is the code is someone want s to see.
PS: Sorry, i´m new and i don t know how to use the ident tool so here it is, again thanks for all the help :smiley:

#include <Servo.h>
#include <Keypad.h>

Servo ServoMotor;
char* password = "53A";  // change the password here, just pick any 3 numbers
int position = 0;
const byte ROWS = 4;
const byte COLS = 4;
char keys[ROWS][COLS] = {
{'1','2','3','A'},
{'4','5','6','B'},
{'7','8','9','C'},
{'*','0','#','D'}
};

byte rowPins[ROWS] = { 8, 7, 6, 9 };
byte colPins[COLS] = { 5, 4, 3, 2 };
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
int RedpinLock = 12;
int GreenpinUnlock = 13;

void setup()
{
ServoMotor.attach(11);
LockedPosition(true);
}

void loop()
{
char key = keypad.waitForKey();
// char key = keypad.getKey();
if (key == '*' || key == '#')
{
position = 0;
LockedPosition(true);
}
if (key == password[position])
{
position ++;
}
 else 
{
 position = 0;
}



if (position == 3)
{
LockedPosition(false);
}
delay(100);
}
void LockedPosition(int locked)
{
if (locked)
{
digitalWrite(RedpinLock, HIGH);
digitalWrite(GreenpinUnlock, LOW);
ServoMotor.write(11);
}
else
{
digitalWrite(RedpinLock, LOW);
digitalWrite(GreenpinUnlock, HIGH);
ServoMotor.write(89);
}
}

PS: Sorry, i´m new and i don t know how to use the ident tool so here it is, again thanks for all the help

In the IDE, use tools -> auto format.