IF statement conditionals

i am quite new to the programming aspect and im making a key pad door lock with a servo and if i enter the correct password it always goes to the incorrect (red) led and Im probably being stupid but i cant work out what the issue is. Thanks if you can help :slight_smile:

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

const byte ROWS = 4; //four rows
const byte COLS = 4; //four columns
int GREEN = 12;
int RED = 13;
int pas_count = -1;
String pas_1;
String pas_2;
String pas_3;
String pas_4;
String FullPassword;
int time_delay = 0;
int pos = 0;
//define the cymbols on the buttons of the keypads
char hexaKeys[ROWS][COLS] = {
  {'1','2','3','A'},
  {'4','5','6','B'},
  {'7','8','9','C'},
  {'*','0','#','D'}
};
byte rowPins[ROWS] = {9, 8, 7, 6}; //connect to the row pinouts of the keypad
byte colPins[COLS] = {5, 4, 3, 2}; //connect to the column pinouts of the keypad

//initialize an instance of class NewKeypad
Keypad customKeypad = Keypad( makeKeymap(hexaKeys), rowPins, colPins, ROWS, COLS); 

void setup(){
  pinMode(GREEN, OUTPUT);
  pinMode(RED, OUTPUT);
  Serial.begin(9600);
  myservo.attach(10);
  myservo.write(pos);
  
  digitalWrite(GREEN, HIGH);
  digitalWrite(RED, HIGH);
  delay(250);
  digitalWrite(GREEN, LOW);
  digitalWrite(RED, LOW);
  delay(250);
  digitalWrite(GREEN, HIGH);
  digitalWrite(RED, HIGH);
  delay(250);
  digitalWrite(GREEN, LOW);
  digitalWrite(RED, LOW);
  delay(250);
  digitalWrite(GREEN, HIGH);
  digitalWrite(RED, HIGH);
  delay(250);
  digitalWrite(GREEN, LOW);
  digitalWrite(RED, LOW);
  delay(250);
  digitalWrite(GREEN, HIGH);
  digitalWrite(RED, HIGH);
  delay(2000);
  digitalWrite(GREEN, LOW);
  digitalWrite(RED, LOW);

  
}
  
void loop(){
  char customKey = customKeypad.getKey();
  
  if (customKey){
    delay(1);
    pas_count += 1;
    if (pas_count == 0){
      pas_1 = customKey;
    }

    if (pas_count == 1){
      pas_2 = customKey;
    }

    if (pas_count == 2){
      pas_3 = customKey;
    }

    if (pas_count == 3){
      pas_4 = customKey;
      FullPassword = pas_1 + pas_2 + pas_3 + pas_4;
      Serial.println(FullPassword);
      
      if (FullPassword == 4089 && pos == 180){
        pos = 0;
        myservo.write(pos);
        pas_count = -1;
        time_delay = 0;
        digitalWrite(GREEN, HIGH);
        delay(2000);
        digitalWrite(GREEN, LOW);
      }
      
      else if (FullPassword == 4089 && pos == 0){
        pos = 180;
        myservo.write(pos);
        pas_count = -1;
        time_delay = 0;
        digitalWrite(GREEN, HIGH);
        delay(2000);
        digitalWrite(GREEN, LOW);
      }

      else if (FullPassword != 4089){
        pas_count = -1;
        time_delay += 5000;
        digitalWrite(RED, HIGH);
        delay(time_delay);
        digitalWrite(RED, LOW);
      }
    
    }
   
}
}

Many comments I would like to make about this code. But lets deal with the issue.

Examine this code

if (FullPassword == 4089 && pos == 180)

You defined FullPassword as a string, yet you are comparing it to a number. Not going to work.
You have 2 ways to fix.

Convert FullPassword to an int and do an int compare.
Compare FullPassword to a string.

Try this

if (FullPassword == "4089" && pos == 180)

Next using String is bad, you need to learn to use c style strings. Google it. Depending on String can lead to hair pulling in the future.

Also, are you sure pos is going to == 180?

Thanks a lot for the help and as you can see im not the best at programming :slight_smile: and yes the servo is a 180 degree one so ranges for a position 0 to 180 and i set pos to 180 after the pos == 0 statment

OP,

Please examine this code.

It needs cleaning and i do not have a keypad so i had to simulate.

but it does show the basics of how you can do this without String and with a cleaner code base.

Please note that function getKeyPress is to simulate your keypad as I do not have one.

*** Updated code with some comments and a failed use case so you have it when you convert this.
To your code.

Notice how cleaner and more compact this code is. It also avoids the use of String. You need to learn that they are bad and should not be used in most cases.

const unsigned int passwordCheck = 4089;
char password[5];
short keyPressCounter = 0;

void setup()
{

  memset(password, 0, sizeof(password)); // set the char array to all nulls.
  Serial.begin(115200);
  Serial.println("Hello!");

}

//My simulatore for your keypad.
char getKeyPress()
{
  switch (keyPressCounter)
  {
    case 0:
      return '4';
    case 1:
      return '0';
    case 2:
      return '8';
    case 3:
      return '9';

  }

}


void loop()
{
  password[keyPressCounter] = getKeyPress(); //Get the keypress and store into char array
  keyPressCounter++; // move counter forward one.
  if (keyPressCounter == 4) //Check to see if we had 4 key presses
  {
    if (atoi(password) == passwordCheck)  //convert password to number and compare numeric
    {
      Serial.print("Password Matches: ");
      Serial.println(password);

    }
    else
    {
      Serial.print("Password Does Not Match: ");
      Serial.println(password);
    }

    keyPressCounter = 0;  //Reset counter for next attempt
    password[0] = '\0'; // Reset char array
  }


}

I have just uploaded the previous code with the "" around the 4089 and it works but i can now see how to do this a simpler way without a load of variables and without the use of strings. Nice to have the experienced help out the learning

thanks again :slight_smile:

obo4:
I have just uploaded the previous code with the "" around the 4089 and it works but i can now see how to do this a simpler way without a load of variables and without the use of strings. Nice to have the experienced help out the learning

thanks again :slight_smile:

obo4:
I have just uploaded the previous code with the "" around the 4089 and it works but i can now see how to do this a simpler way without a load of variables and without the use of strings. Nice to have the experienced help out the learning

thanks again :slight_smile:

obo4:
I have just uploaded the previous code with the "" around the 4089 and it works but i can now see how to do this a simpler way without a load of variables and without the use of strings. Nice to have the experienced help out the learning

thanks again :slight_smile:

OP I hope you see my example above. I did clean it up and comment a bit, please make sure you run it and examine how it works. Please feel free to reach out if you need more help.