[PATCH libinput 1/1] Fix debouncing algorithm
Vicente Bergas
vicencb at gmail.com
Wed Nov 1 23:34:02 UTC 2017
Bouncing can occur when a button toggles, both press and release, and can
bounce multiple times for each toggling.
The algorithm has two states: cancel_noise==false and cancel_noise==true.
When not canceling noise, the default state, any event received is
inmediately committed and state is changed to cancel_noise=true.
The canceling noise state has a timeout. During the limited time of this
state any event received (from the same button) is considered noise and
the last value of the event is saved.
When the timeout expires the state changes back to not canceling noise
and if the last saved event value is different than the committed one,
then a new event is generated to restore equilibrium.
Note that all events are inmediately committed, no delays. This is the
default mode.
A faulty mouse can have bouncing buttons. A worse mouse can also generate
spurious actions. A spurious action is when the button is in steady state
and for a short while it toggles state.
When an spurious action is detected, the algorithm switches to an alternate
mode in which events are committed after noise suppression. This mode is
well suited for both misbehaviours of the button. The drawback is that
there is a delay, so, it is not enabled by default.
The timeout period has been set to 25ms. I have observed that the noisy
period after a button toggle can vary from 8ms to around 30ms, the mean
value at around 20ms. Although those numbers come from one single sample
mouse, 25ms seems a reasonable value, that is 20 clicks per second!
Signed-off-by: Vicente Bergas <vicencb at gmail.com>
---
doc/button_debouncing.dox | 16 +++--
src/evdev-fallback.c | 175 +++++++++++++++++-----------------------------
src/evdev.h | 19 -----
test/litest.c | 2 +-
test/test-pointer.c | 33 ++-------
5 files changed, 82 insertions(+), 163 deletions(-)
diff --git a/doc/button_debouncing.dox b/doc/button_debouncing.dox
index fd52986..fd7a8e6 100644
--- a/doc/button_debouncing.dox
+++ b/doc/button_debouncing.dox
@@ -9,12 +9,16 @@ though the user only pressed or clicked the button once. This effect can be
counteracted by "debouncing" the buttons, usually by ignoring erroneous
events.
-libinput has a built-in debouncing for hardware defects. This feature is
-available for all button-base devices but not active by default. When
-libinput detects a faulty button on a device, debouncing is enabled and a
-warning is printed to the log. Subsequent button events are handled
-correctly in that bouncing button events are ignored, a user should thus see
-the expected behavior.
+libinput has a built-in debouncing for hardware defects. The debouncing
+algorithm can operate in two modes. The first mode is the default, it just
+filters out the bouncing after a press or a release. It has no adverse
+effects like inserting a delay.
+
+Another related issue with buttons is that while pressed they can send
+spurious release events. If libinput detects this kind of fault then it
+enables the second mode of the debouncing algorithm and a warning is printed
+to the log. Subsequent spurious events are handled correctly. This second
+mode inserts a few milliseconds delay.
Note that libinput's debouncing intended to correct hardware damage or
substandard hardware. Debouncing is also used as an accessibility feature
diff --git a/src/evdev-fallback.c b/src/evdev-fallback.c
index 4da07e7..4436c46 100644
--- a/src/evdev-fallback.c
+++ b/src/evdev-fallback.c
@@ -30,7 +30,13 @@
#include "evdev.h"
-#define DEBOUNCE_TIME ms2us(12)
+#define DEBOUNCE_TIME ms2us(25)
+
+enum debounce_mode {
+ /* The debouncing algorithm handles: */
+ DEBOUNCE_MODE_BOUNCE, /* just bounces */
+ DEBOUNCE_MODE_BOUNCE_AND_SPURIOUS, /* bounces and spurious events */
+};
struct fallback_dispatch {
struct evdev_dispatch base;
@@ -89,9 +95,12 @@ struct fallback_dispatch {
bool ignore_events;
struct {
- enum evdev_debounce_state state;
+ bool cancel_noise;
+ int first_value;
+ int last_value;
+ enum debounce_mode debounce_mode;
unsigned int button_code;
- uint64_t button_up_time;
+ uint64_t button_time;
struct libinput_timer timer;
} debounce;
@@ -639,22 +648,47 @@ static inline void
fallback_flush_debounce(struct fallback_dispatch *dispatch,
struct evdev_device *device)
{
- int code = dispatch->debounce.button_code;
- int button;
+ int last_value = dispatch->debounce.last_value;
+ bool commit_last_value;
- if (dispatch->debounce.state != DEBOUNCE_ACTIVE)
+ if (!dispatch->debounce.cancel_noise)
return;
- if (hw_is_key_down(dispatch, code)) {
- button = evdev_to_left_handed(device, code);
+ dispatch->debounce.cancel_noise = false;
+
+ if (dispatch->debounce.debounce_mode ==
+ DEBOUNCE_MODE_BOUNCE_AND_SPURIOUS)
+ /* first_value has not been commited.
+ * Commit last_value only if it is the same.
+ */
+ commit_last_value =
+ dispatch->debounce.first_value == last_value;
+ else
+ /* first_value has already been commited.
+ * Commit last_value only if it has changed.
+ */
+ commit_last_value =
+ dispatch->debounce.first_value != last_value;
+
+ if (commit_last_value) {
+ int code = dispatch->debounce.button_code;
+
evdev_pointer_notify_physical_button(device,
- dispatch->debounce.button_up_time,
- button,
- LIBINPUT_BUTTON_STATE_RELEASED);
- hw_set_key_down(dispatch, code, 0);
+ dispatch->debounce.button_time,
+ evdev_to_left_handed(device, code),
+ last_value ? LIBINPUT_BUTTON_STATE_PRESSED :
+ LIBINPUT_BUTTON_STATE_RELEASED);
+ hw_set_key_down(dispatch, code, last_value);
+ if (dispatch->debounce.debounce_mode !=
+ DEBOUNCE_MODE_BOUNCE_AND_SPURIOUS) {
+ dispatch->debounce.debounce_mode =
+ DEBOUNCE_MODE_BOUNCE_AND_SPURIOUS;
+ evdev_log_info(device,
+ "Enabling spurious event suppression, "
+ "see %sbutton_debouncing.html for details\n",
+ HTTP_DOC_LINK);
+ }
}
-
- dispatch->debounce.state = DEBOUNCE_ON;
}
static void
@@ -668,113 +702,36 @@ fallback_debounce_timeout(uint64_t now, void *data)
}
static bool
-fallback_filter_debounce_press(struct fallback_dispatch *dispatch,
+fallback_filter_debounce(struct fallback_dispatch *dispatch,
struct evdev_device *device,
struct input_event *e,
uint64_t time)
{
- bool filter = false;
- uint64_t tdelta;
-
- /* If other button is pressed while we're holding back the release,
- * flush the pending release (if any) and continue. We don't handle
- * this situation, if you have a mouse that needs per-button
- * debouncing, consider writing to santa for a new mouse.
- */
- if (e->code != dispatch->debounce.button_code) {
- if (dispatch->debounce.state == DEBOUNCE_ACTIVE) {
+ if (dispatch->debounce.cancel_noise) {
+ /* Flush if a button is pressed while holding back another one
+ */
+ if (e->code != dispatch->debounce.button_code) {
libinput_timer_cancel(&dispatch->debounce.timer);
fallback_flush_debounce(dispatch, device);
+ } else {
+ dispatch->debounce.last_value = e->value;
+ return true;
}
- return false;
- }
-
- tdelta = time - dispatch->debounce.button_up_time;
- assert((int64_t)tdelta >= 0);
-
- if (tdelta < DEBOUNCE_TIME) {
- switch (dispatch->debounce.state) {
- case DEBOUNCE_INIT:
- /* This is the first time we debounce, enable proper debouncing
- from now on but filter this press event */
- filter = true;
- evdev_log_info(device,
- "Enabling button debouncing, "
- "see %sbutton_debouncing.html for details\n",
- HTTP_DOC_LINK);
- dispatch->debounce.state = DEBOUNCE_NEEDED;
- break;
- case DEBOUNCE_NEEDED:
- case DEBOUNCE_ON:
- break;
- /* If a release event is pending and, filter press
- * events until we flushed the release */
- case DEBOUNCE_ACTIVE:
- filter = true;
- break;
- }
- } else if (dispatch->debounce.state == DEBOUNCE_ACTIVE) {
- /* call libinput_dispatch() more frequently */
- evdev_log_bug_client(device,
- "Debouncing still active past timeout\n");
}
- return filter;
-}
-
-static bool
-fallback_filter_debounce_release(struct fallback_dispatch *dispatch,
- struct input_event *e,
- uint64_t time)
-{
- bool filter = false;
-
+ dispatch->debounce.cancel_noise = true;
dispatch->debounce.button_code = e->code;
- dispatch->debounce.button_up_time = time;
-
- switch (dispatch->debounce.state) {
- case DEBOUNCE_INIT:
- break;
- case DEBOUNCE_NEEDED:
- filter = true;
- dispatch->debounce.state = DEBOUNCE_ON;
- break;
- case DEBOUNCE_ON:
- libinput_timer_set(&dispatch->debounce.timer,
- time + DEBOUNCE_TIME);
- filter = true;
- dispatch->debounce.state = DEBOUNCE_ACTIVE;
- break;
- case DEBOUNCE_ACTIVE:
- filter = true;
- break;
+ dispatch->debounce.button_time = time;
+ dispatch->debounce.first_value = e->value;
+ dispatch->debounce.last_value = e->value;
+ libinput_timer_set(&dispatch->debounce.timer, time + DEBOUNCE_TIME);
+ if (dispatch->debounce.debounce_mode ==
+ DEBOUNCE_MODE_BOUNCE_AND_SPURIOUS) {
+ /* This could be a spurious event, so, filter it out */
+ return true;
}
- return filter;
-}
-
-static bool
-fallback_filter_debounce(struct fallback_dispatch *dispatch,
- struct evdev_device *device,
- struct input_event *e, uint64_t time)
-{
- bool filter = false;
-
- /* Behavior: we monitor the time deltas between release and press
- * events. Proper debouncing is disabled on init, but the first
- * time we see a bouncing press event we enable it.
- *
- * The first bounced event is simply discarded, which ends up in the
- * button being released sooner than it should be. Subsequent button
- * presses are timer-based and thus released a bit later because we
- * then wait for a timeout before we post the release event.
- */
- if (e->value)
- filter = fallback_filter_debounce_press(dispatch, device, e, time);
- else
- filter = fallback_filter_debounce_release(dispatch, e, time);
-
- return filter;
+ return false;
}
static inline void
diff --git a/src/evdev.h b/src/evdev.h
index 4c42f6e..280b561 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -131,25 +131,6 @@ enum evdev_button_scroll_state {
BUTTONSCROLL_SCROLLING, /* have sent scroll events */
};
-enum evdev_debounce_state {
- /**
- * Initial state, no debounce but monitoring events
- */
- DEBOUNCE_INIT,
- /**
- * Bounce detected, future events need debouncing
- */
- DEBOUNCE_NEEDED,
- /**
- * Debounce is enabled, but no event is currently being filtered
- */
- DEBOUNCE_ON,
- /**
- * Debounce is enabled and we are currently filtering an event
- */
- DEBOUNCE_ACTIVE,
-};
-
struct mt_slot {
int32_t seat_slot;
struct device_coords point;
diff --git a/test/litest.c b/test/litest.c
index 021e19f..baa588f 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -3167,7 +3167,7 @@ litest_timeout_tapndrag(void)
void
litest_timeout_debounce(void)
{
- msleep(15);
+ msleep(28);
}
void
diff --git a/test/test-pointer.c b/test/test-pointer.c
index 25ddc76..f784fab 100644
--- a/test/test-pointer.c
+++ b/test/test-pointer.c
@@ -2287,12 +2287,7 @@ START_TEST(debounce_no_debounce_for_otherbutton)
BTN_LEFT,
LIBINPUT_BUTTON_STATE_RELEASED);
- litest_assert_button_event(li,
- BTN_RIGHT,
- LIBINPUT_BUTTON_STATE_PRESSED);
- litest_assert_button_event(li,
- BTN_RIGHT,
- LIBINPUT_BUTTON_STATE_RELEASED);
+ litest_timeout_debounce();
litest_assert_empty_queue(li);
}
@@ -2323,8 +2318,7 @@ START_TEST(debounce_cancel_debounce_otherbutton)
litest_event(dev, EV_SYN, SYN_REPORT, 0);
litest_event(dev, EV_KEY, BTN_LEFT, 1);
litest_event(dev, EV_SYN, SYN_REPORT, 0);
- /* release is now held back, press was ignored,
- * other button should flush the release */
+ /* press is now held back, other button should flush */
litest_event(dev, EV_KEY, BTN_RIGHT, 1);
litest_event(dev, EV_SYN, SYN_REPORT, 0);
litest_event(dev, EV_KEY, BTN_RIGHT, 0);
@@ -2335,16 +2329,8 @@ START_TEST(debounce_cancel_debounce_otherbutton)
litest_assert_button_event(li,
BTN_LEFT,
LIBINPUT_BUTTON_STATE_PRESSED);
- litest_assert_button_event(li,
- BTN_LEFT,
- LIBINPUT_BUTTON_STATE_RELEASED);
- litest_assert_button_event(li,
- BTN_RIGHT,
- LIBINPUT_BUTTON_STATE_PRESSED);
- litest_assert_button_event(li,
- BTN_RIGHT,
- LIBINPUT_BUTTON_STATE_RELEASED);
+ litest_timeout_debounce();
litest_assert_empty_queue(li);
}
@@ -2374,8 +2360,7 @@ START_TEST(debounce_switch_to_otherbutton)
litest_event(dev, EV_SYN, SYN_REPORT, 0);
litest_event(dev, EV_KEY, BTN_LEFT, 1);
litest_event(dev, EV_SYN, SYN_REPORT, 0);
- /* release is now held back, press was ignored,
- * other button should flush the release */
+ /* press is now held back, other button should flush */
litest_event(dev, EV_KEY, BTN_RIGHT, 1);
litest_event(dev, EV_SYN, SYN_REPORT, 0);
litest_event(dev, EV_KEY, BTN_RIGHT, 0);
@@ -2392,16 +2377,8 @@ START_TEST(debounce_switch_to_otherbutton)
litest_assert_button_event(li,
BTN_LEFT,
LIBINPUT_BUTTON_STATE_PRESSED);
- litest_assert_button_event(li,
- BTN_LEFT,
- LIBINPUT_BUTTON_STATE_RELEASED);
- litest_assert_button_event(li,
- BTN_RIGHT,
- LIBINPUT_BUTTON_STATE_PRESSED);
- litest_assert_button_event(li,
- BTN_RIGHT,
- LIBINPUT_BUTTON_STATE_RELEASED);
+ litest_timeout_debounce();
litest_assert_empty_queue(li);
}
--
2.15.0
More information about the wayland-devel
mailing list