[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