Multiple button code does nothing

I am trying to use (someone elses) code on a Raspberry Pico that is supposed to deal with multiple buttons, and checks wether the buttons were short pressed or long pressed.

The sketch is simple:

#include "ButtonEvents.h"

// define event names for the key events
typedef enum {EVT_NOCHANGE, EVT_PA0_BTNUP, EVT_PA0_LONGPRESS, EVT_PA1_BTNUP, EVT_PA1_LONGPRESS, EVT_PC14_BTNUP, EVT_PC14_LONGPRESS, EVT_PC15_BTNUP, EVT_PC15_LONGPRESS, 
EVT_PB11_BTNUP, EVT_PB11_LONGPRESS, EVT_PA15_BTNUP, EVT_PA15_LONGPRESS, EVT_PA4_BTNUP, EVT_PA4_LONGPRESS, EVT_PB4_BTNUP, EVT_PB4_LONGPRESS , EVT_PB5_BTNUP, EVT_PB5_LONGPRESS } keyEvents;

ButtonEvents b = ButtonEvents(EVT_NOCHANGE);

#define   BTN      13

void setup() {

  while (!Serial) {
  }
  Serial.begin(115200);
  Serial.println("Starting");

  b.add(BTN, EVT_PA1_BTNUP, EVT_PA1_LONGPRESS);
}

void loop() {
  int event = b.getButtonEvent();
  Serial.printf("Event = %i\n", event);
}

Notice that I have added only one button here for test purposes.

The include file (ButtonEvents.h) is a bit more complex:

class ButtonEvents {
  private:
    typedef enum {STATE_UP, STATE_DOWN, STATE_LONGPRESSED} transitionStates; 
          
    struct Button {
      Button* next;
      
      byte pin;
      byte event_button_up;
      byte event_long_press;
      
      // debounce 
      byte stateHistory;
      bool debouncedButtonState;

      // transition handling - button_up, long_press
      transitionStates transitionState;
      int_fast32_t buttonPressedDuration;
    };
    
    Button* next = NULL;
    int noChangeEvent = 0;
    int_fast32_t nextButtonPoll = 0;

    int debounce(Button* b) {
      int buttonPosition = digitalRead(b->pin);

      b->stateHistory = ((b->stateHistory<<1) | buttonPosition) & 0x0f;  
      if (b->stateHistory == 0x0) {b->debouncedButtonState = true;}; 
      if (b->stateHistory == 0xf) {b->debouncedButtonState = false;};
      return b->debouncedButtonState;
    }

    int getButtonEvent(Button* b) {
      bool btn = debounce(b);
      int result = noChangeEvent;
      switch (b->transitionState) {
        case STATE_UP:
          if (btn) {
            b->transitionState = STATE_DOWN;
            b->buttonPressedDuration = millis() + 1000;
          }
          break;
        case STATE_DOWN:
          if (!btn) {
            b->transitionState = STATE_UP;
            result = b->event_button_up;
          }
          if (millis() > b->buttonPressedDuration) {
            b->transitionState = STATE_LONGPRESSED;
            result = b->event_long_press;
          }
          break;
        case STATE_LONGPRESSED:
          if (!btn) {
            b->transitionState = STATE_UP;
          }
          break;
      default:
          b->transitionState = STATE_UP;
      }
      return result;
    }
    
  public:
    ButtonEvents(int noChange) {
      noChangeEvent = noChange;
    };
    
    void add(uint8_t pinNo, int evt_button_up, int evt_long_press) {
      pinMode( pinNo, INPUT_PULLUP);

      // initialise the button structure
      Button* b = (Button*) malloc(sizeof(struct Button));
      //b = {next, pinNo, evt_button_up, evt_long_press, 0, false, STATE_UP, 0};
      b->next = next;
      b->pin = pinNo;
      b->event_button_up = evt_button_up;
      b->event_long_press = evt_long_press;
      b->stateHistory = 0;
      b->debouncedButtonState = false;
      b->transitionState = STATE_UP;
      b->buttonPressedDuration = 0;

      // add it to the head of the chain
      next = b;
    };


    // call this often in the main loop.
    int getButtonEvent()
    {
      // poll the button inputs no closer than every 10 ms
      int_fast32_t currentTime = millis();
      if (currentTime >= nextButtonPoll) {
        nextButtonPoll = currentTime + 10;  
        Button* b = next;
        while(b) {  //scan down the linked list
          int event = getButtonEvent(b);
          if (event != noChangeEvent) {
            Serial.println("New event");
            return event;
          }
          b = b->next;
        }
      }
      return noChangeEvent;
    }
};

The code loads on the Pico without problems, but however short or long I press the button attached to GPIO 13, nothing is returned from the GetButtonEvent function.
However, when I use the simple Button example, the button works, and makes my onboard LED light up when I press the button.

It has to be said that this code was originally written for a Blue Pill (STMF103C8T6), but I see no reason why it should not work on a Pico as well.
Or am I having problems with timing issues here ?

Anyhow, any help would be greatly appreciated.

Post the simple example that works. The code looks plausible, so.

a7

The code that works is the button example supplied with the Arduino IDE:

/*
  Button

  Turns on and off a light emitting diode(LED) connected to digital pin 13,
  when pressing a pushbutton attached to pin 2.

  The circuit:
  - LED attached from pin 13 to ground through 220 ohm resistor
  - pushbutton attached to pin 2 from +5V
  - 10K resistor attached to pin 2 from ground

  - Note: on most Arduinos there is already an LED on the board
    attached to pin 13.

  created 2005
  by DojoDave <http://www.0j0.org>
  modified 30 Aug 2011
  by Tom Igoe

  This example code is in the public domain.

  https://www.arduino.cc/en/Tutorial/BuiltInExamples/Button
*/

// constants won't change. They're used here to set pin numbers:
const int buttonPin = 13;  // the number of the pushbutton pin
const int ledPin = 25;    // the number of the LED pin

// variables will change:
int buttonState = 0;  // variable for reading the pushbutton status

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);
}

void loop() {
  // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);

  // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
  if (buttonState == HIGH) {
    // turn LED on:
    digitalWrite(ledPin, HIGH);
  } else {
    // turn LED off:
    digitalWrite(ledPin, LOW);
  }
}

The only changes made in the example are ledPin set to 25 (led of pico) and buttonPin set to 13 (gpio13 of Pico).
This code works. The code posted in the question does not...

You should post the link to where you got the button-event-library from

to me it seems that the button-event-code that you posted is not the complete
ButtonEvents.h-file

You should always post complete files

For testing I would use all button-events that are available not only the ones you want to have in your final project.

This will give additional information

For future use of any kind of libraries
if a library does not provide example-codes
skip the library and try another one
if you want to do the author of such a library without examples a favor
write a short feedback pointing on this

How have you wired the button? The first program appears to require the button on the low side. The second appears to require the button on the high side with an external pull down resistor.

buttons are typically connected between the pin and ground, the pin configured as INPUT_PULLUP to use the internal pullup resistor which pulls the pin HIGH and when pressed, the button pulls the pin LOW.

Hello phloks

Try and check this proposal:

//https://forum.arduino.cc/t/multiple-button-code-does-nothing/1214958
//https://europe1.discourse-cdn.com/arduino/original/4X/7/e/0/7e0ee1e51f1df32e30893550c85f0dd33244fb0e.jpeg
#define ProjectName "Multiple button code does nothing"
#define NotesOnRelease "Arduino MEGA tested"
// make names
enum TimerEvent {NotExpired, Expired};
enum TimerControl {Halt, Run};
enum ButtonState {BecomesReleased, BecomesPressed};
// make variables
constexpr uint8_t ButtonPins[] {A0};
constexpr uint32_t Short2Long  {1000};
// make structures
struct TIMER
{
  uint32_t interval;
  uint8_t control;
  uint32_t now;
  uint8_t expired(uint32_t currentMillis)
  {
    uint8_t timerEvent = currentMillis - now >= interval and control;
    if (timerEvent == Expired) now = currentMillis;
    return timerEvent;
  }
};
struct BUTTON
{
  uint8_t name;
  uint8_t pin;
  uint8_t stateOld;
  uint32_t clickTime;
  uint32_t   click;
  TIMER   debounce;

  void make (uint8_t name_, uint8_t pin_, uint32_t click_)
  {
    Serial.println(__func__);
    name = name_;
    pin = pin_;
    pinMode(pin, INPUT_PULLUP);
    stateOld = digitalRead(pin) ? LOW : HIGH;
    clickTime = 0;
    click = click_;
    debounce.interval = 20;
    debounce.control = Run;
  }
  void run(uint32_t currentMillis )
  {
    if (debounce.expired(currentMillis) == Expired)
    {
      uint8_t stateNew = digitalRead(pin) ? LOW : HIGH;
      if (stateOld != stateNew)
      {
        stateOld = stateNew;

        switch (stateNew)
        {
          case BecomesPressed:
            Serial.print("Button ");
            clickTime = currentMillis;
            break;
          case BecomesReleased:
            Serial.print (name), Serial.print(" pressed ");
            Serial.println ((currentMillis - clickTime > click) ? "long" : "short");
            break;
        }
      }
    }
  }
} buttons[sizeof(ButtonPins)];

// make support
void heartBeat(const uint8_t LedPin, uint32_t currentMillis)
{
  static bool setUp = false;
  if (setUp == false) pinMode (LedPin, OUTPUT), setUp = true;
  digitalWrite(LedPin, (currentMillis / 500) % 2);
}
// make application
void setup()
{
  Serial.begin(115200);
  Serial.print("Source: "), Serial.println(__FILE__);
  Serial.print(ProjectName), Serial.print(" - "), Serial.println(NotesOnRelease);
  uint8_t element = 0;
  for (auto &button : buttons)
  {
    button.make (element, ButtonPins[element], Short2Long);
    element++;
  }
  Serial.println(" =-> and off we go\n");
}
void loop()
{
  uint32_t currentMillis = millis();
  heartBeat(LED_BUILTIN, currentMillis);
  for (auto &button : buttons)button.run(currentMillis);
}

Have a nice day and enjoy coding in C++.

I agree.
The ButtonEvents.h file does look like this one on Github:
https://github.com/fasteddy516/ButtonEvents
where part of the code in the cpp file has found its way to the .h file.

Sadly, I cannot provide the entire code. The entire, much larger, sketch has been developed over years, and I had to promise not to disclose this sketch.
I intend to keep that promise.
Having said that, all references pertaining to buttons from the original sketch are shown here.

Thanks,

When time permits, I'll look into this.

The button is connected between GPIO13 and ground...

The you are stuck getting help from whomever you made that promise to. This is an open source community. We share code here and help each other with it. If you've got secret code then work on it in secret. We obviously can't help you fix code that we can't see.

Be sure whomever made you promise this understands that distributing code without disclosing the source may be in violation of license agreements on the libraries that they used to write that code.

1 Like

Hello PaulPaulson,

Just tried your code, and it does exactly what would like to achieve.
However, there's one thing I cannot get my head around.

Since I need to call different functions depending on what button was pressed, I replaced the following piece of your code :

case BecomesReleased:
    Serial.print (name), Serial.print(" pressed ");
    Serial.println ((currentMillis - clickTime > click) ? "long" : "short");
    break;

with

case BecomesReleased:
  switch (name)
  {
    case 0:
      Serial.print(name), Serial.print(" pressed ");
      Serial.println((currentMillis - clickTime > click) ? "long" : "short");
    break;
    case 1:
      Serial.print(name), Serial.print(" pressed ");
      Serial.println((currentMillis - clickTime > click) ? "long" : "short");
    break;
    case 2:
      Serial.print(name), Serial.print(" pressed ");
      Serial.println((currentMillis - clickTime > click) ? "long" : "short");
    break;
  }
break;

with the intention of course to replace the print statements with calls to the respective functions.

For some reason I do not understand, after setup and in the very first loop, all cases in the switch(name) loop are traversed. For case 0 nothing is printed.
For case 1 and case 2, the difference between currentMillis and clickTime is > 1000, so I can see the texts

1 pressed long
2 pressed long

But no buttons were pressed yet.
After that first loop, everything works as expected.

So the question is:
Why is the switch(name) statement entered and why are the case 0/1/2 statements executed while no buttons where pressed yet ?

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