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

[Peter Hutterer]

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.

> > > @@ -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.

Cheers,
  Peter