On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote: > 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.
Would just like to point out that there are several places this cast is already in place, and starting to use container_of would introduce inconsistencies. I think this kind of casting isn't necessarily bad for single (or primary) inheritance types of objects. Jonas > > > + 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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel