Help me please!

This is a simulation program error code.
Please see this link. There may be some conditions that need more.

#include <Keypad.h>
#include <LiquidCrystal.h>

#define Password_Lenght 4
LiquidCrystal LCD(A0, A1, A2, A3, A4, A5);
char Data_one[Password_Lenght];
char Data_two[Password_Lenght];

byte maxPasswordLength = 4;
byte currentPasswordLength = 0;
byte data_count_one = 0,data_count_two = 0, master_count = 0;
boolean Pass_is_good1 = false;
boolean Pass_is_good2 = false;
boolean Pass_is_good3 = false;
boolean Pass_is_good4 = false;
char customKey_one,customKey_two;
int state = 0;

const byte ROWS = 4;
const byte COLS = 3;
char keys[ROWS][COLS] = {
{‘1’,‘2’,‘3’},
{‘4’,‘5’,‘6’},
{‘7’,‘8’,‘9’},
{’*’,‘0’,’#’}
};

byte rowPins[ROWS] = {10, 9, 8, 7};
byte colPins[COLS] = {13, 12, 11};

Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
void setup()
{
LCD.begin(16, 2);

}

void lock()
{

}

void unlock()
{

}

void loop()
{
if(state == 0)
{
while(1)
{
customKey_one = keypad.getKey();
if (customKey_one)
{
if(customKey_one == ‘#’)
{
break;
}
Data_one[data_count_one] = customKey_one;
LCD.setCursor(data_count_one,1);
LCD.print(Data_one[data_count_one]);
data_count_one++;

}
}

LCD.clear();
LCD.setCursor(0, 0);
LCD.print(“lock”);
lock();
state = 1;
}
else
{
while(1)
{
customKey_two = keypad.getKey();
if (customKey_two)
{
if(customKey_two == ‘#’)
{
break;
}
Data_two[data_count_two] = customKey_two;
LCD.setCursor(data_count_two,1);
LCD.print(Data_two[data_count_two]);
data_count_two++;

}
}

LCD.clear();
LCD.setCursor(0, 0);

if(Data_two[0] == Data_one[0])
{
Pass_is_good1 = true;
}
else { Pass_is_good1 = false; }
if(Data_two[1] == Data_one[1])
{
Pass_is_good2 = true;
}
else { Pass_is_good2 = false; }
if(Data_two[2] == Data_one[2])
{
Pass_is_good3 = true;
}
else { Pass_is_good3 = false; }
if(Data_two[3] == Data_one[3])
{
Pass_is_good4 = true;
}
else { Pass_is_good4 = false; }

if( (Pass_is_good1 == true)&&(Pass_is_good2 == true)&&(Pass_is_good3 == true)&&(Pass_is_good4 == true) )
{
LCD.print(“Unlock”);
unlock();
state = 0;
}
else
{
LCD.print(“lock”);
lock();
state = 0;
}

}

}

What was the error / problem?

Alright, let’s start easy. If you read the How to use this forum you should have known you should use code tags. Please edit your post and add them to the code. Available in the menu by clicking </> And when you’re already editing, give the post a decent name. We know you want help, that’s why you’re here…

Second, thumbs up for posting a video to show what happens. But big thumbs up by not describing what you think should happen. Al right, here it’s reasonable clear but it’s still a guess.

Alright, because you only vaguely pointed to the problem I’m not going to point to the problem. I’m going to point out things that are wrong or a bad habit in programming (at least C++). Correcting that should make it more clear where the problem is and how to prevent things like that in the future.

Let’s start! Press for fun Ctrl + T in the Arduino IDE. Do you notice the code is wayyyyy more readable all of a sudden?

if(condition){
  //some code
}
//or
if(condition)
{
  //some code
}

Are both valid. But for the sake of keeping code consistent and readable don’t mix them! Choose you’re style and stick with it!

Same goes for variable name. Pick a convention and stick with it! In C++ the default is myVariableName (so smallThenEachBigLetter). Some (but not all) like I do, like to use BigAndThenBigLetter for a const to remind myself I can’t change the value.

Now we’re at the topic of variables, try to pick better names. Yes, it’s harder then you think. But it makes code so much clearer! maxPasswordLength is a good variable name. But Data_one (setting aside all the bad convention practices) is just a crappy name. setPassword and typedPassword already explains a hell lot more! Most names are rubbish.

Now to the loop, what do you think loop means? So why do a ugly while(true) and a break? You can just remove it an use the loop.

So you set a maxPasswordLength variable. Why not use THAT to make the password arrays? Now you have maxPasswordLength and Password_Lenght for the same thing. Skip duplicated variables and use

const byte MaxPasswordLength = 4;
LiquidCrystal LCD(A0, A1, A2, A3, A4, A5);
char Data_one[MaxPasswordLength];
char Data_two[MaxPasswordLength];

Using 4 varaibels to check all the characters is a bit of a wast. You could just loop through each digit and check it in a loop. I would even make it a nice and small function

bool passwordCorrect(){
  for(byte i = 0; i < MaxPasswordLength; i++){
    if(setPassword[i] != typedPassword[i]){
      return false;
    }
  }
  return true;
}

And do you see how I defined the variable i inside the for-loop? I don’t make it global because I don’t need it to be global. You should do the same, define them where you need them. Only make them global if you need them everywhere. But for things like customKey_one and Pass_is_good1 etc I see no reason to not just declare them when you fill them.

And now I’m talking password length (and this is big hint 1), what do you think happens when you do enter a password of 5? You say you have a password maxPasswordLength of 4 but nothing stops me from entering a shorter OR longer password.

And hint 2, you end the password entering with a # and start checking. If it’s correct you unlock it. If it’s not correct you keep it locked. But how does that leave every variable involved with the unlocking? And if you don’t know, maybe start to debug a bit? Print variables to serial monitor for example.

More advanced, don’t you think the sequence to type in the lock and unlock code look an awful lot alike? I would have made one function I could use in the setting the lock code and for typing the unlock code.