bxparks / AceButton

An adjustable, compact, event-driven button library for Arduino that debounces and dispatches events to a user-defined event handler.
MIT License
385 stars 37 forks source link

Button presses not recognized at all or random #19

Closed T-vK closed 5 years ago

T-vK commented 5 years ago

AceButton version: 1.3.2 Arduino IDE version: 1.8.5 Arduino: Arduino Micro (A clone, not the original one. The IDE recognizes the Arduino as an "Arduino Leonardo". )

The click and long press events aren't triggered at all no matter if I press the button for only a few milliseconds or for a long time. Sometimes instead of not reacting at all it just thinks it is constantly receiving presses. It works just fine and as expected when I simply use digitalRead instead.

I minimized the code to a minimum and even took the button and anything else connected to the Arduino out of the equation, I only used a wire to "short" GND to a digital pin (I tried many different pins including 2, 3, 4 and 9). Just to make sure I opened up another brand new Arduino Micro clone, but I had the exact same issues. I also tried instantiating with AceButton leftPedal(LEFT_PEDAL, HIGH); and AceButton leftPedal(LEFT_PEDAL, LOW);. But still it didn't behave correctly. (I'm not using an external pull-down resistor btw, just a wire.)

This works perfectly fine:

const int buttonPin = 2;
int buttonState = 0;

void setup() {
  delay(1000);
  Serial.begin(115200);
  while (! Serial); // Wait until Serial is ready - Leonardo/Micro
  Serial.println(F("setup(): begin"));
  digitalWrite(buttonPin, HIGH);
}

void loop() {
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH) {
    Serial.println(F("HIGH"));
  } else {
    Serial.println(F("LOW"));
  }
}

This code misbehaves as described above:

#include <AceButton.h>
using namespace ace_button;

//uint16_t clickDelay = 100;
//uint16_t debounceDelay = 50;
//uint16_t repeatPressDelay = 500;
//uint16_t repeatPressInterval = 500;

const int LEFT_PEDAL = 2;

AceButton leftPedal(LEFT_PEDAL);

void scrollDownEvent(AceButton*, uint8_t, uint8_t);

void setup() {
  delay(1000); // some microcontrollers reboot twice
  Serial.begin(115200);
  while (! Serial); // Wait until Serial is ready - Leonardo/Micro
  Serial.println(F("setup(): begin"));

  ButtonConfig* leftPedalConfig = leftPedal.getButtonConfig();
  leftPedalConfig->setEventHandler(scrollDownEvent);
  leftPedalConfig->setFeature(ButtonConfig::kFeatureClick);
  //leftPedalConfig->setFeature(ButtonConfig::kFeatureDoubleClick);
  //leftPedalConfig->setFeature(ButtonConfig::kFeatureLongPress);
  leftPedalConfig->setFeature(ButtonConfig::kFeatureRepeatPress);
  //leftPedalConfig->setDebounceDelay(clickDelay);
  //leftPedalConfig->setDebounceDelay(debounceDelay);
  //leftPedalConfig->setRepeatPressDelay(repeatPressDelay);
  //leftPedalConfig->setRepeatPressInterval(repeatPressInterval);

  Serial.println(F("setup(): ready"));
}

void loop() {
  leftPedal.check();
}

void scrollDownEvent(AceButton* /* button */, uint8_t eventType, uint8_t buttonState) {
  switch (eventType) {
    case AceButton::kEventPressed:
      Serial.println(F("kEventPressed: SCROLLDOWN"));
      break;
    case AceButton::kEventRepeatPressed:
      Serial.println(F("kEventRepeatPressed: SCROLLDOWN"));
      break;
  }
}

I spent the last 4 hours desperately trying to get this to work. I would really appreciate it if you could check this out and tell me what the problem is.

bxparks commented 5 years ago

Hi, Without seeing your hardware setup, I can only make educated guesses. From your posted code, my guess is that you forgot to call the pinMode(LEFT_PEDAL, INPUT_PULLUP) method in your setup(). Recall that an Arduino board starts up with its pins in INPUT mode, not INPUT_PULLUP (see https://www.arduino.cc/en/Tutorial/DigitalPins). This means that the pin is in a high impedance state, so that if there is no wire connected to the pin, its voltage level is floating. The logical level of a floating input pin will be completely random. You need to connect an external pullup resistor (so that the button reads HIGH when there is nothing connected to the pin). OR, call pinMode(LEFT_PEDAL, INPUT_PULLUP) in the setup method to tell the microcontroller to enable the internal pull up resistor on the specified pin.

I think the reason your first sample code works is that you make a call todigitalWrite(button, HIGH). My understanding is that this is an old (potentially deprecated?) method of setting the pin into an INPUT_PULLUP mode.

For a problem like this, I recommend going to one of the example code that I provided in the examples/ folder. A good one is examples/SingleButton/SingleButton.ino. Try running it exactly as is, with no changes (except maybe the pin number), and verify that it works. Then take a look at another example that uses the kEventRepeatPressed event, i.e. examples/TunerButtons/TunerButtons.ino, and incrementally add the various lines of code to implement your leftPedal button.

Hope this helps.

T-vK commented 5 years ago

Thanks for the reply! I just read "handles both pull-up and pull-down wiring" in the Readme and in the docs I read "When using a pullup resistor (either external or internal) and the button is connected to ground, this should be set to HIGH.". That lead me to assume that the library would take care of setting the internal pull-up resistor.

Edit: I just tested it with pinMode(LEFT_PEDAL, INPUT_PULLUP); and that totally solved the problem. Thank you so much. Maybe it would make sense to actually handle that within the library or to change the Readme to say something like "compatible with both pull-up and pull-down wiring" instead of "handles both pull-up and pull-down wiring". Or is it just me who thinks this is confusing?

bxparks commented 5 years ago

Glad that it fixed your problem.

I made a somewhat conscious decision to decouple the configuration of the hardware from the AceButton library. The library doesn't really care whether you are using an internal pull-up or an external pull-up resistor. It just needs to know the logical value of the button in the default (unpressed) state. If the library handled the call to pinMode(), it would make the initialization sequence of the button slightly more complicated, since we now need a mechanism to tell the button how it is wired (3 different ways: internal pullup, external pullup, external pull-down). It also seemed to me that most people would want explicit control over the initialization of the hardware pins (and various other hardware connected to the microcontroller), instead of having the library perform that invisibly. Maybe they don't want the pins to be configured at setup(), maybe they want it to be configured dynamically at a later point in their application, and maybe they want it to be turned off when something happens. Since the AceButton library does not care about the hardware configuration, and I cannot anticipate all possible application scenarios, it didn't seem appropriate to have the library handle this.

I think all my examples in the README.md contain a call to pinMode(), so I would have thought that it would be sufficient. But since you were confused by this, it's likely that other people might be confused. I will add a blurb in the README.md to say that the pinMode() must be called explicitly for each button.