[PATCH libinput] test: Add seat slot tests
Jonas Ådahl
jadahl at gmail.com
Mon Feb 24 13:08:37 PST 2014
On Mon, Feb 24, 2014 at 10:52:38AM +1000, Peter Hutterer wrote:
> On Sat, Feb 22, 2014 at 03:38:28PM +0100, Jonas Ådahl wrote:
> > Add one test that checks uniqueness of seat slots when having multiple
> > devices with active touch points.
> >
> > Add one test that checks that libinput drops touch points when it could
> > not represent them with a seat wide slot.
> >
> > This commit also adds support for from a test case add test devices to
> > an existing libinput context. Only litest-wacom-touch supports this so
> > far.
> >
> > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > ---
> >
> > Needs to be applied after 'Split up the touch event into the different
> > touch types'.
> >
> > test/litest-wacom-touch.c | 24 ++++++-
> > test/litest.c | 30 ++++++++-
> > test/litest.h | 13 +++-
> > test/touch.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 217 insertions(+), 6 deletions(-)
> >
> > diff --git a/test/litest-wacom-touch.c b/test/litest-wacom-touch.c
> > index 464d541..a6c22ef 100644
> > --- a/test/litest-wacom-touch.c
> > +++ b/test/litest-wacom-touch.c
> > @@ -24,11 +24,22 @@
> > #include "config.h"
> > #endif
> >
> > +#include <stdio.h>
> > +
> > #include "litest.h"
> > #include "litest-int.h"
> > #include "libinput-util.h"
> >
> > -void litest_wacom_touch_setup(void)
> > +static int device_ids = 0;
> > +
> > +static void
> > +litest_wacom_touch_destroy(struct litest_device *dev)
> > +{
> > + device_ids &= ~dev->device_id;
> > +}
> > +
> > +void
> > +litest_wacom_touch_setup(void)
> > {
> > struct litest_device *d = litest_create_device(LITEST_WACOM_TOUCH);
> > litest_set_current_device(d);
> > @@ -104,14 +115,23 @@ litest_create_wacom_touch(struct litest_device *d)
> > { ABS_MT_TRACKING_ID, 0, 65535, 0 },
> > };
> > struct input_absinfo *a;
> > + char name[256];
> > + int device_id;
> > int rc;
> >
> > d->interface = &interface;
> > + d->destroy = litest_wacom_touch_destroy;
> >
> > dev = libevdev_new();
> > ck_assert(dev != NULL);
> >
> > - libevdev_set_name(dev, "Wacom ISDv4 E6 Finger");
> > + device_id = ffs(~device_ids) - 1;
> > + ck_assert_int_ge(device_id, 0);
> > + device_ids |= 1 << device_id;
> > + d->device_id = device_id;
> > + snprintf(name, sizeof name, "Wacom ISDv4 E6 Finger (%d)", device_id);
> > + libevdev_set_name(dev, name);
> > +
>
> IMO the actual devices should stay as close to the real thing as possible.
> That leaves us, for this test, with two options: put a 1.5s sleep in to
> avoid the uinput duplicate issue, or push this code into the generic touch
> device that you created for the scaling overflow issue. with that we have
> more freedom of messing around with the device name.
Moving to the generic touch device seems like the better way, so I'll do
that. Thanks.
Jonas
>
>
> > libevdev_set_id_bustype(dev, 0x3);
> > libevdev_set_id_vendor(dev, 0x56a);
> > libevdev_set_id_product(dev, 0xe6);
> > diff --git a/test/litest.c b/test/litest.c
> > index 78a0472..e69f354 100644
> > --- a/test/litest.c
> > +++ b/test/litest.c
> > @@ -325,8 +325,15 @@ const struct libinput_interface interface = {
> > .close_restricted = close_restricted,
> > };
> >
> > +const struct libinput_interface *
> > +litest_get_libinput_interface()
> > +{
> > + return &interface;
> > +}
> > +
> > struct litest_device *
> > -litest_create_device(enum litest_device_type which)
> > +litest_create_device_for(struct libinput *libinput,
> > + enum litest_device_type which)
> > {
> > struct litest_device *d = zalloc(sizeof(*d));
> > int fd;
> > @@ -358,7 +365,7 @@ litest_create_device(enum litest_device_type which)
> > rc = libevdev_new_from_fd(fd, &d->evdev);
> > ck_assert_int_eq(rc, 0);
> >
> > - d->libinput = libinput_path_create_context(&interface, NULL);
> > + d->libinput = libinput;
> > ck_assert(d->libinput != NULL);
> >
> > d->libinput_device = libinput_path_add_device(d->libinput, path);
> > @@ -372,6 +379,19 @@ litest_create_device(enum litest_device_type which)
> > return d;
> > }
> >
> > +struct litest_device *
> > +litest_create_device(enum litest_device_type which)
> > +{
> > + struct libinput *libinput;
> > + struct litest_device *d;
> > +
> > + libinput = libinput_path_create_context(&interface, NULL);
> > + d = litest_create_device_for(libinput, which);
> > + d->owns_context = true;
> > +
> > + return d;
> > +}
> > +
> > int
> > litest_handle_events(struct litest_device *d)
> > {
> > @@ -392,8 +412,12 @@ litest_delete_device(struct litest_device *d)
> > if (!d)
> > return;
> >
> > + if (d->destroy)
> > + d->destroy(d);
> > +
> > libinput_device_unref(d->libinput_device);
> > - libinput_destroy(d->libinput);
> > + if (d->owns_context)
> > + libinput_destroy(d->libinput);
> > libevdev_free(d->evdev);
> > libevdev_uinput_destroy(d->uinput);
> > memset(d,0, sizeof(*d));
> > diff --git a/test/litest.h b/test/litest.h
> > index 9cc0ff5..fef051d 100644
> > --- a/test/litest.h
> > +++ b/test/litest.h
> > @@ -60,8 +60,11 @@ struct litest_device {
> > struct libevdev *evdev;
> > struct libevdev_uinput *uinput;
> > struct libinput *libinput;
> > + void (*destroy)(struct litest_device *d);
> > + bool owns_context;
> > struct libinput_device *libinput_device;
> > struct litest_device_interface *interface;
> > + int device_id;
>
> while overkill for now, is this something that we should consider adding a
> void *device_private for?
>
> > };
> >
> > void litest_add(const char *name, void *func,
> > @@ -70,7 +73,11 @@ void litest_add(const char *name, void *func,
> > void litest_add_no_device(const char *name, void *func);
> >
> > int litest_run(int argc, char **argv);
> > -struct litest_device * litest_create_device(enum litest_device_type which);
> > +
> > +const struct libinput_interface *litest_get_libinput_interface(void);
> > +struct litest_device *litest_create_device(enum litest_device_type which);
> > +struct litest_device *litest_create_device_for(struct libinput *libinput,
> > + enum litest_device_type which);
> > struct litest_device *litest_current_device(void);
> > void litest_delete_device(struct litest_device *d);
> > int litest_handle_events(struct litest_device *d);
> > @@ -98,4 +105,8 @@ void litest_button_click(struct litest_device *d,
> > bool is_press);
> > void litest_drain_events(struct libinput *li);
> >
> > +#ifndef ck_assert_notnull
> > +#define ck_assert_notnull(ptr) ck_assert_ptr_ne(ptr, NULL)
> > +#endif
> > +
> > #endif /* LITEST_H */
> > diff --git a/test/touch.c b/test/touch.c
> > index 6833e10..661e295 100644
> > --- a/test/touch.c
> > +++ b/test/touch.c
> > @@ -92,6 +92,159 @@ START_TEST(touch_abs_transform)
> > }
> > END_TEST
> >
> > +START_TEST(touch_seat_slots)
> > +{
> > +
> > + struct libinput *libinput;
> > + struct litest_device *dev1;
> > + struct litest_device *dev2;
> > + struct libinput_event *ev;
> > + struct libinput_event_touch *tev;
> > + int32_t seat_slot;
> > + int32_t last_seat_slot = -1;
> > + bool has_last_seat_slot = false;
> > +
> > + libinput = libinput_path_create_context(litest_get_libinput_interface(),
> > + NULL);
> > + ck_assert_notnull(libinput);
> > +
> > + dev1 = litest_create_device_for(libinput, LITEST_WACOM_TOUCH);
> > + dev2 = litest_create_device_for(libinput, LITEST_WACOM_TOUCH);
>
> just looking at the code here, I wonder if we should wrap the
> create_context call as well:
>
> libinput = litest_create_context()
> dev1 = litest_create_device_for_()
> dev1 = litest_create_device_for_()
>
> this way we don't need to expose the interface, and we can put a assert
> into the litest function to save us all the extra asserts in the tests
> (which we already do for the create_device function, so the checks
> immediately below are superfluous)
>
>
> > + ck_assert_notnull(dev1);
> > + ck_assert_notnull(dev2);
>
>
>
> > +
> > + litest_touch_down(dev1, 0, 0, 0);
> > + litest_touch_down(dev2, 0, 0, 0);
> > +
> > + libinput_dispatch(libinput);
> > +
> > + /*
> > + * Read the two touch down events and ensure their seat slot are
> > + * unique.
> > + */
> > + while ((ev = libinput_get_event(libinput))) {
> > + if (libinput_event_get_type(ev) != LIBINPUT_EVENT_TOUCH_DOWN)
> > + continue;
> > +
> > + tev = libinput_event_get_touch_event(ev);
> > +
> > + ck_assert_int_eq(libinput_event_touch_get_slot(tev), 0);
> > + seat_slot = libinput_event_touch_get_seat_slot(tev);
> > +
>
> add a ck_assert_int_ge(seat_slot, 0); here too
>
> > + if (!has_last_seat_slot) {
> > + has_last_seat_slot = true;
> > + last_seat_slot = seat_slot;
> > + continue;
> > + }
> > +
> > + ck_assert_int_ne(seat_slot, last_seat_slot);
> > +
> > + libinput_dispatch(libinput);
> > + }
> > +
> > + ck_assert(has_last_seat_slot);
> > + ck_assert_int_ne(seat_slot, last_seat_slot);
>
> this seems like a superfluous test, can only happen if has_last_seat_slot is
> false, so we'd abort earlier.
>
> > +
> > + litest_delete_device(dev1);
> > + litest_delete_device(dev2);
> > +
> > + libinput_destroy(libinput);
> > +}
> > +END_TEST
> > +
> > +START_TEST(touch_seat_slot_drop)
> > +{
> > + struct libinput *libinput;
> > + struct litest_device *dev;
> > + struct libinput_event *ev;
> > + struct libinput_event_touch *tev;
> > + struct libinput_device *ldev;
> > + int32_t slot;
> > + int32_t seat_slot;
> > + enum libinput_event_type type;
> > + int found = 0;
> > + size_t i;
> > + const size_t num_tps = 40;
> > + size_t slot_count = 0;
> > + int32_t slots[num_tps];
> > +
> > + dev = litest_current_device();
> > + libinput = dev->libinput;
> > +
> > + for (slot = 0; i < num_tps; ++i)
> > + litest_touch_down(dev, slot, 0, 0);
> > + for (slot = 0; i < num_tps; ++i)
> > + litest_touch_up(dev, slot);
> > +
> > + libinput_dispatch(libinput);
> > +
> > + /* Expect up to 40, but allow fewer. */
> > + memset(slots, 0, sizeof slots);
> > + while ((ev = libinput_get_event(libinput))) {
> > + type = libinput_event_get_type(ev);
> > + if (type == LIBINPUT_EVENT_TOUCH_DOWN) {
> > + tev = libinput_event_get_touch_event(ev);
> > + seat_slot = libinput_event_touch_get_seat_slot(tev);
> > +
> > + ck_assert_int_ge(seat_slot, 0);
> > + ck_assert_int_lt(slot_count, num_tps);
> > + ck_assert_int_eq(slots[slot_count], 0);
> > +
> > + /* Assert uniqueness */
> > + for (i = 0; i < slot_count; ++i)
> > + ck_assert_int_ne(seat_slot, slots[i]);
> > +
> > + slots[slot_count] = seat_slot;
> > + ++slot_count;
> > + } else if (type == LIBINPUT_EVENT_TOUCH_UP) {
> > + tev = libinput_event_get_touch_event(ev);
> > + seat_slot = libinput_event_touch_get_seat_slot(tev);
> > +
> > + --slot_count;
> > + found = 0;
> > + for (i = 0; i < slot_count; ++i)
> > + if (slots[i] == seat_slot)
> > + ++found;
> > + ck_assert_int_eq(found, 1);
> > + break;
> > + }
> > +
> > + libinput_dispatch(libinput);
> > + }
> > +
> > + /* Make sure order was not shuffled and we received up before all
> > + * down, and that we received as many up as down. */
> > + while ((ev = libinput_get_event(libinput))) {
> > + switch (libinput_event_get_type(ev)) {
> > + case LIBINPUT_EVENT_TOUCH_DOWN:
> > + ck_assert(0);
>
> there's a ck_abort() call for this.
>
> > + break;
> > + case LIBINPUT_EVENT_TOUCH_UP:
> > + --slot_count;
> > + found = 0;
> > + for (i = 0; i < slot_count; ++i)
> > + if (slots[i] == seat_slot)
>
> you're not getting the seat_slot from the event here. Which brings me to the
> comment that I wanted to write anyway: I think it's better if you write this
> test as two loops:
>
> while () {
> if (touch down)
> handle event
> else if (touch up)
> break;
> }
>
> do {
> if (touch down)
> abort()
> else if (touch up)
> handle event
> } while (...);
>
>
> this way you only have one block that handles touch down events. on the
> first run, the event is already set from the previous while loop (though you
> need some checks that the loop didn't end on its own account).
>
>
> > + ++found;
> > + ck_assert_int_eq(found, 1);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + if (slot_count == 0)
> > + break;
> > +
> > + libinput_dispatch(libinput);
> > + }
> > +
>
> maybe add a libinput_dispatch() here, just in case.
>
> > + /* Make sure we didn't receive too many up. */
> > + while ((ev = libinput_get_event(libinput))) {
> > + type = libinput_event_get_type(ev);
> > + ck_assert_int_ne(type, LIBINPUT_EVENT_TOUCH_DOWN);
> > + ck_assert_int_ne(type, LIBINPUT_EVENT_TOUCH_UP);
> > + }
>
> I think you still need a ck_assert_eq(slot_count, 0) here to make sure you
> did get all up events and no slot is still pending.
>
> Cheers,
> Peter
>
> > +}
> > +END_TEST
> >
> > int
> > main(int argc, char **argv)
> > @@ -99,6 +252,9 @@ main(int argc, char **argv)
> > litest_add("touch:frame", touch_frame_events, LITEST_TOUCH, LITEST_ANY);
> > litest_add("touch:abs-transform", touch_abs_transform,
> > LITEST_TOUCH, LITEST_ANY);
> > + litest_add_no_device("touch:seat-slot", touch_seat_slots);
> > + litest_add("touch:seat-slot-drop", touch_seat_slot_drop,
> > + LITEST_TOUCH, LITEST_ANY);
> >
> > return litest_run(argc, argv);
> > }
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
More information about the wayland-devel
mailing list