[PATCH libevdev] Don't treat devices with (ABS_MT_SLOT - 1) as multitouch devices

[Peter Hutterer]

On Tue, Dec 10, 2013 at 10:07:35AM +0100, David Herrmann wrote:
> Hi
> 
> On Tue, Dec 10, 2013 at 2:02 AM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > Some devices (PS3 sixaxis controller) merely have a bunch of axes, without the
> > semantic information that linux/input.h requires. For those, the ABS_MT range
> > may be merely another axis, not the special range that we need to treat it
> > with.
> >
> > Use a simple heuristic: if ABS_MT_SLOT - 1 is enabled, don't treat ABS_MT as
> > multitouch axes. The ABS_MT_SLOT - 1 axis is not used for a real axis.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  libevdev/libevdev.c            |  5 +++--
> >  libevdev/libevdev.h            |  9 +++++++++
> >  test/test-libevdev-has-event.c | 33 +++++++++++++++++++++++++++++++++
> >  3 files changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> > index 60bbbfc..6967e4b 100644
> > --- a/libevdev/libevdev.c
> > +++ b/libevdev/libevdev.c
> > @@ -333,7 +333,8 @@ libevdev_set_fd(struct libevdev* dev, int fd)
> >                                 goto out;
> >
> >                         dev->abs_info[i] = abs_info;
> > -                       if (i == ABS_MT_SLOT) {
> > +                       if (i == ABS_MT_SLOT &&
> > +                           !libevdev_has_event_code(dev, EV_ABS, ABS_MT_SLOT - 1)) {
> 
> Short comment here maybe? Not obvious at all and not everyone looks up
> the comments in the header when reading code.

jawoi. added locally, thanks.

> 
> >                                 dev->num_slots = abs_info.maximum + 1;
> >                                 dev->current_slot = abs_info.value;
> >                         }
> > @@ -627,7 +628,7 @@ update_key_state(struct libevdev *dev, const struct input_event *e)
> >  static int
> >  update_mt_state(struct libevdev *dev, const struct input_event *e)
> >  {
> > -       if (e->code == ABS_MT_SLOT) {
> > +       if (e->code == ABS_MT_SLOT && dev->num_slots > -1) {
> >                 int i;
> >                 dev->current_slot = e->value;
> >                 /* sync abs_info with the current slot values */
> > diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
> > index 2b6f48a..44aa3a6 100644
> > --- a/libevdev/libevdev.h
> > +++ b/libevdev/libevdev.h
> > @@ -361,6 +361,15 @@ extern "C" {
> >   *
> >   * As with @ref bits, the logical state of the device as seen by the library
> >   * depends on the caller using libevdev_next_event().
> > + *
> > + * The Linux kernel requires all axes on a device to have a semantic
> > + * meaning, matching the axis names in linux/input.h. Some devices merely
> > + * export a number of axes beyond the available axis list. For those
> > + * devices, the multitouch information is invalid. Specfically, if a device
> > + * provides the ABS_MT_SLOT axis AND also the (ABS_MT_SLOT - 1) axis, the
> > + * device is not treated as multitouch device. No slot information is
> > + * available and the ABS_MT axis range for these devices is treated as all
> > + * other EV_ABS axes.
> 
> Stupid HID core uses ABS_MISC+x, yeah. But I remember Benjamin trying
> to fix that recently. 

yeah, the idea was to add  INPUT_PROP_MT and label all devices that are
known MT as such. patch is stuck with Dmitry though (see message-id
528E2C91.4000505 at redhat.com) and looks like it may not go anywhere.

Cheers,
   Peter


> Anyhow, patch looks good. I think I will also
> send a patch to linux-input at vger to add a comment to linux/input.h
> about that.
> 
> Reviewed-by: David Herrmann <dh.herrmann at gmail.com>
> 
> Thanks
> David
> 
> >   */
> >
> >  /**
> > diff --git a/test/test-libevdev-has-event.c b/test/test-libevdev-has-event.c
> > index 3aca23b..8beb8c4 100644
> > --- a/test/test-libevdev-has-event.c
> > +++ b/test/test-libevdev-has-event.c
> > @@ -425,6 +425,38 @@ START_TEST(test_slot_number)
> >  }
> >  END_TEST
> >
> > +START_TEST(test_invalid_mt_device)
> > +{
> > +       struct uinput_device* uidev;
> > +       struct libevdev *dev;
> > +       int rc;
> > +       const int nslots = 4;
> > +       int value;
> > +       struct input_absinfo abs[] = {  { ABS_X, 0, 2 },
> > +               { ABS_Y, 0, 2 },
> > +               { ABS_MT_POSITION_X, 0, 2 },
> > +               { ABS_MT_POSITION_Y, 0, 2 },
> > +               { ABS_MT_SLOT - 1, 0, 2 },
> > +               { ABS_MT_SLOT, 0, nslots - 1 }};
> > +
> > +       rc = test_create_abs_device(&uidev, &dev, 6, abs,
> > +                       -1);
> > +       ck_assert_msg(rc == 0, "Failed to uinput device: %s", strerror(-rc));
> > +
> > +       ck_assert_int_eq(libevdev_get_num_slots(dev), -1);
> > +       ck_assert_int_eq(libevdev_get_current_slot(dev), -1);
> > +       ck_assert_int_eq(libevdev_set_slot_value(dev, 0, ABS_MT_POSITION_X, 0), -1);
> > +       ck_assert_int_eq(libevdev_fetch_slot_value(dev, 0, ABS_MT_POSITION_X, &value), 0);
> > +
> > +       ck_assert(libevdev_has_event_code(dev, EV_ABS, ABS_MT_SLOT - 1));
> > +       ck_assert(libevdev_has_event_code(dev, EV_ABS, ABS_MT_SLOT));
> > +
> > +       ck_assert_int_eq(libevdev_set_event_value(dev, EV_ABS, ABS_MT_SLOT, 1), 0);
> > +       ck_assert(libevdev_get_event_value(dev, EV_ABS, ABS_MT_SLOT) == 1);
> > +
> > +       uinput_device_free(uidev);
> > +} END_TEST
> > +
> >
> >  START_TEST(test_device_name)
> >  {
> > @@ -1129,6 +1161,7 @@ libevdev_has_event_test(void)
> >         tcase_add_test(tc, test_no_slots);
> >         tcase_add_test(tc, test_slot_number);
> >         tcase_add_test(tc, test_slot_init_value);
> > +       tcase_add_test(tc, test_invalid_mt_device);
> >         suite_add_tcase(s, tc);
> >
> >         tc = tcase_create("device info");
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > Input-tools mailing list
> > Input-tools at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/input-tools