Want to multiply a button debounce routine neatly

Hi I've finally succeeded with writing a buttondebounce routine.

I think I must use a pointer to the debounceroutine to reuse the routine for more buttons so that I can do something like

if (button1ReallyPressed)
a();
if (button2ReallyPressed)
b();
if (button3ReallyPressed)
c();

How can I do this modyfing the code:

const int buttonPin = 2;    // the number of the pushbutton pin
const int ledPin = 13;      // the number of the LED pin
int ledState;
int buttonReallyPressed(int *i); //output-signal from the statemachine



void setup() {
  ledState=LOW;
  pinMode(buttonPin, INPUT);
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, ledState);
  Serial.begin(9600);
}
void loop() {
  int reading = digitalRead(buttonPin); 
  if(buttonReallyPressed(&reading) == HIGH){
    ledState=!ledState;
          digitalWrite(ledPin,ledState);      
  }
}//end loop
int buttonReallyPressed(int *reading){
//buttonstates - possible states in statemachine
/* Description of statemachine
button not pressed and "released" is true => State = "start"
button pressed => State goes from "start" to "firstButtonPressDetected". Timer starts.
button still pressed => "firstButtonPressDetected" goes immediately to "countingMilliseconds". Go back to "start" if button released to early.
if button pressed longer than 50ms go to "countingBiggerThanButtonDelayTime". Use this state for your outputfunction.
if button still not released go to "countingBiggerThanButtonDelayTimeNoReleaseYet" and do nothing wait for release.
When button released go to "start".
*/
    const int start=0;
    const int firstButtonPressDetected=1;
    const int countingMilliseconds=2;
    const int countingBiggerThanButtonDelayTime =3;
    const int countingBiggerThanButtonDelayTimeNoReleaseYet=4;
    const long debounceDelay = 50;    // the debounce time; increase if the output flickers
    long startTime=0;
    long timeButtonPressed; 
static int state;//Current state in statemachine

    switch (state){
    case start:
         if (*reading == HIGH){
             state = firstButtonPressDetected;
         }
         return LOW;
      break;
    case firstButtonPressDetected:
         if (*reading == HIGH){
             startTime=millis();
             state = countingMilliseconds;
         }
         else{
           state = start;
           timeButtonPressed=0;
         }
         return LOW;
       break;
     case  countingMilliseconds:
           if (*reading == HIGH){
             timeButtonPressed=millis()-startTime;
             if (timeButtonPressed > debounceDelay){
                state = countingBiggerThanButtonDelayTime;
             }
           }else{
               state = start;
               timeButtonPressed=0;
           }
           return LOW;
       break;
     case countingBiggerThanButtonDelayTime:
           if (*reading == HIGH){
             state = countingBiggerThanButtonDelayTimeNoReleaseYet;
             return HIGH;
           }
           else{
               state = start;
               timeButtonPressed=0;
               return LOW;
           }
        break;
     case countingBiggerThanButtonDelayTimeNoReleaseYet:
        if (*reading == LOW){
           state = start;
           timeButtonPressed=0;
        }
        return LOW;
        break;
     default:
          return LOW;
          break;                             
  }//end switch 
  
  }

I think I must use a pointer to the debounceroutine to reuse the routine for more buttons so that I can do something like

I don't. Your function is (poorly) written to deal with one switch.

First, don't use a pointer when what you want is a reference variable.

int buttonReallyPressed(int &reading)
{

Second, don't use pointers or references when the function does not modify the value passed in.

The function should not have any static variables. It should not use any global variables.

It should take the current state of the switch and the state of the switch last time.

Then, the caller is responsible for maintaining the state, in a global or static array.

Thank for answering Paul. Your programming practises are very sound, but I don't want to do it that way.

I want each instance of the buttondebounceroutine to contain the states of the specific button it belongs to.

I want each instance of the buttondebounceroutine to contain the states of the specific button it belongs to.

If you need to change something, you'll need to change it in every routine. That way leads to bugs, where you miss a change in one of several nearly identical functions.

But, hey, it's your code. Do it your way.

Hi all, I got the structure that I want. Grabbed info on function pointers from Nick Gammon

But I have compilation errors...

sketch_dec25b.ino: In function 'void loop()':
sketch_dec25b:29: error: 'Button1Pressed' was not declared in this scope
sketch_dec25b:34: error: 'Button2Pressed' was not declared in this scope

Can somebody help?

const int button1Pin = 2;    // the number of the pushbutton pin
const int button2Pin = 3;
const int ledPin1 = 13;      // the number of the LED pin
const int ledPin2 = 12;
int ledState1;
int ledState2;
typedef int (*ButtonDebounceFunction) ( int arg1);
int buttonReallyPressed(int i); 



void setup() {
  ButtonDebounceFunction Button1Pressed = buttonReallyPressed;
  ButtonDebounceFunction Button2Pressed = buttonReallyPressed;
  ledState1=LOW;
  ledState2=LOW;
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  digitalWrite(ledPin1, ledState1);
  digitalWrite(ledPin2, ledState2);
  Serial.begin(9600);
}
void loop() {
  int reading1 = digitalRead(button1Pin); 
  if(Button1Pressed(reading1) == HIGH){
    ledState1=!ledState1;
          digitalWrite(ledPin1,ledState1); 
     
 int reading2 = digitalRead(button2Pin); 
  if(Button2Pressed(reading2) == HIGH){
    ledState2=!ledState2;
          digitalWrite(ledPin2,ledState2);         
  }
}//end loop

int buttonReallyPressed(int reading){
//buttonstates - possible states in statemachine
/* Description of statemachine
button not pressed and "released" is true => State = "start"
button pressed => State goes from "start" to "firstButtonPressDetected". Timer starts.
button still pressed => "firstButtonPressDetected" goes immediately to "countingMilliseconds". Go back to "start" if button released to early.
if button pressed longer than 50ms go to "countingBiggerThanButtonDelayTime". Use this state for your outputfunction.
if button still not released go to "countingBiggerThanButtonDelayTimeNoReleaseYet" and do nothing wait for release.
When button released go to "start".
*/
    const int start=0;
    const int firstButtonPressDetected=1;
    const int countingMilliseconds=2;
    const int countingBiggerThanButtonDelayTime =3;
    const int countingBiggerThanButtonDelayTimeNoReleaseYet=4;
    const long debounceDelay = 50;    // the debounce time; increase if the output flickers
    long startTime=0;
    long timeButtonPressed; 
static int state;//Current state in statemachine

    switch (state){
    case start:
         if (reading == HIGH){
             state = firstButtonPressDetected;
         }
         return LOW;
      break;
    case firstButtonPressDetected:
         if (reading == HIGH){
             startTime=millis();
             state = countingMilliseconds;
         }
         else{
           state = start;
           timeButtonPressed=0;
         }
         return LOW;
       break;
     case  countingMilliseconds:
           if (reading == HIGH){
             timeButtonPressed=millis()-startTime;
             if (timeButtonPressed > debounceDelay){
                state = countingBiggerThanButtonDelayTime;
             }
           }else{
               state = start;
               timeButtonPressed=0;
           }
           return LOW;
       break;
     case countingBiggerThanButtonDelayTime:
           if (reading == HIGH){
             state = countingBiggerThanButtonDelayTimeNoReleaseYet;
             return HIGH;
           }
           else{
               state = start;
               timeButtonPressed=0;
               return LOW;
           }
        break;
     case countingBiggerThanButtonDelayTimeNoReleaseYet:
        if (reading == LOW){
           state = start;
           timeButtonPressed=0;
        }
        return LOW;
        break;
     default:
          return LOW;
          break;                             
  }//end switch 
  
  }
  ButtonDebounceFunction Button1Pressed = buttonReallyPressed;
  ButtonDebounceFunction Button2Pressed = buttonReallyPressed;

The key to the message is the phrase "in this scope". The scope of these variables is that of the setup() function. After setup() ends, these variables go away.

Yes Paul, you're right. No compilation errors!

But the code is not doing anything anymore. Do you know why?

const int button1Pin = 2;    // the number of the pushbutton pin
const int button2Pin = 3;
const int ledPin1 = 13;      // the number of the LED pin
const int ledPin2 = 12;
int ledState1;
int ledState2;
typedef int (*ButtonDebounceFunction) ( int arg1);
int buttonReallyPressed(int i); 
ButtonDebounceFunction Button1Pressed = buttonReallyPressed;
ButtonDebounceFunction Button2Pressed = buttonReallyPressed;


void setup() {
  ledState1=LOW;
  ledState2=LOW;
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  digitalWrite(ledPin1, ledState1);
  digitalWrite(ledPin2, ledState2);
  Serial.begin(9600);
}
void loop() {
  int reading1 = digitalRead(button1Pin); 
  if(Button1Pressed(reading1) == HIGH){
    ledState1=!ledState1;
          digitalWrite(ledPin1,ledState1); 
  }
 int reading2 = digitalRead(button2Pin); 
  if(Button2Pressed(reading2) == HIGH){
    ledState2=!ledState2;
          digitalWrite(ledPin2,ledState2);         
  }
}//end loop

int buttonReallyPressed(int reading){
//buttonstates - possible states in statemachine
/* Description of statemachine
button not pressed and "released" is true => State = "start"
button pressed => State goes from "start" to "firstButtonPressDetected". Timer starts.
button still pressed => "firstButtonPressDetected" goes immediately to "countingMilliseconds". Go back to "start" if button released to early.
if button pressed longer than 50ms go to "countingBiggerThanButtonDelayTime". Use this state for your outputfunction.
if button still not released go to "countingBiggerThanButtonDelayTimeNoReleaseYet" and do nothing wait for release.
When button released go to "start".
*/
    const int start=0;
    const int firstButtonPressDetected=1;
    const int countingMilliseconds=2;
    const int countingBiggerThanButtonDelayTime =3;
    const int countingBiggerThanButtonDelayTimeNoReleaseYet=4;
    const long debounceDelay = 50;    // the debounce time; increase if the output flickers
    long startTime=0;
    long timeButtonPressed; 
static int state;//Current state in statemachine

    switch (state){
    case start:
         if (reading == HIGH){
             state = firstButtonPressDetected;
         }
         return LOW;
      break;
    case firstButtonPressDetected:
         if (reading == HIGH){
             startTime=millis();
             state = countingMilliseconds;
         }
         else{
           state = start;
           timeButtonPressed=0;
         }
         return LOW;
       break;
     case  countingMilliseconds:
           if (reading == HIGH){
             timeButtonPressed=millis()-startTime;
             if (timeButtonPressed > debounceDelay){
                state = countingBiggerThanButtonDelayTime;
             }
           }else{
               state = start;
               timeButtonPressed=0;
           }
           return LOW;
       break;
     case countingBiggerThanButtonDelayTime:
           if (reading == HIGH){
             state = countingBiggerThanButtonDelayTimeNoReleaseYet;
             return HIGH;
           }
           else{
               state = start;
               timeButtonPressed=0;
               return LOW;
           }
        break;
     case countingBiggerThanButtonDelayTimeNoReleaseYet:
        if (reading == LOW){
           state = start;
           timeButtonPressed=0;
        }
        return LOW;
        break;
     default:
          return LOW;
          break;                             
  }//end switch 
  
  }

IMHO you first need to consider whether, and why, any debounce code is needed.

You only get button bounce problems if the unwanted switch reading causes your program to do something undesirable. In most cases the normal management of the code (for example the time for an LED to light and be visible) comfortably deals with bounce issues without any special code being needed.

...R

This has to be the BIGGEST debounce routine I've seen (so far). :slight_smile:

Yes people but it seems that i don't receive separate instances of the debounce-routine(contains a static variable) when I do

typedef int (*ButtonDebounceFunction) ( int arg1);
int buttonReallyPressed(int i); 
ButtonDebounceFunction Button1Pressed = buttonReallyPressed;
ButtonDebounceFunction Button2Pressed = buttonReallyPressed;

shoud I use new(), malloc etc. somehow?

laskar01:
shoud I use new(), malloc etc. somehow?

No.

...R

The neatest way is to create a class, structure or some other sort of library code to encapsulate the data required for the debouncing. There are a couple of examples of doing this for Analog and Digital switches in my code libraries (links below).

Thank you all, and thank you marco_c, I will look into your libraries.
I thought one couldn't use structs or classes in the Arduino IDE ???
Because I wrote "struct" and it was not highlighted, so I didn't manage to encapsulate the way I wanted.
So I had to save all info about the buttons outside the statemachine that takes care of the debouncing...
AND THIS IS NOT WHAT I WANTED :frowning: Buhuu Cry, cry

I wanted it clean with no variables visible. I wanted to do this:

const int button1Pin = 2;    // the number of the pushbutton pin
const int button2Pin = 3;
const int ledPin1 = 13;      // the number of the LED pin
const int ledPin2 = 12;
int ledState1;
int ledState2;
typedef int (*ButtonDebounceFunction) ( int arg1);
int buttonReallyPressed(int i); //output-signal from the statemachine
//DOES NOT WORK! Two pointers to same function, not getting separate instances of same function
  ButtonDebounceFunction Button1Pressed = buttonReallyPressed;
  ButtonDebounceFunction Button2Pressed = buttonReallyPressed;

....
void loop() {
   ; 
  if(Button1Pressed(digitalRead(button1Pin)) == HIGH){
      ledState1=!ledState1;
      digitalWrite(ledPin1,ledState1);      
  }
  
  if(Button2Pressed(digitalRead(button2Pin)) == HIGH){
      ledState2=!ledState2;
      digitalWrite(ledPin2,ledState2);      
  }
}//end loop

This would have been pretty and clean, with a lot of info encaspulated.

I ended up with this mess with all details dispalyed, but it works though...

const int button1Pin = 2;    // the number of the pushbutton pin
const int button2Pin = 3;
const int ledPin1 = 13;      // the number of the LED pin
const int ledPin2 = 12;
int ledState1;
int ledState2;
const int numberOfButtons=2;
int buttonState[numberOfButtons];
long timeButtonPressed[numberOfButtons];
long startTime[numberOfButtons];
int ButtonPressed(int, int*, long*, long*);

void setup() {
  ledState1=LOW;
  ledState2=LOW;
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  digitalWrite(ledPin1, ledState1);
  digitalWrite(ledPin2, ledState2);
  Serial.begin(9600);
  int i=0;
  for (i = 0; i<numberOfButtons;i++)
    buttonState[i]=0;
    timeButtonPressed[i]=0;
    startTime[i]=0;
}
void loop() {
  int reading1 = digitalRead(button1Pin); 
  if(ButtonPressed(reading1,&buttonState[0],&startTime[0],&timeButtonPressed[0]) == HIGH){
          ledState1=!ledState1;
          digitalWrite(ledPin1,ledState1); 
  }
 int reading2 = digitalRead(button2Pin); 
  if(ButtonPressed(reading2,&buttonState[1],&startTime[1],&timeButtonPressed[1]) == HIGH){
      ledState2=!ledState2;
      digitalWrite(ledPin2,ledState2);         
  }
}//end loop

int ButtonPressed(int reading, int *state,long *startTime ,long *timeButtonPressed){
//buttonstates - possible states in statemachine
/* Description of statemachine
button not pressed and "released" is true => State = "start"
button pressed => State goes from "start" to "firstButtonPressDetected". Timer starts.
button still pressed => "firstButtonPressDetected" goes immediately to "countingMilliseconds". Go back to "start" if button released to early.
if button pressed longer than 50ms go to "countingBiggerThanButtonDelayTime". Use this state for your outputfunction.
if button still not released go to "countingBiggerThanButtonDelayTimeNoReleaseYet" and do nothing wait for release.
When button released go to "start".
*/
    const int start=0;
    const int firstButtonPressDetected=1;
    const int countingMilliseconds=2;
    const int countingBiggerThanButtonDelayTime =3;
    const int countingBiggerThanButtonDelayTimeNoReleaseYet=4;
    const long debounceDelay = 50;    // the debounce time; increase if the output flickers
    

    switch (*state){
    case start:
         if (reading == HIGH){
             *state = firstButtonPressDetected;
         }
         return LOW;
      break;
    case firstButtonPressDetected:
         if (reading == HIGH){
             *startTime=millis();
             *state = countingMilliseconds;
         }
         else{
           *state = start;
           *timeButtonPressed=0;
         }
         return LOW;
       break;
     case  countingMilliseconds:
           if (reading == HIGH){
             *timeButtonPressed=millis()-*startTime;
             if (*timeButtonPressed > debounceDelay){
                *state = countingBiggerThanButtonDelayTime;
             }
           }else{
               *state = start;
               *timeButtonPressed=0;
           }
           return LOW;
       break;
     case countingBiggerThanButtonDelayTime:
           if (reading == HIGH){
             *state = countingBiggerThanButtonDelayTimeNoReleaseYet;
             return HIGH;
           }
           else{
               *state = start;
               *timeButtonPressed=0;
               return LOW;
           }
        break;
     case countingBiggerThanButtonDelayTimeNoReleaseYet:
        if (reading == LOW){
           *state = start;
           *timeButtonPressed=0;
        }
        return LOW;
        break;
     default:
          return LOW;
          break;                             
  }//end switch 
  
  }