[PATCH] Fix debouncing algorithm

[Vicente Bergas]

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 at both the press and release events and can
bounce multiple times.
Also, the timeout period was missing more than 50% 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: idle and active.
When in idle state, the default, any event received is inmediately
processed and state is changed to active.
The active state has a timeout. During the limited time of this state
any event received (from the same button) is considered noise, the
timeout is reset and the state of the event is stored.
When the timeout expires the state changes back to idle and if the last
stored event state is different than the committed one, then a new event
is generated to restore equilibrium.

Note that all events are inmediately committed by default. There is no
need to dynamically enable that feature because of the adverse effects
of the previously fixed delay.

The timeout period has been set to 25ms. I have observed that the noisy
period after a button toggle can vary from 8ms to almost 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>
---
 src/evdev-fallback.c | 149 +++++++++++++--------------------------------------
 1 file changed, 37 insertions(+), 112 deletions(-)

diff --git a/src/evdev-fallback.c b/src/evdev-fallback.c
index 13e130a..a236f73 100644
--- a/src/evdev-fallback.c
+++ b/src/evdev-fallback.c
@@ -30,7 +30,7 @@
 
 #include "evdev.h"
 
-#define	DEBOUNCE_TIME ms2us(12)
+#define	DEBOUNCE_TIME ms2us(25)
 
 struct fallback_dispatch {
 	struct evdev_dispatch base;
@@ -91,7 +91,9 @@ struct fallback_dispatch {
 	struct {
 		enum evdev_debounce_state state;
 		unsigned int button_code;
-		uint64_t button_up_time;
+		bool button_committed_value;
+		bool button_last_value;
+		uint64_t button_time;
 		struct libinput_timer timer;
 	} debounce;
 
@@ -639,19 +641,19 @@ static inline void
 fallback_flush_debounce(struct fallback_dispatch *dispatch,
 			struct evdev_device *device)
 {
-	int code = dispatch->debounce.button_code;
-	int button;
+	bool last_value = dispatch->debounce.button_last_value;
 
-	if (dispatch->debounce.state != DEBOUNCE_ACTIVE)
-		return;
-
-	if (hw_is_key_down(dispatch, code)) {
-		button = evdev_to_left_handed(device, code);
+	if (dispatch->debounce.button_committed_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 revert %u\n", last_value);
+	} else {
+		evdev_log_debug(device, "debounce same   %u\n", last_value);
 	}
 
 	dispatch->debounce.state = DEBOUNCE_ON;
@@ -668,113 +670,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) {
-			libinput_timer_cancel(&dispatch->debounce.timer);
+	if (dispatch->debounce.state == DEBOUNCE_ACTIVE) {
+		/* Re-arm timer by first canceling it and then setting it again */
+		libinput_timer_cancel(&dispatch->debounce.timer);
+		/* If a button is pressed while we're holding back another one, flush. */
+		if (e->code != dispatch->debounce.button_code) {
+			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");
-	}
-
-	return filter;
-}
-
-static bool
-fallback_filter_debounce_release(struct fallback_dispatch *dispatch,
-				 struct input_event *e,
-				 uint64_t time)
-{
-	bool filter = false;
-
-	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:
+		evdev_log_debug(device, "debounce noise  %u %8lu\n",
+			e->value, time - dispatch->debounce.button_time);
 		libinput_timer_set(&dispatch->debounce.timer,
-				   time + DEBOUNCE_TIME);
-		filter = true;
-		dispatch->debounce.state = DEBOUNCE_ACTIVE;
-		break;
-	case DEBOUNCE_ACTIVE:
-		filter = true;
-		break;
+			time + DEBOUNCE_TIME);
+		dispatch->debounce.button_time = time;
+		dispatch->debounce.button_last_value = e->value;
+		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;
+	evdev_log_debug(device, "debounce start  %u\n", e->value);
+	dispatch->debounce.state = DEBOUNCE_ACTIVE;
+	dispatch->debounce.button_code = e->code;
+	dispatch->debounce.button_time = time;
+	dispatch->debounce.button_last_value = e->value;
+	dispatch->debounce.button_committed_value = e->value;
+	libinput_timer_set(&dispatch->debounce.timer, time + DEBOUNCE_TIME);
+	return false;
 }
 
 static inline void
-- 
2.14.2