[PATCH libinput 2/2] pad: use libwacom to get the evdev to button number mapping

Jason Gerecke killertofu at gmail.com
Fri Oct 27 20:26:40 UTC 2017


On Wed, Oct 25, 2017 at 10:24 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> Some of wacom's tablets, notably the Bamboo series, have a non-predictable
> scheme of mapping the buttons to numeric button numbers in libwacom. Since we
> promise sequential button numbers, we need to have those identical to
> libwacom, otherwise it's impossible to map the two together.
>
> Most tablets have a predictable mapping, so this does not affect the majority
> of devices.
>
> For the old-style bamboos, this swaps the buttons around with the buttons
> being ordered in a vertical order.

This language gives me pause because I know under the covers the order
ultimately comes down to whatever the libwacom SVGs specify. Still, it
looks like all the Bamboo SVGs do indeed use a vertical order so you
can leave it as-is if you'd like.

>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  meson.build            |   9 +++
>  src/evdev-tablet-pad.c |  74 +++++++++++++++++++--
>  test/test-pad.c        | 171 ++++++++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 233 insertions(+), 21 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index f93cde70..72bd90b0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -66,6 +66,15 @@ if have_libwacom
>                           name : 'libwacom_get_paired_device check',
>                           dependencies : dep_libwacom)
>         config_h.set10('HAVE_LIBWACOM_GET_PAIRED_DEVICE', result)
> +
> +       code = '''
> +       #include <libwacom/libwacom.h>
> +       int main(void) { libwacom_get_button_evdev_code(NULL, 'A'); }
> +       '''
> +       result = cc.links(code,
> +                         name : 'libwacom_get_button_evdev_code check',
> +                         dependencies : dep_libwacom)
> +       config_h.set10('HAVE_LIBWACOM_GET_BUTTON_EVDEV_CODE', result)
>  else
>         dep_libwacom = declare_dependency()
>  endif
> diff --git a/src/evdev-tablet-pad.c b/src/evdev-tablet-pad.c
> index 82604d70..2e85f7ac 100644
> --- a/src/evdev-tablet-pad.c
> +++ b/src/evdev-tablet-pad.c
> @@ -27,6 +27,10 @@
>  #include <stdbool.h>
>  #include <string.h>
>
> +#if HAVE_LIBWACOM
> +#include <libwacom/libwacom.h>
> +#endif
> +
>  #define pad_set_status(pad_,s_) (pad_)->status |= (s_)
>  #define pad_unset_status(pad_,s_) (pad_)->status &= ~(s_)
>  #define pad_has_status(pad_,s_) (!!((pad_)->status & (s_)))
> @@ -517,17 +521,61 @@ static struct evdev_dispatch_interface pad_interface = {
>         .get_switch_state = NULL,
>  };
>
> +static bool
> +pad_init_buttons_from_libwacom(struct pad_dispatch *pad,
> +                              struct evdev_device *device)
> +{
> +       bool rc = false;
> +#if HAVE_LIBWACOM_GET_BUTTON_EVDEV_CODE
> +       WacomDeviceDatabase *db = NULL;
> +       WacomDevice *tablet = NULL;
> +       int num_buttons;
> +       int map = 0;
> +
> +       db = libwacom_database_new();
> +       if (!db) {
> +               evdev_log_info(device,
> +                              "Failed to initialize libwacom context.\n");
> +               goto out;
> +       }
> +
> +       tablet = libwacom_new_from_usbid(db,
> +                                        evdev_device_get_id_vendor(device),
> +                                        evdev_device_get_id_product(device),
> +                                        NULL);
> +       if (!tablet)
> +               goto out;
> +
> +       num_buttons = libwacom_get_num_buttons(tablet);
> +       for (int i = 0; i < num_buttons; i++) {
> +               unsigned int code;
> +
> +               code = libwacom_get_button_evdev_code(tablet, 'A' + i);
> +               if (code == 0)
> +                       continue;
> +
> +               pad->button_map[code] = map++;
> +       }
> +
> +       pad->nbuttons = map;
> +
> +       rc = true;
> +out:
> +       if (tablet)
> +               libwacom_destroy(tablet);
> +       if (db)
> +               libwacom_database_destroy(db);
> +#endif
> +       return rc;
> +}
> +
>  static void
> -pad_init_buttons(struct pad_dispatch *pad,
> -                struct evdev_device *device)
> +pad_init_buttons_from_kernel(struct pad_dispatch *pad,
> +                              struct evdev_device *device)
>  {
>         unsigned int code;
> -       size_t i;
>         int map = 0;
>
> -       for (i = 0; i < ARRAY_LENGTH(pad->button_map); i++)
> -               pad->button_map[i] = -1;
> -
>         /* we match wacom_report_numbered_buttons() from the kernel */
>         for (code = BTN_0; code < BTN_0 + 10; code++) {
>                 if (libevdev_has_event_code(device->evdev, EV_KEY, code))
> @@ -553,6 +601,20 @@ pad_init_buttons(struct pad_dispatch *pad,
>  }
>
>  static void
> +pad_init_buttons(struct pad_dispatch *pad,
> +                struct evdev_device *device)
> +{
> +       size_t i;
> +
> +       for (i = 0; i < ARRAY_LENGTH(pad->button_map); i++)
> +               pad->button_map[i] = -1;
> +
> +       if (!pad_init_buttons_from_libwacom(pad, device))
> +               pad_init_buttons_from_kernel(pad, device);
> +
> +}
> +
> +static void
>  pad_init_left_handed(struct evdev_device *device)
>  {
>         if (evdev_tablet_has_left_handed(device))
> diff --git a/test/test-pad.c b/test/test-pad.c
> index 085d6c58..3383c60f 100644
> --- a/test/test-pad.c
> +++ b/test/test-pad.c
> @@ -29,6 +29,10 @@
>  #include <unistd.h>
>  #include <stdbool.h>
>
> +#if HAVE_LIBWACOM
> +#include <libwacom/libwacom.h>
> +#endif
> +
>  #include "libinput-util.h"
>  #include "litest.h"
>
> @@ -119,6 +123,35 @@ START_TEST(pad_time)
>  }
>  END_TEST
>
> +START_TEST(pad_num_buttons_libwacom)
> +{
> +#if HAVE_LIBWACOM
> +       struct litest_device *dev = litest_current_device();
> +       struct libinput_device *device = dev->libinput_device;
> +       WacomDeviceDatabase *db = NULL;
> +       WacomDevice *wacom = NULL;
> +       unsigned int nb_lw, nb;
> +
> +       db = libwacom_database_new();
> +       ck_assert_notnull(db);
> +
> +       wacom = libwacom_new_from_usbid(db,
> +                                       libevdev_get_id_vendor(dev->evdev),
> +                                       libevdev_get_id_product(dev->evdev),
> +                                       NULL);
> +       ck_assert_notnull(wacom);
> +
> +       nb_lw = libwacom_get_num_buttons(wacom);
> +       nb = libinput_device_tablet_pad_get_num_buttons(device);
> +
> +       ck_assert_int_eq(nb, nb_lw);
> +
> +       libwacom_destroy(wacom);
> +       libwacom_database_destroy(db);
> +#endif
> +}
> +END_TEST
> +
>  START_TEST(pad_num_buttons)
>  {
>         struct litest_device *dev = litest_current_device();
> @@ -141,33 +174,42 @@ START_TEST(pad_num_buttons)
>  }
>  END_TEST
>
> -START_TEST(pad_button)
> +START_TEST(pad_button_intuos)
>  {
> +#if !HAVE_LIBWACOM_GET_BUTTON_EVDEV_CODE
>         struct litest_device *dev = litest_current_device();
>         struct libinput *li = dev->libinput;
>         unsigned int code;
>         unsigned int expected_number = 0;
>         struct libinput_event *ev;
>         struct libinput_event_tablet_pad *pev;
> +       unsigned int count;
> +
> +       /* Intuos button mapping is sequential up from BTN_0 and continues
> +        * with BTN_A */
> +
> +       if (!libevdev_has_event_code(dev->evdev, EV_KEY, BTN_0))
> +               return;
>
>         litest_drain_events(li);
>
> -       for (code = BTN_0; code < KEY_MAX; code++) {
> -               if (!libevdev_has_event_code(dev->evdev, EV_KEY, code))
> +       for (code = BTN_0; code < BTN_DIGI; code++) {
> +               /* SKip over the BTN_MOUSE and BTN_JOYSTICK range */
> +               if (code >= BTN_MOUSE && code < BTN_A) {
> +                       ck_assert(!libevdev_has_event_code(dev->evdev,
> +                                                          EV_KEY, code));

Will this get tripped up by the ExpressKey remote? It uses BTN_BASE
and BTN_BASE2 for its two highest-numbered buttons (following
BTN_0..BTN_9, BTN_A..BTN_Z). No other device uses those codes from
what I can tell, but a hypothetical future tablet with enough buttons
would.

>                         continue;
> -
> -               litest_button_click(dev, code, 1);
> -               litest_button_click(dev, code, 0);
> -               libinput_dispatch(li);
> -
> -               switch (code) {
> -               case BTN_STYLUS:
> -                       litest_assert_empty_queue(li);
> -                       continue;
> -               default:
> -                       break;
>                 }
>
> +               if (!libevdev_has_event_code(dev->evdev, EV_KEY, code))
> +                       return;
> +
> +               litest_button_click(dev, code, 1);
> +               litest_button_click(dev, code, 0);
> +               libinput_dispatch(li);
> +
> +               count++;
> +
>                 ev = libinput_get_event(li);
>                 pev = litest_is_pad_button_event(ev,
>                                                  expected_number,
> @@ -186,6 +228,102 @@ START_TEST(pad_button)
>         }
>
>         litest_assert_empty_queue(li);
> +
> +       ck_assert_int_gt(count, 3);
> +#endif
> +}
> +END_TEST
> +
> +START_TEST(pad_button_bamboo)
> +{
> +#if !HAVE_LIBWACOM_GET_BUTTON_EVDEV_CODE
> +       struct litest_device *dev = litest_current_device();
> +       struct libinput *li = dev->libinput;
> +       unsigned int code;
> +       unsigned int expected_number = 0;
> +       struct libinput_event *ev;
> +       struct libinput_event_tablet_pad *pev;
> +       unsigned int count;
> +
> +       if (!libevdev_has_event_code(dev->evdev, EV_KEY, BTN_LEFT))
> +               return;

Pre-3.17 kernels combine the pen and pad on the same device. A device
like the Intuos4 will declare the BTN_0 range for its ExpressKeys, as
well as the BTN_LEFT range for its puck. Not sure if that'll be
problematic...

> +
> +       litest_drain_events(li);
> +
> +       for (code = BTN_LEFT; code < BTN_JOYSTICK; code++) {
> +               if (!libevdev_has_event_code(dev->evdev, EV_KEY, code))
> +                       return;
> +
> +               litest_button_click(dev, code, 1);
> +               litest_button_click(dev, code, 0);
> +               libinput_dispatch(li);
> +
> +               count++;
> +
> +               ev = libinput_get_event(li);
> +               pev = litest_is_pad_button_event(ev,
> +                                                expected_number,
> +                                                LIBINPUT_BUTTON_STATE_PRESSED);
> +               ev = libinput_event_tablet_pad_get_base_event(pev);
> +               libinput_event_destroy(ev);
> +
> +               ev = libinput_get_event(li);
> +               pev = litest_is_pad_button_event(ev,
> +                                                expected_number,
> +                                                LIBINPUT_BUTTON_STATE_RELEASED);
> +               ev = libinput_event_tablet_pad_get_base_event(pev);
> +               libinput_event_destroy(ev);
> +
> +               expected_number++;
> +       }
> +
> +       litest_assert_empty_queue(li);
> +
> +       ck_assert_int_gt(count, 3);
> +#endif
> +}
> +END_TEST
> +
> +START_TEST(pad_button_libwacom)
> +{
> +#if HAVE_LIBWACOM_GET_BUTTON_EVDEV_CODE
> +       struct litest_device *dev = litest_current_device();
> +       struct libinput *li = dev->libinput;
> +       WacomDeviceDatabase *db = NULL;
> +       WacomDevice *wacom = NULL;
> +
> +       db = libwacom_database_new();
> +       assert(db);
> +
> +       wacom = libwacom_new_from_usbid(db,
> +                                       libevdev_get_id_vendor(dev->evdev),
> +                                       libevdev_get_id_product(dev->evdev),
> +                                       NULL);
> +       assert(wacom);
> +
> +       litest_drain_events(li);
> +
> +       for (int i = 0; i < libwacom_get_num_buttons(wacom); i++) {
> +               unsigned int code;
> +
> +               code = libwacom_get_button_evdev_code(wacom, 'A' + i);
> +
> +               litest_button_click(dev, code, 1);
> +               litest_button_click(dev, code, 0);
> +               libinput_dispatch(li);
> +
> +               litest_assert_pad_button_event(li,
> +                                              i,
> +                                              LIBINPUT_BUTTON_STATE_PRESSED);
> +               litest_assert_pad_button_event(li,
> +                                              i,
> +                                              LIBINPUT_BUTTON_STATE_RELEASED);
> +       }
> +
> +
> +       libwacom_destroy(wacom);
> +       libwacom_database_destroy(db);
> +#endif
>  }
>  END_TEST
>
> @@ -788,7 +926,10 @@ litest_setup_tests_pad(void)
>         litest_add("pad:time", pad_time, LITEST_TABLET_PAD, LITEST_ANY);
>
>         litest_add("pad:button", pad_num_buttons, LITEST_TABLET_PAD, LITEST_ANY);
> -       litest_add("pad:button", pad_button, LITEST_TABLET_PAD, LITEST_ANY);
> +       litest_add("pad:button", pad_num_buttons_libwacom, LITEST_TABLET_PAD, LITEST_ANY);
> +       litest_add("pad:button", pad_button_intuos, LITEST_TABLET_PAD, LITEST_ANY);
> +       litest_add("pad:button", pad_button_bamboo, LITEST_TABLET_PAD, LITEST_ANY);
> +       litest_add("pad:button", pad_button_libwacom, LITEST_TABLET_PAD, LITEST_ANY);
>         litest_add("pad:button", pad_button_mode_groups, LITEST_TABLET_PAD, LITEST_ANY);
>
>         litest_add("pad:ring", pad_has_ring, LITEST_RING, LITEST_ANY);
> --
> 2.13.6
>

Those concerns aside, for the set:

Acked-by: Jason Gerecke <jason.gerecke at wacom.com>

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....


More information about the wayland-devel mailing list