Locking out inputs and recalling states. I have the stupid. Please help.

Projects Programming
1

Hello

I've got another one that's really hurting and would dearly love some help as I've been struggling for about ten hours with this and am chasing my tail with what should quite simple things.

I've cobbled the following code together out of the larger sketch I'm working on to demonstrate my lack of understanding and dimness a little clearer.

Please can somebody help me ...

The "locking out" and "recalling last state" are the problems.

There are three toggling inputs with no bounce:

(pin A1)- Lock mode/ free mode
(pin A2)- RED led on/off
(pin A3)- GREEN led on/off

I have three LEDs representing three situations.

LED 1 is "lock" mode.(pin 3)
LED 2 RED (pin 5)
LED 3 GREEN (pin 6)

Rules:
When LED 1 is on , RED and GREEN must turn off (if they are on).
When LED 1 is on, RED and GREEN must not be able to turn on.

When LED 1 is turned off, the RED and GREEN must revert to the on/off state they were in before the LED 1 was turned on- and toggle on off from that.

Powers up in "unlock" mode with Red and Green switching on/off with first button presses.

I'm pretty upset with myself for not being able to solve this. I love writing sketches but honestly have some kind of dyslexia with it.
As I said, this sketch summarises the problem I'm having in a larger sketch so don't want to re-write in too much of a different way..and I know there are much shorter and more efficient ways. Simple to understand is key for me. :astonished:

Any help truly appreciated.

int state = LOW;     
int reading;          
int previous = LOW;    
int remlock = HIGH;   
int stater = HIGH;   
int readingr;
int previousr = LOW;

int stateg = HIGH;
int readingg;
int previousg = LOW;




void setup()
{
 
  pinMode(A0, INPUT);
  pinMode(A1, INPUT);
  pinMode(A2, INPUT);

  
  digitalWrite(A0, HIGH);
  digitalWrite(A1, HIGH);
  digitalWrite(A2, HIGH);

  
  pinMode(3, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);

 
  digitalWrite(3, HIGH);
  digitalWrite(5, HIGH);
  digitalWrite(6, HIGH);
  digitalWrite(9, HIGH);
  digitalWrite(10, HIGH);
  digitalWrite(11, HIGH);


}


void loop() {

  if  (digitalRead(A0) == LOW) {
    remote();
  }

  else if (digitalRead(A1) == LOW) {
    red();
  }

  else if (digitalRead(A2) == LOW) {
    green();
  }

}



void remote () {
  delay(1); 

  reading = digitalRead (A0);
  if (reading == LOW && previous == HIGH) {
    state = !state; 
  }

  if (state == LOW) 
  { digitalWrite(3, 0); 
    digitalWrite(5, 1); 
    digitalWrite(6, 1);


}

  if (state == HIGH) 
  { digitalWrite(3, 1); 
    digitalWrite(5, stater); 
    digitalWrite(6, stateg); 
   
   
    //digitalWrite(5, previousr);
    //digitalWrite(6, previousg);
    //digitalWrite(5, readingr);
    //digitalWrite(6, readingg);
    // none work
  }
  previous = reading;

}

void red() {
  delay(1); 
  readingr = digitalRead (A1);
  if (readingr == LOW && previousr == HIGH) {
    stater = !stater; 
  }

  if (stater == LOW) digitalWrite(5, HIGH);
  if (stater == HIGH) digitalWrite(5, LOW);

  previousr = readingr;
  
}


void green () {
  delay(1); 
  readingg = digitalRead (A2);
  if (readingg == LOW && previousg == HIGH) {
    stateg = !stateg;
  }

  if (stateg == LOW) digitalWrite(6, HIGH);
  if (stateg == HIGH) digitalWrite(6, LOW);

  previousg = readingg;

}
2

Get rid of the commented out code!

Get rid of the goto statements.

The code does something. What it actually does is quite unclear.

3

PaulS:
Get rid of the commented out code! Done

Get rid of the goto statements.

The one commented out "goto 10" was a bit tongue in cheek. Done

The code does something. What it actually does is quite unclear.

It works to a point. It toggles 3 LEDs on and off with conditions. Can you run it?

I hope you can help with what I'm having problems with.

4

I guess I need to add some more detail..

There are three LEDS on three different outputs.

There are 3 toggling inputs.

The inputs toggle the LEDs on and off.

All this works okay.

When led number 1 is turned on, I want to turn off the other two, and not let them turn on again until led 1 is turned off.
When led number 1 is turned off, it recalls the state of the other two - and they can toggle on and off again freely.

This is what I'm having problems with.

5

Have you read about "state machines"?

6

I don't like to provide "turnkey" code since I think it's better to learn by doing but I myself am just getting my mind round state machines, so I coded the following solution.

I'm glad your buttons don't bounce: mine do and it really needs debouncing...

Anyhoo, here's my solution.

//forum thread http://forum.arduino.cc/index.php?topic=454813.0

bool locked = false;
bool redState = false;
bool redStateOld;
bool greenState = false;
bool greenStateOld;

byte lockBtn = 15; // A1
byte redBtn = 16; //A2
byte greenBtn = 17; //A3

byte lockLed = 3;
byte redLed = 5;
byte greenLed = 6;

int debounceInt=200; //poor man's debouncer

void setup() {
  // put your setup code here, to run once:

  pinMode(lockBtn, INPUT_PULLUP);
  pinMode(redBtn, INPUT_PULLUP);
  pinMode(greenBtn, INPUT_PULLUP);

  pinMode(lockLed, OUTPUT);
  pinMode(redLed, OUTPUT);
  pinMode(greenLed, OUTPUT);

  digitalWrite(lockLed, locked);
  digitalWrite(redLed, redState);
  digitalWrite(greenLed, greenState);
}

void loop() {
  // put your main code here, to run repeatedly:
  //remember inputs are active low


  //if system is not locked, three things are allowed:
  //     toggle red, or
  //     toggle green, or
  //     lock system, which means
  //         on the lock led and set locked true
  //         save red and green led states for later
  //         off the red and green leds
  if (!locked) //not locked
  {
    if (!digitalRead(redBtn)) //toggle red 
    {
      redState = !redState;
      digitalWrite(redLed, redState);
      delay(debounceInt);
    } //red

    if (!digitalRead(greenBtn)) //toggle green
    {
      greenState = !greenState;
      digitalWrite(greenLed, greenState);
      delay(debounceInt);
    } //green

    if (!digitalRead(lockBtn)) //lock pressed means lock
    {
      redStateOld = redState;
      redState = false;
      digitalWrite(redLed, redState);

      greenStateOld = greenState;
      greenState = false;
      digitalWrite(greenLed, greenState);

      locked = true;
      digitalWrite(lockLed, locked);
      delay(debounceInt);
    } //lock
  } //!locked

  //if system is locked, only action is to unlock
  //     set locked to false
  //     restore red and green leds
  if (locked)
  {
    if (!digitalRead(lockBtn)) //lock pressed means unlock
    {
      locked = false;
      digitalWrite(lockLed, locked);

      redState = redStateOld;
      digitalWrite(redLed, redState);

      greenState = greenStateOld;
      digitalWrite(greenLed, greenState);
      delay(debounceInt);
    }//unlock
  } //locked

} //loop
7

if I may offer a constructive comment, those pins would be better declared as const byte

byte lockBtn = 15; // A1
byte redBtn = 16; //A2
byte greenBtn = 17; //A3

byte lockLed = 3;
byte redLed = 5;
byte greenLed = 6;

and usually it's better to use the word Pin into the variable name to ensure you remember what is a Pin versus what is the status of the pin for example.

Also you are taking conceptual shortcuts between booleans and pin states. Sure true is 1 and so is HIGH but that makes code not as precise as it would need to be. Also that will lead to confusion when you have INPUT_PULLUP with reversed logic. better separate the logic from pin value for readability and maintainability.

8

Thankyou Manor Royal!!

The "if locked/ locked" condition in the main loop was what I totally missed... along with the state machine aspect. So much simpler!
It's so easy to get caught in a rut with your thinking and not think of other options. Not seeing wood for the trees and all that.

I like the way you comment at the last braces too. I'm going to do that from now on as I always get caught out there.

I sincerely thank you for taking the time to write that sketch as I have learnt a lot from it!!

Karma to you Mr Royal!

9

You're welcome: so many things benefit from that approach, and I like to do those just for the heck of it to help me get my mind round the idea.

Take heed of JML's valid points though.

10

I've been pondering this a bit further.

Strictly speaking, there are not just 2 states unlocked and locked. As well as locked, there are unlocked/no leds, unlocked/red only, unlocked/green only, and unlocked/red & green leds. I ignored that fact in my coding, and when unlocked, turning either the red or green led on or off, stayed in the simple state of unlocked regardless of how many of the 2 leds were on.

Turning a led on or off should really take us to a new state; any legal action should.

(edit: That said, in a way I have distinguished the states, since within unlocked I do have the red and green states explicitly set. But even so, there's no explicit unlocked/red, unlocked/green etc states.)

With only 2 leds a graphical model of that would be doable; with 10 leds in any combination, that would be a nightmare.

I'm wondering what the thinking is about modeling such complex (albeit finite and definable) states in real-world systems.

11

Seems I'm not the first to ponder that complexity :wink: which makes sense since I'm a civil engineer by original qual not a computer scientist.

Wikipedia says this:

Classic state diagrams require the creation of distinct nodes for every valid combination of parameters that define the state. This can lead to a very large number of nodes and transitions between nodes for all but the simplest of systems

I think that's what I was saying about having the 2 or more LEDs above. The red led can be toggled from any other valid position within "unlocked": it can be toggled on from when there are no leds, on from when there's blue only, toggled off from red only and toggled off from red and blue together. Wiki uses the example of the "clear" button on a calculator, which needs to work no matter where you are in the state diagram.

But that's enough hi-jacking of Laserbee's thread. For my purposes it's ok (to me) to have it the way I coded it, and to visualize it like that, but in a complex, real-life, safety-critical application it would obviously need a more rigorous approach.