[PATCH libinput 0/6] Add dynamic devices to the path backend

[Peter Hutterer]

On Thu, Feb 06, 2014 at 10:23:57PM +0100, Jonas Ådahl wrote:
> On Thu, Feb 06, 2014 at 02:13:04PM +1000, Peter Hutterer wrote:
> > 
> > This patchset revamps the path backend to allow for more than one path-based
> > device per context. I thought the initial approach of having one context per
> > device is sufficient but there are a few use-cases that can really only be
> > solved by having libinput control all devices. A common example is disabling
> > the touchpad while typing, or making the trackstick buttons on the Lenovo
> > T440s useful.
> > 
> > So for my little libinput-based xorg driver [1] I need to have a shared
> > context between the various devices added. The change looks bigger than it
> > is, it largely just replicates what the udev-seat backend already does
> > anyway.
> > 
> > Most notable there are a few API changes:
> > - libinput_create_from_path() has been removed, a caller should now use
> >   libinput_path_create_context(), then libinput_path_add_device()
> 
> Why not just libinput_path_create()?

I wanted to make it explicit that all this call does is create the context,
as opposed to e.g. udev_create_... which adds devices too.

> >   I found this to be nicer in the caller code than having
> >   libinput_path_create_from_device() and then adding devices.
> > - more devices can be added or removed with libinput_path_add_device and
> >   libinput_path_remove_device
> > - to ensure proper namespacing, libinput_create_from_udev is now
> >   libinput_udev_create_for_seat()
> 
> These API changes looks good to me, but should maybe suspend and resume
> behaviour be documented? Is it required to re-add devices after having
> resumed?

it's not required, the behaviour is that suspend/resume removes and re-adds
all devices, or fails if a device cannot be added. exactly the same behavour
as the udev seat. I'll expand on the suspend/resume documentation in a
separate patch.

> > So far this looks flexible enough for the xorg drivers which have different
> > use-cases than in weston, specifically each device can be disabled and
> > enabled individually. I just call remove/add device when that happens.
> > 
> > What's not so nice is a shortcut in libinput_path_add_device(). Instead of
> > just succeeding and letting the caller handle the DEVICE_ADDED event, it
> > returns the struct libinput_device directly. At least within the xorg driver
> > context I found it too hard to work with the events (long description on
> > request) but the short summary is that I'd need to cache any events before
> > DEVICE_ADDED and use timers to re-trigger the read once I have the device,
> > aside from the problem of not being able to figure out which device is which
> > based on the events only (we don't expose the devnode to the caller).
> 
> So I guess you can't just add and then flush and process the queue right
> there, as the DEVICE_ADDED event will already have been queued when
> calling libinput_path_add_device()?

Not without doing some work to the server, the enable/disable bits are in a
quirky order and called from different places than the actual event
processing. So without auditing the call paths I can't guarantee that the
server is in the correct state for handling events when you get the first
couple of events. Returning the device pointer was the less-intrusive but
still somewhat-sensible approach :)

> We could also simply expose the devnode to the caller as well,
> caching and matching later is not very nice.

IMO we should do this anyway, otherwise we'd require the caller to use udev
and subsystem guessing* to match it up themselves. At least a devnode is
non-ambiguous.

Cheers,
   Peter

* probably not much of an issue since we don't deal with serial devices, so
we can assume subsystem is always "input"