I have 4 switches that need to be activated in a certain order to trigger the maglock. The code as-is works fine but my problem is that if the switches are triggered out of sequence they all need to be turned off before restarting. I tried using the break function in an if nested if statement but it seems to exit all of the while loops.
How can I make the code only back down only one level at a time and still have the previous switch sequence valid?
Instead of reading the switches inside the conditional, read all 4 switches into ints, essentially taking a snapshot, then evaluate based on the 4 together. When they are inside the conditional, they are called one at a time based on order of operations.
P.S. avoid using "while" and make your conditionals if statements.
P.P.S avoid using "delay" and make your delays conditional if statements based on millis().
Kirkg:
I have 4 switches that need to be activated in a certain order to trigger the maglock. The code as-is works fine but my problem is that if the switches are triggered out of sequence they all need to be turned off before restarting.
That sounds like a standard security feature.
If you really do want to deal with things separately then I would use IF everywhere you have WHILE. And that may need some other changes.
Why don't you use buttons rather than switches, and look for them to become pressed, then you don't have to switch them back later on: they'll be released right after being pressed.
I would take a totally different approach to yours.
I would store the target pin sequence in an array. (Which as a side benefit makes it easier to change the password one day!) Have a boolean flag allCorrectSoFar, initialised true.
Also have the button pins themselves in an array.
Have a counter for how many positions have been checked.
At the top of loop look for any button to show a state change: from its position in the button array, you will know which button it is, and from the counter you will know its position in the sequence. Compare the actual button to the target button. If match, do nothing. If no match, set the allCorrectSoFar flag to false. (But don't alert the user, else they could eventually figure out the sequence.)
Repeat until all positions are checked. Then if flag is still true, Open Sesame. If any of the positions caused the flag to go false, remain in the off state, and the buttons are ready for the next attempt.
Robin2:
That sounds like a standard security feature.
If you really do want to deal with things separately then I would use IF everywhere you have WHILE. And that may need some other changes.
...R
I actually started the code using the IF statement. The problem was that it would no account for the sequence. for example, if the first one was is off and the next 3 are on once the first is turned on it unlocks not caring that the other three dint come next.
Your right maybe in just overthinking it..."it does sound like a standers security feature".
// https://forum.arduino.cc/index.php?topic=638948
// assume locked when Arduino powers up
// press 4 buttons in sequence to unlock
// and added a 5th button to re-lock
// has bwod pulse on L13
bool seeDebugPrints = false; //set this true or false depending on if you want to see the keys as they are hit
//states
enum {ST_LOCKED, ST_UNLOCKED} currentState = ST_LOCKED;
bool newlyArrivedInthisState = true;
// the buttons
byte buttonPins[] = {9, 10, 11, 12}; //the buttons must be wired from pin to ground, pinmodes are input_pullup
const byte howManyButtons(sizeof(buttonPins) / sizeof(byte));
bool buttonStates[howManyButtons]; // current state of the button
bool lastButtonStates[howManyButtons]; // previous state of the button
bool aButtonWasPressed = false;
byte thisButtonWasPressed;
byte targetSequence[] = {3, 2, 1, 0}; // these are indices, not pin numbers (which are acually 12,11,10,9)
byte howManyButtonsEntered;
bool allButtonsCorrect = true;
// and the re-lock button
byte reLockButton = 8;
//the pulse led
int pulseLedInterval = 500;
unsigned long previousMillisPulse;
bool pulseState = false;
//the lock leds
const byte unlockedLed = 16;
const byte lockedLed = 14;
void setup()
{
// initialize serial communication:
Serial.begin(9600);
Serial.println("setup() ... ");
Serial.print(howManyButtons);
Serial.println(" buttons in sequence to unlock");
Serial.print("Compiler: ");
Serial.print(__VERSION__);
Serial.print(", Arduino IDE: ");
Serial.println(ARDUINO);
Serial.print("Created: ");
Serial.print(__TIME__);
Serial.print(", ");
Serial.println(__DATE__);
Serial.println(__FILE__);
Serial.println(" ");
// initialize the button pins as input with pullup so active low
// make sure the button is from pin to ground
for (int i = 0; i < howManyButtons; i++)
{
pinMode(buttonPins[i], INPUT_PULLUP);
//initialize button states
buttonStates[i] = digitalRead(buttonPins[i]);
lastButtonStates[i] = buttonStates[i];
}
pinMode(reLockButton, INPUT_PULLUP);
//initialise leds
pinMode(LED_BUILTIN, OUTPUT);
digitalWrite(LED_BUILTIN, pulseState);
pinMode(lockedLed, OUTPUT);
pinMode(unlockedLed, OUTPUT);
Serial.print("Debug prints are ");
if (seeDebugPrints) Serial.println("ON");
else Serial.println("OFF");
Serial.println("setup() done");
Serial.println(" ");
}//setup
void loop()
{
pulse();
checkForButtonStateChange();
manageTheLock();
} //loop
void pulse()
{
if (millis() - previousMillisPulse >= pulseLedInterval)
{
previousMillisPulse = millis();
pulseState = !pulseState;
digitalWrite(LED_BUILTIN, pulseState);
}
} //pulse
void checkForButtonStateChange()
{
for (int i = 0; i < howManyButtons; i++)
{
buttonStates[i] = digitalRead(buttonPins[i]);
// compare the buttonState to its previous state
if (buttonStates[i] != lastButtonStates[i]) // means it changed... but which way?
{
if (buttonStates[i] == LOW) // changed to pressed
{
aButtonWasPressed = true;
thisButtonWasPressed = i;
if (seeDebugPrints)
{
Serial.print("Newly pressed ");
Serial.println(thisButtonWasPressed);
}
}
// poor man's de-bounce
delay(50);
}
// save the current state as the last state, for next time through the loop
lastButtonStates[i] = buttonStates[i];
}
} // checkForButtonStateChange()
void manageTheLock()
{
switch (currentState)
{
case ST_LOCKED:
if (newlyArrivedInthisState)
{
Serial.println("Door is LOCKED");
Serial.print(" Press ");
Serial.print(howManyButtons);
Serial.println(" keys in the correct sequence to unlock...");
newlyArrivedInthisState = false;
digitalWrite(unlockedLed, LOW);
digitalWrite(lockedLed, HIGH);
}
if (aButtonWasPressed)
{
aButtonWasPressed = false;
if (seeDebugPrints)
{
Serial.print("Entered ");
Serial.print(thisButtonWasPressed);
Serial.print(", expecting ");
Serial.println(targetSequence[howManyButtonsEntered]);
}
else
{
Serial.print("*");
}
if (thisButtonWasPressed != targetSequence[howManyButtonsEntered]) allButtonsCorrect = false;
howManyButtonsEntered++;
}
if (howManyButtonsEntered == 4)
{
Serial.println(" ");
Serial.print(howManyButtons);
Serial.print(" buttons entered...");
if (!allButtonsCorrect)
{
currentState = ST_LOCKED;
Serial.println(" but you messed up");
allButtonsCorrect = true;
}
else
{
currentState = ST_UNLOCKED;
Serial.println(" correctly");
}
newlyArrivedInthisState = true;
howManyButtonsEntered = 0;
}
break;
case ST_UNLOCKED:
if (newlyArrivedInthisState)
{
Serial.println("Door is UNLOCKED");
Serial.println(" Press the re-lock button to lock");
digitalWrite(unlockedLed, HIGH);
digitalWrite(lockedLed, LOW);
newlyArrivedInthisState = false;
}
if (!digitalRead(reLockButton))
{
Serial.println(" Locking the door...");
currentState = ST_LOCKED;
newlyArrivedInthisState = true;
}
break;
}//switch
}//manageTheLock
[/code
Kirkg:
I actually started the code using the IF statement. The problem was that it would no account for the sequence. for example, if the first one was is off and the next 3 are on once the first is turned on it unlocks not caring that the other three dint come next.
Your right maybe in just overthinking it..."it does sound like a standers security feature".
If you need the sequence to be specific, are the inputs momentary or latching? Ideally, I would not use blocking code such as while and delay, and momentary inputs (that reset themselves with each push) along with hardware and software debounce, and take each input into a series of if statements with global flags, not digitalRead(), that would be reset after a time out interval.
Kirkg:
The issue I found using break; is that it seems to break all the nested while loops at once so then the program doesn't know the original order.
Seems to ? It doesn't. Its very explicitly breaking out of one loop or switch only, the innermost one, just like return breaks out of one function call only, the innermost one.
Why in heaven's name do you have 3 nested while loops? What are you even trying to do? Whatever you think you're doing, I can guarantee there's an infinitely better way.
The entire point of the loop function is to loop. Putting a near-endless loop inside of it is unnecessary.
Kirkg:
Your right maybe in just overthinking it..."it does sound like a standers security feature".
That has to be a joke. And even if it is a security feature you want, it should be a deliberate part of the design and not an accident of poor code structure.
Just to add, whenever I ‘need’ to nest loops (why?), or use recursion (ok), I keep a local static counter in the function - to keep track of the depth of activity.
This can be used for many purposes - not the least to avoid runaway memory problems, or detect position within parsers etc.
Jiggy-Ninja:
What are you even trying to do? Whatever you think you're doing, I can guarantee there's an infinitely better way.
Pretty much an x-y problem: trying to kluge one's way out of a situation one could have avoided.
Even if the contacts need to be switches rather than the buttons I used in my code a few posts above, my code can be adapted for that. Simply prevent the switches being read for an unlock in ST_LOCKED until the switches have all been read as open in ST_LOCKED, to "prime" the system for an unlock sequence. (From a practical point of view, probably by just adding a new state. Move out of ST_LOCKED to ST_PRIMED by clearing the switches, and reading them for an unlock in ST_PRIMED rather than ST_LOCKED, then into ST_UNLOCKED as before.)
Code below implements previous post. Only goes to PRIMED to accept new unlock sequence when the switches are all found to be open in LOCKED.
// https://forum.arduino.cc/index.php?topic=638948
// VERSION 2
// assume locked when Arduino powers up
// unlock with 4 switches in sequence (not buttons as before)
// *** switches must all be opened after a locking, to prime for next unlock ***
// and added a 5th button to re-lock
// has bwod pulse on L13
bool seeDebugPrints = true; //set this true or false depending on if you want to see the keys as they are hit
//states
enum {ST_LOCKED, ST_PRIMED, ST_UNLOCKED} currentState = ST_LOCKED;
bool newlyArrivedInthisState = true;
// the buttons
byte buttonPins[] = {9, 10, 11, 12}; //the buttons must be wired from pin to ground, pinmodes are input_pullup
const byte howManyButtons(sizeof(buttonPins) / sizeof(byte));
bool buttonStates[howManyButtons]; // current state of the button
bool lastButtonStates[howManyButtons]; // previous state of the button
bool aButtonWasPressed = false;
byte thisButtonWasPressed;
byte targetSequence[] = {3, 2, 1, 0}; // these are indices, not pin numbers (which are acually 12,11,10,9)
byte howManyButtonsEntered;
bool allButtonsCorrect = true;
bool allButtonsOpen;
// and the re-lock button
byte reLockButton = 8;
//the pulse led
int pulseLedInterval = 500;
unsigned long previousMillisPulse;
bool pulseState = false;
//the lock leds
const byte unlockedLed = 16;
const byte lockedLed = 14;
const byte primedLed = 15;
void setup()
{
// initialize serial communication:
Serial.begin(9600);
Serial.println("setup() ... ");
Serial.print(howManyButtons);
Serial.println(" switches in sequence to unlock");
Serial.print("Compiler: ");
Serial.print(__VERSION__);
Serial.print(", Arduino IDE: ");
Serial.println(ARDUINO);
Serial.print("Created: ");
Serial.print(__TIME__);
Serial.print(", ");
Serial.println(__DATE__);
Serial.println(__FILE__);
Serial.println(" ");
// initialize the button pins as input with pullup so active low
// make sure the button is from pin to ground
for (int i = 0; i < howManyButtons; i++)
{
pinMode(buttonPins[i], INPUT_PULLUP);
//initialize button states
buttonStates[i] = digitalRead(buttonPins[i]);
lastButtonStates[i] = buttonStates[i];
}
pinMode(reLockButton, INPUT_PULLUP);
//initialise leds
pinMode(LED_BUILTIN, OUTPUT);
digitalWrite(LED_BUILTIN, pulseState);
pinMode(lockedLed, OUTPUT);
pinMode(unlockedLed, OUTPUT);
pinMode(primedLed, OUTPUT);
Serial.print("Debug prints are ");
if (seeDebugPrints) Serial.println("ON");
else Serial.println("OFF");
Serial.println("setup() done");
Serial.println(" ");
}//setup
void loop()
{
pulse();
checkForButtonStateChange();
manageTheLock();
} //loop
void pulse()
{
if (millis() - previousMillisPulse >= pulseLedInterval)
{
previousMillisPulse = millis();
pulseState = !pulseState;
digitalWrite(LED_BUILTIN, pulseState);
}
} //pulse
void checkForButtonStateChange()
{
for (int i = 0; i < howManyButtons; i++)
{
buttonStates[i] = digitalRead(buttonPins[i]);
// compare the buttonState to its previous state
if (buttonStates[i] != lastButtonStates[i]) // means it changed... but which way?
{
if (buttonStates[i] == LOW) // changed to pressed
{
aButtonWasPressed = true;
thisButtonWasPressed = i;
if (seeDebugPrints)
{
Serial.print("Newly pressed ");
Serial.println(thisButtonWasPressed);
}
}
// poor man's de-bounce
delay(50);
}
// save the current state as the last state, for next time through the loop
lastButtonStates[i] = buttonStates[i];
}
} // checkForButtonStateChange()
void manageTheLock()
{
switch (currentState)
{
case ST_LOCKED:
if (newlyArrivedInthisState)
{
Serial.println("Door is LOCKED");
Serial.print(" Open ");
Serial.print(howManyButtons);
Serial.println(" keys to prime system for unlocking...");
newlyArrivedInthisState = false;
digitalWrite(unlockedLed, LOW);
digitalWrite(lockedLed, HIGH);
}
//check that all buttons are open
allButtonsOpen = true;
for (int i = 0; i < howManyButtons; i++)
{
buttonStates[i] = digitalRead(buttonPins[i]);
if (!buttonStates[i]) allButtonsOpen = false;
}
if (allButtonsOpen)
{
currentState = ST_PRIMED;
Serial.println(" All switches open....");
newlyArrivedInthisState = true;
}
break;
case ST_PRIMED:
if (newlyArrivedInthisState)
{
Serial.println("Door is LOCKED, system is primed");
Serial.print(" Press ");
Serial.print(howManyButtons);
Serial.println(" keys in the correct sequence to unlock...");
newlyArrivedInthisState = false;
digitalWrite(primedLed, HIGH);
}
if (aButtonWasPressed)
{
aButtonWasPressed = false;
if (seeDebugPrints)
{
Serial.print("Entered ");
Serial.print(thisButtonWasPressed);
Serial.print(", expecting ");
Serial.println(targetSequence[howManyButtonsEntered]);
}
else
{
Serial.print("*");
}
if (thisButtonWasPressed != targetSequence[howManyButtonsEntered]) allButtonsCorrect = false;
howManyButtonsEntered++;
}
if (howManyButtonsEntered == howManyButtons)
{
Serial.println(" ");
Serial.print(howManyButtons);
Serial.print(" buttons entered...");
if (!allButtonsCorrect)
{
currentState = ST_LOCKED;
Serial.println(" but you messed up");
allButtonsCorrect = true;
}
else
{
currentState = ST_UNLOCKED;
Serial.println(" correctly");
}
newlyArrivedInthisState = true;
howManyButtonsEntered = 0;
}
break;
case ST_UNLOCKED:
if (newlyArrivedInthisState)
{
Serial.println("Door is UNLOCKED");
Serial.println(" Press the re-lock button to lock");
digitalWrite(unlockedLed, HIGH);
digitalWrite(lockedLed, LOW);
digitalWrite(primedLed, LOW);
newlyArrivedInthisState = false;
}
if (!digitalRead(reLockButton))
{
Serial.println(" Locking the door...");
currentState = ST_LOCKED;
newlyArrivedInthisState = true;
}
break;
}//switch
}//manageTheLock