Help with debouncing button to change to different part of code

Hi,

I am having an issue with my code after adding a denounce. Before I added the debounce to my code it would work where it would loop and change the valveStatus with a button press. Now it won't change the valveStatus but I can see that the button is working with the debounce.

Any help would be much appreciated.

Thanks,
Mike.

Code before debounce

int LED1 = 8; // Indicates Valve Closed
int LED2 = 9; // Indicates Valve Open
int LED3 = 10; // Power/Programme Running
int delayPress = 500;
const int  buttonPin = 3;    // The pin that the pushbutton is attached to
const int valvepin = 1;      // The pin that the valves are attached to



// Variables will change:
int buttonState;         // Current state of the button
int lastButtonState;     // Previous state of the button
bool valveStatus;        // checks the status of the valves


void setup()
{
  Serial.begin(9600);          //  Setup serial
  Serial.println("Exhaust Valves ");
  
  // Initialize the button pin as a input with pullup, active low
  pinMode(buttonPin, INPUT_PULLUP);
  
  //Initialize button states
  buttonState = digitalRead(buttonPin);
  lastButtonState = buttonState;

  
  pinMode (LED1, OUTPUT);
  pinMode (LED2, OUTPUT);
  digitalWrite (LED3, HIGH);

}

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

  // Compare the buttonState to its previous state
  if (buttonState != lastButtonState) //Changed
  {
    if (buttonState == LOW) //New press, so change valve flag
    {
      valveStatus = !valveStatus;
    }
    delay(delayPress);
  }

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


  if (valveStatus) //Positions the valve
  {
    
    Serial.write(0x56);
    Serial.write(0x31);
    Serial.write(0x0A);

    Serial.print("Valve Status: ");
    Serial.print(" Valve Open");
    Serial.println(" (Loud Mode)");
    digitalWrite (LED1, LOW);
    digitalWrite (LED2, HIGH);
     {
while (digitalRead(buttonPin) == HIGH);
  }
    delay (delayPress);

  }
  else
  {
    
    Serial.write(0x56);
    Serial.write(0x30);
    Serial.write(0x0A);

    Serial.print("Valve Status: ");
    Serial.print(" Valve Closed");
    Serial.println(" (Quiet Mode)");
    digitalWrite (LED2, LOW);
    digitalWrite (LED1, HIGH);
     {
while (digitalRead(buttonPin) == HIGH);
  }
    delay (delayPress);

  }

}

Code with debounce added

// constants won't change. They're used here to set pin numbers:
const int LED1 = 8; // Indicates Valve Closed
const int LED2 = 9; // Indicates Valve Open
const int LED3 = 10; // Power/Programme Running
const int buttonPin = 3;    // the number of the pushbutton pin
const int valvepin = 1;      // The pin that the valves are attached to

// Variables will change:
bool valveStatus;         // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin

// the following variables are unsigned longs because the time, measured in
// milliseconds, 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() {
  Serial.begin(9600);          //  Setup serial
  Serial.println("Exhaust Valves ");

  // Initialize the button pin as a input with pullup, active low
  pinMode(buttonPin, INPUT_PULLUP);

  //Initialize button states
  buttonState = digitalRead(buttonPin);
  lastButtonState = buttonState;


  pinMode (LED1, OUTPUT);
  pinMode (LED2, OUTPUT);
  digitalWrite (LED3, HIGH);
}

void loop() {
  // read the state of the switch into a local variable:
  int buttonState = digitalRead(buttonPin);

  // check to see if you just pressed the button
  // (i.e. the input went from LOW to HIGH), and you've waited long enough
  // since the last press to ignore any noise:

  // If the switch changed, due to noise or pressing:
  if (buttonState != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:

    // if the button state has changed:
    if (buttonState != buttonState) {


      // only toggle the LED if the new button state is HIGH
      if (buttonState == LOW) {
        valveStatus = !valveStatus;
      }
    }
  }

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


  if (valveStatus) //Positions the valve
  {

    Serial.write(0x56);
    Serial.write(0x31);
    Serial.write(0x0A);

    Serial.print("Valve Status: ");
    Serial.print(" Valve Open");
    Serial.println(" (Loud Mode)");
    digitalWrite (LED1, LOW);
    digitalWrite (LED2, HIGH);
    {
      while (digitalRead(buttonPin) == HIGH);
    }



  }
  else
  {

    Serial.write(0x56);
    Serial.write(0x30);
    Serial.write(0x0A);

    Serial.print("Valve Status: ");
    Serial.print(" Valve Closed");
    Serial.println(" (Quiet Mode)");
    digitalWrite (LED2, LOW);
    digitalWrite (LED1, HIGH);
    {
      while (digitalRead(buttonPin) == HIGH);
    }


    // save the reading. Next time through the loop, it'll be the lastButtonState:
    lastButtonState = buttonState;
  }
}

Please explain this:

    {
      while (digitalRead(buttonPin) == HIGH);
    }

First of all, the braces do nothing so remove them.

Why do you even look at the button pin outside of the button press logic, and why would you freeze the program while the switch is HIGH? I really can't imagine your reason.

Also, instead of the undocumented cryptic

    Serial.write(0x56);
    Serial.write(0x31);
    Serial.write(0x0A);

why not just send "what it really is"?

Serial.println("V1");

Are you just trying to toggle the valve? Believe me, it doesn't have to be this convoluted. Separate the button and valve code completely. You already created a boolean 'valveStatus' that you can use perfectly well for that purpose.

Also, instead of the undocumented cryptic

why not just send "what it really is"?

Serial.println("V1");

Yes, I am trying to toggle the valves over serial. I need to send these values to operate the valves.

This code opens the valves

  Serial.write(0x56);   
  Serial.write(0x31);   
  Serial.write(0x0A);

This code closes the valves

  Serial.write(0x56);   
  Serial.write(0x30);   
  Serial.write(0x0A);

I have removed these parts of the code. I'm not sure why they were there.

  {
      while (digitalRead(buttonPin) == HIGH);
    }

Thanks,
Mike.

Mike, The series of bytes you send with write(), is just a plain vanilla human readable ASCII string. 0x56 is ‘V’ in the ASCII table.
So what you are really sending is

  Serial.write('V');  
  Serial.write('1');  
  Serial.write(\n);

and you can more easily send it as:

Serial.println("V1")

because println() adds the carriage return character itself.

If you want to avoid writing clear, understandable code that is of course, your choice.

It’s not clear whether your problem was resolved. If not, please repost your latest effort. The most important advice was at the end of my post, and you haven’t responded to it at all.

Thanks for your reply,

I am not entirely sure what you mean by this

Are you just trying to toggle the valve? Believe me, it doesn't have to be this convoluted. Separate the button and valve code completely. You already created a boolean 'valveStatus' that you can use perfectly well for that purpose.

I will try the println and have a play then I will report back,

Thanks,
Mike.

"convoluted" means roughly: winding, twisted, unnecessarily complicated for the task it has to do. But instead of focusing on that, how about just considering the specific advice I gave you?

Separate the code. Here you can see how you've combined switch code with valve code:

  if (valveStatus) //Positions the valve
  {

 ...    {
      while (digitalRead(buttonPin) == HIGH);
    }

My tutorial on debouncing switches provide a debounce class that has methods like
isDown(); // switch is debouced down
isChanged(); // switch up/down just changed state

That may simplify your logic.

Ok, I have made progress. I removed the else statement and made it an if statement. Now it toggles between the two if statements which is what I want it to do.

// constants won't change. They're used here to set pin numbers:
const int LED1 = 8; // Indicates Valve Closed
const int LED2 = 9; // Indicates Valve Open
const int LED3 = 10; // Power/Programme Running
const int buttonPin = 3;    // the number of the pushbutton pin

// Variables will change:
bool valveStatus;         // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin

// the following variables are unsigned longs because the time, measured in
// milliseconds, 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() {
  Serial.begin(9600);          //  Setup serial
  Serial.println("Exhaust Valves ");

  // Initialize the button pin as a input with pullup, active low
  pinMode(buttonPin, INPUT_PULLUP);

  //Initialize button states
  buttonState = digitalRead(buttonPin);
  lastButtonState = buttonState;


  pinMode (LED1, OUTPUT);
  pinMode (LED2, OUTPUT);
  digitalWrite (LED3, HIGH);
}

void loop() {
  // read the state of the switch into a local variable:
  buttonState = digitalRead(buttonPin);

  // check to see if you just pressed the button
  // (i.e. the input went from LOW to HIGH), and you've waited long enough
  // since the last press to ignore any noise:

  // If the switch changed, due to noise or pressing:
  if (buttonState != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:

    // if the button state has changed:
    if (lastButtonState != buttonState) {
      buttonState = lastButtonState;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == LOW) {
        valveStatus = !valveStatus;
      }
    }
  }

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


  if (valveStatus = valveStatus) //Positions the valve
  {

    Serial.write(0x56);
    Serial.write(0x31);
    Serial.write(0x0A);

    Serial.print("Valve Status: ");
    Serial.print(" Valve Open");
    Serial.println(" (Loud Mode)");
    digitalWrite (LED1, LOW);
    digitalWrite (LED2, HIGH);
    {
      while (digitalRead(buttonPin) == HIGH);
    }



  }
  if (valveStatus = !valveStatus)
  {

    Serial.write(0x56);
    Serial.write(0x30);
    Serial.write(0x0A);

    Serial.print("Valve Status: ");
    Serial.print(" Valve Closed");
    Serial.println(" (Quiet Mode)");
    digitalWrite (LED2, LOW);
    digitalWrite (LED1, HIGH);
   {
    
      while (digitalRead(buttonPin) == HIGH);
   }
  
}
      

}

BUT

Instead of just cycling between valve open and valve closed I get.

V1
Valve Status:  Valve Open (Loud Mode)
V0
Valve Status:  Valve Closed (Quiet Mode)
V1
Valve Status:  Valve Open (Loud Mode)

and

V0
Valve Status:  Valve Closed (Quiet Mode)
V1
Valve Status:  Valve Open (Loud Mode)
V0
Valve Status:  Valve Closed (Quiet Mode)

It seems to bounce back to Open then back to Closed and vice verse when I press the button.

If I don't keep this in the code it just continuously toggles between Open and Closed.

{
      while (digitalRead(buttonPin) == HIGH);
    }
  if (valveStatus = valveStatus) //Positions the valve

did you mean

  if (valveStatus == valveStatus) //Positions the valve

This is wrong, you should NOT do this every time through loop.

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

Simplified a bit with debounce added.

#include <mechButton.h>


int LED1 = 8; // Indicates Valve Closed
int LED2 = 9; // Indicates Valve Open
int LED3 = 10; // Power/Programme Running
const int  buttonPin = 3;    // The pin that the pushbutton is attached to

// Variables will change:
bool        valveOpen;        // Holds the status of the valves
mechButton  cutoutButton(buttonPin);


void setup() {
   Serial.begin(57600);          //  Setup serial
   Serial.println("Exhaust Valves ");
   pinMode (LED1, OUTPUT);
   pinMode (LED2, OUTPUT);
   pinMode (LED3, OUTPUT);
   digitalWrite(LED3, HIGH);
   cutoutButton.setCallback(buttonClick);     // Set up our callback. (Also calls hookup() for idling.)
   valveOpen = false;
}


void buttonClick(void) {

   if (!cutoutButton.trueFalse()) {
      if (valveOpen) {
         Serial.write(0x56);
         Serial.write(0x30);
         Serial.write(0x0A);
         Serial.print("Valve Status: ");
         Serial.print(" Valve Closed");
         Serial.println(" (Quiet Mode)");
         digitalWrite (LED2, LOW);
         digitalWrite (LED1, HIGH);
         valveOpen = false;
      } else {
         Serial.write(0x56);
         Serial.write(0x31);
         Serial.write(0x0A);
         
         Serial.print("Valve Status: ");
         Serial.print(" Valve Open");
         Serial.println(" (Loud Mode)");
         digitalWrite (LED1, LOW);
         digitalWrite (LED2, HIGH);
         valveOpen = true;
      }
   }
}


void loop() { idle(); } // Run the background stuff, like mechButton.

Library manager in your IDE. Install LC_baseTools if you’d like to try this.

-jim lee

Thank you jimlee that worked perfectly. I probably won't use this exact code in my project as I am learning I will attempt to do my own version, but this is a great example for me to build on.

Thanks,
Mike.

You're really quite close. You just need to go through the logic step by step and remove a few errors.

s200bym:
Thank you jimlee that worked perfectly. I probably won’t use this exact code in my project as I am learning I will attempt to do my own version, but this is a great example for me to build on.

Thanks,
Mike.

You are very welcome. Good luck with the project!

-jim lee

aarg:
You’re really quite close. You just need to go through the logic step by step and remove a few errors.

We have good progress.
I finally managed to get the code to work. I had a go at writing a library and added that to the debounce sketch, whether I needed to do that or not but I am on a learning curve so I just did it.

Is it possible to simplify the code further or is that the correct way for debouncing?

One thing I want to try and sort out is the constant sending of the serial signal which is seen scrolling on the serial monitor and visible on an oscilloscope. I am guessing it is to do with the loop function?

I noticed on jimlee’s code that it is in its own void and the serial signal is only sent when the button is pushed and then sits at logic level (idle) until the button is pushed. I tried putting my code in its own void and calling it in the loop but I just get the same result.

#include<exhaustValve.h>

// constants won't change. They're used here to set pin numbers:
const int LED1 = 8; // Indicates Valve Closed
const int LED2 = 9; // Indicates Valve Open
const int LED3 = 10; // Power/Programme Running
const int buttonPin = 3;    // the number of the pushbutton pin

// Variables will change:
bool valveStatus;         // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState;   // the previous reading from the input pin
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

exhaustValve myExhaustValve;

void setup() {

  //  Setup serial
  Serial.begin(9600);
  Serial.println("Exhaust Valves ");

  // Initialize the button pin as a input with pullup, active low
  pinMode(buttonPin, INPUT_PULLUP);

  //Initialize button states
  buttonState = digitalRead(buttonPin);
  lastButtonState = buttonState;

  //Initialize LED states
  pinMode (LED1, OUTPUT);
  pinMode (LED2, OUTPUT);
  digitalWrite (LED3, HIGH);
}

void loop() {

  // read the state of the switch into a local variable:
  buttonState = digitalRead(buttonPin);


  // If the switch changed, due to noise or pressing:
  if (buttonState != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:
  }

  // 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 == LOW) {
      valveStatus = ! valveStatus;
    }
    // save the current state as the last state, for next time through the loop
    lastButtonState = buttonState;
  }


  //  Changes the status of the valve
  if (valveStatus) {
    myExhaustValve.Open();

  }
  else
  {
    myExhaustValve.Close();
  }


}

Thanks guys,
Mike.

In my code, if you look at the mechButton.cpp file.. mechButton is a class derived from idler. Therefore it already "knows" how to hook-in to the background idle queue. It actually hooks itself in when you assign its callback function.

I use the idle queue to hide stuff from the human that can run on its own. This way, what I'm working on in loop(), stays far less cluttered up with "boilerplate" code. Making life a lot less complicated.

-jim lee

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.