Arduino Project Issues with IF Statements

Question

Currently, I am making a project for my school and I am having an issue when the code runs multiple times. I am using an Arduino Mega 2560. The project I am doing is making a safe/box that requires a PIN to open it. In order of how things are supposed to operate it follows:

1. The IR receiver receives a signal from a remote that activates the signal. When the system is active it turns on a blue LED. When the system is inactive it does nothing.
2. When the system is active, it can read inputs from the 4x4 pin pad. If it gets a wrong code it turns on a red LED then clears the data so it can take in the code again. If the correct code is given then a green LED is turned on and turns a servo motor 90 degrees.
3. After the correct code is turned on, a button is pressed to return the motor to 0 degrees and deactivates the system.

The issue is that after the button is pressed I cannot reactivate the system. It basically becomes a 1 and done. The code for the project can be found below.

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

Servo servo;

const byte ROWS = 4; 
const byte COLS = 4; 

#define Passcode_L 5

char Data[Passcode_L];
char Master[Passcode_L]="14B6";

char hexaKeys[ROWS][COLS] = {
   {'1', '2', '3', 'A'},
   {'4', '5', '6', 'B'},
   {'7', '8', '9', 'C'},
   {'*', '0', '#', 'D'}
 };
  
byte rowPins[ROWS] = {22, 24, 26, 28}; 
byte colPins[COLS] = {30, 32, 34, 36}; 
byte data_count = 0;
char customKey;
  
Keypad customKeypad = Keypad(makeKeymap(hexaKeys), rowPins, colPins, ROWS, COLS); 

const int led_blue = 23;
const int led_green = 25;
const int led_red = 27;


const int BUTTON = 47;
int BUTTONstate = 0;


const int RECV_PIN = 44;
int togglestate = 0;
IRrecv irrecv(RECV_PIN);
decode_results results;

void setup(){
  Serial.begin(9600);
  irrecv.enableIRIn();
  servo.attach(38);
  servo.write(0);
  pinMode(led_blue, OUTPUT);
  pinMode(led_green, OUTPUT);
  pinMode(led_red, OUTPUT);
  pinMode(BUTTON, INPUT);
}
    
void loop(){
  char customKey = customKeypad.getKey();

  BUTTONstate = digitalRead(BUTTON);
  if (irrecv.decode(&results)){
    switch(results.value){
      case 0xFFA25D:
      // Toggle BLUE LED On or Off
      if(togglestate==0){
        digitalWrite(led_blue, HIGH);
        togglestate=1;
      }
      else {
        digitalWrite(led_blue, LOW);
        togglestate=0;
      }
    break;  
    }
    irrecv.resume();
  }
  
  if (customKey){
    Data[data_count] = customKey;
    data_count++;
  }
  
  if (data_count == Passcode_L - 1 && togglestate==1) {
     if (!strcmp(Data, Master)) {
      //password is correct
      digitalWrite(led_green, HIGH);
      delay(500);
      servo.write(90);
      delay(1500);
      digitalWrite(led_green, LOW);
     }
     else {
      // Password is incorrect
      digitalWrite(led_red, HIGH);
      delay(1000);
      digitalWrite(led_red, LOW);
    }
    clearData();
 }
 
 if (BUTTONstate !=0){
  servo.write(0);
  togglestate=0;
  digitalWrite(led_blue, LOW);
  }


  
}

void clearData() {
  // Go through array and clear data
  while (data_count != 0) {
    Data[data_count--] = 0;
  }
  return;
}

Answer 1

The logic in your program is fairly hard to follow, and change without breaing everything... I think the problem is a design issue. Your code is written to react to events, but does not properly track in which state you device is. And state is. Keeping track of state is very important. It's probably the most important thing to do when programming embedded devices. And for a lock, I really think it definitely is the most important thing to keep track of.

Try this: you have a lock, it has these distinct states:

• locked (stable state)
• entering passcode (transitional -> error or unlocking) (needs aborted entry timeout?)
• show passcode error (transitional -> locked)
• unlocking (transitional -> unlocked)
• unlocked (stable)
• locking (transitional -> locked)

If you keep track of the state of your device, and separating the logic for each state, the logic will become much easier to manage and debug. It also makes adding features and extra logic almost trivial. I thin you should try something like this:

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

const byte ROWS = 4; 
const byte COLS = 4; 
  
// IO pins

byte rowPins[ROWS] = {22, 24, 26, 28}; 
byte colPins[COLS] = {30, 32, 34, 36}; 
  
const int led_blue = 23;
const int led_green = 25;
const int led_red = 27;
const int BUTTON = 47;
const int RECV_PIN = 44;

// devices

Servo servo;
IRrecv irrecv(RECV_PIN);

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

Keypad customKeypad = Keypad(makeKeymap(hexaKeys), rowPins, colPins, ROWS, COLS); 

// lock states    
class enum LockState
{
    locked,
    passcodeEntry,
    passcodeError,
    unlocking,
    unlocked,
    locking,
};

// lock data
LockState lockState = locked;
byte data_count = 0;
bool passcodeOK = false;
unsigned int timeoutTS;

const char Master[] = "14B6";
#define Passcode_L (sizeof(Master) - 1)
#define TIMEOUT (30000)  // 30 seconds timeout for data entry

void setup()
{
  Serial.begin(9600);
  irrecv.enableIRIn();
  servo.attach(38);
  servo.write(0);
  pinMode(led_blue, OUTPUT);
  pinMode(led_green, OUTPUT);
  pinMode(led_red, OUTPUT);
  pinMode(BUTTON, INPUT);
}
    
void loop() 
{
  switch (lockState) 
  {
  default:
  case LockState::locked:
    {
      // locked, resting.  turn LEDs off, and clear received key code.
      digitalWrite(led_blue, LOW);
      digitalWrite(led_red, LOW);
      digitalWrite(led_green, LOW);
      data_count = 0;

      decode_results results;
      if (irrecv.decode(&results) && results.value == 0xFFA25D) 
      {
        timeoutTS = (unsigned int)millis();
        lockState = LockState::passcodeEntry;
      }
    }
    break;
  
  case LockState::passcodeEntry:
    {
      // read keypad, blue LED is on.  
      digitalWrite(led_blue, HIGH);

      // this assumes customKeypad debounces the key and returns a non-zero
      // value only once for each key, when it is initially pressed.
      char customKey = customKeypad.getKey(); 
      if (customKey)
      {
        timeoutTS = (unsigned int)millis();

        if (data_count == 0)
          passcodeOK = true;

        // check passcode.
        passcodeOK &= (customKey == Master[data_count++]);
        if (data_count >= Passcode_L) 
        {
          if (passcodeOK)
            lockState = LockState::unlocking;
          else
            lockState = LockState::passcodeError;
        }

        // check for timeout, we wouldn't want to let anyone enter codes
        // if the owner leaves the room now with the IR remote in his pocket
        if ((unsigned int)millis() - timeoutTS > TIMEOUT) 
            lockState = LockState::locked;
    } 
    break;

  case LockState::passcodeError:
    {
      // turn on red LED for 1 second. then let use retry a passcode. 
      digitalWrite(led_red, HIGH);
      delay(1000);
      digitalWrite(led_red, LOW);
      data_count = 0;
      lockState = LockState::passcodeEntry;
    }
    break;

  case LockState::unlocking:
    {
      digitalWrite(led_green, HIGH);
      servo.write(90);
      delay(1500);
      digitalWrite(led_green, LOW);
      lockState = LockState::unlocked;
    }
    break;

  case LockState::unlocked:
    {
      // wait for button press.
      if (digitalRead(BUTTON))
        lockState = LockState::locking;
    }
    break;

  case LockState::locking:
    {
       // turn the lock back, go back to locked state
       servo.write(0);
       loackState = LockState::locked;
    }
    break;
  }
}

I don't have an arduino with me, and am not setup at the moment to compile a sketch, so there may be a couple compile errors, but the logic should be very close to what you want to achieve.