[PATCH 04/18] Add tap-on-LED feature support

Peter Hutterer peter.hutterer at who-t.net
Tue Oct 12 20:29:48 PDT 2010


On Fri, Oct 08, 2010 at 07:22:28PM +0200, Takashi Iwai wrote:
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>  include/synaptics-properties.h |    3 +
>  man/synaptics.man              |   17 +++++++
>  src/properties.c               |   27 +++++++++++
>  src/synaptics.c                |   97 +++++++++++++++++++++++++++++++++++++++-
>  src/synapticsstr.h             |    6 +++
>  tools/synclient.c              |    1 +
>  6 files changed, 149 insertions(+), 2 deletions(-)
> 
> diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> index dd7e259..1343e6d 100644
> --- a/include/synaptics-properties.h
> +++ b/include/synaptics-properties.h
> @@ -161,4 +161,7 @@
>  /* 8 bit (BOOL), led_status (on/off) */
>  #define SYNAPTICS_PROP_LED_STATUS "Synaptics LED Status"
>  
> +/* 8 bit (BOOL), double-tap action on LED corner (on/off) */
> +#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Dobule Tap"
> +

SYNAPTICS_PROP_GESTURES is essentially there for things like this, no need
for a new property, just append it there.

>  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> diff --git a/man/synaptics.man b/man/synaptics.man
> index 8a9767d..d8afa1a 100644
> --- a/man/synaptics.man
> +++ b/man/synaptics.man
> @@ -553,6 +553,19 @@ coordinates are less than MaxTapMove units apart.
>  A "touch" event happens when the Z value goes above FingerHigh, and an
>  "untouch" event happens when the Z value goes below FingerLow.
>  .
> +.TP
> +.BI "Option \*qLEDDoubleTap\*q \*q" boolean \*q
> +.
> +Enables/disables the touchpad-control by double-tapping 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 whether this action is enabled or not.
> +The double-tap size is same as specified in MaxDoubleTapTime.
> +The default value is ON.
> +Property: "Synaptics LED Double Tap"
> +.
>  .LP
>  The MaxDoubleTapTime parameter has the same function as the MaxTapTime
>  parameter, but for the second, third, etc tap in a tap sequence.
> @@ -918,6 +931,10 @@ LED support or not.
>  .BI "Synaptics LED Status"
>  8 bit (BOOL), the light status of the embedded LED.
>  
> +.TP 7
> +.BI "Synaptics LED Double Tap"
> +8 bit (BOOL), enable/disable the double-tap on LED.
> +
>  .SH "NOTES"
>  There is an example hal policy file in
>  .I ${sourcecode}/fdi/11-x11-synaptics.fdi
> diff --git a/src/properties.c b/src/properties.c
> index 57db0ec..6862a09 100644
> --- a/src/properties.c
> +++ b/src/properties.c
> @@ -84,6 +84,7 @@ Atom prop_resolution            = 0;
>  Atom prop_area                  = 0;
>  Atom prop_led                   = 0;
>  Atom prop_led_status            = 0;
> +Atom prop_led_double_tap        = 0;
>  
>  static Atom
>  InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
> @@ -283,6 +284,9 @@ InitDeviceProperties(InputInfoPtr pInfo)
>  
>      prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, &para->has_led);
>      prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 8, 1, &para->led_status);
> +
> +    prop_led_double_tap = InitAtom(local->dev, SYNAPTICS_PROP_LED_DOUBLE_TAP, 8, 1, &para->led_double_tap);
> +
>  }
>  
>  int
> @@ -508,6 +512,19 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
>              return BadValue;
>  
>          para->touchpad_off = off;
> +        if (!checkonly && para->has_led &&
> +	    para->led_status != para->touchpad_off) {
> +            para->led_status = para->touchpad_off;
> +            if (priv->proto_ops && priv->proto_ops->UpdateLED)
> +                priv->proto_ops->UpdateLED(local);
> +        }
> +    } else if (property == prop_led_double_tap)
> +    {
> +        if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
> +            return BadMatch;
> + 
> +        para->led_double_tap = *(CARD8*)prop->data;
> +
>      } else if (property == prop_gestures)
>      {
>          BOOL *gestures;
> @@ -669,3 +686,13 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
>      return Success;
>  }
>  
> +void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off)
> +{
> +        uint8_t val;
> +
> +        if (!prop_off)
> +                return;
> +        val = off;
> +        XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8,
> +                               PropModeReplace, 1, &val, FALSE);
> +}

last argument must be TRUE, we need to notify the clients about this change. 

> diff --git a/src/synaptics.c b/src/synaptics.c
> index 6f57d81..26ef666 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -142,6 +142,7 @@ static void CalculateScalingCoeffs(SynapticsPrivate *priv);
>  void InitDeviceProperties(InputInfoPtr pInfo);
>  int SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
>                  BOOL checkonly);
> +void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off);
>  
>  InputDriverRec SYNAPTICS = {
>      1,
> @@ -608,6 +609,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
>      pars->tap_and_drag_gesture = xf86SetBoolOption(opts, "TapAndDragGesture", TRUE);
>      pars->resolution_horiz = xf86SetIntOption(opts, "HorizResolution", horizResolution);
>      pars->resolution_vert = xf86SetIntOption(opts, "VertResolution", vertResolution);
> +    pars->led_double_tap = xf86SetBoolOption(opts, "LEDDoubleTap", TRUE);
>  
>      /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge parameters */
>      if (pars->top_edge > pars->bottom_edge) {
> @@ -894,6 +896,10 @@ DeviceOn(DeviceIntPtr dev)
>      xf86AddEnabledDevice(pInfo);
>      dev->public.on = TRUE;
>  
> +    /* update LED */
> +    if (priv->proto_ops && priv->proto_ops->UpdateLED)
> +        priv->proto_ops->UpdateLED(local);
> +
>      return Success;
>  }
>  
> @@ -1101,6 +1107,72 @@ DeviceInit(DeviceIntPtr dev)
>      return Success;
>  }
>  
> +#define LED_TOGGLE_X_AREA	0.10
> +#define LED_TOGGLE_Y_AREA	0.08
> +
> +static int
> +in_led_toggle_area(LocalDevicePtr local, struct SynapticsHwState *hw)
> +{
> +    SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> +    int click_led_x, click_led_y;
> +
> +    click_led_x = (priv->maxx - priv->minx) * LED_TOGGLE_X_AREA + priv->minx;
> +    click_led_y = (priv->maxy - priv->miny) * LED_TOGGLE_Y_AREA + priv->miny;
> +    return (hw->x < click_led_x && hw->y < click_led_y);
> +}
> +
> +/* clicpad button toggle point:


typo


> + * some devices have a LED at the upper-left corner, and double-tapping it
> + * toggles the touchpad enable/disable
> + */
> +static int
> +handle_toggle_led(LocalDevicePtr local, struct SynapticsHwState *hw, int finger)
> +{
> +    SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> +    SynapticsParameters *para = &priv->synpara;
> +    int diff;
> +
> +    if (finger) {
> +        if (!in_led_toggle_area(local, hw)) {
> +            /* outside the toggle area */
> +            priv->led_touch_state = FALSE;
> +            priv->led_tapped = FALSE;
> +            return finger;
> +        }
> +        if (!priv->led_touch_state) {
> +            /* touch start */
> +            priv->led_touch_millis = hw->millis;
> +            priv->led_touch_state = TRUE;
> +        }
> +        return 0; /* already processed; ignore this finger event */
> +    }
> +
> +    if (!priv->led_touch_state)
> +        return finger; /* nothing happened */
> +
> +    /* touch-released */
> +    priv->led_touch_state = FALSE;
> +    diff = TIME_DIFF(priv->led_touch_millis + para->tap_time, hw->millis);
> +    if (diff < 0) { /* non-tap? */
> +	priv->led_tapped = FALSE;
> +        return finger;
> +    }
> +    if (priv->led_tapped) {
> +        /* double-tapped? */
> +        diff = TIME_DIFF(priv->led_tap_millis + para->tap_time_2, hw->millis);
> +        if (diff >= 0) {
> +            para->touchpad_off = !para->touchpad_off;
> +            if (priv->proto_ops && priv->proto_ops->UpdateLED)
> +                priv->proto_ops->UpdateLED(local);
> +	    priv->prop_change_pending = 1;
> +            priv->led_tapped = FALSE;
> +        }
> +    } else
> +        priv->led_tapped = TRUE;
> +    priv->led_tap_millis = hw->millis;
> +    return 0; /* already processed; ignore this finger event */
> +}
> +

two tab indentations in this hunk.

>  /* clickpad event handling */
>  static void
>  handle_clickpad(LocalDevicePtr local, struct SynapticsHwState *hw)
> @@ -1261,6 +1333,7 @@ timerFunc(OsTimerPtr timer, CARD32 now, pointer arg)
>  {
>      InputInfoPtr pInfo = arg;
>      SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
> +    SynapticsParameters *para = &priv->synpara;
>      struct SynapticsHwState hw;
>      int delay;
>      int sigstate;
> @@ -1272,6 +1345,13 @@ timerFunc(OsTimerPtr timer, CARD32 now, pointer arg)
>      hw.millis = now;
>      delay = HandleState(pInfo, &hw);
>  
> +#if GET_ABI_MAJOR(ABI_XINPUT_VERSION) >= 3
> +    if (priv->prop_change_pending & 1) {
> +	SynapticsToggleOffProperty(local->dev, para->touchpad_off);
> +	priv->prop_change_pending = 0;
> +    }
> +#endif
> +
>      /*
>       * Workaround for wraparound bug in the TimerSet function. This bug is already
>       * fixed in CVS, but this driver needs to work with XFree86 versions 4.2.x and
> @@ -1322,6 +1402,10 @@ ReadInput(InputInfoPtr pInfo)
>  	hw.millis = GetTimeInMillis();
>  	priv->hwState = hw;
>  	delay = HandleState(pInfo, &hw);
> +#if GET_ABI_MAJOR(ABI_XINPUT_VERSION) >= 3
> +	if (priv->prop_change_pending)
> +	    delay = MIN(10, delay);
> +#endif
>  	newDelay = TRUE;
>      }
>  
> @@ -2306,7 +2390,7 @@ update_hw_button_state(const InputInfoPtr pInfo, struct SynapticsHwState *hw, in
>      SynapticsParameters *para = &priv->synpara;
>  
>      /* Clickpad handling for button area */
> -    if (priv->is_clickpad)
> +    if (para->touchpad_off != 1 && priv->is_clickpad)
>  	handle_clickpad(local, hw);
>  
>      /* Treat the first two multi buttons as up/down for now. */
> @@ -2424,7 +2508,7 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
>      update_shm(pInfo, hw);
>  
>      /* If touchpad is switched off, we skip the whole thing and return delay */
> -    if (para->touchpad_off == 1)
> +    if (para->touchpad_off == 1 && !(para->has_led && para->led_double_tap))
>  	return delay;
>  
>      inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
> @@ -2460,6 +2544,15 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
>  	finger = SynapticsDetectFinger(priv, hw);
>      }
>  
> +    if (para->has_led && para->led_double_tap) {
> +	if (inside_active_area)
> +		finger = handle_toggle_led(local, hw, finger);
> +        if (para->touchpad_off == 1) {
> +            priv->finger_state = finger;
> +            return delay;
> +        }
> +    }
> +

tab/space indentation mixed up. also, are you sure this is what you want
here? the led only toggled if it's inside the user's active area? Given that
this is a hardware feature more than a software feature, it should likely be
triggerable regardless of the area defined, isn't it?

>      /* tap and drag detection. Needs to be performed even if the finger is in
>       * the dead area to reset the state. */
>      timeleft = HandleTapProcessing(priv, hw, edge, finger, inside_active_area);
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index 88ca9de..05308c1 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -162,6 +162,7 @@ typedef struct _SynapticsParameters
>      int area_left_edge, area_right_edge, area_top_edge, area_bottom_edge; /* area coordinates absolute */
>      Bool has_led;                           /* has an embedded LED */
>      Bool led_status;                        /* Current status of LED (1=on) */
> +    Bool led_double_tap;		    /* double-tap period in ms for touchpad LED control */


spaces only please

meat of the patch looks good, bar the few comments above, you have my ack.

Cheers,
  Peter

>  } SynapticsParameters;
>  
>  
> @@ -239,6 +240,11 @@ typedef struct _SynapticsPrivateRec
>      Bool has_scrollbuttons;		/* device has physical scrollbuttons */
>      Bool is_clickpad;			/* is Clickpad device (one-button) */
>      struct SynapticsHwState prev_hw;	/* previous h/w state (for clickpad) */
> +    int prop_change_pending;
> +    Bool led_touch_state;
> +    Bool led_tapped;
> +    int led_touch_millis;
> +    int led_tap_millis;
>  
>      enum TouchpadModel model;          /* The detected model */
>  } SynapticsPrivate;
> diff --git a/tools/synclient.c b/tools/synclient.c
> index 539eefb..f7d93e0 100644
> --- a/tools/synclient.c
> +++ b/tools/synclient.c
> @@ -144,6 +144,7 @@ static struct Parameter params[] = {
>      {"AreaTopEdge",           PT_INT,    0, 10000, SYNAPTICS_PROP_AREA,	32,	2},
>      {"AreaBottomEdge",        PT_INT,    0, 10000, SYNAPTICS_PROP_AREA,	32,	3},
>      {"LEDStatus",             PT_BOOL,   0, 1,     SYNAPTICS_PROP_LED_STATUS,	8,	0},
> +    {"LEDDoubleTap",          PT_BOOL,   0, 1,     SYNAPTICS_PROP_LED_DOUBLE_TAP,	8,	0},
>      { NULL, 0, 0, 0, 0 }
>  };
>  
> -- 
> 1.7.3.1
> 


More information about the xorg-devel mailing list