On Mon, Feb 03, 2014 at 06:12:15PM -0800, Ping Cheng wrote:
> 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.
what about HardwareTouchSwitchState then? this is read-only anyway, so the
extra couple of characters don't hurt and it makes it 100% clear :)
Cheers,
Peter
> 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
