Help me debug a switch polling function

[330R]

I'm using an arduino nano. As a little test for a bigger project of mine, I've made a little program to interface an lcd and a couple of buttons. The polling of the buttons is handled by a function, which is called once for each button in the main loop. My issue is this: when I call the function to monitor just one switch, all goes well and my counter is incremented by one however long I keep the button pressed. However, when I call another instance of the same function to monitor a second button, the first one keeps triggering and increments the counter as long as the button is kept pressed, which is not the way it should behave. The serial monitor shows that, in this case, the button_flag variable continually oscillates between true and false, and, therefore, each true reading increments my counter. I've also tried to implement the same idea with return values from my polling function instead of pointers and and global variables, but the problem stays the same. Making a small library and implementing the polling function as a class seems overkill for such a trivial case. When I comment out the "crazy lane" (#49), all goes well. My code follows.

#include <LiquidCrystal.h>

const uint8_t rs = 12, en = 13, d4 = 8, d5 = 9, d6 = 10, d7 = 11;
LiquidCrystal lcd(rs, en, d4, d5, d6, d7);
// end LiquidCrystal

#define BUTTON_PIN 3
#define COUNTER_RESET_PIN 4
#define DEBOUNCE_DELAY 6

#define DEBUG 1

// Global varaibles

char gTimeString[9];
char gCounterString[9];
unsigned int gButtonCounter = 0;
bool gPushbutton, gReset;

void setup()
{

    pinMode(BUTTON_PIN, INPUT_PULLUP);
    pinMode(COUNTER_RESET_PIN, INPUT_PULLUP);

    #if DEBUG
    Serial.begin(9600);
    #endif
    
    // Lcd setup
    lcd.begin(16,2);
    delay(50);
    lcd.clear();
    delay(10);
    // end setup

    lcd.print("Uptime: ");
    lcd.setCursor(0,1);
    lcd.print("Counter: ");
}

void loop()
{  
    updateTimeString((unsigned int) (millis() / 1000));
    updateCounterString(gButtonCounter);
    
    
    pollButton(BUTTON_PIN, &gPushbutton);
    pollButton(COUNTER_RESET_PIN, &gReset); // <-- *** CRAZY LINE ***
    
    if (gPushbutton)
    {
        gButtonCounter += 1;

        #if DEBUG
        Serial.println("Pushbutton = true.");
        #endif
    }
    else
    {
        #if DEBUG
        Serial.println("Pushbutton = false.");
        #endif
    } // end gPushbutton
   
    if (gReset)
        gButtonCounter = 0;
    

    lcd.setCursor(8,0);
    lcd.print(gTimeString);

    lcd.setCursor(9,1);
    lcd.print(gCounterString);

    #if DEBUG
    // slow down so we can follow it on monitor
    delay (500);
    #endif

}

void pollButton (int button_pin, bool * button_flag)
{
    static bool last_state, current_state;

    *button_flag = false;    
    current_state = digitalRead(button_pin);

    if ( current_state != last_state )
    {
        delay(DEBOUNCE_DELAY);
        
        if (current_state == 0) // active low
            *button_flag = true;
    } // end outer if

    last_state = current_state;
    return;
}

void updateTimeString (unsigned int seconds)
{
    unsigned int h, m, s;

    h = seconds / 3600;
    m = (seconds / 60) % 60;
    s = seconds % 60;
   
    sprintf (gTimeString, "%02u:%02u:%02u", h, m, s);

    gTimeString[8] = '\0';
    return;
}

void updateCounterString (unsigned int number)
{
    sprintf (gCounterString, "%7u", number);
    return;
}

[Danois90]

In "pollButton" you have "static bool last_state, current_state". "current_state" should not be static and "last_state" must be maintained "per button", meaning that you have to save the last state for each button if it should work. In fact, you could use argument "* button_flag" instead of "last_state" with some minor modifications.

[gcjr]

consider

#undef MyHW
#ifdef MyHW
# define BUTTON_PIN          A1
# define COUNTER_RESET_PIN   A2

#else
# define BUTTON_PIN          3
# define COUNTER_RESET_PIN   4
#endif

#define DEBOUNCE_DELAY 6
#define DEBUG 1

bool gPushbutton, gReset;

bool
pollButton (
    int   button_pin,
    bool *button_flag)
{
    bool but = digitalRead (button_pin);
    if (*button_flag != but) {
        *button_flag = but;
        delay (DEBOUNCE_DELAY);

        if (LOW == but)
            return true;
    }
    return false;
}

void loop ()
{
    if (pollButton (BUTTON_PIN, &gPushbutton))
        Serial.println ("pushButton");

    if (pollButton (COUNTER_RESET_PIN, &gReset))
        Serial.println ("reset Button");
}

void setup ()
{
#if DEBUG
    Serial.begin (9600);
#endif

    pinMode (BUTTON_PIN,         INPUT_PULLUP);
    gPushbutton = digitalRead (BUTTON_PIN);

    pinMode (COUNTER_RESET_PIN,  INPUT_PULLUP);
    gReset      = digitalRead (COUNTER_RESET_PIN);
}

[330R]

@gcjr Thanks, that's what I needed to make it work. It looks like I had to merge my two approaches, i.e. both changing a global variable by reference and having the function return a boolean flag.

@Danois90 Now indeed each button is kept track of into it's own global variable. The fact that current_state be static or not doesn't change much (I've run some experiments with both versions) since it gets reset as soon as the function is called anyway.

I still don't understand what exactly was amiss with my code, but hey, I got it alright now!