From 0cb570addd126648cd53d54591c6c34810738092 Mon Sep 17 00:00:00 2001 From: satrmb <10471-satrmb_true-email-is-private_contact-via-web@gitlab.freedesktop.org> Date: Wed, 19 May 2021 10:13:06 +0200 Subject: evdev: restart debouncing timers after every event Signed-off-by: satrmb <10471-satrmb@users.noreply.gitlab.freedesktop.org> --- doc/button-debouncing-state-machine.svg | 2 +- src/evdev-debounce.c | 12 ++- test/test-pointer.c | 129 ++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 2 deletions(-) diff --git a/doc/button-debouncing-state-machine.svg b/doc/button-debouncing-state-machine.svg index 53c08305..d9f30770 100644 --- a/doc/button-debouncing-state-machine.svg +++ b/doc/button-debouncing-state-machine.svg @@ -1,3 +1,3 @@ -

Entry states: IS_UP, IS_DOWN

Assumption: state is stored per-button, and OTHER BUTTON events are always processed before the actual button. Stored state per button is a single bit (up/down), a single state for the state machine across the device is sufficient.

Start the state machine with IS_UP or IS_DOWN based on the button's bit, any OTHER BUTTON event will reset it to that state anyway, so the state can be re-used for the new button.

[Not supported by viewer]

Entry state: DISABLED

Only set on devices that have button debouncing disabled. This state is effectively a noop, it just forwards the events as they come in and returns back to the same state.
[Not supported by viewer]
button
press
button<br>press
button
release
button<br>release
DISABLED
DISABLED
button
press
button<br>press
notify
button
press
[Not supported by viewer]
button
release
button<br>release
notify
button
release
[Not supported by viewer]
other
button
other<br>button
IS_UP
IS_UP
button
press
button<br>press
IS_DOWN_WAITING
IS_DOWN_WAITING
IS_UP_DELAYING
IS_UP_DELAYING
notify
button
release
[Not supported by viewer]
notify
button
press
[Not supported by viewer]
IS_UP
IS_UP
notify
button
release
[Not supported by viewer]
IS_DOWN_DETECTING_SPURIOUS
IS_DOWN_DETECTING_SPURIOUS
spurious
enabled?
spurious<br>enabled?
no
no
yes
yes
set
timer
set<br>timer
notify
button
press
[Not supported by viewer]
other
button
other<br>button
enable
spurious
enable<br>spurious
IS_DOWN
IS_DOWN
IS_DOWN
IS_DOWN
button
release
button<br>release
set
timer
set<br>timer
set short
timer
set short<br>timer
other
button
other<br>button
IS_UP_DELAYING_SPURIOUS
IS_UP_DELAYING_SPURIOUS
IS_UP_DETECTING_SPURIOUS
IS_UP_DETECTING_SPURIOUS
button
press
button<br>press
timeout
short
timeout<br>short
other
button
other<br>button
timeout
timeout
button
release
button<br>release
timeout
short
timeout<br>short
other
button
other<br>button
timeout
timeout
timeout
timeout
other
button
other<br>button
timeout
timeout
other
button
other<br>button
IS_DOWN_DELAYING
IS_DOWN_DELAYING
notify
button
press
[Not supported by viewer]
timeout
timeout
button
press
button<br>press
other
button
other<br>button
notify
button
release
[Not supported by viewer]
notify
button
release
[Not supported by viewer]
timeout
short
timeout<br>short
button
press
button<br>press
other
button
other<br>button
IS_UP_WAITING
IS_UP_WAITING
button
release
button<br>release
timeout
timeout
other
button
other<br>button \ No newline at end of file +

Entry states: IS_UP, IS_DOWN

Assumption: state is stored per-button, and OTHER BUTTON events are always processed before the actual button. Stored state per button is a single bit (up/down), a single state for the state machine across the device is sufficient.

Start the state machine with IS_UP or IS_DOWN based on the button's bit, any OTHER BUTTON event will reset it to that state anyway, so the state can be re-used for the new button.

Entry states: IS_UP, IS_DOWN...

Entry state: DISABLED

Only set on devices that have button debouncing disabled. This state is effectively a noop, it just forwards the events as they come in and returns back to the same state.
Entry state: DISABLED...
button
press
button...
button
release
button...
DISABLED
DISABLED
button
press
button...
notify
button
press
notify...
button
release
button...
notify
button
release
notify...
other
button
other...
IS_UP
IS_UP
button
press
button...
IS_DOWN_WAITING
IS_DOWN_WAITING
IS_UP_DELAYING
IS_UP_DELAYING
notify
button
release
notify...
notify
button
press
notify...
IS_UP
IS_UP
notify
button
release
notify...
IS_DOWN_DETECTING_SPURIOUS
IS_DOWN_DETECTING_SPURIOUS
spurious
enabled?
spurious...
no
no
yes
yes
set
timer
set...
notify
button
press
notify...
other
button
other...
enable
spurious
enable...
IS_DOWN
IS_DOWN
IS_DOWN
IS_DOWN
button
release
button...
set
timer
set...
set short
timer
set short...
other
button
other...
IS_UP_DELAYING_SPURIOUS
IS_UP_DELAYING_SPURIOUS
IS_UP_DETECTING_SPURIOUS
IS_UP_DETECTING_SPURIOUS
button
press
button...
timeout
short
timeout...
other
button
other...
timeout
timeout
button
release
button...
timeout
short
timeout...
other
button
other...
timeout
timeout
timeout
timeout
other
button
other...
timeout
timeout
other
button
other...
IS_DOWN_DELAYING
IS_DOWN_DELAYING
notify
button
press
notify...
timeout
timeout
button
press
button...
other
button
other...
notify
button
release
notify...
notify
button
release
notify...
timeout
short
timeout...
button
press
button...
other
button
other...
IS_UP_WAITING
IS_UP_WAITING
button
release
button...
timeout
timeout
other
button
other...
set
timer
set...
set
timer
set...
set
timer
set...
set
timer
set...
set
timer
set...
set short
timer
set short...
set
timer
set...
set short
timer
set short...Viewer does not support full SVG 1.1 \ No newline at end of file diff --git a/src/evdev-debounce.c b/src/evdev-debounce.c index 0c520c8c..32ca0eb3 100644 --- a/src/evdev-debounce.c +++ b/src/evdev-debounce.c @@ -57,7 +57,9 @@ 7 and 8 are cases where the first event happens within the first timeout but the second event is outside that timeout (but within the timeout of - the second event). These cases are currently unhandled. + the second event). These cases are handled by restarting the timer on every + event that could be part of a bouncing sequence, which makes these cases + indistinguishable from 5 and 6. */ enum debounce_event { @@ -236,6 +238,7 @@ debounce_is_down_waiting_handle_event(struct fallback_dispatch *fallback, enum d log_debounce_bug(fallback, event); break; case DEBOUNCE_EVENT_RELEASE: + debounce_set_timer(fallback, time); debounce_set_state(fallback, DEBOUNCE_STATE_IS_UP_DELAYING); /* Note: In the debouncing RPR case, we use the last * release's time stamp */ @@ -258,6 +261,7 @@ debounce_is_up_delaying_handle_event(struct fallback_dispatch *fallback, enum de { switch (event) { case DEBOUNCE_EVENT_PRESS: + debounce_set_timer(fallback, time); debounce_set_state(fallback, DEBOUNCE_STATE_IS_DOWN_WAITING); break; case DEBOUNCE_EVENT_RELEASE: @@ -304,6 +308,8 @@ debounce_is_up_detecting_spurious_handle_event(struct fallback_dispatch *fallbac { switch (event) { case DEBOUNCE_EVENT_PRESS: + debounce_set_timer(fallback, time); + debounce_set_timer_short(fallback, time); /* Note: in a bouncing PRP case, we use the last press * event time */ fallback->debounce.button_time = time; @@ -332,6 +338,8 @@ debounce_is_down_detecting_spurious_handle_event(struct fallback_dispatch *fallb log_debounce_bug(fallback, event); break; case DEBOUNCE_EVENT_RELEASE: + debounce_set_timer(fallback, time); + debounce_set_timer_short(fallback, time); debounce_set_state(fallback, DEBOUNCE_STATE_IS_UP_DETECTING_SPURIOUS); break; case DEBOUNCE_EVENT_TIMEOUT_SHORT: @@ -355,6 +363,7 @@ debounce_is_up_waiting_handle_event(struct fallback_dispatch *fallback, enum deb { switch (event) { case DEBOUNCE_EVENT_PRESS: + debounce_set_timer(fallback, time); /* Note: in a debouncing PRP case, we use the last press' * time */ fallback->debounce.button_time = time; @@ -379,6 +388,7 @@ debounce_is_down_delaying_handle_event(struct fallback_dispatch *fallback, enum log_debounce_bug(fallback, event); break; case DEBOUNCE_EVENT_RELEASE: + debounce_set_timer(fallback, time); debounce_set_state(fallback, DEBOUNCE_STATE_IS_UP_WAITING); break; case DEBOUNCE_EVENT_TIMEOUT_SHORT: diff --git a/test/test-pointer.c b/test/test-pointer.c index a4be655a..6ed0a37b 100644 --- a/test/test-pointer.c +++ b/test/test-pointer.c @@ -2719,6 +2719,64 @@ START_TEST(debounce_bounce) } END_TEST +START_TEST(debounce_bounce_high_delay) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + unsigned int button = _i; /* ranged test */ + + if (!libinput_device_pointer_has_button(dev->libinput_device, + button)) + return; + + litest_disable_middleemu(dev); + disable_button_scrolling(dev); + litest_drain_events(li); + + /* Debouncing timeout is 25ms after a button down or up. Make sure we go + * over 25ms for the total bouncing duration, but stay under 25ms for + * each single event. */ + litest_event(dev, EV_KEY, button, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(15); + litest_event(dev, EV_KEY, button, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(15); + litest_event(dev, EV_KEY, button, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + litest_timeout_debounce(); + libinput_dispatch(li); + + litest_assert_button_event(li, + button, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_empty_queue(li); + + litest_event(dev, EV_KEY, button, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(15); + litest_event(dev, EV_KEY, button, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(15); + litest_event(dev, EV_KEY, button, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + litest_timeout_debounce(); + libinput_dispatch(li); + + litest_assert_button_event(li, + button, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + START_TEST(debounce_bounce_check_immediate) { struct litest_device *dev = litest_current_device(); @@ -2908,6 +2966,75 @@ START_TEST(debounce_spurious_multibounce) } END_TEST +START_TEST(debounce_spurious_trigger_high_delay) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + + litest_disable_middleemu(dev); + litest_drain_events(li); + + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + litest_timeout_debounce(); + libinput_dispatch(li); + + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + + /* Spurious timeout is 12ms after a button down or up. Make sure we go + * over 12ms for the total bouncing duration, but stay under 12ms for + * each single event. */ + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(5); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(5); + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + msleep(5); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + + litest_timeout_debounce(); + libinput_dispatch(li); + + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + + /* gets filtered now */ + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + litest_timeout_debounce(); + litest_assert_empty_queue(li); + + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + litest_timeout_debounce(); + libinput_dispatch(li); + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + litest_assert_empty_queue(li); +} +END_TEST + START_TEST(debounce_spurious_dont_enable_on_otherbutton) { struct litest_device *dev = litest_current_device(); @@ -3231,9 +3358,11 @@ TEST_COLLECTION(pointer) litest_add(pointer_time_usec, LITEST_RELATIVE, LITEST_ANY); litest_add_ranged(debounce_bounce, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE, &buttons); + litest_add_ranged(debounce_bounce_high_delay, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE, &buttons); litest_add(debounce_bounce_check_immediate, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE); litest_add_ranged(debounce_spurious, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE, &buttons); litest_add(debounce_spurious_multibounce, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE); + litest_add(debounce_spurious_trigger_high_delay, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE); litest_add(debounce_spurious_dont_enable_on_otherbutton, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE); litest_add(debounce_spurious_cancel_debounce_otherbutton, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE); litest_add(debounce_spurious_switch_to_otherbutton, LITEST_BUTTON, LITEST_TOUCHPAD|LITEST_NO_DEBOUNCE); -- cgit v1.2.1