On Sun, Jan 5, 2014 at 8:55 PM, Peter Hutterer <peter.hutte...@who-t.net>wrote:
> 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"
>
Will update this.
>
> > +.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.
Good question. There might have had a reason. But the holiday resets my
memory to ...
> 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.
>
Ok.
>
> > 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
>
if you say so...
>
> > + 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?
>
We may be able to use wcmTouchDevice. I'll have to test it to be sure.
> + }
> > + }
> > +
> > /* 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"?
>
Thank you for reviewing the patch. I'll make a v2 to incorporate all your
comments.
Ping
> + 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
