Skip to content

Use a more updated Synaptics driver #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 56 commits into
base: master
Choose a base branch
from

Conversation

Chosko
Copy link

@Chosko Chosko commented May 16, 2015

This fixes #1

whot and others added 30 commits January 17, 2014 08:24
This reverts commit 0903d99.

Scroll buttons are still present in some modern devices, e.g. the Fujitsu
Lifebook E782 and others in the series.

Conflicts:
	include/synaptics.h
	man/synaptics.man
	src/synaptics.c
This suite was never really maintained anyway, and it is quite hard to do so
anyway. The driver is linked to the server's API too tightly to easily do
independent testing. We need to re-implement stubs for the API the driver
uses, have to track API changes, etc. Not worth the effort.

Signed-off-by: Peter Hutterer <[email protected]>
This reverts commit 3b02e7f.

Signed-off-by: Peter Hutterer <[email protected]>

Conflicts:
	man/synaptics.man
	src/synaptics.c

Acked-by: Daniel Stone <[email protected]>
6d47d33 disallows a zero value for horizontal/vertical scroll deltas but the
man page wasn't updated. We've added separate toggles to enable/disable
scrolling a few years ago, setting the distance to 0 is not recommended.

X.Org Bug 75074 <http://bugs.freedesktop.org/show_bug.cgi?id=75074>

Signed-off-by: Peter Hutterer <[email protected]>
… area

synaptics offers an option to make parts of the touchpad insensitive. This
is ie useful to do palm avoidance rather then palm detection (which may be
unreliable) by disabling an area of 15% on the right and left side of the
touchpad.

Currently a motion which has started inside the active area, stops as soon
as it moves outside of the active area.

If a motion started inside the active area and thus has already generated some
move events, this makes no sense. If the user moves outside of the active
area in this case, this is very likely because the user wants to continue
the motion.

This commit allows such motions to continue normally.

I would like to thank Juerd Waalboer for the basic idea, some coding and lots
of testing for this fix.

Cc: Juerd Waalboer <[email protected]>
Reported-by: Juerd Waalboer <[email protected]>
Tested-by: Juerd Waalboer <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
Reviewed-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
New generation of laptops with trackstick do not have physical buttons
associated with the trackstick, but instead rely on software buttons at
the top of the clickpad.
Adding a secondary software button area for this purpose.
As we're likely detecting the devices that need it based on udev tags
and MatchTag configuration items, this area doesn't need to be exposed
through properties. So static configuration is fine.

Signed-off-by: Benjamin Tissoires <[email protected]>
Reviewed-by: Peter Hutterer <[email protected]>

[couple of man-page additions and rewrites]

Signed-off-by: Peter Hutterer <[email protected]>
Signed-off-by: Keith Packard <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
If we try to grab the evdev device before we've set the new file
descriptor, libevdev_grab returns -EFAULT, which causes DeviceOn to
fail.

Signed-off-by: Keith Packard <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
…licks

On a new set of laptops like the Lenovo T440 the trackstick does not have
physical buttons. Instead, the touchpad's top edge is supposed to acts
software button area. To avoid spurious cursor jumps when the trackstick is in
use and the finger is resting on the touchpad, add another mode that disables
motion events.

Enabled by syndaemon with -t click-only, the default stays unchanged. No
specific integration with the traditional disable-while-typing is needed. On
such touchpads, disabling motion events is sufficient to avoid spurious
events and we don't want to stop HW buttons to send events.

Note that this only adds the new state to the driver and to syndaemon, there
is nothing hooked up otherwise to actually monitor the trackstick.

Special note for syndaemon: optional arguments are a GNU extension, so work
around it by messing with an optstring starting with ":" which allows us to
manually parse the options.

Original version of this patch by John Pham <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
While at it also move the enum for the soft button edges out of
is_inside_button_area() so that it can be used elsewhere too.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
We only use it to store button state which we already have in
priv->lastButtons.

While at it also properly indent the code block checking the various
soft button areas.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Unless the motion has started outside the soft-button area.

Note that we must start reporting motions regardless of whether we think we're
in the button area or not as soon as we've switched to using cumulative
coordinates, since then the coordinates are no longer absolute.

This fixes the reporting of unintended motion just before a click in a soft
button area which sometimes causes mis-clicks.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
This fixes my felipejfc#1 anoyance with clickpads, where 2 out of 3 clicks turn into
a click + drag unless I hold my finger really really still.

Signed-off-by: Hans de Goede <[email protected]>

Replaced property with a hardcoded 100ms. This is not something that we should
expose as property, we should find a delay that works best and live with it.

Signed-off-by: Peter Hutterer <[email protected]>
When a button click and new coordinates get reported in one go we sync the
cumulative coordinates to the old x and y, rather then the newly reported ones.

This keeping of the old coordinates causes the following issue:
-touch the touchpad in its right click area
-let go of the touchpad
-rapidly click in the left click area (or middle area), so that the
 new location and the click get reported in one syn (may require some
 practicing with evemu-record to reproduce)
-the driver registers the click as a right click because it uses the
 old coordinates from the cumulative coordinates to determine the
 click location

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
…lick

It is possible for a click to get reported before any related touch events
get reported, here is the relevant part of an evemu-record session on a T440s:

E: 3.985585 0000 0000 0000	# ------------ SYN_REPORT (0) ----------
E: 3.997419 0003 0039 -001	# EV_ABS / ABS_MT_TRACKING_ID   -1
E: 3.997419 0001 014a 0000	# EV_KEY / BTN_TOUCH            0
E: 3.997419 0003 0018 0000	# EV_ABS / ABS_PRESSURE         0
E: 3.997419 0001 0145 0000	# EV_KEY / BTN_TOOL_FINGER      0
E: 3.997419 0000 0000 0000	# ------------ SYN_REPORT (0) ----------
E: 5.117881 0001 0110 0001	# EV_KEY / BTN_LEFT             1
E: 5.117881 0000 0000 0000	# ------------ SYN_REPORT (0) ----------
E: 5.133422 0003 0039 0187	# EV_ABS / ABS_MT_TRACKING_ID   187
E: 5.133422 0003 0035 3098	# EV_ABS / ABS_MT_POSITION_X    3098
E: 5.133422 0003 0036 3282	# EV_ABS / ABS_MT_POSITION_Y    3282
E: 5.133422 0003 003a 0046	# EV_ABS / ABS_MT_PRESSURE      46
E: 5.133422 0001 014a 0001	# EV_KEY / BTN_TOUCH            1
E: 5.133422 0003 0000 3102	# EV_ABS / ABS_X                3102
E: 5.133422 0003 0001 3282	# EV_ABS / ABS_Y                3282
E: 5.133422 0003 0018 0046	# EV_ABS / ABS_PRESSURE         46
E: 5.133422 0001 0145 0001	# EV_KEY / BTN_TOOL_FINGER      1
E: 5.133422 0000 0000 0000	# ------------ SYN_REPORT (0) ----------

Notice the BTN_LEFT event all by itself!

If this happens, it may lead to the following problem scenario:
-touch the touchpad in its right click area
-let go of the touchpad
-rapidly click in the middle area, so that BTN_LEFT gets reported before the
 new coordinates (such as seen in the trace above, this may require some
 practicing with evemu-record to reproduce)
-the driver registers the click as a right click because it uses the
 old coordinates from the cumulative coordinates to determine the
 click location

This commit fixes this by:
1) Resetting the cumulative coordinates not only when no button is pressed,
   but also when there is no finger touching the touchpad, so that when
   we do get a touch the cumulative coordinates start at the right place
2) Delaying processing the BTN_LEFT down transition if there is no finger
   touching the touchpad

This approach has one downside, if we wrongly identify a touchpad as
a clickpad, then the left button won't work unless the user touches the
touchpad while clicking the left button.

If we want we can fix this by doing something like this:
1) Making update_hw_button_state return a delay; and
2) Tracking that we've delayed BTN_LEFT down transition processing; and
3) When we've delayed BTN_LEFT down transition return a small delay value; and
4) If when we're called again we still don't have a finger down, just
   treat the click as a BTN_LEFT

But this is not worth the trouble IMHO, the proper thing to do in this
scenario is to fix the mis-identification of the touchpad as a clickpad.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
leftover from an earlier revision

Signed-off-by: Peter Hutterer <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
This was originally intended as a fixed xorg.conf option only (and still
largely is seen as such). Secondary software button are required only on a specific series
of touchpads and should be pre-configured by the system and/or the
distribution. As such, the property will not be initialized if it is not set
in the xorg.conf and will thus not respond to runtime changes.

Exposing the property in this way gives clients a chance of detecting if a top
software button area is present and thus adjust their behaviour accordingly.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
This was supposed to emulate a SYN_REPORT event so that the upper layers
process what's in the queue.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Reviewed-by: Benjamin Tissoires <[email protected]>
The kernel guarantees slots start at 0

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Reviewed-by: Benjamin Tissoires <[email protected]>
When checking the device don't open a new mtdev instance, use the existing
libevdev struct.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Reviewed-by: Benjamin Tissoires <[email protected]>
All kernel touchpad devices now support slots, there isn't really a need to
support protocol A devices in synaptics. If such devices exist, we just treat
them as non-multitouch devices.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Reviewed-by: Benjamin Tissoires <[email protected]>
Signed-off-by: Stephen Chandler Paul <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Clicking in the top soft button area causes the trackpad to begin
registering motion, even if the finger never leaves the top soft button
area. We don't want this kind of behavior for the top soft button area,
since it makes clicking and dragging items much more difficult when
using a pointing stick.

Signed-off-by: Stephen Chandler Paul <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
…button clicks"

This third state is not needed, the behaviour of the touchpad driver is now
good enough to not need an external syndaemon instance to toggle this third
state.

This reverts commit eea7335.

Conflicts:
	man/synaptics.man
	src/synaptics.c

Signed-off-by: Peter Hutterer <[email protected]>
Bad fdi file, type="string" is missing and it wouldn't merge properly.

This reverts commit a35b2d6.
Enabling clicks in off mode also allows for the new Lenovo *40 series to use
the top software buttons while the touchpad is disabled. This benefits those
that usually disable touchpads altogether but still need the buttons for the
trackstick.

This changes existing behaviour, but TouchpadOff was always intended to stop
erroneous events while typing. Physical button presses are hard to trigger
accidentally. On the touchpads that TouchpadOff concept was originally
designed for the buttons are nowhere near the keyboard and are physically
separated from the touchpad anyway. On Clickpads, triggering a physical
click requires more force than accidentally touching the surface.

https://bugs.freedesktop.org/show_bug.cgi?id=76156

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
whot and others added 26 commits March 18, 2014 07:28
Signed-off-by: Peter Hutterer <[email protected]>
This was required when we started supporting hotplugging to avoid duplicate
events. These days the drawback of not being able to record events in the case
of a bug is significant.

Check the configuration source on init. If the device was hotplugged through a
a server config backend, disable the grab. If the device was statically
configured through an xorg.conf then leave the default grab enabled to avoid
a duplicate device.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
last_mt_vals_slot is only used in one location and there we can just use
cur_slot

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Once the sync finishes, we get -EAGAIN. This only indicates the sync is done,
but some events may still be waiting in the pipe for us to read. We must read
those now, otherwise select may not trigger on further data.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
This is a bit problematic: libevdev only has one global log handler.
So if we have another driver use libevdev, we'll either overwrite that handler
or get overwritten, whichever comes first. So we need to re-set the handler
every time we get an event to make sure we log through our handler.
Likewise, if we ever drop the device, we need to unset the log handler back to
NULL because we may unload the module and our handler may disappear.

Use the lowest logging priority, let the server filter based on the verbosity
level instead.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Everytime I look at this I get confused about OPEN_EMPTY vs EMPTY. Let's fix
that.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
And expand DMI strings to more precise matches

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
When trying to do a 3 fingerclick on a touchpad which only tracks 2 touches,
this may register as a 3 or 2 fingerclick depending on the order in which
the touchpad detects the fingers. If the 2 outer fingers of the 3 get seen
first, then the 2 touches will be too far apart for the heuristic to see
them as being close together, and the click gets counted as a 2 finger click.

A user will likely never do a 2 finger click with a 3th finger resting
somewhere else on the pad, where-as the above misdetection of the clicks is
a real issue, so simply always count a click with trippletap set as a
3 finger click on pads which track less then 3 touches.

https://bugzilla.redhat.com/show_bug.cgi?id=1086218

Signed-off-by: Hans de Goede <[email protected]>
Reviewed-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Add a HasSecondaryButtons boolean config option which defaults to true for
devices with the INPUT_PROP_TOPBUTTONPAD and false for all other devices.

Only parse the SecondarySoftButtonAreas when this option is true, effectively
disabling the top buttons when it is false. Likewise, only initialize the
SecondarySoftButtonAreas property if we enable support for it.

This means that it is now safe to always set a SecondarySoftButtonAreas
default in 50-synaptics.conf, and that he section which was intended for
use with future pnp-id matching can be dropped, as that is now all handled
in the kernel.

While at also remove the comment about disabling the bottom edge area, as that
is now done automatically.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Some Macbooks are being tagged as MODEL_UNIBODY_MACBOOKs when they should not
be. This causes the default sensitivity to be very low for them, making the
touchpad almost unusable. This change puts those devices into the correct
bucket again.

Signed-off-by: Clinton Sprain <[email protected]>
Reviewed-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
Rely on INPUT_PROP_TOP_BUTTONPAD and default button areas as well.

Signed-off-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
We had reports that the top software button area is hard to hit for those
using the trackpoint and clicking the buttons with their thumb.

Analysis of event recordings (3 different people) for left, right and middle
clicks shows that there is a significant amount of events up to about 10mm
(with outliers up to 12mm) from the top of the touchpad. That maps to 15%.

Interestingly, the middle button does not seem to need this, presumably the
haptic feedback of the little dots sticking out from the surface make hitting
the button easier. Its size is increased to 15% anyway, for simplicity and
because a sample set of 3 is too small to be definitive about this.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
(cherry picked from commit 730101223432f60397c61f74a5e6789b3ee34ecd)
When two fingers are used, the coordinates of only one of them is taken into
account. This can lead to sudden variations of the absolute coordinates when
two-fingers taps are performed if the finger considered changes.

Take into account coordinates variations to prevent unwanted taps only if
the number of fingers doesn't change.

Reviewed-by: Peter Hutterer <[email protected]>
Signed-off-by: Peter Hutterer <[email protected]>
(cherry picked from commit 7d0ff39519e4d3760722b914883bee276035061c)
In file included from /usr/include/string.h:634:0,
                 from /usr/include/xorg/os.h:53,
                 from /usr/include/xorg/misc.h:115,
                 from /usr/include/xorg/xf86str.h:37,
                 from /usr/include/xorg/xf86Xinput.h:54,
                 from synproto.h:36,
                 from synproto.c:24:
/usr/include/xorg/os.h:579:1: error: expected identifier or '(' before '__extension__'
 strndup(const char *str, size_t n);

See http://lists.freedesktop.org/archives/xorg-devel/2014-July/043070.html

Signed-off-by: Peter Hutterer <[email protected]>
(cherry picked from commit 96e60a4ea242d2decf109835981ae186cc36f642)
Default on evdev devices is CLOCK_REALTIME. If that clock falls behind the
server's CLOCK_MONOTONIC, motion after a clickpad click may be delayed by the
difference in the clocks.

In detail:
When the timer func is triggered, GetTimeInMillis() which is CLOCK_MONOTONIC,
is stored as hwState->millis. The eventcomm backend uses struct
input_event time (CLOCK_REALTIME).

When we read events from the device, if the evdev time is less than the server
time, the fix for (#48777) sets the current event time to hwState->millis.
Until the evdev time overtakes that stored time, all events have the
hwState->millis time.

If during that time a clickpad triggers a physical click,
clickpad_click_millis is set to hwState->millis + the ignore-motion timeout.
Thus, all motion is ignored until the event time overtakes that stored
time.

The whole issue is further enhanced by us unconditionally setting the timer
func if we get any events, which is a separate issue anyway.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
(cherry picked from commit 90d19302306f49722e210227b2fb5161e6f51880)
And warn when we run out of labels.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Keith Packard <[email protected]>
(cherry picked from commit d239f831f17ccf5468f5dc6b2f199a9c1f6e35af)
open_slots holds the slot index, resetting it to 0 is a bad idea. And make
sure that we do reset after DEVICE_INIT. We already do so on DEVICE_CLOSE, but
after the first DEVICE_ON the data could still be random.

Signed-off-by: Peter Hutterer <[email protected]>
Reviewed-by: Keith Packard <[email protected]>
(cherry picked from commit afbbcfa10eb3a2295823720907f35bb59972dd82)
Default resolution is 1, don't allow setting 0 to avoid divisions by 0 or
just general weirdness.

Signed-off-by: Peter Hutterer <[email protected]>
(cherry picked from commit 049611bd7f04e285909c55807478306cce83385f)
Signed-off-by: Peter Hutterer <[email protected]>
@felipejfc
Copy link
Owner

have you tested it?

@Chosko
Copy link
Author

Chosko commented May 17, 2015

I have successfully tested it on Ubuntu 15.04
sabato, 16 maggio 2015, 11:59PM +02:00 da Felipe Cavalcanti [email protected]:

have you tested it?

Reply to this email directly or  view it on GitHub .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rebase to an updated version
8 participants