Re: [PATCH libinput 0/5] Disable laptop touchpad on lid close
On Thu, Jan 5, 2017 at 4:15 PM, James Ye wrote: Although a laptop touchpad is usually not accessible when the lid is closed, some laptop models suffer from a hardware bug where the touchpad can be activated even if the lid is closed. This bug can be worked around by disabling the touchpad when the lid is closed. This patch set adds: 1: hwdb patch to mark switches as input devices (needs to go to systemd) 2: switch interface[1] 3: evdev dispatch interface for laptop lid switches 4: mechanism for pairing touchpad with lid, and disabling the touchpad 5: test cases Best regards, James Ye I did a bit of testing on a laptop which suffers from this issue. The good news is that it works as intended on a real device, which is expected. The bad news, however, is that on that laptop (ASUS ZENBOOK UX32VD) the lid switch does not reliably engage when you close the lid. The hardware design is similar across a few models from that generation of laptops, so other models may be affected too. In any case, I'll send in revised patches which fix some incorrect assumptions about events soon. Cheers, James [1]: https://lists.freedesktop.org/archives/wayland-devel/2016-January/026349.html James Ye (5): udev: mark switches as input devices Add a "switch" interface for parts of the SW_* range Add evdev_dispatch interface for lid switch Pair touchpad and lid_switch for disable test: add tests for lid switch doc/Makefile.am| 1 + doc/switches.dox | 13 +++ src/evdev-mt-touchpad.c| 47 src/evdev-mt-touchpad.h| 5 + src/evdev.c| 92 ++- src/evdev.h| 8 ++ src/libinput-private.h | 5 + src/libinput.c | 103 + src/libinput.h | 107 + src/libinput.sym | 9 ++ test/Makefile.am | 4 +- test/lid.c | 228 + test/litest-device-lid-switch.c| 58 ++ test/litest.c | 31 + test/litest.h | 10 ++ tools/event-debug.c| 34 ++ udev/90-libinput-model-quirks.hwdb | 6 + 17 files changed, 758 insertions(+), 3 deletions(-) create mode 100644 doc/switches.dox create mode 100644 test/lid.c create mode 100644 test/litest-device-lid-switch.c -- 2.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 0/5] Disable laptop touchpad on lid close
On Thu, Jan 05, 2017 at 06:00:05PM +0100, Carlos Garnacho wrote: > Hey, > > Thanks James for these patches! > > On Thu, Jan 5, 2017 at 11:02 AM, Peter Hutterer > wrote: > > On Thu, Jan 05, 2017 at 05:11:24PM +1100, James Ye wrote: > >> Although a laptop touchpad is usually not accessible when the lid is > >> closed, > >> some laptop models suffer from a hardware bug where the touchpad can be > >> activated even if the lid is closed. This bug can be worked around by > >> disabling > >> the touchpad when the lid is closed. > >> > >> This patch set adds: > >> 1: hwdb patch to mark switches as input devices (needs to go to systemd) > >> 2: switch interface[1] > >> 3: evdev dispatch interface for laptop lid switches > >> 4: mechanism for pairing touchpad with lid, and disabling the touchpad > >> 5: test cases > >> > >> Best regards, > >> James Ye > > > > thanks for picking this up, much appreciated. The patches look good bar a > > few minor nitpicks and a few test cases we should add. But the series even > > *has* test cases, so there's really very little I can complain about :) > > > > One thing I found missing when I reviewed the tests: we don't sync the > > status of the switch on startup. There are two options here (either can be a > > follow-up patch): > > * send a switch toggle event immediately after DEVICE_ADDED on startup. > > That's in-line with the other events and iirc also what we do for > > proximity if a tablet tool is already in proximity on device added. > > * add a libinput_device_switch_get_state() function for callers to query the > > state if they need it and only send the actual changes after we have a > > context initialised. That has a small potential for race conditions, so > > I'm tending to the first option > > > > Carlos, Jonas, Hans, any opinions? > > I tend to favor the first option as well, at least seems more > consistent. Another thing I'd maybe miss is separating toggle on/off > events, so it could be documented that you're always guaranteed to > receive the other event next without fiddling on the event contents > themselves (although you still need to lookup lid/tablet-mode...). We don't do this for other events except touch/gesture events. I'd like to keep this consistent with the key and button events which are closer to switch events. And as you said, since you still need to check which switch it is, you don't get around looking at the event itself anyway. Agree with the documentation though, James, please add a paragraph that libinput guarantees that the event sequence is always correct (i.e. that it will never send two switch on or two switch off in a row for the same switch). Cheers, Peter > > Other than that, the patches look correct to me. > > Cheers, > Carlos ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 0/5] Disable laptop touchpad on lid close
Hey, Thanks James for these patches! On Thu, Jan 5, 2017 at 11:02 AM, Peter Hutterer wrote: > On Thu, Jan 05, 2017 at 05:11:24PM +1100, James Ye wrote: >> Although a laptop touchpad is usually not accessible when the lid is closed, >> some laptop models suffer from a hardware bug where the touchpad can be >> activated even if the lid is closed. This bug can be worked around by >> disabling >> the touchpad when the lid is closed. >> >> This patch set adds: >> 1: hwdb patch to mark switches as input devices (needs to go to systemd) >> 2: switch interface[1] >> 3: evdev dispatch interface for laptop lid switches >> 4: mechanism for pairing touchpad with lid, and disabling the touchpad >> 5: test cases >> >> Best regards, >> James Ye > > thanks for picking this up, much appreciated. The patches look good bar a > few minor nitpicks and a few test cases we should add. But the series even > *has* test cases, so there's really very little I can complain about :) > > One thing I found missing when I reviewed the tests: we don't sync the > status of the switch on startup. There are two options here (either can be a > follow-up patch): > * send a switch toggle event immediately after DEVICE_ADDED on startup. > That's in-line with the other events and iirc also what we do for > proximity if a tablet tool is already in proximity on device added. > * add a libinput_device_switch_get_state() function for callers to query the > state if they need it and only send the actual changes after we have a > context initialised. That has a small potential for race conditions, so > I'm tending to the first option > > Carlos, Jonas, Hans, any opinions? I tend to favor the first option as well, at least seems more consistent. Another thing I'd maybe miss is separating toggle on/off events, so it could be documented that you're always guaranteed to receive the other event next without fiddling on the event contents themselves (although you still need to lookup lid/tablet-mode...). Other than that, the patches look correct to me. Cheers, Carlos ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 0/5] Disable laptop touchpad on lid close
On Thu, Jan 05, 2017 at 05:11:24PM +1100, James Ye wrote: > Although a laptop touchpad is usually not accessible when the lid is closed, > some laptop models suffer from a hardware bug where the touchpad can be > activated even if the lid is closed. This bug can be worked around by > disabling > the touchpad when the lid is closed. > > This patch set adds: > 1: hwdb patch to mark switches as input devices (needs to go to systemd) > 2: switch interface[1] > 3: evdev dispatch interface for laptop lid switches > 4: mechanism for pairing touchpad with lid, and disabling the touchpad > 5: test cases > > Best regards, > James Ye thanks for picking this up, much appreciated. The patches look good bar a few minor nitpicks and a few test cases we should add. But the series even *has* test cases, so there's really very little I can complain about :) One thing I found missing when I reviewed the tests: we don't sync the status of the switch on startup. There are two options here (either can be a follow-up patch): * send a switch toggle event immediately after DEVICE_ADDED on startup. That's in-line with the other events and iirc also what we do for proximity if a tablet tool is already in proximity on device added. * add a libinput_device_switch_get_state() function for callers to query the state if they need it and only send the actual changes after we have a context initialised. That has a small potential for race conditions, so I'm tending to the first option Carlos, Jonas, Hans, any opinions? Cheers, Peter > > [1]: > https://lists.freedesktop.org/archives/wayland-devel/2016-January/026349.html > > James Ye (5): > udev: mark switches as input devices > Add a "switch" interface for parts of the SW_* range > Add evdev_dispatch interface for lid switch > Pair touchpad and lid_switch for disable > test: add tests for lid switch > > doc/Makefile.am| 1 + > doc/switches.dox | 13 +++ > src/evdev-mt-touchpad.c| 47 > src/evdev-mt-touchpad.h| 5 + > src/evdev.c| 92 ++- > src/evdev.h| 8 ++ > src/libinput-private.h | 5 + > src/libinput.c | 103 + > src/libinput.h | 107 + > src/libinput.sym | 9 ++ > test/Makefile.am | 4 +- > test/lid.c | 228 > + > test/litest-device-lid-switch.c| 58 ++ > test/litest.c | 31 + > test/litest.h | 10 ++ > tools/event-debug.c| 34 ++ > udev/90-libinput-model-quirks.hwdb | 6 + > 17 files changed, 758 insertions(+), 3 deletions(-) > create mode 100644 doc/switches.dox > create mode 100644 test/lid.c > create mode 100644 test/litest-device-lid-switch.c > > -- > 2.9.3 > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 0/5] Disable laptop touchpad on lid close
Although a laptop touchpad is usually not accessible when the lid is closed, some laptop models suffer from a hardware bug where the touchpad can be activated even if the lid is closed. This bug can be worked around by disabling the touchpad when the lid is closed. This patch set adds: 1: hwdb patch to mark switches as input devices (needs to go to systemd) 2: switch interface[1] 3: evdev dispatch interface for laptop lid switches 4: mechanism for pairing touchpad with lid, and disabling the touchpad 5: test cases Best regards, James Ye [1]: https://lists.freedesktop.org/archives/wayland-devel/2016-January/026349.html James Ye (5): udev: mark switches as input devices Add a "switch" interface for parts of the SW_* range Add evdev_dispatch interface for lid switch Pair touchpad and lid_switch for disable test: add tests for lid switch doc/Makefile.am| 1 + doc/switches.dox | 13 +++ src/evdev-mt-touchpad.c| 47 src/evdev-mt-touchpad.h| 5 + src/evdev.c| 92 ++- src/evdev.h| 8 ++ src/libinput-private.h | 5 + src/libinput.c | 103 + src/libinput.h | 107 + src/libinput.sym | 9 ++ test/Makefile.am | 4 +- test/lid.c | 228 + test/litest-device-lid-switch.c| 58 ++ test/litest.c | 31 + test/litest.h | 10 ++ tools/event-debug.c| 34 ++ udev/90-libinput-model-quirks.hwdb | 6 + 17 files changed, 758 insertions(+), 3 deletions(-) create mode 100644 doc/switches.dox create mode 100644 test/lid.c create mode 100644 test/litest-device-lid-switch.c -- 2.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
