[PATCH libinput v3 0/1] Fix debouncing algorithm

[Peter Hutterer]

On Sat, Nov 11, 2017 at 03:44:42PM +0100, Vicente Bergas wrote:
> On Wed, 8 Nov 2017 21:19:04 +1000, Peter Hutterer wrote:
> >On Tue, Nov 07, 2017 at 10:26:11PM +0100, Vicente Bergas wrote:
> >>Hi Peter,
> >>
> >>On Thu, Nov 2, 2017 at 2:04 AM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> >>> On Thu, Nov 02, 2017 at 12:34:01AM +0100, Vicente Bergas wrote:
> >>...
> >>>> I hope it is fixed, but not been able to run the test-suite.
> >>>> Please, can you confirm if the new test-suite passes?
> >>>
> >>> running the full test suite should be a simple sudo ninja test, or you run
> >>> it directly with
> >>>  sudo ./build/libinput-test-suite-runner --filter-test="*sometestname*"
> >>> That is particularly useful when you are working on a specific feature and
> >>> want a quick test turnover time (the full suite can take up 20 min or
> >>> longer)
> >>>
> >>> https://wayland.freedesktop.org/libinput/doc/latest/test-suite.html
> >>> has more documentation and we can add to that what's missing
> >>
> >>I ran the test suite and it failed :(
> >>One of the reasons it failed is because the time parameter passed to
> >>fallback_interface_process is far behind the current time.
> >>Is that expected?
> >>I expected that the time parameter would be the same (or very close
> >>to) the current time.
> >
> >yeah, it should be. in most case you get an  error message like this:
> >litest error libinput bug: timer event19 debounce: offset negative (-3165)
> >where the 'debounce' bit is the timer's name that gets wrongly set. For the
> >test cases it's often a sign of a missing libinput_dispatch()
> >e.g. for the timer_debounce it works with this diff applied:
> >
> >diff --git a/test/test-pointer.c b/test/test-pointer.c
> >index 25ddc765..3ce80840 100644
> >--- a/test/test-pointer.c
> >+++ b/test/test-pointer.c
> >@@ -2154,6 +2154,7 @@ START_TEST(debounce_timer)
> >        litest_event(dev, EV_SYN, SYN_REPORT, 0);
> >        litest_event(dev, EV_KEY, BTN_LEFT, 0);
> >        litest_event(dev, EV_SYN, SYN_REPORT, 0);
> >+       libinput_dispatch(li);
> >
> >        litest_timeout_debounce();
> >        litest_drain_events(li);
> >
> 
> Now that I have been able to run the test suite and having your hint about
> libinput_dispatch, the new test suite is now fixed, it passes all bouncing
> tests.

confirmed, but there are still a few other tests that fail with v3, those
need to be fixed too. But see below.

> You can ignore v2 and go straight to v3.

doh. and I was halfway through reviewing v2 :)
I'll send the patch out with my comments for v2 anyway, just so you have a
record of things to consider for future patches.

The long story short is though that I was in the middle of fixing up things
with your patch and addressing the various comments when the whole thing got
too complicated for me to keep in my head. I found I had to write down way
too much to consider the various cases and there were some unfixable cases
(or at least some that required a lot more work for the corner cases).

So I decided to switch the whole thing to a full state machine similar to
the tapping code. Debugging is a lot easier this way and adding special
behaviours can be added relatively easily too.
https://github.com/whot/libinput/tree/wip/button-debouncing-v3
e.g. the above branch uses your algorithm for most events but uses
spurious mode for spurious release events once the button is held down.

Please have a look at the patches for that branch:
https://lists.freedesktop.org/archives/wayland-devel/2017-November/035782.html

Cheers,
   Peter

> 
> >
> >but I also see failures in e.g. trackpoint_topsoftbuttons_left_handed_both,
> >touchpad_left_handed and pointer_left_handed which have no obvious error in
> >the test case. After debugging this for way too long I discovered/remembered
> >that litest_button_click() forces a debounce timeout (15ms). That's likely
> >the source of your offsets, any test with that call can cause the test case
> >failure. The debounce timeout was added there because otherwise we'd have to
> >update a lot of test cases for debouncing - I guess with your new algorithm
> >we'll have to do this anyway.
> >
> >Cheers,
> >   Peter
> 
> Regards,
>   Vicente.
> 
> Vicente Bergas (1):
>   Fix debouncing algorithm
> 
>  doc/button_debouncing.dox |  16 ++--
>  src/evdev-fallback.c      | 180 +++++++++++++----------------------
>  src/evdev.h               |  19 ----
>  test/litest.c             |   2 +-
>  test/litest.h             |   4 +-
>  test/test-pointer.c       | 237 ++++++++++++++++------------------------------
>  6 files changed, 163 insertions(+), 295 deletions(-)
> 
> -- 
> 2.15.0
>