(simple) Debouncing 2 button state machine

Hi, I have a problem. I keep failing with using debouncing to cycle a state machine.
In this example, I want to have one button to go up one state, and one button to go down one state, with debounce features. This should be obvious; I just don't understand enough yet.

When I press either of the two buttons, serial shows going through all the states to the max, or min value. Why does it skip through them, and not cycle each state individually? I don't understand what I've missed. Thank you.


#include <Bounce2.h>
#define BUTTON_PIN_1 2
#define BUTTON_PIN_2 3
#define LED_PIN 13
//global ints
int state = 0;
int old = 0;
Bounce debouncer1 = Bounce(); 
Bounce debouncer2 = Bounce(); 
void setup() {
  Serial.begin(9600);
   // Setup the first button with an internal pull-up :
  pinMode(BUTTON_PIN_1,INPUT_PULLUP);
  // After setting up the button, setup the Bounce instance :
  debouncer1.attach(BUTTON_PIN_1);
  debouncer1.interval(50); // interval in ms
  
   // Setup the second button with an internal pull-up :
  pinMode(BUTTON_PIN_2,INPUT_PULLUP);
  // After setting up the button, setup the Bounce instance :
  debouncer2.attach(BUTTON_PIN_2);
  debouncer2.interval(50); // interval in ms


  //Setup the LED :
  pinMode(LED_PIN,OUTPUT);

}

void loop() {
  
  // Update the Bounce instances :
  debouncer1.update();
  debouncer2.update();

  // Get the updated value :
  int value1 = debouncer1.read();
  int value2 = debouncer2.read();

  // Turn on the LED if either button is pressed :
  if ( value1 == LOW ) {
    digitalWrite(LED_PIN, HIGH );
  state = old + 1;
  } 
  else {
    digitalWrite(LED_PIN, LOW );
  }
if ( value2 == LOW ) {
    digitalWrite(LED_PIN, HIGH );
  state = old - 1;
  } 
  else {
    digitalWrite(LED_PIN, LOW );
  }
switch (state) {
  case 1: 
  Serial.println("one");
  old = 1;
  break;
  case 2:
  Serial.println("two");
  old = 2;
  break;
  case 3:
  Serial.println("three");
  old = 3;
  break;
  case 4:
  Serial.println("four");
  old = 4;
  break;
}}
  



Welcome and well done for the code tags on your first post.

I don't know what is in the code Bounce2 as you have not shared it, but I strongly suspect from what I can see that you are detecting when a button is pressed rather than when it becomes pressed. That means each time the loop function goes round it detects the button being pressed and increments or decrements the state machine again for as long as you hold the button. There are tutorials on this: in the IDE there is the debouce example, there is my tutorial Buttons and other electro-mechanical inputs (introduction) - #2 by PerryBebbington and if you do some searching there are other tutorials.

What is this for:

int old = 0;

?
Any of the following will add 1 to the variable state

state = state + 1;
state += 1;
state++;
++state;

Hey, I'm certainly not claiming my way is correct/best but here's how I personally would approach a simple state machine: A Simple State Machine - Wokwi Arduino Simulator

You're welcome to take this (and the button lib I wrote, which has debouncing built-in) and run with it if you like or just see how I've structured the code.

i've found that in most applications debouncing just requires a short delay

consider the following that monitors several buttons and toggles a corresponding LED when a button is pressed

// check multiple buttons and toggle LEDs

enum { Off = HIGH, On = LOW };

byte pinsLed [] = { 10, 11, 12 };
byte pinsBut [] = { A1, A2, A3 };
#define N_BUT   sizeof(pinsBut)

byte butState [N_BUT];

// -----------------------------------------------------------------------------
int
chkButtons ()
{
    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        byte but = digitalRead (pinsBut [n]);

        if (butState [n] != but)  {
            butState [n] = but;

            delay (10);     // debounce

            if (On == but)
                return n;
        }
    }
    return -1;
}

// -----------------------------------------------------------------------------
void
loop ()
{
    switch (chkButtons ())  {
    case 2:
        digitalWrite (pinsLed [2], ! digitalRead (pinsLed [2]));
        break;

    case 1:
        digitalWrite (pinsLed [1], ! digitalRead (pinsLed [1]));
        break;

    case 0:
        digitalWrite (pinsLed [0], ! digitalRead (pinsLed [0]));
        break;
    }
}

// -----------------------------------------------------------------------------
void
setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        pinMode (pinsBut [n], INPUT_PULLUP);
        butState [n] = digitalRead (pinsBut [n]);
    }

    for (unsigned n = 0; n < sizeof(pinsLed); n++)  {
        digitalWrite (pinsLed [n], Off);
        pinMode      (pinsLed [n], OUTPUT);
    }
}

do you need to recognize a change in button state rather than simply that it is being pressed?

Thank you for the tips! I have been reading through your links. I think I tested your debounce before, and thought that library was actually it. I don't really know where that library came from honestly.

I like the use of bool

int old = 0;

This is just from what I saw others doing, I was questioning what the purpose was. The state list options you gave me were perfect. I will eventually make the current state int store in EEProm.

My guess was that this debounce library was doing something odd, so I played with the MS....

debouncer1.interval(50);

Ranging from 5 to 200 ms values had no change in state change behavior. However, when I tried another debounce library I wasn't even getting an input signal. So I came here to see if there was something glaring that was off.

I am going to test this again, with a hodge podge from what was posted, to see if there is a hardware issue or if it's purely software.

I really like the layout of what you've done, and arrays are a lot more attractive, but I can't use delay for this. I will be adding to this basic structure as I learn. The delay although it works, I would like to learn to not have to rely on.

why? is 10 msec too long?
what real-time requirement prevents this?

consider (no delay()) (requirement)?

// check multiple buttons and toggle LEDs

enum { Off = HIGH, On = LOW };

byte pinsLed [] = { 10, 11, 12 };
byte pinsBut [] = { A1, A2, A3 };
#define N_BUT   sizeof(pinsBut)

byte butState [N_BUT];

// -----------------------------------------------------------------------------
#define DebounceMsec 10
#define NoBut        -1
int
chkButtons ()
{
    static unsigned long msecLst = 0;
           unsigned long msec    = millis ();

    if (msecLst && (msec - msecLst) < DebounceMsec)
        return NoBut;

    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        byte but = digitalRead (pinsBut [n]);

        if (butState [n] != but)  {
            butState [n] = but;

            msecLst = msec ? msec : 1;      // not zero

            if (On == but)
                return n;
        }
    }
    return NoBut;
}

// -----------------------------------------------------------------------------
void
loop ()
{
    switch (chkButtons ())  {
    case 2:
        digitalWrite (pinsLed [2], ! digitalRead (pinsLed [2]));
        break;

    case 1:
        digitalWrite (pinsLed [1], ! digitalRead (pinsLed [1]));
        break;

    case 0:
        digitalWrite (pinsLed [0], ! digitalRead (pinsLed [0]));
        break;
    }
}

// -----------------------------------------------------------------------------
void
setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        pinMode (pinsBut [n], INPUT_PULLUP);
        butState [n] = digitalRead (pinsBut [n]);
    }

    for (unsigned n = 0; n < sizeof(pinsLed); n++)  {
        digitalWrite (pinsLed [n], Off);
        pinMode      (pinsLed [n], OUTPUT);
    }
}

From the library docs at Bounce2/src at master · thomasfredericks/Bounce2 · GitHub

@brief Returns true if pin signal transitions from high to low.
*/
bool fell() const;

/**
@brief Returns true if pin signal transitions from low to high.
*/
bool rose() const;

You can use these two methods to detect a rising or falling input state

Thanks for making that! I definitely will learn how you did all of that. I'm in a weird stage of learning where I know how some things work probably better than my novice level, but others I'm really lost. So this is a perfect example for me to compare to.

I am curious what does this line do at the end of the enumerated section?
}; States state;

Course, States if you see, is what we named our enum. So consider it similar to a data type like float, String, char, int etc we created a variable called 'state' of type 'States' that we will use to store the current state, which will be equal to one of the enumerations we declared

No, my code has nothing to do with a library. I like people to understand how code works, which means NOT hiding it in a library. I understand that does not suit everyone though, for some people a library is just what they need.

I'm not clear from reading all your replies to me and others that you have understood the importance of detecting when a button becomes pressed as opposed to detecting when it is pressed.

2 Likes

I don't think I understand exactly either. I understand that we are looking to see that state change initially and comparing to that time in terms of millis() for debouncing purposes, but I gather there is more to the button pressing state change that you mean?

I guess it would make more sense if I knew which code was which, in terms of what you're referring to... I could definitely work backwards from there and make sense of it in this specific case.
Your code is oriented around checking for the state change cycle AND THEN outputting a button signal. And that in this case, you are suggesting the reason why my code (with use of that library) was skipping was that the debounce protocol was effectively not detecting the state change cycle of the button, just that the button is pressed.... and since the button was still bouncing, it would come back to check the state of the button, to see it is different than reference, so it continually sends out a signal?

I would have maybe more intuitively seen that, however I did test this with 200ms of debounce and the results were the same. So that's where I'm confused. I suppose the issue is that I don't understand the library I used. But again, 200ms is high for any button.

However I do appreciate you bringing that point back up, because after some thought I think I understood your point..

To me, personally, the literal explanation of button being pressed vs is pressed was a bit confusing. And I find the idea of calling it a button state change cycle a bit more precise. But that is just my honest reflection

The problem with 10 msec is that I will be adding PID control to this. And the process that this prototype board will be controlling actually occurs in approximately 40-80ms depending on which state is changing to which.

Or rather just say like in your code notes "has been pressed"

I had read through your other posts to use the code and understand it, but it has been a few long days. And my mind is a bit fatigued. I had an infusion today so I'm tired to say the least. But I think I understand now, and that is great help. I will be next looking to see if I can potentially nest an if statement inside one of my states, for holding one button down, and tapping the other, to toggle between internal states, then exit to the next state upon release.
But, admittedly first I need to fix the switch case portion by removing it from my loop, and putting it in a void of it's own.

@blackbird78 am I to understand that you now appreciate the difference between when a button becomes pressed and is pressed? Do I need to explain that any more?

I don't know the library, but I can infer things from how you used it. I think you are referring to when I said in my reply #2:

That means each time the loop function goes round it detects the button being pressed and increments or decrements the state machine again for as long as you hold the button.

You understand that the loop function (they are called functions, not 'voids') goes round and round very fast, yes? So it gets to the bit where value1 and value2 are updated, here:

  int value1 = debouncer1.read();
  int value2 = debouncer2.read();

Then just after that you take action depending on whether the values are low or high. Shortly after that loop goes around again, checks the buttons, sees they are still pressed and does the same thing again. There is no memory of the fact that the action was taken last time for the same button press, so it gets repeated until you let go of the button. This is NOT a debounce problem, that's a separate issue (with similar but not the same symptoms). Debounce is about what happens in the first 20ms (roughly) of pressing the button.

@gcjr suggested 10ms delay. The use of delay comes up often in discussions on here (do some searching if you want to read the full debate, do pour yourself a pint of beer / glass of wine / other favourite drink before you start reading). I am firmly in the 'never use delay' camp, others don't agree. You have found for yourself what problems it causes with your concern over whether PID will work properly.

I will be next looking to see if I can potentially nest an if statement inside one of my states, for holding one button down, and tapping the other, to toggle between internal states, then exit to the next state upon release.

Something to consider, and I am not saying this is the right (or wrong) thing to do, if you have switch states you could have separate states for the button becoming pressed, being pressed and being released, this will be 2 or 3 states depending on exactly how you do it. One of those states will take the desired action and then immediately change the state to the next one. Might be worth playing with even if only to learn something.

I need to fix the switch case portion by removing it from my loop, and putting it in a void of it's own.

As already mentioned, they are called functions. The word to the left is the type of variable the function returns, with 'void' meaning the function does not return any variable.

Do you need me to clarify anything else?

it depends on the requirements.

in this case, the PID requirements suggests (??) not using a delay and i showed button code not using delay

but the comments from the OP are confusing

No that's been very helpful Perry, thank you for taking the time. Exactly what I needed.

Yes I am aware that they are not "voids" and are functions. And I am aware that the loop section repeats at (or below obviously depending on the loop contents) clock speed. Like I said, long week, long day. People don't quite understand how strong infusion (chemo/blood based) drugs are. They all hit quite hard on the body. I am loose and fast with language sometimes, but the core ideas I understand.

Thanks for going over it. The debounce section was based off an example, so I definitely also learned not to copy paste junk as well. Makes sense now that I have a better understanding how that library was likely working.

For now this is more than enough help, I have digesting and tinkering to do. I'm sure I'll have more questions down the road.

I also agree with never using delay.

For a toy or a hobby fun project, delay is fine. Lazy, but fine.

But any kind of application where safety is involved, it's flat out unacceptable. There are many applications where this platform fits in amazingly well, that controls events down to the 10ms range.
For my intended application, adding unnecessary blockages to the throughput, would compromise or delay the system's ability to react, can prove catastrophic. I know because I engineered the hydraulics and mechanical components that I intend to control.

Do real world parts move as fast as 10 ms? No. But, when you have to implement a complex inertial based control strategy, with multiple inputs, and PID features... 10 ms in the chain of mechanical events that aggregate to a total of less than 100ms, is a lot. And that can mean compromising the optimized control strategy (not quite LQR) and longevity of the system.

Instead, just do it the right way.

Rather than type that out, or even be more specific, I just tend towards leaving that info, and my opinion aside. I appreciate your help, and will use your example to further my understanding, but delay simply put isn't an option for me to use.