Two Button State Detection Error

Hello everyone,

I am using the state change detection example to increment the count from 0-4 and from 4-0. When using serial monitor, I am able to increment from 0-4 but I can not decrement and it outputs the word “off.”

Does anybody see any mistakes with my code? I can not see why my current state is always LOW. I still get the same error, even if I comment out “buttonPushCounter = lowerLimit;”

Code:

// Variables will change:
int buttonPushCounter = 0;            // counter for the number of button presses
int buttonState = 0;                  // current state of the button counting up
int buttonState2 = 0;                 // current state of the button counting down
int lastButtonState = 0;              // previous state of the button
int lastButtonState2 = 0;             // previous state of the button
int upperLimit = 3;
int lowerLimit = 0;


void setup() {
  pinMode(buttonPin, INPUT);                     // initialize the button pin as a input:
  pinMode(buttonPin2, INPUT);                    // initialize the button pin 2 as a input:
  Serial.begin(9600);                            // initialize serial communication:

}

void loop() {
  buttonState = digitalRead(buttonPin);          // read the pushbutton input pin:
  if (buttonState != lastButtonState) {          // compare the buttonState to its previous state
    if (buttonState == HIGH) {                   // if the state has changed, increment the counter
      buttonPushCounter++;                       // if the current state is HIGH then the button
      if (buttonPushCounter > upperLimit) {
        buttonPushCounter = upperLimit;
      }
    }
    buttonState2 = digitalRead(buttonPin2);
    if (buttonState2 != lastButtonState2) {
      if (buttonState2 == HIGH) {
        buttonPushCounter--;
        if (buttonPushCounter <= lowerLimit) {
          buttonPushCounter = lowerLimit;
        }
      }
    }
    // wend from off to on:
    Serial.println("on");
    Serial.print("number of button pushes:  ");
    Serial.println(buttonPushCounter);
  } else {
    // if the current state is LOW then the button
    // wend from on to off:
    Serial.println("off");
  }
  delay(50);                                   // Delay a little bit to avoid bouncing

  // save the current state as the last state,
  //for next time through the loop
  lastButtonState = buttonState;
  lastButtonState2 = buttonState2;
}

Thank you very much!

Use CTRL-T to format your code. Notice how it seems to scramble your code a little? That means you've got the braces ({) in the wrong places.

The "off" is printed anytime the first button doesn't change.

Thank you for the advice.

I don't see why the "off" is only printed when the first button doesn't change. Shouldn't it be printed when either button isn't changing?

Tell us which if() the else is attached to?

.

It should correspond to this:

if (buttonState == HIGH) {

And that seems to be part of my mistake. Getting closer now :wink:

It is now counting up and down as desired, but it is not debouncing. It counts about 5 digits every time the button is pressed down.

// this constant won't change:
const int  buttonPin = 4;    // the pin that the pushbutton is attached to
const int buttonPin2 = 5;

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button
int lastButtonState2 = 0;     // previous state of the button
int buttonState2 = 0;
int upperLimit = 3;
int lowerLimit = 0;

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin, INPUT);
  // initialize the LED as an output:
  pinMode(buttonPin2, INPUT);
  // initialize serial communication:
  Serial.begin(9600);
}


void loop() {
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }

  //////////////////////////////////////////////
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin2);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter--;
      Serial.println("on");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(50);

    // save the current state as the last state,
    //for next time through the loop
    lastButtonState = buttonState;




  }
}

Why do you have one variable (lastButtonState) trying to hold the state of both buttons? So if either button changes or isn't the same as the other, then it counts.

Imagine, button1 is LOW but button2 is HIGH. lastButtonState gets the HIGH since it is set after reading button 2. But then you compare that to what you read from button1 to see if it has changed. You're comparing the state of button1 to the last state of button2 at that point. It's different so it keeps on counting.

That was the only way I got it to count up and down. I tried to use different variables such as in the code below, but it stopped counting down after doing so.

// this constant won't change:
const int  buttonPin = 4;    // the pin that the pushbutton is attached to
const int buttonPin2 = 5;

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
int buttonState2 = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button
int lastButtonState2 = 0;     // previous state of the button
int upperLimit = 3;
int lowerLimit = 0;

// the following variables are unsigned long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin, INPUT);
  // initialize the LED as an output:
  pinMode(buttonPin2, INPUT);
  // initialize serial communication:
  Serial.begin(9600);
}


void loop() {
  
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }

  //////////////////////////////////////////////
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin2);

  // compare the buttonState to its previous state
  if (buttonState2 != lastButtonState2) {
    // if the state has changed, increment the counter
    if (buttonState2 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter--;
      Serial.println("on");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(50);

    // save the current state as the last state,
    //for next time through the loop
    lastButtonState = buttonState;




  }
}
const int  buttonPin = 4;    // the pin that the pushbutton is attached to
const int buttonPin2 = 5;

Don't add a suffix to one name in a set. Add suffixes to ALL of the names in a set, or use an array.

Does that comment really add any value? If it does, why don't both lines have useless comments?

  // initialize the button pin as a input:
  pinMode(buttonPin, INPUT);
  // initialize the LED as an output:
  pinMode(buttonPin2, INPUT);

More useless comments that aren't even accurate.

Just how ARE the switches wired? Why aren't you using the internal pullup resistors, instead? If you have to ask "Instead of what?", your switches are NOT wired correctly.

  buttonState = digitalRead(buttonPin2);

  // compare the buttonState to its previous state
  if (buttonState2 != lastButtonState2) {

This is EXACTLY why suffixes on all variables in a set are useful. The state of the second pin should be stored in buttonState2, not buttonState or buttonState1, especially since you then compare the state2 variable.

Paul -
Yeah, the comments could use a little help.

The switches are working now with my new code, but I am now see some of the mistakes I was making. Thank you very much!

// this constant won't change:
const int buttonPin = 4; // the pin that the pushbutton is attached to
const int buttonPin2 = 5;

// Variables will change:
int buttonPushCounter = 0; // counter for the number of button presses
int flagButton1 = 0; // current state of the button
int flagButton2 = 0; // current state of the button
int upperLimit = 3;
int lowerLimit = 0;

void setup() {
// initialize the button pin as a input:
pinMode(buttonPin, INPUT);
// initialize the LED as an output:
pinMode(buttonPin2, INPUT);
// initialize serial communication:
Serial.begin(9600);
}

void loop() 
{

if (digitalRead(buttonPin) && flagButton1==1)
{
if(buttonPushCounter<upperLimit)
{
buttonPushCounter++;
Serial.println("Button1 pressed to increment");
Serial.print("number of button pushes: ");
Serial.println(buttonPushCounter);
flagButton1=0;
}
else
{
Serial.print("Counter value reached the upper limit which is: ");
Serial.println(upperLimit);
buttonPushCounter=upperLimit;
}
delay(100);
} 
if (!digitalRead(buttonPin) && flagButton1==0)
{
Serial.println("Button1 not pressed!");
flagButton1=1;
delay(100);
}

//////////////////////////////////////////////
if (digitalRead(buttonPin2) && flagButton2==1)
{
if(buttonPushCounter>=lowerLimit)
{
buttonPushCounter--;
Serial.println("Button2 pressed to decrement");
Serial.print("number of button pushes: ");
Serial.println(buttonPushCounter);
flagButton2=0;
}
else
{
Serial.print("Counter value reached the lower limit which is: ");
Serial.println(lowerLimit);
buttonPushCounter=lowerLimit;
}
delay(100);
} 
if (!digitalRead(buttonPin2) && flagButton2==0)
{
Serial.println("Button2 not pressed!");
flagButton2=1;
delay(100);
}
}

But, now appears that my buttons do not work while the processor is using the delay function.

Seems like I'll have to find a way to use the function millis.

Voltimetro:
But, now appears that my buttons do not work while the processor is using the delay function.

Seems like I'll have to find a way to use the function millis.

Yes, but you shouldn't have to look far. There are examples built into the IDE and about a hundred thousand great tutorials on the topic.

Here’s an update of where I am now.

After looking through some of the examples, I was able to come up with a code that counts up and down. However, I am now having trouble interfacing it to the part of my code that does controls a DC motor. My goal is to have four different states of time. If button is pressed once, have the motor run for a second (time not defined just yet); press the button twice and have the motor run for two seconds, and so on.

After going trough a few tutorials, and doing some research I am now a little acquainted with the Millis function, but I still don’t understand how to use it to its full extent.

Here are some of the problems that I’m currently having:

1- My motor is activated with a PIR sensor, and it re-triggers the motor as soon as it finishes with a cycle. The sensor has a hardware (pin selector) to not be re-triggered; yet, it still does…

2- Sometimes the buttons will not denounce. They will randomly skip the counter from 2-4. While this won’t affect my application at all, I am still curious why it might be happening.

3- The different states of time do not work at all. The motor will not change the time that it operates for, regardless of how many times the buttons have been pressed.

Questions:

Should I use the delay or the millis function for my PWM? I had the impression that using the delay function did not affect my button inputs anywhere as much as I thought that it would.

Will I consume less energy by ramping up my motor? This will be a battery operated gadget. From what I understand, DC motors will draw more current when they first start and I’m thinking that gradually increasing the voltage will help it decrease that spike of current at t=0. Please correct me if that’s not right.

Again, thank you for your valuable advice.

Code:

//This Code Is For PWM With a Controlled Time (Millis) Using Buttons//
// These constant won't change:
const int buttonPin = 4;                              // the pin that the pushbutton is attached to
const int buttonPin2 = 5;                             // the pin that the pushbutton2 is attached to
int PIR = 2;                                          // give a name to digital pin 2, which has a pushbutton attached             
int motorControl = 3;                                 // the transistor which controls the motor will be attached to digital pin 3
 
// Variables will change for button limits:
unsigned long currentMillis = 0;                     
unsigned long previousMillis = 0;
unsigned long finalMillis = 0;
int buttonPushCounter = 0;                            // counter
int upperLimit = 4;                                   // counter max limit
int lowerLimit = 2;                                   // counter min limit
//

void setup() {
pinMode(buttonPin, INPUT);                            // initialize the button pin as a input:
digitalWrite(buttonPin, HIGH);
pinMode(buttonPin2, INPUT);                           // initialize the buttonPin2 as an output:
digitalWrite(buttonPin2, HIGH);
pinMode(PIR, INPUT);                                  // make PIR pin an input:
pinMode(motorControl, OUTPUT);                        // make the transistor's pin an output:
Serial.begin(9600);                                   // initialize serial communication:
}

void loop() {
  
///////////////////////////////////////////////////////
//////////This part is for the buttons.////////////////
////////////////Counting UP////////////////////////////
///////////////////////////////////////////////////////
 if (digitalRead(buttonPin) == LOW)
  if(buttonPushCounter<upperLimit)
  {
    buttonPushCounter++;
    Serial.println("on");
    Serial.print("number of button pushes:  ");
    Serial.println(buttonPushCounter);

    while(digitalRead(buttonPin) == LOW)
    {
      currentMillis = millis();
    }

    finalMillis = currentMillis - previousMillis;  
    //Serial.println(finalMillis);

    //previousMillis = finalMillis;
    //finalMillis = 0;
  }
  /////////////////////
  if (digitalRead(buttonPin2) == LOW)
  if(buttonPushCounter>=lowerLimit)
  {
    buttonPushCounter--;
    Serial.println("on");
    Serial.print("number of button pushes:  ");
    Serial.println(buttonPushCounter);

    while(digitalRead(buttonPin2) == LOW)
    {
      currentMillis = millis();
    }

    finalMillis = currentMillis - previousMillis;  
    Serial.println(finalMillis);

    //previousMillis = finalMillis;
    //finalMillis = 0;
  }


///////////////////////////////////////////////////////
///////////////Button Conclusion///////////////////////
///////////////////////////////////////////////////////

///////////////////////////////////////////////////////
///////////////Motor Control///////////////////////////
///////////////////////////////////////////////////////

if(digitalRead(PIR) == HIGH){                 // read the state of the button and check if it is pressed
   
for(int x = 0; x <= 255; x++){                // ramp up the motor speed
 analogWrite(motorControl, x); 
 delay(10);
    }

    for(int x = 255; x >= 0; x--){            // ramp down the motor speed 
      if(buttonPushCounter = 4){
      delay(4000);
      }
      if(buttonPushCounter = 3){
      delay(300);
      }
      if(buttonPushCounter = 2){
      delay(200);
      }
      if(buttonPushCounter = 1){
      delay(100);
      }           
      analogWrite(motorControl, x);
      
    }
    
    if(digitalRead(PIR) == LOW){
    analogWrite(motorControl, 0);     
    }

  delay(5000);                              // delay in between reads for stability
}
//Conclusion for motor control.//
}

  1. Set a boolean variable to true once it has been triggered and false when the motor turns off and you want to allow it to trigger again. Test against that variable when you get a trigger to see if the software should react.
boolean triggeredAlready = false;

//......//

if(pirIsTriggered() && triggeredAlready == false ){
    startMotor();
    triggeredAlready = true;
}
  1. I don’t see any code to debounce the button. You can do it with code or with hardware (a cap across the button) but it isn’t going to just debounce itself.

  2. That’s terrible. Maybe someone who can see the current version of the code can help. But you’d have to post that here in order for any of us to help debug it.

Thank you, I will try to use a boolean variable and also see how the capacitors affect my buttons input. :wink:

I was editing the post when you replied. Thank you for your prompt response, the code has now been added.

if(buttonPushCounter = 4){

Ooopsies. Assignment vs. Comparison. Common trap.

  delay(5000);                              // delay in between reads for stability[/code[/quote]

Your comment is wrong. It should say "Sick your head in the sand and forget about reading any buttons or anything"

Go back and re-read Planning and Implementing an Arduino Program Follow its recommendations.

Another useful one might be http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html You've started describing the behaviour of your system as "states". One way to fix it is to make everything a state. "motor running for 2 sec" is a state. When the timer gets to 2 seconds you leave that state and go to another one, which might be "motor prevented from running for 2 sec". But that state can still respond to inputs such as an emergency-stop or reset button. It can keep counting with the counter so you can make a couple of steps with the up/down buttons and then when it leaves that state, it will go to the correct state for that counter.