evert-arias / EasyButton

Arduino library for debouncing momentary contact switches, detect press, release, long press and sequences with event definitions and callbacks.
https://easybtn.earias.me
MIT License
452 stars 63 forks source link

spurious pressed-for events #5

Closed lexelby closed 5 years ago

lexelby commented 5 years ago

Hi! Thanks for writing this, it's designed to do exactly what I want.

I noticed a weird issue where mashing on the button seems to cause spurious pressed-for events. I set my debounce time to 50ms and the pressed-for duration to 5000ms.

I did a bunch of debugging and determined that when one of these spurious pressed-for events happens, _last_change is less than _time -- always by one ms. Since they're unsigned ints, subtracting them results in a huge numbers, so the test for a "pressed for" event passes and the callback is called.

It's late and I haven't managed to dig all the way through the logic to figure out what's happening but updating _time at the start of read() right after read_start_ms = millis() seems to fix the problem.

lexelby commented 5 years ago

Here's what I think happens:

  1. Button release event happens.
  2. More time than the debounce interval passes.
  3. read() is called.
  4. Nothing changes, and _time is updated at the end of read() -- let's say to 10999.
  5. millis() ticks over to 11000.
  6. User presses the button
  7. read() is called.
  8. A change in state is detected and _last_change is set to 11000. _current_state is true.
  9. The else if is evaluated, and _time - _last_change is 10999 - 11000, which is 4294967295.
  10. 4294967295 is most definitely greater than the _held_threshold and the pressed-for event fires.

I think I may be especially susceptible to this because my loop() has a few things in it:

  server.handleClient();
  ArduinoOTA.handle();
  button.read();

I log lines to a remote log server using syslog, so network-related interrupts on my esp8366 are also firing. All that could easily add up to >= 1ms between button.read() calls.

I've created a PR in #6 -- hopefully I haven't missed anything in my logic :)

evert-arias commented 5 years ago

@lexelby Hi! I've done some tests and it looks like your fix solves the "spurious pressed-for events". Thank you very much for your collaboration https://github.com/evert-arias/EasyButton/pull/6 👍

commit: https://github.com/evert-arias/EasyButton/commit/d670ce1cd4484ee440d716151fad94d8776e4d18

lexelby commented 5 years ago

Hooray!! Thanks again for writing this library :)

evert-arias commented 5 years ago

I will close this issue since it has been solved