[PATCH libinput] Fix debouncing algorithm

Vicente Bergas vicencb at gmail.com
Wed Oct 25 19:12:53 UTC 2017


The algorithm assumed that bouncing can only occur once when pressing a
button.
This approach did not work with my mouse because that is not 100%
correct. Bouncing can occur when a button toggles, both press and release
and can bounce multiple times. Mine bounces mostly on the release event.
Also, the timeout period was missing more than half of the bounces.

This patch changes the algorithm so that there is no difference between
press and release events, they are treated the same. The algorithm has
two states: cancel_noise==false and cancel_noise==true.
When not canceling noise, the default state, any event received is
inmediately processed 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. There is no need to
dynamically enable that feature because of the adverse effects of the
fixed delay from the previous algorithm.

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. A bounce generates an odd number
of events in fast sequence, eg. press-release-press. A spurious action
generates an even number of events in fast sequence, eg. release-press.

The new algorithm is well suited for debouncing because events are
inmediately committed. But it is not well suited for filtering out
spurious actions.
There is a macro named DEBOUNCE_AFTER that changes the algorithm in a way
that events are committed after noise suppression. In that mode it is
well suited for both misbehaviours of the button. The drawback is that
there is a delay in that mode, 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!

It would be interesting to have the DEBOUNCE_TIME and DEBOUNCE_AFTER as
configurable parameters, but that is not part of this patch.

Signed-off-by: Vicente Bergas <vicencb at gmail.com>
---
 src/evdev-fallback.c | 158 +++++++++++++++------------------------------------
 src/evdev.h          |  19 -------
 2 files changed, 45 insertions(+), 132 deletions(-)

diff --git a/src/evdev-fallback.c b/src/evdev-fallback.c
index 13e130a..44cb900 100644
--- a/src/evdev-fallback.c
+++ b/src/evdev-fallback.c
@@ -30,7 +30,16 @@
 
 #include "evdev.h"
 
-#define	DEBOUNCE_TIME ms2us(12)
+/* Time window after an event to ignore extra events */
+#define	DEBOUNCE_TIME ms2us(25)
+
+/* Commit an event before or after DEBOUNCE_TIME
+ *   before: events are notified without any delay
+ *           spurious events are also notified
+ *   after: events are notified after a delay
+ *          spurious events are filtered out
+ */
+#define	DEBOUNCE_AFTER false
 
 struct fallback_dispatch {
 	struct evdev_dispatch base;
@@ -89,9 +98,11 @@ struct fallback_dispatch {
 	bool ignore_events;
 
 	struct {
-		enum evdev_debounce_state state;
+		bool cancel_noise;
+		bool first_value;
+		bool last_value;
 		unsigned int button_code;
-		uint64_t button_up_time;
+		uint64_t button_time;
 		struct libinput_timer timer;
 	} debounce;
 
@@ -639,22 +650,24 @@ static inline void
 fallback_flush_debounce(struct fallback_dispatch *dispatch,
 			struct evdev_device *device)
 {
-	int code = dispatch->debounce.button_code;
-	int button;
-
-	if (dispatch->debounce.state != DEBOUNCE_ACTIVE)
-		return;
+	bool last_value = dispatch->debounce.last_value;
 
-	if (hw_is_key_down(dispatch, code)) {
-		button = evdev_to_left_handed(device, code);
+	if (DEBOUNCE_AFTER !=
+		(dispatch->debounce.first_value != 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);
+		evdev_log_debug(device, "debounce commit %u\n", last_value);
+	} else {
+		evdev_log_debug(device, "debounce ignore %u\n", last_value);
 	}
 
-	dispatch->debounce.state = DEBOUNCE_ON;
+	dispatch->debounce.cancel_noise = false;
 }
 
 static void
@@ -668,113 +681,32 @@ 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);
+			evdev_log_debug(device, "debounce reset\n");
 			fallback_flush_debounce(dispatch, device);
+			return false;
 		}
-		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");
+		evdev_log_debug(device, "debounce noise  %u %8lu\n",
+			e->value, time - dispatch->debounce.button_time);
+		dispatch->debounce.last_value = e->value;
+		return true;
 	}
-
-	return filter;
-}
-
-static bool
-fallback_filter_debounce_release(struct fallback_dispatch *dispatch,
-				 struct input_event *e,
-				 uint64_t time)
-{
-	bool filter = false;
-
+	evdev_log_debug(device, "debounce start  %u\n", e->value);
+	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;
-	}
-
-	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;
+	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);
+	return DEBOUNCE_AFTER;
 }
 
 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;
-- 
2.14.3



More information about the wayland-devel mailing list