[PATCH libinput 0/5] Disable laptop touchpad on lid close

[Peter Hutterer]

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
> <peter.hutterer at who-t.net> 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