[PATCH libevdev 2/3] test: devices with abs axes need to be created properly
Benjamin Tissoires
benjamin.tissoires at gmail.com
Mon Aug 12 02:24:44 PDT 2013
On Fri, Aug 9, 2013 at 8:40 AM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> Simply enabling the bits is not enough, we need to provide axis
> information too if we want to enable this properly.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
I'm a little bit worried about this patch. It does not looks very consistent:
sometimes, you use "int code = 0", in other places "int code = ABS_X"
and in some others "struct input_absinfo abs = { ABS_X, 0, 2, 0, 0,
0};".
See my comments below:
> ---
> test/test-libevdev-has-event.c | 141 ++++++++++++++++++++++++-----------------
> 1 file changed, 83 insertions(+), 58 deletions(-)
>
> diff --git a/test/test-libevdev-has-event.c b/test/test-libevdev-has-event.c
> index f96bbc3..c1bfd0d 100644
> --- a/test/test-libevdev-has-event.c
> +++ b/test/test-libevdev-has-event.c
> @@ -45,9 +45,16 @@ START_TEST(test_has_ev_bit)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int i, rc;
> + int code = 0;
Why this variable is required?
>
> - rc = test_create_device(&uidev, &dev,
> - *evbit, 0,
> + if (*evbit == EV_ABS) {
> + struct input_absinfo abs = { code, 0, 2, 0, 0, 0};
I would directly use: "struct input_absinfo abs = { ABS_X, 0, 2, 0, 0,
0};" instead.
> + rc = test_create_abs_device(&uidev, &dev,
> + 1, &abs,
> + -1);
> + } else
> + rc = test_create_device(&uidev, &dev,
> + *evbit, code,
> -1);
> ck_assert_msg(rc == 0, "%s: Failed to create device with: %s",
> libevdev_get_event_type_name(*evbit),
> @@ -79,10 +86,17 @@ START_TEST(test_ev_bit_limits)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int rc;
> + int code = 0;
ditto
>
> - rc = test_create_device(&uidev, &dev,
> - *evbit, 0,
> - -1);
> + if (*evbit == EV_ABS) {
> + struct input_absinfo abs = { code, 0, 2, 0, 0, 0};
ditto
> + rc = test_create_abs_device(&uidev, &dev,
> + 1, &abs,
> + -1);
> + } else
> + rc = test_create_device(&uidev, &dev,
> + *evbit, code,
> + -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> ck_assert_int_eq(libevdev_has_event_type(dev, EV_MAX + 1), 0);
> @@ -114,9 +128,15 @@ START_TEST(test_event_codes)
> max = libevdev_get_event_type_max(*evbit);
>
> for (code = 1; code < max; code += 10) {
> - rc = test_create_device(&uidev, &dev,
> - *evbit, code,
> - -1);
> + if (*evbit == EV_ABS) {
> + struct input_absinfo abs = { code, 0, 2, 0, 0, 0};
> + rc = test_create_abs_device(&uidev, &dev,
> + 1, &abs,
> + -1);
> + } else
> + rc = test_create_device(&uidev, &dev,
> + *evbit, code,
> + -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> ck_assert_msg(libevdev_has_event_type(dev, *evbit), "for event type %d\n", *evbit);
> @@ -143,6 +163,7 @@ START_TEST(test_event_code_limits)
> struct libevdev *dev;
> int rc;
> int max;
> + int code = 1;
>
> if (*evbit == EV_SYN) {
> evbit++;
> @@ -152,8 +173,14 @@ START_TEST(test_event_code_limits)
> max = libevdev_get_event_type_max(*evbit);
> ck_assert(max != -1);
>
> - rc = test_create_device(&uidev, &dev,
> - *evbit, 1,
> + if (*evbit == EV_ABS) {
> + struct input_absinfo abs = { code, 0, 2, 0, 0, 0};
> + rc = test_create_abs_device(&uidev, &dev,
> + 1, &abs,
> + -1);
> + } else
> + rc = test_create_device(&uidev, &dev,
> + *evbit, code,
> -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> @@ -213,11 +240,10 @@ START_TEST(test_input_props)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int rc, i;
> + struct input_absinfo abs = {0, 0, 2, 0, 0};
I would rather see ABS_X there (instead of the first 0)
>
> uidev = uinput_device_new(TEST_DEVICE_NAME);
> - rc = uinput_device_set_event_bits(uidev,
> - EV_ABS, ABS_X,
> - -1);
> + rc = uinput_device_set_abs_bit(uidev, ABS_X, &abs);
> ck_assert_int_eq(rc, 0);
> uinput_device_set_prop(uidev, INPUT_PROP_DIRECT);
> uinput_device_set_prop(uidev, INPUT_PROP_BUTTONPAD);
> @@ -248,6 +274,7 @@ START_TEST(test_set_input_props)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int rc, fd;
> + struct input_absinfo abs = {0, 0, 2, 0, 0};
ditto
>
> dev = libevdev_new();
> ck_assert_int_eq(libevdev_enable_property(dev, INPUT_PROP_MAX + 1), -1);
> @@ -257,9 +284,7 @@ START_TEST(test_set_input_props)
> ck_assert_int_eq(libevdev_has_property(dev, INPUT_PROP_BUTTONPAD), 1);
>
> uidev = uinput_device_new(TEST_DEVICE_NAME);
> - rc = uinput_device_set_event_bits(uidev,
> - EV_ABS, ABS_X,
> - -1);
> + rc = uinput_device_set_abs_bit(uidev,ABS_X, &abs);
nitpick: missing space before ABS_X :)
> ck_assert_int_eq(rc, 0);
> uinput_device_set_prop(uidev, INPUT_PROP_BUTTONPAD);
> rc = uinput_device_create(uidev);
> @@ -288,14 +313,7 @@ START_TEST(test_slot_init_value)
> int fd;
>
> uidev = uinput_device_new(TEST_DEVICE_NAME);
> - rc = uinput_device_set_event_bits(uidev,
> - EV_ABS, ABS_X,
> - EV_ABS, ABS_Y,
> - EV_ABS, ABS_MT_SLOT,
> - EV_ABS, ABS_MT_TRACKING_ID,
> - EV_ABS, ABS_MT_POSITION_X,
> - EV_ABS, ABS_MT_POSITION_Y,
> - -1);
> + rc = uinput_device_set_event_bits(uidev, -1);
Hmm... how can you test against ABS_X, ABS_Y, ABS_MT_POSITION_*,
etc... if you disable all the event bits?
ok, nevermind, I re-read the test and it is ok.
Maybe you should also re-write the assignments of abs with a simple
declaration like you do later:
struct input_absinfo abs[] = { { ABS_X, 0, 1000 },
{ ABS_Y, 0, 1000 },
{ ABS_MT_POSITION_X, 0, 1000 },
{ ABS_MT_POSITION_Y, 0, 1000 },
{ ABS_MT_TRACKING_ID, -1, 2 },
{ ABS_MT_SLOT, 0, 1 }};
> ck_assert_int_eq(rc, 0);
>
> memset(abs, 0, sizeof(abs));
> @@ -360,12 +378,17 @@ START_TEST(test_no_slots)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int rc;
> - rc = test_create_device(&uidev, &dev,
> - EV_ABS, ABS_X,
> - EV_ABS, ABS_Y,
> - EV_ABS, ABS_MT_POSITION_X,
> - EV_ABS, ABS_MT_POSITION_Y,
> - -1);
> + struct input_absinfo abs[] = { { ABS_X, 0, 2 },
> + { ABS_Y, 0, 2 },
> + { ABS_MT_POSITION_X, 0, 2 },
> + { ABS_MT_POSITION_Y, 0, 2 }};
> +
> + rc = test_create_abs_device(&uidev, &dev, 4, abs,
> + EV_ABS, ABS_X,
> + EV_ABS, ABS_Y,
> + EV_ABS, ABS_MT_POSITION_X,
> + EV_ABS, ABS_MT_POSITION_Y,
> + -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> ck_assert_int_eq(libevdev_get_num_slots(dev), -1);
> @@ -381,17 +404,23 @@ START_TEST(test_slot_number)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int rc;
> + const int nslots = 4;
> + 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, 0, nslots - 1 }};
>
> - rc = test_create_device(&uidev, &dev,
> - EV_ABS, ABS_X,
> - EV_ABS, ABS_Y,
> - EV_ABS, ABS_MT_POSITION_X,
> - EV_ABS, ABS_MT_POSITION_Y,
> - EV_ABS, ABS_MT_SLOT,
> - -1);
> + rc = test_create_abs_device(&uidev, &dev, 5, abs,
> + EV_ABS, ABS_X,
> + EV_ABS, ABS_Y,
> + EV_ABS, ABS_MT_POSITION_X,
> + EV_ABS, ABS_MT_POSITION_Y,
> + EV_ABS, ABS_MT_SLOT,
> + -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_num_slots(dev), nslots);
> ck_assert_int_eq(libevdev_get_current_slot(dev), 0);
>
> uinput_device_free(uidev);
> @@ -694,12 +723,12 @@ START_TEST(test_device_enable_bit)
> {
> struct uinput_device* uidev;
> struct libevdev *dev, *dev2;
> - struct input_absinfo abs;
> + struct input_absinfo abs = {ABS_X, 0, 2};
> int rc;
>
> - rc = test_create_device(&uidev, &dev,
> - EV_ABS, ABS_X,
> - -1);
> + rc = test_create_abs_device(&uidev, &dev, 1, &abs,
> + EV_ABS, ABS_X,
> + -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> ck_assert(!libevdev_has_event_code(dev, EV_ABS, ABS_Y));
> @@ -740,12 +769,12 @@ START_TEST(test_device_enable_bit_invalid)
> {
> struct uinput_device* uidev;
> struct libevdev *dev;
> - struct input_absinfo abs = {0};
> + struct input_absinfo abs = {ABS_X, 0, 1};
> int rc;
>
> - rc = test_create_device(&uidev, &dev,
> - EV_ABS, ABS_X,
> - -1);
> + rc = test_create_abs_device(&uidev, &dev, 1, &abs,
> + EV_ABS, ABS_X,
> + -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> ck_assert_int_eq(libevdev_enable_event_code(dev, EV_ABS, ABS_MAX + 1, &abs), -1);
> @@ -766,13 +795,13 @@ START_TEST(test_device_disable_bit)
> struct uinput_device* uidev;
> struct libevdev *dev, *dev2;
> int rc;
> + struct input_absinfo abs[2] = {{ABS_X, 0, 1}, {ABS_Y, 0, 1}};
>
> - rc = test_create_device(&uidev, &dev,
> - EV_ABS, ABS_X,
> - EV_ABS, ABS_Y,
> - EV_REL, REL_X,
> - EV_REL, REL_Y,
> - -1);
> + rc = test_create_abs_device(&uidev, &dev,
> + 2, abs,
> + EV_REL, REL_X,
> + EV_REL, REL_Y,
Seems like you are missing the EV_ABS bits here... :(
> + -1);
> ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
>
> ck_assert(libevdev_has_event_code(dev, EV_ABS, ABS_X));
> @@ -814,14 +843,10 @@ START_TEST(test_device_disable_bit_invalid)
> struct uinput_device* uidev;
> struct libevdev *dev;
> int rc;
> + struct input_absinfo abs = {ABS_X, 0, 1};
>
> - rc = uinput_device_new_with_events(&uidev, TEST_DEVICE_NAME, DEFAULT_IDS,
> - EV_ABS, ABS_X,
> - -1);
> + rc = test_create_abs_device(&uidev, &dev, 1, &abs, -1);
> ck_assert_msg(rc == 0, "Failed to create uinput device: %s", strerror(-rc));
> - rc = libevdev_new_from_fd(uinput_device_get_fd(uidev), &dev);
> - ck_assert_msg(rc == 0, "Failed to init device: %s", strerror(-rc));
> -
>
> ck_assert_int_eq(libevdev_disable_event_code(dev, EV_ABS, ABS_MAX + 1), -1);
> ck_assert_int_eq(libevdev_disable_event_code(dev, EV_MAX + 1, ABS_MAX + 1), -1);
> --
> 1.8.2.1
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools
Cheers,
Benjamin
More information about the Input-tools
mailing list