On Tue, Dec 24, 2013 at 10:04:34AM -0800, Ping Cheng wrote:
> New Intuos series introduced a hardware switch to turn touch
> events on/off. This patch retrieves its state from kernel by
> checking if SW_MUTE_DEVICE is declared and reporting the
> value if it is supported.     
> 
> A new input property, WACOM_PROP_HARDWARE_TOUCH,
> is added to post touch status changed by end user through
> touch switch.
> 
> A new xsetwacom option, HardwareTouch, is also added. This
> option is read-only since the state can only be changed by end
> users. This option is independent from the existing Touch option,
> which can be considered as a software touch switch.
> 
> Signed-off-by: Ping Cheng <pi...@wacom.com>

sorry about the delay, apparently there was some festive season or somesuch
:)

patch looks good, thanks. just a few minor nits

> ---
>  include/wacom-properties.h |  3 ++
>  man/xsetwacom.man          |  6 ++++
>  src/wcmUSB.c               | 33 ++++++++++++++++++
>  src/wcmXCommand.c          | 83 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  src/xf86Wacom.h            |  5 +++
>  src/xf86WacomDefs.h        |  3 ++
>  tools/xsetwacom.c          | 11 +++++-
>  7 files changed, 143 insertions(+), 1 deletion(-)
> 
> diff --git a/include/wacom-properties.h b/include/wacom-properties.h
> index dd7e7f3..419251a 100644
> --- a/include/wacom-properties.h
> +++ b/include/wacom-properties.h
> @@ -78,6 +78,9 @@
>  /* BOOL, 1 value */
>  #define WACOM_PROP_TOUCH "Wacom Enable Touch"
>  
> +/* BOOL, 1 value */
> +#define WACOM_PROP_HARDWARE_TOUCH "Wacom Hardware Touch Switch"
> +
>  /* 8 bit, 1 values */
>  #define WACOM_PROP_ENABLE_GESTURE "Wacom Enable Touch Gesture"
>  
> diff --git a/man/xsetwacom.man b/man/xsetwacom.man
> index 978b104..80bc52b 100644
> --- a/man/xsetwacom.man
> +++ b/man/xsetwacom.man
> @@ -209,6 +209,12 @@ If on, touch events are reported to userland, i.e., 
> system cursor moves when
>  user touches the tablet. If off, touch events are ignored. Default: on for
>  devices that support touch; off for all other models.
>  .TP
> +\fBHardwareTouch\fR on|off
> +If on, it means touch switch is turned off. That is, touch events are 
> reported
> +to userland. If off, touch switch is turned on, i.e., touch events are 
> ignored.
> +This is a read-only parameter. Initial touch switch state is retrieved from
> +kernel when X driver starts.

"from the kernel"

> +.TP
>  \fBCursorProximity\fR distance
>  sets the max distance from tablet to stop reporting movement for cursor in
>  relative mode. Default for Intuos series is 10, for Graphire series 
> (including
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index cf27a10..6455e1a 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -518,6 +518,7 @@ int usbWcmGetRanges(InputInfoPtr pInfo)
>       struct input_absinfo absinfo;
>       unsigned long ev[NBITS(EV_MAX)] = {0};
>       unsigned long abs[NBITS(ABS_MAX)] = {0};
> +     unsigned long sw[NBITS(SW_MAX)] = {0};
>       WacomDevicePtr priv = (WacomDevicePtr)pInfo->private;
>       WacomCommonPtr common = priv->common;
>       wcmUSBData* private = common->private;
> @@ -744,6 +745,27 @@ int usbWcmGetRanges(InputInfoPtr pInfo)
>       if (!ISBITSET(abs, ABS_MISC))
>               common->wcmProtocolLevel = WCM_PROTOCOL_GENERIC;
>  
> +     if (ioctl(pInfo->fd, EVIOCGBIT(EV_SW, sizeof(sw)), sw) < 0)
> +     {
> +             xf86Msg(X_ERROR, "%s: usbProbeKeys unable to ioctl "
> +                     "sw bits.\n", pInfo->name);
> +             return 0;
> +     }
> +     else if (ISBITSET(sw, SW_MUTE_DEVICE))
> +             common->wcmTouchSwitchEnabled = TRUE;
> + 
> +     if (common->wcmTouchSwitchEnabled)
> +     {
> +             memset(sw, 0, sizeof(sw));
> +
> +             ioctl(priv->pInfo->fd, EVIOCGSW(sizeof(sw)), sw);
> +
> +             if (ISBITSET(sw, SW_MUTE_DEVICE))
> +                     common->wcmHWTouch = 0;
> +             else
> +                     common->wcmHWTouch = 1;
> +     }
> +

any reason you don't just add this to the ISBITSET condition? there's no
other way wcmTouchSwitchEnabled can be true anyway. Also, I'd prefer
the names wcmHaveHWTouchSwitch and wcmHWTouchSwitchState or something like
that. wcmTouchSwitchEnabled sounds like it's the variable used to trigger
the state.

>       usbWcmInitPadState(pInfo);
>  
>       return Success;
> @@ -1682,6 +1704,17 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>               if (usbFilterEvent(common, event))
>                       continue;
>  
> +             if (common->wcmTouchSwitchEnabled)
> +             {
> +                     if ((event->type == EV_SW) &&
> +                         (event->code == SW_MUTE_DEVICE))
> +                     {
> +                             int touch = (event->value == 0);
> +                             if (touch != common->wcmHWTouch)

"touch" -> "enabled" would be more expressive, imo 

> +                                     wcmUpdateHWTouchProperty(pInfo, touch);

is is possible to get the right device here so we don't need to loop through
the devices to find the touch device in the timer func?

> +                     }
> +             }
> +
>               /* absolute events */
>               if (event->type == EV_ABS)
>               {
> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> index b2ba5a5..2f37458 100644
> --- a/src/wcmXCommand.c
> +++ b/src/wcmXCommand.c
> @@ -92,6 +92,7 @@ Atom prop_cursorprox;
>  Atom prop_threshold;
>  Atom prop_suppress;
>  Atom prop_touch;
> +Atom prop_hardware_touch;
>  Atom prop_gesture;
>  Atom prop_gesture_param;
>  Atom prop_hover;
> @@ -264,6 +265,11 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
>       values[0] = common->wcmTouch;
>       prop_touch = InitWcmAtom(pInfo->dev, WACOM_PROP_TOUCH, XA_INTEGER, 8, 
> 1, values);
>  
> +     if (common->wcmTouchSwitchEnabled && IsTouch(priv)) {
> +             values[0] = common->wcmHWTouch;
> +             prop_hardware_touch = InitWcmAtom(pInfo->dev, 
> WACOM_PROP_HARDWARE_TOUCH, XA_INTEGER, 8, 1, values);
> +     }
> +
>       if (IsStylus(priv)) {
>               values[0] = !common->wcmTPCButton;
>               prop_hover = InitWcmAtom(pInfo->dev, WACOM_PROP_HOVER, 
> XA_INTEGER, 8, 1, values);
> @@ -601,6 +607,70 @@ void wcmUpdateRotationProperty(WacomDevicePtr priv)
>       }
>  }
>  
> +static CARD32
> +touchTimerFunc(OsTimerPtr timer, CARD32 now, pointer arg)
> +{
> +     InputInfoPtr pInfo = arg;
> +     WacomDevicePtr priv = pInfo->private;
> +     WacomCommonPtr common = priv->common;
> +     XIPropertyValuePtr prop;
> +     CARD8 prop_value;
> +     int sigstate;
> +     int rc;
> +
> +     /* make sure we set hardware touch to touch device */

.. "set hardware touch on the touch device"?

Cheers,
   Peter

> +     if (!IsTouch(priv))
> +     {
> +             WacomToolPtr tool = NULL;
> +             for (tool = common->wcmTool; tool; tool = tool->next)
> +             {
> +                     if (tool->typeid == TOUCH_ID)
> +                             break;
> +             }
> +             if (tool)
> +                     pInfo = tool->device;
> +     }
> +
> +     sigstate = xf86BlockSIGIO();
> +
> +     rc = XIGetDeviceProperty(pInfo->dev, prop_hardware_touch, &prop);
> +     if (rc != Success || prop->format != 8 || prop->size != 1)
> +     {
> +             xf86Msg(X_ERROR, "%s: Failed to update hardware touch state.\n",
> +                     pInfo->name);
> +             return 0;
> +     }
> +
> +     prop_value = common->wcmHWTouch;
> +     XIChangeDeviceProperty(pInfo->dev, prop_hardware_touch, XA_INTEGER,
> +                            prop->format, PropModeReplace,
> +                            prop->size, &prop_value, TRUE);
> +
> +     xf86UnblockSIGIO(sigstate);
> +
> +     return 0;
> +}
> +
> +/**
> + * Update HW touch property when its state is changed by touch switch
> + */
> +void
> +wcmUpdateHWTouchProperty(InputInfoPtr pInfo, int touch)
> +{
> +     WacomDevicePtr priv = pInfo->private;
> +     WacomCommonPtr common = priv->common;
> +
> +     if (touch == common->wcmHWTouch)
> +             return;
> +
> +     common->wcmHWTouch = touch;
> +
> +     /* This function is called during SIGIO. Schedule timer for property
> +      * event delivery outside of signal handler. */
> +     priv->touch_timer = TimerSet(priv->touch_timer, 0 /* reltime */,
> +                                   1, touchTimerFunc, pInfo);
> +}
> +
>  /**
>   * Only allow deletion of a property if it is not being used by any of the
>   * button actions.
> @@ -781,6 +851,19 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> XIPropertyValuePtr prop,
>  
>               if (!checkonly && common->wcmTouch != values[0])
>                       common->wcmTouch = values[0];
> +     } else if (property == prop_hardware_touch)
> +     {
> +             if (common->wcmTouchSwitchEnabled)
> +             {
> +                     /* If we get here from wcmUpdateHWTouchProperty,
> +                      * we know the wcmHWTouch has been set internally
> +                      * already, so we can reply with success. */
> +                     if (prop->size == 1 && prop->format == 8)
> +                             if (((CARD8*)prop->data)[0] == 
> common->wcmHWTouch)
> +                                     return Success;
> +             }
> +
> +             return BadValue; /* read-only */
>       } else if (property == prop_gesture)
>       {
>               CARD8 *values = (CARD8*)prop->data;
> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
> index 587bf48..8ea69b5 100644
> --- a/src/xf86Wacom.h
> +++ b/src/xf86Wacom.h
> @@ -43,6 +43,10 @@
>  #define LogMessageVerbSigSafe xf86MsgVerb
>  #endif
>  
> +#ifndef SW_MUTE_DEVICE
> +#define SW_MUTE_DEVICE       0x0e
> +#endif
> +
>  
> /*****************************************************************************
>   * Unit test hack
>   
> ****************************************************************************/
> @@ -175,6 +179,7 @@ extern int wcmDeleteProperty(DeviceIntPtr dev, Atom 
> property);
>  extern void InitWcmDeviceProperties(InputInfoPtr pInfo);
>  extern void wcmUpdateRotationProperty(WacomDevicePtr priv);
>  extern void wcmUpdateSerial(InputInfoPtr pInfo, unsigned int serial, int id);
> +extern void wcmUpdateHWTouchProperty(InputInfoPtr pInfo, int touch);
>  
>  /* Utility functions */
>  extern Bool is_absolute(InputInfoPtr pInfo);
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 3a64fd6..e563de6 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -310,6 +310,7 @@ struct _WacomDeviceRec
>  
>       OsTimerPtr serial_timer; /* timer used for serial number property 
> update */
>       OsTimerPtr tap_timer;   /* timer used for tap timing */
> +     OsTimerPtr touch_timer; /* timer used for touch switch property update 
> */
>  };
>  
>  
> /******************************************************************************
> @@ -431,6 +432,8 @@ struct _WacomCommonRec
>       WacomDevicePtr wcmTouchDevice; /* The pointer for pen to access the
>                                         touch tool of the same device id */
>       Bool wcmPenInProx;      /* Keep pen in-prox state for touch tool */
> +     Bool wcmTouchSwitchEnabled;     /* Tablet has a touch on/off switch */
> +     int wcmHWTouch;                 /* touch event disable/enabled by 
> hardware switch */
>  
>       /* These values are in tablet coordinates */
>       int wcmMaxX;                 /* tablet max X value */
> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
> index bbe25ea..b9cf76b 100644
> --- a/tools/xsetwacom.c
> +++ b/tools/xsetwacom.c
> @@ -204,6 +204,15 @@ static param_t parameters[] =
>               .prop_flags = PROP_FLAG_BOOLEAN
>       },
>       {
> +             .name = "HardwareTouch",
> +             .desc = "Touch events turned on/off by hardware switch. ",
> +             .prop_name = WACOM_PROP_HARDWARE_TOUCH,
> +             .prop_format = 8,
> +             .prop_offset = 0,
> +             .arg_count = 1,
> +             .prop_flags = PROP_FLAG_READONLY
> +     },
> +     {
>               .name = "Gesture",
>               .desc = "Turns on/off multi-touch gesture events "
>               "(default is on). ",
> @@ -2791,7 +2800,7 @@ static void test_parameter_number(void)
>        * deprecated them.
>        * Numbers include trailing NULL entry.
>        */
> -     assert(ARRAY_SIZE(parameters) == 37);
> +     assert(ARRAY_SIZE(parameters) == 38);
>       assert(ARRAY_SIZE(deprecated_parameters) == 17);
>  }
>  
> -- 
> 1.8.3.2
> 

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to