On Fri, Jun 04, 2010 at 03:40:31PM -0700, Ping Cheng wrote:
> If a tool is on an USB tablet before the driver is loaded, the driver
> loses the initial state of the tool since kernel input device driver
> filters repeated events. To initialize the tool with the proper
> device type, we call EVIOCGKEY to retrieve it from the kernel.
>
> Signed-off-by: Ping Cheng <[email protected]>
thanks, principle looks good but I have a few comments inline.
> ---
> src/wcmCommon.c | 7 ++++---
> src/wcmUSB.c | 45 +++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index eb985e4..96bd48c 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1297,9 +1297,7 @@ static void commonDispatchDevice(WacomCommonPtr common,
> unsigned int channel,
>
> if (!ds->device_type && ds->proximity)
> {
> - /* Tool may be on the tablet when X starts.
> - * Figure out device type by device id
> - */
> + /* something went wrong. Figure out device type by device id */
> switch (ds->device_id)
> {
> case STYLUS_DEVICE_ID:
> @@ -1314,6 +1312,9 @@ static void commonDispatchDevice(WacomCommonPtr common,
> unsigned int channel,
> case TOUCH_DEVICE_ID:
> ds->device_type = TOUCH_ID;
> break;
> + case PAD_DEVICE_ID:
> + ds->device_type = PAD_ID;
> + break;
This hunk looks out of place. shouldn't this be a separate "add missing
PAD_DEVICE_ID to device type conversion" patch?
> default:
> ds->device_type = idtotype(ds->device_id);
> }
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 249a7bf..7128fae 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -869,6 +869,24 @@ skipEvent:
> common->wcmEventCnt = 0;
> }
>
> +static struct
> +{
> + __u16 device_type;
> + __u16 tool_key;
__u16 is rather kernel specific, I think we should be using either normal
unsigned ints/shorts or if the width is to be enforced uint16_t.
> +} wcmTypeToKey [] =
> +{
> + { STYLUS_ID, BTN_TOOL_PEN },
> + { STYLUS_ID, BTN_TOOL_PENCIL },
> + { STYLUS_ID, BTN_TOOL_BRUSH },
> + { STYLUS_ID, BTN_TOOL_AIRBRUSH },
> + { ERASER_ID, BTN_TOOL_RUBBER },
> + { CURSOR_ID, BTN_TOOL_MOUSE },
> + { CURSOR_ID, BTN_TOOL_LENS },
> + { TOUCH_ID, BTN_TOOL_DOUBLETAP },
> + { TOUCH_ID, BTN_TOOL_TRIPLETAP },
> + { PAD_ID, BTN_TOOL_FINGER }
> +};
> +
> static void usbParseChannel(LocalDevicePtr local, int channel)
> {
> int i, shift, nkeys;
> @@ -876,6 +894,8 @@ static void usbParseChannel(LocalDevicePtr local, int
> channel)
> struct input_event* event;
> WacomDevicePtr priv = (WacomDevicePtr)local->private;
> WacomCommonPtr common = priv->common;
> + WacomChannelPtr pChannel = common->wcmChannel + channel;
> + WacomDeviceState dslast = pChannel->valid.state;
>
> DBG(6, common, "%d events received\n", common->wcmEventCnt);
> #define MOD_BUTTONS(bit, value) do { \
> @@ -996,8 +1016,6 @@ static void usbParseChannel(LocalDevicePtr local, int
> channel)
> }
> else if (event->code == BTN_TOOL_DOUBLETAP)
> {
> - WacomChannelPtr pChannel = common->wcmChannel +
> channel;
> - WacomDeviceState dslast = pChannel->valid.state;
> DBG(6, common,
> "USB Touch detected %x (value=%d)\n",
> event->code, event->value);
> @@ -1023,8 +1041,6 @@ static void usbParseChannel(LocalDevicePtr local, int
> channel)
> }
> else if (event->code == BTN_TOOL_TRIPLETAP)
> {
> - WacomChannelPtr pChannel = common->wcmChannel +
> channel;
> - WacomDeviceState dslast = pChannel->valid.state;
> DBG(6, common,
> "USB Touch second finger detected %x
> (value=%d)\n",
> event->code, event->value);
> @@ -1066,6 +1082,27 @@ static void usbParseChannel(LocalDevicePtr local, int
> channel)
> }
> } /* next event */
>
> + /* device type unknown? Tool may be on the tablet when X starts. */
> + if (!ds->device_type && !dslast.proximity)
> + {
> + __u16 keys[NBITS(KEY_MAX)];
we use unsigned long for the bits, should be using the same here if you want
to use the ISBITSET macro.
also, if you do a
unsigned long keys[NBITS(KEY_MAX)] = { 0 };
you can save yourself the memset.
Cheers,
Peter
> + int i;
> +
> + memset(keys, 0, sizeof(keys));
> +
> + /* Retrieve the type by asking a resend from the kernel */
> + ioctl(common->fd, EVIOCGKEY(sizeof(keys)), keys);
> +
> + for (i=0; i<sizeof(wcmTypeToKey) / sizeof(wcmTypeToKey[0]); i++)
> + {
> + if (ISBITSET(keys, wcmTypeToKey[i].tool_key))
> + {
> + ds->device_type = wcmTypeToKey[i].device_type;
> + break;
> + }
> + }
> + }
> +
> /* don't send touch event when touch isn't enabled */
> if ((ds->device_type == TOUCH_ID) && !common->wcmTouch)
> return;
> --
> 1.6.6.1
------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
