[PATCH libinput] touchpad: require at least 3 events before enabling trackpoint palm detection

[Hans de Goede]

Hi,

On 08-09-16 07:43, Peter Hutterer wrote:
> On Wed, Sep 07, 2016 at 10:22:22AM +1000, Peter Hutterer wrote:
>> Some trackpoints, notably the one on the Lenovo T460s have a tendency to send
>> the odd event even when they're not actually used. Trackpoint events trigger
>> palm detection (see 0210f1fee193) and thus effectively disable the touchpad,
>> causing the touchpad to appear nonresponsive.
>>
>> Fix this by requiring at least 3 events from a trackpoint before palm
>> detection is enabled. For normal use it's hard enough to trigger a single
>> event anyway so this should not affect the normal use-case.
>>
>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>
> follow-up: this is actually this bug here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1364850
> after looking at the wrong thing for a while I found that comment 3 shows
> that single event from event5 (the trackpoint node).

I somehow missed the original posting of this patch, so
I'm replying this mail instead.

Patch LGTM:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans



>
> Cheers,
>    Peter
>
>> ---
>>  src/evdev-mt-touchpad.c |  9 ++++++++-
>>  src/evdev-mt-touchpad.h |  1 +
>>  test/trackpoint.c       | 26 ++++++++++++++++++++++++++
>>  3 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
>> index 65b0abf..8f7dbf0 100644
>> --- a/src/evdev-mt-touchpad.c
>> +++ b/src/evdev-mt-touchpad.c
>> @@ -1302,6 +1302,7 @@ tp_trackpoint_timeout(uint64_t now, void *data)
>>
>>  	tp_tap_resume(tp, now);
>>  	tp->palm.trackpoint_active = false;
>> +	tp->palm.trackpoint_event_count = 0;
>>  }
>>
>>  static void
>> @@ -1314,6 +1315,13 @@ tp_trackpoint_event(uint64_t time, struct libinput_event *event, void *data)
>>  	if (event->type == LIBINPUT_EVENT_POINTER_BUTTON)
>>  		return;
>>
>> +	tp->palm.trackpoint_last_event_time = time;
>> +	tp->palm.trackpoint_event_count++;
>> +
>> +	/* Require at least three events before enabling palm detection */
>> +	if (tp->palm.trackpoint_event_count < 3)
>> +		return;
>> +
>>  	if (!tp->palm.trackpoint_active) {
>>  		tp_edge_scroll_stop_events(tp, time);
>>  		tp_gesture_cancel(tp, time);
>> @@ -1321,7 +1329,6 @@ tp_trackpoint_event(uint64_t time, struct libinput_event *event, void *data)
>>  		tp->palm.trackpoint_active = true;
>>  	}
>>
>> -	tp->palm.trackpoint_last_event_time = time;
>>  	libinput_timer_set(&tp->palm.trackpoint_timer,
>>  			   time + DEFAULT_TRACKPOINT_ACTIVITY_TIMEOUT);
>>  }
>> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
>> index 8a8d2db..c2edb83 100644
>> --- a/src/evdev-mt-touchpad.h
>> +++ b/src/evdev-mt-touchpad.h
>> @@ -337,6 +337,7 @@ struct tp_dispatch {
>>  		struct libinput_event_listener trackpoint_listener;
>>  		struct libinput_timer trackpoint_timer;
>>  		uint64_t trackpoint_last_event_time;
>> +		uint32_t trackpoint_event_count;
>>  		bool monitor_trackpoint;
>>  	} palm;
>>
>> diff --git a/test/trackpoint.c b/test/trackpoint.c
>> index b92b994..e9ba027 100644
>> --- a/test/trackpoint.c
>> +++ b/test/trackpoint.c
>> @@ -349,6 +349,31 @@ START_TEST(trackpoint_palmdetect_resume_touch)
>>  }
>>  END_TEST
>>
>> +START_TEST(trackpoint_palmdetect_require_min_events)
>> +{
>> +	struct litest_device *trackpoint = litest_current_device();
>> +	struct litest_device *touchpad;
>> +	struct libinput *li = trackpoint->libinput;
>> +
>> +	touchpad = litest_add_device(li, LITEST_SYNAPTICS_I2C);
>> +	litest_drain_events(li);
>> +
>> +	/* A single event does not trigger palm detection */
>> +	litest_event(trackpoint, EV_REL, REL_X, 1);
>> +	litest_event(trackpoint, EV_REL, REL_Y, 1);
>> +	litest_event(trackpoint, EV_SYN, SYN_REPORT, 0);
>> +	libinput_dispatch(li);
>> +	litest_drain_events(li);
>> +
>> +	litest_touch_down(touchpad, 0, 30, 30);
>> +	litest_touch_move_to(touchpad, 0, 30, 30, 80, 80, 10, 1);
>> +	litest_touch_up(touchpad, 0);
>> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
>> +
>> +	litest_delete_device(touchpad);
>> +}
>> +END_TEST
>> +
>>  void
>>  litest_setup_tests_trackpoint(void)
>>  {
>> @@ -362,4 +387,5 @@ litest_setup_tests_trackpoint(void)
>>
>>  	litest_add("trackpoint:palmdetect", trackpoint_palmdetect, LITEST_POINTINGSTICK, LITEST_ANY);
>>  	litest_add("trackpoint:palmdetect", trackpoint_palmdetect_resume_touch, LITEST_POINTINGSTICK, LITEST_ANY);
>> +	litest_add("trackpoint:palmdetect", trackpoint_palmdetect_require_min_events, LITEST_POINTINGSTICK, LITEST_ANY);
>>  }
>> --
>> 2.7.4
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>