On Mon, Feb 3, 2014 at 5:51 PM, Peter Hutterer <peter.hutte...@who-t.net>wrote:

> On Mon, Feb 03, 2014 at 04:22:11PM -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.
> >
> > A new input property, WACOM_PROP_HARDWARE_TOUCH, is intoduced
> > to report touch status changed by end user through touch switch.
> >
> > HardwareTouch, a new xsetwacom option, is 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>
> > ---
> >  include/wacom-properties.h |  3 ++
> >  man/xsetwacom.man          |  6 ++++
> >  src/wcmUSB.c               | 36 ++++++++++++++++++++++++
> >  src/wcmXCommand.c          | 69
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  src/xf86Wacom.h            |  5 ++++
> >  src/xf86WacomDefs.h        |  3 ++
> >  tools/xsetwacom.c          | 11 +++++++-
> >  7 files changed, 132 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..9a7f194 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 the
> > +kernel when X driver starts.
> > +.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..333f413 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,26 @@ 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->wcmHasHWTouchSwitch = TRUE;
> > +
> > +             memset(sw, 0, sizeof(sw));
> > +
> > +             ioctl(pInfo->fd, EVIOCGSW(sizeof(sw)), sw);
> > +
> > +             if (ISBITSET(sw, SW_MUTE_DEVICE))
> > +                     common->wcmHasHWTouchSwitch = 0;
> > +             else
> > +                     common->wcmHasHWTouchSwitch = 1;
>
> uhm, did you test this code? you're setting the wrong variable here, this
> should be wcmHWTouchSwitchState.
>

Good question! After held by this set for so long, I was too eager to push
things out....


> with that change, Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
> though as a last-minute change I'd rename the xsetwacom parameter from
> HardwareTouch to HardwareTouchSwitch to avoid potential confusion.
>

Can we use HardwareTouchState instead? When the touch switch is on
(SW_MUTE_DEVICE is set), we do not receive touch data. That leads to
wcmHWTouchSwitchState to 0. HardwareTouchSwitch sounds like an indication
of the state of the hardware itself, which is 1.

Anyway, it's a bit twisted. The switch's state reported from the kernel is
opposite to the touch data state used in X driver, like wcmTouch == 1 means
touch is enabled.

Looks like we need to add comments here.

I'll update and test the patchset after hear from you.

Cheers,

Ping


>
> > +     }
> > +
> >       usbWcmInitPadState(pInfo);
> >
> >       return Success;
> > @@ -1682,6 +1703,21 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> >               if (usbFilterEvent(common, event))
> >                       continue;
> >
> > +             if (common->wcmHasHWTouchSwitch)
> > +             {
> > +                     if ((event->type == EV_SW) &&
> > +                         (event->code == SW_MUTE_DEVICE))
> > +                     {
> > +                             int touch_enabled = (event->value == 0);
> > +
> > +                             if (touch_enabled !=
> common->wcmHWTouchSwitchState)
> > +                             /* this property is only set for touch
> device */
> > +                                     wcmUpdateHWTouchProperty(
> > +                                             common->wcmTouchDevice,
> > +                                             touch_enabled);
> > +                     }
> > +             }
> > +
> >               /* absolute events */
> >               if (event->type == EV_ABS)
> >               {
> > diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> > index b2ba5a5..aa2aecb 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->wcmHasHWTouchSwitch && IsTouch(priv)) {
> > +             values[0] = common->wcmHWTouchSwitchState;
> > +             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,56 @@ 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;
> > +
> > +     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->wcmHWTouchSwitchState;
> > +     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(WacomDevicePtr priv, int hw_touch)
> > +{
> > +     WacomCommonPtr common = priv->common;
> > +
> > +     if (hw_touch == common->wcmHWTouchSwitchState)
> > +             return;
> > +
> > +     common->wcmHWTouchSwitchState = hw_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, priv->pInfo);
> > +}
> > +
> >  /**
> >   * Only allow deletion of a property if it is not being used by any of
> the
> >   * button actions.
> > @@ -781,6 +837,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->wcmHasHWTouchSwitch)
> > +             {
> > +                     /* If we get here from wcmUpdateHWTouchProperty,
> we know
> > +                      * the wcmHWTouchSwitchState has been set
> internally
> > +                      * already, so we can reply with success. */
> > +                     if (prop->size == 1 && prop->format == 8)
> > +                             if (((CARD8*)prop->data)[0] ==
> common->wcmHWTouchSwitchState)
> > +                                     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..90d2499 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(WacomDevicePtr priv, int touch);
> >
> >  /* Utility functions */
> >  extern Bool is_absolute(InputInfoPtr pInfo);
> > diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> > index 3a64fd6..d1a3859 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 wcmHasHWTouchSwitch;    /* Tablet has a touch on/off switch */
> > +     int wcmHWTouchSwitchState;   /* 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
> >
>
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121051231&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