[PATCH 2/2] Support LED and tap-on-LED feature

[Takashi Iwai]

At Wed, 21 Apr 2010 16:53:14 +1000,
Peter Hutterer wrote:
> 
> On Wed, Apr 21, 2010 at 08:26:41AM +0200, Takashi Iwai wrote:
> > At Wed, 21 Apr 2010 14:16:29 +1000,
> > Peter Hutterer wrote:
> > > 
> > > On Thu, Apr 15, 2010 at 06:12:11PM +0200, Takashi Iwai wrote:
> > > > This patch adds the control of the embedded LED on the top-left corner
> > > > of new Synaptics devices.  For LED control, it requires the patch to
> > > > Linux synaptics input driver,
> > > > 	https://patchwork.kernel.org/patch/92434/
> > > > 
> > > > When evdev reports the presense of LED and LED_MUTE bits, the driver
> > > > assumes it supports the embeded LED control.  This corresponds to the
> > > > touchpad-off state.  When touchpad is disabled, LED is turned on.
> > > > 
> > > > For linking between the touchpad state and the LED state, a new callback
> > > > UpdateHardware is introduced.  Only eventcomm.c supports this (naturally).
> > > > 
> > > > A new feature for the LED-equipped device is that user can double-tap
> > > > on the LED to toggle the touchpad state on the fly.  This is also linked
> > > > with the touchpad-off state.
> > > > 
> > > > There is a new parameter for controlling the LED double-tap behavior, too.
> > > > It specifies the double-tap time.  Passing zero disables the double-tap
> > > > feature.
> > > > 
> > > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > > > ---
> > > >  include/synaptics-properties.h |    3 ++
> > > >  man/synaptics.man              |   13 +++++++
> > > >  src/eventcomm.c                |   39 +++++++++++++++++++++-
> > > >  src/properties.c               |   26 ++++++++++++++
> > > >  src/synaptics.c                |   73 ++++++++++++++++++++++++++++++++++++++-
> > > >  src/synapticsstr.h             |    6 +++
> > > >  src/synproto.h                 |    1 +
> > > >  tools/synclient.c              |    1 +
> > > >  8 files changed, 159 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > > > index c77afd3..56c3e1d 100644
> > > > --- a/include/synaptics-properties.h
> > > > +++ b/include/synaptics-properties.h
> > > > @@ -161,4 +161,7 @@
> > > >  /* 8 bit (BOOL) */
> > > >  #define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
> > > >  
> > > > +/* 32 bit */
> > > > +#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
> > > > +
> > > >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > > > diff --git a/man/synaptics.man b/man/synaptics.man
> > > > index f92ea0c..baa7e8f 100644
> > > > --- a/man/synaptics.man
> > > > +++ b/man/synaptics.man
> > > > @@ -530,6 +530,19 @@ clicking the button.  On the other hand, this reduces the available
> > > >  space on the touchpad.  The default is on.
> > > >  Property: "Synaptics Touch Button Sense"
> > > >  .
> > > > +.TP
> > > > +.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
> > > > +.
> > > > +The double-tap time for toggling the touchpad-control on the top-left
> > > > +corner LED.
> > > > +.
> > > > +Some devices have an LED on the top-left corner to indicate the
> > > > +touchpad state.  User can double-tap on the LED to toggle the touchpad
> > > > +state.  This option controls the double-tap time in milli-seconds.
> > > > +When it's zero, the LED-tapping feature is disabled.  The default
> > > > +value is 400.
> > > > +Property: "Synaptics LED Double Tap"
> > > 
> > > a few questions to the principle:
> > > - any specific reason this is a double-tap?
> > 
> > A single-tap is often too sensitive.  User may toggle it mistakenly,
> > and wonders what happens suddenly.  Also, Windows driver uses the
> > double-tap, too.
> > 
> > > - the corner might be used for the same feature even if there is no LED
> > >   present, right?
> > 
> > The check is done only when has_led flag is set, so it's exclusive for
> > the device with LED control.
> > 
> > > as I read this patch, the LED updating could be split out
> > >   from the actual gesture of disabling the touchpad by clicking into the
> > >   corner.
> > 
> > In general yes, the double-tapping to toggle touchpad is an individual
> > feature.  But this becomes intuitive when coupled with LED, thus
> > rather somewhat special for LED-equipped device, I suppose.
>  
> gnome has a feature that allows you to disable the touchpad hitting Fn+F8,
> and it then displays a popup box to notify the user of this. The same could
> be done for this feature by monitoring the off property, even if there's no
> LED present. So I think there are use-cases that warrant this feature
> without the tight LED integration.

Right, but the positioned double-tapping (that is featured on Windows)
seems coupled with LED, I suppose.

> > > > @@ -496,6 +500,11 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
> > > >              return BadValue;
> > > >  
> > > >          para->touchpad_off = off;
> > > > +	if (!checkonly) {
> > > > +	    if (priv->proto_ops && priv->proto_ops->UpdateHardware)
> > > > +		priv->proto_ops->UpdateHardware(local);
> > > > +	}
> > > > +
> > > 
> > > indentation.
> > 
> > What is wrong here?
> 
> the property code is one of the few places where the indentation is
> consistent (mainly because I wrote most of it :). Please use spaces here
> instead of tabs, which generally is my preference wherever in doubt.

OK.  I'm a kernel guy, so I prefer differently, but hey, it's just a
personal preference :)


thanks,

Takashi