Hi,
On 06/03/2014 07:34 AM, Peter Hutterer wrote:
> Now that we have run-time changes of the tap.enabled state move the check
> to the IDLE state only. Otherwise the tap machine may hang if tapping is
> disabled while a gesture is in progress.
>
> Two basic tests are added to check for the tap default setting - which is now
> "tap disabled by default", because a bike shed's correct color is green.
>
> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
> ---
> src/evdev-mt-touchpad-tap.c | 56
> +++++++++++++++++++++++++++++++++++++++------
> src/evdev-mt-touchpad.h | 1 +
> test/touchpad.c | 32 ++++++++++++++++++++++++++
> 3 files changed, 82 insertions(+), 7 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
> index eee334f..24ea8c3 100644
> --- a/src/evdev-mt-touchpad-tap.c
> +++ b/src/evdev-mt-touchpad-tap.c
> @@ -438,13 +438,13 @@ static void
> tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
> time)
> {
> enum tp_tap_state current;
> - if (!tp->tap.enabled)
> - return;
>
> current = tp->tap.state;
>
> switch(tp->tap.state) {
> case TAP_STATE_IDLE:
> + if (!tp->tap.enabled)
> + break;
> tp_tap_idle_handle_event(tp, event, time);
> break;
> case TAP_STATE_TOUCH:
> @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
> unsigned int
> tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
> {
> - if (!tp->tap.enabled)
> - return 0;
> -
> if (tp->tap.timeout && tp->tap.timeout <= time) {
> tp_tap_clear_timer(tp);
> tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
> @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
> time)
> return tp->tap.timeout;
> }
>
> +static int
> +tp_tap_config_count(struct libinput_device *device)
> +{
> + struct evdev_dispatch *dispatch = ((struct evdev_device
> *)device)->dispatch;
> + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> +
> + return min(tp->ntouches, 3); /* we only do up to 3 finger tap */
> +}
> +
> +static enum libinput_config_status
> +tp_tap_config_enable(struct libinput_device *device, int enabled)
> +{
> + struct evdev_dispatch *dispatch = ((struct evdev_device
> *)device)->dispatch;
Please user container_of here for proper type checking rather then
just a blatant cast of $random_type_foo to $random_type_bar. Same for all
the other occurrences of the same pattern below.
> + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> +
> + if (tp_tap_config_count(device) == 0)
> + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
This is a repeat of the test done by the higher level wrapper, as such
this seems unnecessary by me.
Regards,
Hans
> +
> + tp->tap.enabled = enabled;
> +
> + return LIBINPUT_CONFIG_STATUS_SUCCESS;
> +}
> +
> +static int
> +tp_tap_config_is_enabled(struct libinput_device *device)
> +{
> + struct evdev_dispatch *dispatch = ((struct evdev_device
> *)device)->dispatch;
> + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> +
> + return tp->tap.enabled;
> +}
> +
> +static void
> +tp_tap_config_reset(struct libinput_device *device)
> +{
> + struct evdev_dispatch *dispatch = ((struct evdev_device
> *)device)->dispatch;
> + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> +
> + tp->tap.enabled = false;
> +}
> +
> int
> tp_init_tap(struct tp_dispatch *tp)
> {
> + tp->tap.config.count = tp_tap_config_count;
> + tp->tap.config.enable = tp_tap_config_enable;
> + tp->tap.config.is_enabled = tp_tap_config_is_enabled;
> + tp->tap.config.reset = tp_tap_config_reset;
> + tp->device->base.config.tap = &tp->tap.config;
> +
> tp->tap.state = TAP_STATE_IDLE;
> tp->tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
>
> @@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
> return -1;
> }
>
> - tp->tap.enabled = 1; /* FIXME */
> -
> return 0;
> }
>
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index d514ed6..0b31e9a 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -203,6 +203,7 @@ struct tp_dispatch {
>
> struct {
> bool enabled;
> + struct libinput_device_config_tap config;
> int timer_fd;
> struct libinput_source *source;
> unsigned int timeout;
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 35781c3..060b529 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
> struct libinput *li = dev->libinput;
> struct libinput_event *event;
>
> + libinput_device_config_tap_enable(dev->libinput_device, 1);
> +
> litest_drain_events(li);
>
> litest_touch_down(dev, 0, 50, 50);
> @@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag)
> struct libinput *li = dev->libinput;
> struct libinput_event *event;
>
> + libinput_device_config_tap_enable(dev->libinput_device, 1);
> +
> litest_drain_events(li);
>
> litest_touch_down(dev, 0, 50, 50);
> @@ -194,6 +198,8 @@ START_TEST(touchpad_2fg_tap)
> struct libinput *li = dev->libinput;
> struct libinput_event *event;
>
> + libinput_device_config_tap_enable(dev->libinput_device, 1);
> +
> litest_drain_events(dev->libinput);
>
> litest_touch_down(dev, 0, 50, 50);
> @@ -349,6 +355,30 @@ START_TEST(clickpad_click_n_drag)
> }
> END_TEST
>
> +START_TEST(touchpad_tap_is_available)
> +{
> + struct litest_device *dev = litest_current_device();
> +
> +
> ck_assert_int_ge(libinput_device_config_tap_get_finger_count(dev->libinput_device),
> 1);
> +
> ck_assert_int_eq(libinput_device_config_tap_is_enabled(dev->libinput_device),
> 0);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_tap_is_not_available)
> +{
> + struct litest_device *dev = litest_current_device();
> +
> +
> ck_assert_int_eq(libinput_device_config_tap_get_finger_count(dev->libinput_device),
> 0);
> +
> ck_assert_int_eq(libinput_device_config_tap_is_enabled(dev->libinput_device),
> 0);
> +
> ck_assert_int_eq(libinput_device_config_tap_enable(dev->libinput_device, 1),
> + LIBINPUT_CONFIG_STATUS_UNSUPPORTED);
> +
> + /* doesn't do anything but tests for null-pointer derefernce */
> + libinput_device_config_tap_reset(dev->libinput_device);
> +}
> +END_TEST
> +
> +
> int main(int argc, char **argv) {
>
> litest_add("touchpad:motion", touchpad_1fg_motion, LITEST_TOUCHPAD,
> LITEST_ANY);
> @@ -357,6 +387,8 @@ int main(int argc, char **argv) {
> litest_add("touchpad:tap", touchpad_1fg_tap, LITEST_TOUCHPAD,
> LITEST_ANY);
> litest_add("touchpad:tap", touchpad_1fg_tap_n_drag, LITEST_TOUCHPAD,
> LITEST_ANY);
> litest_add("touchpad:tap", touchpad_2fg_tap, LITEST_TOUCHPAD,
> LITEST_SINGLE_TOUCH);
> + litest_add("touchpad:tap", touchpad_tap_is_available, LITEST_TOUCHPAD,
> LITEST_ANY);
> + litest_add("touchpad:tap", touchpad_tap_is_not_available, LITEST_ANY,
> LITEST_TOUCHPAD);
>
> litest_add_no_device("touchpad:clickfinger", touchpad_1fg_clickfinger);
> litest_add_no_device("touchpad:clickfinger", touchpad_2fg_clickfinger);
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
