[PATCH libinput] Introduce device groups to group logical devices together
Peter Hutterer
peter.hutterer at who-t.net
Thu Feb 5 16:58:12 PST 2015
On Thu, Feb 05, 2015 at 07:14:54PM +0800, Jonas Ådahl wrote:
> On Thu, Feb 05, 2015 at 02:30:58PM +1000, Peter Hutterer wrote:
> > Devices like Wacom tablets have multiple event nodes (touch, pad and stylus).
> > This requires some logical grouping, e.g. setting an Intuos 5 tablet
> > left-handed effectively turns it upside down. That then applies to both the
> > stylus and the touch device.
> >
> > Merging the devices into one struct libinput_device is not feasable, it
> > complicates the API for little benefit. A caller would still need access to
> > all subdevices to get udev handles, etc. Some configuration options apply to
> > the whole device (left-handed) but some (may) only apply to a single subdevice
> > (calibration, natural scrolling).
> >
> > Addressing this would make the libinput API unwieldly and hard to use.
> >
> > Instead, add a device group concept. Each device is a member of a device
> > group - a singleton for most devices. Wacom tablets will have a single group
> > across multiple devices, allowing the caller to associate the devices together
> > if needed.
> >
> > The API is intentionally very simple and requires the caller to keep track of
> > groups and which/how many devices are in it. The caller has more powerful
> > libraries available to do that than we have.
> >
> > This patch does not address the actual merging of devices into the same
> > device group, it simply creates a new group for each new device.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>
> This is nicer indeed.
>
> Can be considered Reviewed-by: Jonas Ådahl <jadahl at gmail.com> as well,
> well, with a comment inline addressed.
[...]
> I think it would be nice to say whether these groups are "recycled" in
> any way, like our seats are. I guess it makes no sense if groups are
> intended to be just dump containers, but since we have this behavior
> elsewhere it is at least good to me precise about it in the API.
thanks, I added a comment that device groups are _not_ recycled and
re-plugging a device will cause new groups to appear.
Cheers,
Peter
>
> > ---
> > Many thanks to Jonas, Hans, Benjamin for making me realise that merged
> > device won't work.
> >
> > Also, this obsoletes the patchset for capability events as well as the
> > "get_udev_devices()" patch
> >
> > src/evdev.c | 10 ++++++
> > src/libinput-private.h | 13 +++++++
> > src/libinput.c | 64 +++++++++++++++++++++++++++++++++
> > src/libinput.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > src/libinput.sym | 5 +++
> > test/device.c | 51 +++++++++++++++++++++++++++
> > 6 files changed, 239 insertions(+)
> >
> > diff --git a/src/evdev.c b/src/evdev.c
> > index 6e318dc..6024d38 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -1572,6 +1572,7 @@ evdev_device_create(struct libinput_seat *seat,
> > int fd;
> > int unhandled_device = 0;
> > const char *devnode = udev_device_get_devnode(udev_device);
> > + struct libinput_device_group *group;
> >
> > /* Use non-blocking mode so that we can loop on read on
> > * evdev_device_data() until all events on the fd are
> > @@ -1642,6 +1643,12 @@ evdev_device_create(struct libinput_seat *seat,
> > if (!device->source)
> > goto err;
> >
> > + group = libinput_device_group_create();
> > + if (!group)
> > + goto err;
> > + libinput_device_set_device_group(&device->base, group);
> > + libinput_device_group_unref(group);
> > +
> > list_insert(seat->devices_list.prev, &device->base.link);
> >
> > evdev_tag_device(device);
> > @@ -2123,6 +2130,9 @@ evdev_device_destroy(struct evdev_device *device)
> > if (dispatch)
> > dispatch->interface->destroy(dispatch);
> >
> > + if (device->base.group)
> > + libinput_device_group_unref(device->base.group);
> > +
> > filter_destroy(device->pointer.filter);
> > libinput_seat_unref(device->base.seat);
> > libevdev_free(device->evdev);
> > diff --git a/src/libinput-private.h b/src/libinput-private.h
> > index b938ed0..23f66e4 100644
> > --- a/src/libinput-private.h
> > +++ b/src/libinput-private.h
> > @@ -165,8 +165,14 @@ struct libinput_device_config {
> > struct libinput_device_config_click_method *click_method;
> > };
> >
> > +struct libinput_device_group {
> > + int refcount;
> > + void *user_data;
> > +};
> > +
> > struct libinput_device {
> > struct libinput_seat *seat;
> > + struct libinput_device_group *group;
> > struct list link;
> > struct list event_listeners;
> > void *user_data;
> > @@ -240,6 +246,13 @@ void
> > libinput_device_init(struct libinput_device *device,
> > struct libinput_seat *seat);
> >
> > +struct libinput_device_group *
> > +libinput_device_group_create(void);
> > +
> > +void
> > +libinput_device_set_device_group(struct libinput_device *device,
> > + struct libinput_device_group *group);
> > +
> > void
> > libinput_device_add_event_listener(struct libinput_device *device,
> > struct libinput_event_listener *listener,
> > diff --git a/src/libinput.c b/src/libinput.c
> > index 7456b90..76ecab1 100644
> > --- a/src/libinput.c
> > +++ b/src/libinput.c
> > @@ -1277,6 +1277,12 @@ libinput_device_get_context(struct libinput_device *device)
> > return libinput_seat_get_context(device->seat);
> > }
> >
> > +LIBINPUT_EXPORT struct libinput_device_group *
> > +libinput_device_get_device_group(struct libinput_device *device)
> > +{
> > + return device->group;
> > +}
> > +
> > LIBINPUT_EXPORT const char *
> > libinput_device_get_sysname(struct libinput_device *device)
> > {
> > @@ -1387,6 +1393,64 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event)
> > return &event->base;
> > }
> >
> > +LIBINPUT_EXPORT struct libinput_device_group *
> > +libinput_device_group_ref(struct libinput_device_group *group)
> > +{
> > + group->refcount++;
> > + return group;
> > +}
> > +
> > +struct libinput_device_group *
> > +libinput_device_group_create(void)
> > +{
> > + struct libinput_device_group *group;
> > +
> > + group = zalloc(sizeof *group);
> > + if (group)
> > + group->refcount = 1;
> > + return group;
> > +}
> > +
> > +void
> > +libinput_device_set_device_group(struct libinput_device *device,
> > + struct libinput_device_group *group)
> > +{
> > + device->group = group;
> > + libinput_device_group_ref(group);
> > +}
> > +
> > +static void
> > +libinput_device_group_destroy(struct libinput_device_group *group)
> > +{
> > + free(group);
> > +}
> > +
> > +LIBINPUT_EXPORT struct libinput_device_group *
> > +libinput_device_group_unref(struct libinput_device_group *group)
> > +{
> > + assert(group->refcount > 0);
> > + group->refcount--;
> > + if (group->refcount == 0) {
> > + libinput_device_group_destroy(group);
> > + return NULL;
> > + } else {
> > + return group;
> > + }
> > +}
> > +
> > +LIBINPUT_EXPORT void
> > +libinput_device_group_set_user_data(struct libinput_device_group *group,
> > + void *user_data)
> > +{
> > + group->user_data = user_data;
> > +}
> > +
> > +LIBINPUT_EXPORT void *
> > +libinput_device_group_get_user_data(struct libinput_device_group *group)
> > +{
> > + return group->user_data;
> > +}
> > +
> > LIBINPUT_EXPORT const char *
> > libinput_config_status_to_str(enum libinput_config_status status)
> > {
> > diff --git a/src/libinput.h b/src/libinput.h
> > index c986bfe..d5d12f0 100644
> > --- a/src/libinput.h
> > +++ b/src/libinput.h
> > @@ -195,6 +195,16 @@ struct libinput;
> > struct libinput_device;
> >
> > /**
> > + * @ingroup device
> > + * @struct libinput_device_group
> > + *
> > + * A base handle for accessing libinput device groups. This struct is
> > + * refcounted, use libinput_device_group_ref() and
> > + * libinput_device_group_unref().
> > + */
> > +struct libinput_device_group;
> > +
> > +/**
> > * @ingroup seat
> > * @struct libinput_seat
> > *
> > @@ -1409,6 +1419,35 @@ libinput_device_get_context(struct libinput_device *device);
> > /**
> > * @ingroup device
> > *
> > + * Get the device group this device is assigned to. Some physical
> > + * devices like graphics tablets are represented by multiple kernel
> > + * devices and thus by multiple struct libinput_device.
> > + *
> > + * libinput assigns these devices to the same libinput_device_group
> > + * allowing the caller to identify such devices and adjust configuration
> > + * settings accordingly. For example, setting a tablet to left-handed often
> > + * means turning it upside down. A touch device on the same tablet would
> > + * need to be turned upside down too to work correctly.
> > + *
> > + * All devices are part of a device group though for most devices the group
> > + * will be a singleton. A device is assigned to a device group on @ref
> > + * LIBINPUT_EVENT_DEVICE_ADDED and removed from that group on @ref
> > + * LIBINPUT_EVENT_DEVICE_REMOVED. It is up to the caller to track how many
> > + * devices are in each device group.
> > + *
> > + * The returned device group is not refcounted and may become invalid after
> > + * the next call to libinput. Use libinput_device_group_ref() and
> > + * libinput_device_group_unref() to continue using the handle outside of the
> > + * immediate scope.
>
> I think it would be nice to say whether these groups are "recycled" in
> any way, like our seats are. I guess it makes no sense if groups are
> intended to be just dump containers, but since we have this behavior
> elsewhere it is at least good to me precise about it in the API.
>
> > + *
> > + * @return The device group this device belongs to
> > + */
> > +struct libinput_device_group *
> > +libinput_device_get_device_group(struct libinput_device *device);
> > +
> > +/**
> > + * @ingroup device
> > + *
> > * Get the system name of the device.
> > *
> > * To get the descriptive device name, use libinput_device_get_name().
> > @@ -1594,6 +1633,63 @@ int
> > libinput_device_has_button(struct libinput_device *device, uint32_t code);
> >
> > /**
> > + * @ingroup device
> > + *
> > + * Increase the refcount of the device group. A device group will be freed
> > + * whenever the refcount reaches 0. This may happen during dispatch if all
> > + * devices of this group were removed from the system. A caller must ensure
> > + * to reference the device group correctly to avoid dangling pointers.
> > + *
> > + * @param group A previously obtained device group
> > + * @return The passed device group
> > + */
> > +struct libinput_device_group *
> > +libinput_device_group_ref(struct libinput_device_group *group);
> > +
> > +/**
> > + * @ingroup device
> > + *
> > + * Decrease the refcount of the device group. A device group will be freed
> > + * whenever the refcount reaches 0. This may happen during dispatch if all
> > + * devices of this group were removed from the system. A caller must ensure
> > + * to reference the device group correctly to avoid dangling pointers.
> > + *
> > + * @param group A previously obtained device group
> > + * @return NULL if the device group was destroyed, otherwise the passed
> > + * device group
> > + */
> > +struct libinput_device_group *
> > +libinput_device_group_unref(struct libinput_device_group *group);
> > +
> > +/**
> > + * @ingroup device
> > + *
> > + * Set caller-specific data associated with this device group. libinput does
> > + * not manage, look at, or modify this data. The caller must ensure the
> > + * data is valid.
> > + *
> > + * @param group A previously obtained device group
> > + * @param user_data Caller-specific data pointer
> > + * @see libinput_device_group_get_user_data
> > + */
> > +void
> > +libinput_device_group_set_user_data(struct libinput_device_group *group,
> > + void *user_data);
> > +
> > +/**
> > + * @ingroup device
> > + *
> > + * Get the caller-specific data associated with this input device group, if
> > + * any.
> > + *
> > + * @param group A previously obtained group
> > + * @return Caller-specific data pointer or NULL if none was set
> > + * @see libinput_device_group_set_user_data
> > + */
> > +void *
> > +libinput_device_group_get_user_data(struct libinput_device_group *group);
> > +
> > +/**
> > * @defgroup config Device configuration
> > *
> > * Enable, disable, change and/or check for device-specific features. For
> > diff --git a/src/libinput.sym b/src/libinput.sym
> > index 77fccb8..535ff9b 100644
> > --- a/src/libinput.sym
> > +++ b/src/libinput.sym
> > @@ -125,4 +125,9 @@ global:
> > libinput_device_config_click_get_method;
> > libinput_device_config_click_get_methods;
> > libinput_device_config_click_set_method;
> > + libinput_device_get_device_group;
> > + libinput_device_group_get_user_data;
> > + libinput_device_group_ref;
> > + libinput_device_group_set_user_data;
> > + libinput_device_group_unref;
> > } LIBINPUT_0.8.0;
> > diff --git a/test/device.c b/test/device.c
> > index 76486f0..e72ef19 100644
> > --- a/test/device.c
> > +++ b/test/device.c
> > @@ -661,6 +661,54 @@ START_TEST(device_context)
> > }
> > END_TEST
> >
> > +START_TEST(device_group_get)
> > +{
> > + struct litest_device *dev = litest_current_device();
> > + struct libinput_device_group *group;
> > +
> > + int userdata = 10;
> > +
> > + group = libinput_device_get_device_group(dev->libinput_device);
> > + ck_assert_notnull(group);
> > +
> > + libinput_device_group_ref(group);
> > +
> > + libinput_device_group_set_user_data(group, &userdata);
> > + ck_assert_ptr_eq(&userdata,
> > + libinput_device_group_get_user_data(group));
> > +
> > + libinput_device_group_unref(group);
> > +}
> > +END_TEST
> > +
> > +START_TEST(device_group_ref)
> > +{
> > + struct libinput *li = litest_create_context();
> > + struct litest_device *dev = litest_add_device(li,
> > + LITEST_MOUSE);
> > + struct libinput_device *device = dev->libinput_device;
> > + struct libinput_device_group *group;
> > +
> > + group = libinput_device_get_device_group(device);
> > + ck_assert_notnull(group);
> > + libinput_device_group_ref(group);
> > +
> > + libinput_device_ref(device);
> > + litest_drain_events(li);
> > + litest_delete_device(dev);
> > + litest_drain_events(li);
> > +
> > + /* make sure the device is dead but the group is still around */
> > + ck_assert(libinput_device_unref(device) == NULL);
> > +
> > + libinput_device_group_ref(group);
> > + ck_assert_notnull(libinput_device_group_unref(group));
> > + ck_assert(libinput_device_group_unref(group) == NULL);
> > +
> > + libinput_unref(li);
> > +}
> > +END_TEST
> > +
> > int main (int argc, char **argv)
> > {
> > litest_add("device:sendevents", device_sendevents_config, LITEST_ANY, LITEST_TOUCHPAD);
> > @@ -686,5 +734,8 @@ int main (int argc, char **argv)
> >
> > litest_add("device:udev", device_get_udev_handle, LITEST_ANY, LITEST_ANY);
> >
> > + litest_add("device:group", device_group_get, LITEST_ANY, LITEST_ANY);
> > + litest_add_no_device("device:group", device_group_ref);
> > +
> > return litest_run(argc, argv);
> > }
> > --
> > 2.1.0
> >
More information about the wayland-devel
mailing list