[PATCH libinput] touchpad: allow for multiple paired keyboards

Peter Hutterer peter.hutterer at who-t.net
Tue Oct 31 23:21:59 UTC 2017


needed for the razer blade keybard which provides multiple event nodes for
one physical device but it's hard/impossible to identify which one is the real
event node we care about.

https://bugs.freedesktop.org/show_bug.cgi?id=103156

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
 src/evdev-mt-touchpad.c |  53 ++++++++++-------
 src/evdev-mt-touchpad.h |  17 ++++--
 test/test-touchpad.c    | 151 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 196 insertions(+), 25 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 1ad66fb0..749f9413 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1588,6 +1588,8 @@ tp_interface_process(struct evdev_dispatch *dispatch,
 static void
 tp_remove_sendevents(struct tp_dispatch *tp)
 {
+	struct paired_keyboard *kbd;
+
 	libinput_timer_cancel(&tp->palm.trackpoint_timer);
 	libinput_timer_cancel(&tp->dwt.keyboard_timer);
 
@@ -1596,9 +1598,10 @@ tp_remove_sendevents(struct tp_dispatch *tp)
 		libinput_device_remove_event_listener(
 					&tp->palm.trackpoint_listener);
 
-	if (tp->dwt.keyboard)
-		libinput_device_remove_event_listener(
-					&tp->dwt.keyboard_listener);
+	ARRAY_FOR_EACH(tp->dwt.paired_keyboard, kbd) {
+		if (kbd->device)
+			libinput_device_remove_event_listener(&kbd->listener);
+	}
 
 	if (tp->lid_switch.lid_switch)
 		libinput_device_remove_event_listener(
@@ -1964,9 +1967,8 @@ tp_dwt_pair_keyboard(struct evdev_device *touchpad,
 		     struct evdev_device *keyboard)
 {
 	struct tp_dispatch *tp = (struct tp_dispatch*)touchpad->dispatch;
-
-	if (tp->dwt.keyboard)
-		return;
+	struct paired_keyboard *kbd;
+	bool found = false;
 
 	if ((keyboard->tags & EVDEV_TAG_KEYBOARD) == 0)
 		return;
@@ -1974,16 +1976,25 @@ tp_dwt_pair_keyboard(struct evdev_device *touchpad,
 	if (!tp_want_dwt(touchpad, keyboard))
 		return;
 
-	libinput_device_add_event_listener(&keyboard->base,
-					   &tp->dwt.keyboard_listener,
-					   tp_keyboard_event, tp);
-	tp->dwt.keyboard = keyboard;
-	tp->dwt.keyboard_active = false;
+	ARRAY_FOR_EACH(tp->dwt.paired_keyboard, kbd) {
+		if (kbd->device)
+			continue;
 
-	evdev_log_debug(touchpad,
-			"palm: dwt activated with %s<->%s\n",
-			touchpad->devname,
-			keyboard->devname);
+		found = true;
+		libinput_device_add_event_listener(&keyboard->base,
+						   &kbd->listener,
+						   tp_keyboard_event, tp);
+		kbd->device = keyboard;
+		evdev_log_debug(touchpad,
+				"palm: dwt activated with %s<->%s\n",
+				touchpad->devname,
+				keyboard->devname);
+		break;
+	}
+
+	if (!found)
+		evdev_log_bug_libinput(touchpad,
+				       "too many internal keyboards for dwt\n");
 }
 
 static void
@@ -2121,6 +2132,7 @@ tp_interface_device_removed(struct evdev_device *device,
 			    struct evdev_device *removed_device)
 {
 	struct tp_dispatch *tp = (struct tp_dispatch*)device->dispatch;
+	struct paired_keyboard *kbd;
 
 	if (removed_device == tp->buttons.trackpoint) {
 		/* Clear any pending releases for the trackpoint */
@@ -2134,11 +2146,12 @@ tp_interface_device_removed(struct evdev_device *device,
 		tp->buttons.trackpoint = NULL;
 	}
 
-	if (removed_device == tp->dwt.keyboard) {
-		libinput_device_remove_event_listener(
-					&tp->dwt.keyboard_listener);
-		tp->dwt.keyboard = NULL;
-		tp->dwt.keyboard_active = false;
+	ARRAY_FOR_EACH(tp->dwt.paired_keyboard, kbd) {
+		if (kbd->device == removed_device) {
+			libinput_device_remove_event_listener(&kbd->listener);
+			kbd->device = NULL;
+			tp->dwt.keyboard_active = false;
+		}
 	}
 
 	if (removed_device == tp->lid_switch.lid_switch) {
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 9d9f0826..9459e76e 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -383,13 +383,20 @@ struct tp_dispatch {
 		struct libinput_device_config_dwt config;
 		bool dwt_enabled;
 
-		bool keyboard_active;
-		struct libinput_event_listener keyboard_listener;
-		struct libinput_timer keyboard_timer;
-		struct evdev_device *keyboard;
+		/* We have to allow for more than one device node to be the
+		 * internal dwt keyboard (Razer Blade). But they're the same
+		 * physical device, so we don't care about per-keyboard
+		 * key/modifier masks.
+		 */
+		struct paired_keyboard {
+			struct evdev_device *device;
+			struct libinput_event_listener listener;
+		} paired_keyboard[3];
+
 		unsigned long key_mask[NLONGS(KEY_CNT)];
 		unsigned long mod_mask[NLONGS(KEY_CNT)];
-
+		bool keyboard_active;
+		struct libinput_timer keyboard_timer;
 		uint64_t keyboard_last_press_time;
 	} dwt;
 
diff --git a/test/test-touchpad.c b/test/test-touchpad.c
index 6d2aee57..895de1f6 100644
--- a/test/test-touchpad.c
+++ b/test/test-touchpad.c
@@ -4232,6 +4232,152 @@ START_TEST(touchpad_dwt_acer_hawaii)
 }
 END_TEST
 
+START_TEST(touchpad_dwt_multiple_keyboards)
+{
+	struct litest_device *touchpad = litest_current_device();
+	struct litest_device *k1, *k2;
+	struct libinput *li = touchpad->libinput;
+
+	ck_assert(has_disable_while_typing(touchpad));
+
+	enable_dwt(touchpad);
+
+	k1 = litest_add_device(li, LITEST_KEYBOARD);
+	k2 = litest_add_device(li, LITEST_KEYBOARD);
+
+	litest_keyboard_key(k1, KEY_A, true);
+	litest_keyboard_key(k1, KEY_A, false);
+	litest_drain_events(li);
+
+	litest_touch_down(touchpad, 0, 50, 50);
+	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 10, 1);
+	litest_touch_up(touchpad, 0);
+	litest_assert_empty_queue(li);
+
+	litest_timeout_dwt_short();
+
+	litest_keyboard_key(k2, KEY_A, true);
+	litest_keyboard_key(k2, KEY_A, false);
+	litest_drain_events(li);
+
+	litest_touch_down(touchpad, 0, 50, 50);
+	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 10, 1);
+	litest_touch_up(touchpad, 0);
+	litest_assert_empty_queue(li);
+
+	litest_timeout_dwt_short();
+
+	litest_delete_device(k1);
+	litest_delete_device(k2);
+}
+END_TEST
+
+START_TEST(touchpad_dwt_multiple_keyboards_bothkeys)
+{
+	struct litest_device *touchpad = litest_current_device();
+	struct litest_device *k1, *k2;
+	struct libinput *li = touchpad->libinput;
+
+	ck_assert(has_disable_while_typing(touchpad));
+
+	enable_dwt(touchpad);
+
+	k1 = litest_add_device(li, LITEST_KEYBOARD);
+	k2 = litest_add_device(li, LITEST_KEYBOARD);
+
+	litest_keyboard_key(k1, KEY_A, true);
+	litest_keyboard_key(k1, KEY_A, false);
+	litest_keyboard_key(k2, KEY_B, true);
+	litest_keyboard_key(k2, KEY_B, false);
+	litest_drain_events(li);
+
+	litest_touch_down(touchpad, 0, 50, 50);
+	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 10, 1);
+	litest_touch_up(touchpad, 0);
+	litest_assert_empty_queue(li);
+
+	litest_delete_device(k1);
+	litest_delete_device(k2);
+}
+END_TEST
+
+START_TEST(touchpad_dwt_multiple_keyboards_bothkeys_modifier)
+{
+	struct litest_device *touchpad = litest_current_device();
+	struct litest_device *k1, *k2;
+	struct libinput *li = touchpad->libinput;
+
+	ck_assert(has_disable_while_typing(touchpad));
+
+	enable_dwt(touchpad);
+
+	k1 = litest_add_device(li, LITEST_KEYBOARD);
+	k2 = litest_add_device(li, LITEST_KEYBOARD);
+
+	litest_keyboard_key(k1, KEY_RIGHTCTRL, true);
+	litest_keyboard_key(k1, KEY_RIGHTCTRL, false);
+	litest_keyboard_key(k2, KEY_B, true);
+	litest_keyboard_key(k2, KEY_B, false);
+	litest_drain_events(li);
+
+	/* If the keyboard is a single physical device, the above should
+	 * trigger the modifier behavior for dwt. But libinput views it as
+	 * two separate devices and this is such a niche case that it
+	 * doesn't matter. So we test for the easy behavior:
+	 * ctrl+B across two devices is *not* a dwt modifier combo
+	 */
+	litest_touch_down(touchpad, 0, 50, 50);
+	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 10, 1);
+	litest_touch_up(touchpad, 0);
+	litest_assert_empty_queue(li);
+
+	litest_delete_device(k1);
+	litest_delete_device(k2);
+}
+END_TEST
+
+START_TEST(touchpad_dwt_multiple_keyboards_remove)
+{
+	struct litest_device *touchpad = litest_current_device();
+	struct litest_device *keyboards[2];
+	struct libinput *li = touchpad->libinput;
+	int which = _i; /* ranged test */
+	struct litest_device *removed, *remained;
+
+	ck_assert_int_le(which, 1);
+
+	ck_assert(has_disable_while_typing(touchpad));
+
+	enable_dwt(touchpad);
+
+	keyboards[0] = litest_add_device(li, LITEST_KEYBOARD);
+	keyboards[1] = litest_add_device(li, LITEST_KEYBOARD);
+
+	litest_keyboard_key(keyboards[0], KEY_A, true);
+	litest_keyboard_key(keyboards[0], KEY_A, false);
+	litest_keyboard_key(keyboards[1], KEY_B, true);
+	litest_keyboard_key(keyboards[1], KEY_B, false);
+	litest_drain_events(li);
+
+	litest_timeout_dwt_short();
+
+	removed = keyboards[which % 2];
+	remained = keyboards[(which + 1) % 2];
+
+	litest_delete_device(removed);
+	litest_keyboard_key(remained, KEY_C, true);
+	litest_keyboard_key(remained, KEY_C, false);
+	litest_drain_events(li);
+
+	litest_touch_down(touchpad, 0, 50, 50);
+	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 10, 1);
+	litest_touch_up(touchpad, 0);
+	litest_assert_empty_queue(li);
+
+	litest_delete_device(remained);
+}
+END_TEST
+
 static int
 has_thumb_detect(struct litest_device *dev)
 {
@@ -5493,6 +5639,7 @@ void
 litest_setup_tests_touchpad(void)
 {
 	struct range axis_range = {ABS_X, ABS_Y + 1};
+	struct range twice = {0, 2 };
 
 	litest_add("touchpad:motion", touchpad_1fg_motion, LITEST_TOUCHPAD, LITEST_ANY);
 	litest_add("touchpad:motion", touchpad_2fg_no_motion, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
@@ -5620,6 +5767,10 @@ litest_setup_tests_touchpad(void)
 	litest_add("touchpad:dwt", touchpad_dwt_remove_kbd_while_active, LITEST_TOUCHPAD, LITEST_ANY);
 	litest_add_for_device("touchpad:dwt", touchpad_dwt_apple, LITEST_BCM5974);
 	litest_add_for_device("touchpad:dwt", touchpad_dwt_acer_hawaii, LITEST_ACER_HAWAII_TOUCHPAD);
+	litest_add_for_device("touchpad:dwt", touchpad_dwt_multiple_keyboards, LITEST_SYNAPTICS_I2C);
+	litest_add_for_device("touchpad:dwt", touchpad_dwt_multiple_keyboards_bothkeys, LITEST_SYNAPTICS_I2C);
+	litest_add_for_device("touchpad:dwt", touchpad_dwt_multiple_keyboards_bothkeys_modifier, LITEST_SYNAPTICS_I2C);
+	litest_add_ranged_for_device("touchpad:dwt", touchpad_dwt_multiple_keyboards_remove, LITEST_SYNAPTICS_I2C, &twice);
 
 	litest_add("touchpad:thumb", touchpad_thumb_begin_no_motion, LITEST_CLICKPAD, LITEST_ANY);
 	litest_add("touchpad:thumb", touchpad_thumb_update_no_motion, LITEST_CLICKPAD, LITEST_ANY);
-- 
2.13.6



More information about the wayland-devel mailing list