On Sun, Jul 27, 2014 at 11:28:30PM +0200, Jonas Ådahl wrote: > When removing a device, its not guaranteed that all button or key > presses have been released, resulting in an invalid seat wide button > count. > > Note that kernel devices normally will send release events when being > unplugged, but this won't happen when removing a device from the path > backend. > > Signed-off-by: Jonas Ådahl <jad...@gmail.com> > --- > src/evdev.c | 47 +++++++++++++++++++++++ > test/keyboard.c | 116 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > test/pointer.c | 90 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 253 insertions(+) > > diff --git a/src/evdev.c b/src/evdev.c > index f656a5e..ade7eec 100644 > --- a/src/evdev.c > +++ b/src/evdev.c > @@ -60,6 +60,12 @@ is_key_pressed(struct evdev_device *device, int code) > } > > static int > +get_key_pressed_count(struct evdev_device *device, int code)
here too: pressed vs down > +{ > + return device->key_count[code]; > +} do we still need is_key_pressed then? can we drop the mask? > + > +static int > update_key_pressed_count(struct evdev_device *device, int code, int pressed) > { > assert(code >= 0 && code < KEY_CNT); > @@ -331,6 +337,45 @@ get_key_type(uint16_t code) > } > > static void > +release_pressed_keys(struct evdev_device *device) > +{ > + struct libinput *libinput = device->base.seat->libinput; > + struct timespec ts; > + uint64_t time; > + int code; > + > + if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) { > + log_bug_libinput(libinput, "clock_gettime: %s\n", > strerror(errno)); > + return; > + } > + > + time = ts.tv_sec * 1000ULL + ts.tv_nsec / 1000000; > + > + for (code = 0; code < KEY_CNT; code++) { > + if (get_key_pressed_count(device, code) > 0) { > + switch (get_key_type(code)) { > + case EVDEV_KEY_TYPE_NONE: > + break; > + case EVDEV_KEY_TYPE_KEY: > + evdev_keyboard_notify_key( > + device, > + time, > + code, > + LIBINPUT_KEY_STATE_RELEASED); > + break; > + case EVDEV_KEY_TYPE_BUTTON: > + evdev_pointer_notify_button( > + device, > + time, > + code, > + LIBINPUT_BUTTON_STATE_RELEASED); > + break; > + } > + } > + } > +} > + > +static void > evdev_process_touch_button(struct evdev_device *device, > uint64_t time, int value) > { > @@ -1008,6 +1053,8 @@ evdev_device_remove(struct evdev_device *device) > libinput_remove_source(device->base.seat->libinput, > device->source); > > + release_pressed_keys(device); > + > if (device->mtdev) > mtdev_close_delete(device->mtdev); > close_restricted(device->base.seat->libinput, device->fd); > diff --git a/test/keyboard.c b/test/keyboard.c > index a7f500c..dc9c4ea 100644 > --- a/test/keyboard.c > +++ b/test/keyboard.c > @@ -25,6 +25,7 @@ > #include <check.h> > #include <stdio.h> > > +#include "libinput-util.h" > #include "litest.h" > > START_TEST(keyboard_seat_key_count) > @@ -163,11 +164,126 @@ START_TEST(keyboard_ignore_no_pressed_release) > } > END_TEST > > +static void > +test_key_event(struct litest_device *dev, unsigned int key, int state) > +{ > + struct libinput *li = dev->libinput; > + struct libinput_event *event; > + struct libinput_event_keyboard *kevent; > + > + litest_event(dev, EV_KEY, key, state); > + litest_event(dev, EV_SYN, SYN_REPORT, 0); > + > + libinput_dispatch(li); > + > + event = libinput_get_event(li); > + ck_assert(event != NULL); > + ck_assert_int_eq(libinput_event_get_type(event), > LIBINPUT_EVENT_KEYBOARD_KEY); > + > + kevent = libinput_event_get_keyboard_event(event); > + ck_assert(kevent != NULL); > + ck_assert_int_eq(libinput_event_keyboard_get_key(kevent), key); > + ck_assert_int_eq(libinput_event_keyboard_get_key_state(kevent), > + state ? LIBINPUT_KEY_STATE_PRESSED : > + LIBINPUT_KEY_STATE_RELEASED); > + libinput_event_destroy(event); > +} > + > +START_TEST(keyboard_key_auto_release) > +{ > + struct libinput *libinput; > + struct litest_device *dev; > + struct libinput_event *event; > + enum libinput_event_type type; > + struct libinput_event_keyboard *kevent; > + struct { > + int code; > + int released; > + } keys[] = { > + { .code = KEY_A, }, > + { .code = KEY_S, }, > + { .code = KEY_D, }, > + { .code = KEY_G, }, > + { .code = KEY_Z, }, > + { .code = KEY_DELETE, }, > + { .code = KEY_F24, }, > + }; > + int events[2 * (ARRAY_LENGTH(keys) + 1)]; > + unsigned i; > + int key; > + int valid_code; > + > + /* Enable all tested keys on the device */ > + for (i = 0; i < 2 * ARRAY_LENGTH(keys);) { > + key = keys[i / 2].code; > + events[i++] = EV_KEY; > + events[i++] = key; hmm, advancing i manually in a for loop is normally a big no-no. any reason you don't just use a while loop here? > + } > + events[i++] = -1; > + events[i++] = -1; > + > + libinput = litest_create_context(); > + dev = litest_add_device_with_overrides(libinput, > + LITEST_KEYBOARD, > + "Generic keyboard", > + NULL, NULL, events); > + > + litest_drain_events(libinput); > + > + /* Send pressed events, without releasing */ > + for (i = 0; i < ARRAY_LENGTH(keys); ++i) { > + test_key_event(dev, keys[i].code, 1); > + } > + > + litest_drain_events(libinput); > + > + /* "Disconnect" device */ > + litest_delete_device(dev); > + > + /* Mark all released keys until device is removed */ > + while (1) { > + event = libinput_get_event(libinput); > + ck_assert_notnull(event); > + type = libinput_event_get_type(event); > + > + if (type == LIBINPUT_EVENT_DEVICE_REMOVED) { > + libinput_event_destroy(event); > + break; > + } > + > + ck_assert_int_eq(type, LIBINPUT_EVENT_KEYBOARD_KEY); > + kevent = libinput_event_get_keyboard_event(event); > + ck_assert_int_eq(libinput_event_keyboard_get_key_state(kevent), > + LIBINPUT_KEY_STATE_RELEASED); > + key = libinput_event_keyboard_get_key(kevent); > + > + valid_code = 0; > + for (i = 0; i < ARRAY_LENGTH(keys); ++i) { > + if (keys[i].code == key) { > + ck_assert_int_eq(keys[i].released, 0); > + keys[i].released = 1; > + valid_code = 1; > + } > + } just fyi, if you name the struct you could use ARRAY_FOR_EACH here, not that it matters that much. with the for/while loop change (in both tests): Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net> Cheers, Peterk > + ck_assert_int_eq(valid_code, 1); > + libinput_event_destroy(event); > + } > + > + /* Check that all pressed keys has been released. */ > + for (i = 0; i < ARRAY_LENGTH(keys); ++i) { > + ck_assert_int_eq(keys[i].released, 1); > + } > + > + libinput_unref(libinput); > +} > +END_TEST > + > int > main(int argc, char **argv) > { > litest_add_no_device("keyboard:seat key count", > keyboard_seat_key_count); > litest_add_no_device("keyboard:ignore no pressed release", > keyboard_ignore_no_pressed_release); > + litest_add_no_device("keyboard:key_auto_release", > keyboard_key_auto_release); > > return litest_run(argc, argv); > } > diff --git a/test/pointer.c b/test/pointer.c > index aa75274..c0af460 100644 > --- a/test/pointer.c > +++ b/test/pointer.c > @@ -152,6 +152,95 @@ START_TEST(pointer_button) > } > END_TEST > > +START_TEST(pointer_button_auto_release) > +{ > + struct libinput *libinput; > + struct litest_device *dev; > + struct libinput_event *event; > + enum libinput_event_type type; > + struct libinput_event_pointer *pevent; > + struct { > + int code; > + int released; > + } buttons[] = { > + { .code = BTN_LEFT, }, > + { .code = BTN_MIDDLE, }, > + { .code = BTN_EXTRA, }, > + { .code = BTN_SIDE, }, > + { .code = BTN_BACK, }, > + { .code = BTN_FORWARD, }, > + { .code = BTN_4, }, > + }; > + int events[2 * (ARRAY_LENGTH(buttons) + 1)]; > + unsigned i; > + int button; > + int valid_code; > + > + /* Enable all tested buttons on the device */ > + for (i = 0; i < 2 * ARRAY_LENGTH(buttons);) { > + button = buttons[i / 2].code; > + events[i++] = EV_KEY; > + events[i++] = button; > + } > + events[i++] = -1; > + events[i++] = -1; > + > + libinput = litest_create_context(); > + dev = litest_add_device_with_overrides(libinput, > + LITEST_MOUSE, > + "Generic mouse", > + NULL, NULL, events); > + > + litest_drain_events(libinput); > + > + /* Send pressed events, without releasing */ > + for (i = 0; i < ARRAY_LENGTH(buttons); ++i) { > + test_button_event(dev, buttons[i].code, 1); > + } > + > + litest_drain_events(libinput); > + > + /* "Disconnect" device */ > + litest_delete_device(dev); > + > + /* Mark all released buttons until device is removed */ > + while (1) { > + event = libinput_get_event(libinput); > + ck_assert_notnull(event); > + type = libinput_event_get_type(event); > + > + if (type == LIBINPUT_EVENT_DEVICE_REMOVED) { > + libinput_event_destroy(event); > + break; > + } > + > + ck_assert_int_eq(type, LIBINPUT_EVENT_POINTER_BUTTON); > + pevent = libinput_event_get_pointer_event(event); > + > ck_assert_int_eq(libinput_event_pointer_get_button_state(pevent), > + LIBINPUT_BUTTON_STATE_RELEASED); > + button = libinput_event_pointer_get_button(pevent); > + > + valid_code = 0; > + for (i = 0; i < ARRAY_LENGTH(buttons); ++i) { > + if (buttons[i].code == button) { > + ck_assert_int_eq(buttons[i].released, 0); > + buttons[i].released = 1; > + valid_code = 1; > + } > + } > + ck_assert_int_eq(valid_code, 1); > + libinput_event_destroy(event); > + } > + > + /* Check that all pressed buttons has been released. */ > + for (i = 0; i < ARRAY_LENGTH(buttons); ++i) { > + ck_assert_int_eq(buttons[i].released, 1); > + } > + > + libinput_unref(libinput); > +} > +END_TEST > + > static void > test_wheel_event(struct litest_device *dev, int which, int amount) > { > @@ -300,6 +389,7 @@ int main (int argc, char **argv) { > > litest_add("pointer:motion", pointer_motion_relative, LITEST_POINTER, > LITEST_ANY); > litest_add("pointer:button", pointer_button, LITEST_BUTTON, > LITEST_CLICKPAD); > + litest_add_no_device("pointer:button_auto_release", > pointer_button_auto_release); > litest_add("pointer:scroll", pointer_scroll_wheel, LITEST_WHEEL, > LITEST_ANY); > litest_add_no_device("pointer:seat button count", > pointer_seat_button_count); > > -- > 1.8.5.1 > > _______________________________________________ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel