3 step button - weird behaviour

First of all, hello :slight_smile:
My name is Bram i’m from the netherlands.

I recently bought an Arduino Uno to work out an 3 step/level start button for my car and later on i want to add some more stuff…
Now i work with php/javascript etc… on a daily basis and know the basics of C++…

But for some reason i can’t make this work.

I got a simple button which is pressed… But the first press the first led has to come on. The second press the second les has to come on. And the 3th (long)press the 3th led has to come on. Sounds quite simple… Atleast that was what i thought…

Here is one of my test cases which didn’t succeed… could you guys help me out?
When i press the button the lights go on sequentiall… so the first light allready is lit up (that’s correct). When i press the button the second light lights up. But the thirth one also does… (and back to off within a second)…

int pwrLed = 13;
int accLed = 12;
int ignLed = 11;
int strLed = 10;

int btn = 2;

int pwrLedState = HIGH;
int accLedState = HIGH;
int ignLedState = LOW;
int strLedState = LOW;

void setup() { 
  pinMode(pwrLed,OUTPUT);
  pinMode(accLed,OUTPUT);
  pinMode(ignLed,OUTPUT);
  pinMode(strLed,OUTPUT);

  pinMode(btn,INPUT);
}

void loop(){

  if(digitalRead(btn)==HIGH && accLedState==HIGH && ignLedState==HIGH && strLedState==HIGH){
    strLedState = LOW;
    ignLedState = LOW;
    digitalWrite(strLed,LOW);
    digitalWrite(ignLed,LOW);
  }else
  if(digitalRead(btn)==HIGH && accLedState==HIGH && ignLedState==HIGH){
    strLedState = HIGH;
    digitalWrite(strLed,HIGH);
  }else
  if(digitalRead(btn)==HIGH && accLedState==HIGH && ignLedState==LOW){
    ignLedState = HIGH;
    digitalWrite(ignLed,HIGH);
  }
}


some more attempts below (allready deleted a few…) and some with power led also :stuck_out_tongue:

// power led
int power = 13;
// first stage
int lvl1 = 5;
//second stage
int lvl2 = 11;
// thirth stage(start)
int lvl3 = 10;

int state = 1;

// button
int btn = 6;
int btnSwitch = 3;

// locked or unlocked
int lock = 9;
int unlock = 8;

// state
int pwr = HIGH;

void setup() {                
  pinMode(btn, OUTPUT);     
  pinMode(power, OUTPUT);       
  pinMode(lvl1, OUTPUT);        
  pinMode(lvl2, OUTPUT);        
  pinMode(lvl3, OUTPUT);  

  //check if locked or unlocked
  pinMode(lock,INPUT);
  pinMode(unlock,INPUT);
  pinMode(btnSwitch,INPUT);
}

// the loop routine runs over and over again forever:
void loop() {
  
  int doUnLock = digitalRead(unlock);
  if(doUnLock==HIGH)
    pwr=HIGH;
    
  int doLock = digitalRead(lock);
  if(doLock==HIGH)
    pwr=LOW;

  int doSwitch = digitalRead(btnSwitch);
  if(doSwitch==HIGH){
    if(state==2)
      state=3;
    if(state==1)
      state=2;
  }else{
    if(state>=3)
      state=1;
  }

  if(state>=1)
    digitalWrite(lvl1,pwr);
  if(state>=2)
    digitalWrite(lvl2,pwr);
  if(state>=3)
    digitalWrite(lvl3,pwr);
    
  digitalWrite(btn,pwr);
  
  //digitalWrite(9, HIGH);   // always power 9
  digitalWrite(power,HIGH); // show if power

  delay(1000);
}

thanks in advance… and I hope this isn’t to much of a noob question :blush:

you have no real control over the states and if it was not for the delay the whole 3 states would be done and over with in a blink

you really need to look at the button being pressed and the button being released. Also if you want to hold the button on the third step then you will need to learn how to use a millis timer.

Also when using input you will need to add a pull down resistor or the button will give a false read. Look at references to PULL_UP and consider using that instead

If you can run this test program and use serial monitor to check. Watch the PULLUP as its wired different from a normal pin (pin on arduino to dc neg via a button)

Once tested tear the program to bits and play with the code until you understand how it works. Then write something better

const byte btnSwitch = 3;//pin to ground via button

byte prevDoSwitch;//remember what the pin done last
byte state;//4 states 0,1,2,3
unsigned long previousMillis = 0;//remember the time
unsigned long interval = 100;//how long to time

void setup() {

  Serial.begin(9600);//set up to monitor
  pinMode(btnSwitch, INPUT_PULLUP);//this is a pullup wire pin to ground via button
}
void loop() {
  // put your main code here, to run repeatedly:
  unsigned long currentMillis = millis();//used by timer
  int doSwitch = digitalRead(btnSwitch);//read button on pin 3

  if (state == 2) {//next state change timer to 1 second
    interval = 1000;
  }
  else {
    interval = 100;//all other changes use 10th of a second
  }

  //PullUP reverses button so low is pressed
  if ((doSwitch == LOW) && (prevDoSwitch == 0)) {
    if (currentMillis - previousMillis > interval) {//check timer
      state++;//change state if timer done
      prevDoSwitch = 1;//set the prev to block the button
    }
  }
  if (doSwitch == HIGH) {//button not pressed
    prevDoSwitch = 0;//remove the block on the button
    previousMillis = currentMillis; //keep reseting timer while not pressed
  }
  if (state >= 4) {//roll state back to 0
    state = 0;
  }
  Serial.println (state);
}

1) you are reading the button three times in your loop. what happens if the button is pressed while your code is running? I mean - it won't hurt, but the point is that you are not handling it. Solution - read the button once, put it in a boolean.

2) Typically, buttons are tied to the +5v rail via INPUT_PULLUP when they are not pressed, and go to ground when they are pressed. That is: pressed is LOW, unpressed is HIGH.

3) The main trick to buttons is that you mustn't ask "is the button HIGH", you need to ask "has the button changed from LOW to HIGH since the last time I looked"? This means you need to keep a record of what the button was last time the loop ran. The pattern is:

boolean buttonStateWas = HIGH;
void setup() {
  pinMode(btn,INPUT_PULLUP);
}

void loop() {
  // read sensors
  boolean buttonStateIs = digitalRead(btn);
  // manage state changes
  boolean buttonPressed = (buttonStateWas==HIGH) && (buttonStateIs==LOW);
  buttonStateWas = buttonStateIs; // save state

  if(buttonPressed && accLedState==HIGH && ignLedState==HIGH && strLedState==HIGH){
... etc

4) you use three variables to hold your state, yielding 8 possible conditions, and you only code for three of them. It's a pretty sure-fire way to accidentally overlook something.

4a) I notice that your conditions check accLedState==HIGH, but there is nothing in your code that ever sets it to low, so the check is always redundant.

4b) If your system has three states, then use a variable that has three values to keep track of what state you are in. Use an int, or use an enum if you are feeling daring.

5) You are going to run into switch bouncing - the wonderful world of physical components. My debouncing code is here: http://forum.arduino.cc/index.php?topic=339321 .

Thanks a bunch guys!! Will look into it tonight. And i can work wit millis() but will tear yout code apart gpop1. And look at your points murray :)

Thanks alot. Will post outcome here tonight

So i got it all working :) Thanks alot guys.. Here is the code.. Tweaked it a bit. Added a bit..

Nothing difficult just basics.. But a good start. Going to add different things to it now :)

const byte btnSwitch = 2;//pin to ground via button
const byte lockPulse = 3;//to lock the car and shut of the script/button
const byte unlockPulse = 4;//to unlock the car and start accled and stuff

byte prevDoSwitch;//remember what the pin done last
byte state;//4 states 0=off 1=accu stage 2=ignition stage 3 = start the b*tch
unsigned long previousMillis = 0;//remember the time
unsigned long interval = 100;//how long to time
unsigned long starter = 0; //returns 5000 or more if engine started
//@todo:  have to change starter to real 'started engine' wire to 
//        check if engine succesfully started

const byte accuLed = 13;
const byte ignLed = 12;
const byte startLed = 11;
const byte pwrLed = 10; //illuminate bttn

byte locked; // 0 = locked 1=unlocked
byte started; // 0 is not started 1=started

void setup() {

  Serial.begin(9600);//set up to monitor
  pinMode(unlockPulse, INPUT_PULLUP);// get pulse from lock device that it's unlocking the door
  pinMode(lockPulse, INPUT_PULLUP); // get pulse that it's locking the doors
  pinMode(btnSwitch, INPUT_PULLUP);//this is a pullup wire pin to ground via button

  // now only LED's within a few days actuall wires to start things :)
  pinMode(accuLed,OUTPUT);
  pinMode(ignLed,OUTPUT);
  pinMode(startLed,OUTPUT);
  pinMode(pwrLed,OUTPUT);
}
void loop() {

  unsigned long currentMillis = millis();//used by timer
  int doSwitch = digitalRead(btnSwitch);//read button on pin 2
  int doLock = digitalRead(lockPulse);//read button on pin 3
  int doUnlock = digitalRead(unlockPulse);//read button on pin 4

  if(doLock==LOW && started==0){// only accept lock signal if engine is off
    locked = 0;
    state = 0;
  }
  if(doUnlock==LOW){// car is unlocked activate the start button
    locked = 1;
    state = 1;
  }
  
  if(locked == 1){// is unlocked so continue
    digitalWrite(pwrLed,HIGH); //illuminate button @todo: maybe implement flashing if engine is not started?
  
    //PullUP reverses button so low is pressed
    if ((doSwitch == LOW) && (prevDoSwitch == 0)) {
      if (currentMillis - previousMillis > interval) {//check timer
        state++;//change state if timer done
        prevDoSwitch = 1;//set the prev to block the button
        if(state==3) //if state3 and button pressed set starter to 1
          starter++;
      }
    }
    if(starter > 0 && state==3)
      starter++; //check how long it has been pressed
    if (doSwitch == HIGH) {//button not pressed
      if(state==3){
        state--; //go back to normal
        if(starter>5000)// if run for 5000 times (pseudo for engine start) it is started
          started = 1;
        else
          starter = 1;
      }
      prevDoSwitch = 0;//remove the block on the button
      previousMillis = currentMillis; //keep reseting timer while not pressed
    }
    
    if(state==3 && started==1){// if started and again pressed go back to beginning
      state = 1;
      started = 0;
      starter = 0;
    }
    if (state >= 4) {//roll state back to 0
      state = 1;
    }
  
    // some LED fun :)
    if(state >= 1) {
      digitalWrite(accuLed,HIGH);
    }else{
      digitalWrite(accuLed,LOW);
    }
    
    if(state >= 2) {
      digitalWrite(ignLed,HIGH);
    }else{
      digitalWrite(ignLed,LOW);
    }
    
    if(state >= 3) {
      digitalWrite(startLed,HIGH);
    }else{
      digitalWrite(startLed,LOW);
    }
  }else{
    // car is locked and engine off so turn off all led's
    digitalWrite(pwrLed,LOW);
    digitalWrite(accuLed,LOW);
    digitalWrite(ignLed,LOW);
    digitalWrite(startLed,LOW);
  }
}

EDIT: added my new code.. Adds a little more functionality :)