On Tue, Mar 4, 2014 at 5:16 AM, Egbert Eich <e...@freedesktop.org> wrote:
> From: Egbert Eich <e...@suse.com>
>
> Tool type detection is done in two places: early in usbDispatchEvents()
> by calling usbInitToolType() and later on in the same function when the
> events are parsed.
> usbInitToolType() is used to set (wcmUSBData*)->wcmDeviceType, the
> detection that happens later when the events are parsed sets
> (WacomDeviceState).device_type. These variables are matched against each
> to find the right channel for a device.
> If the algorithms used for both set of tool type detection diverge
> undesirable effects may happen. Therefore it is advisable to determine the
> tool type only once ie. in usbInitToolType() and copy the result to
> (WacomDeviceState).device_type if this value is unset.
> Since we now have brought the algorithm in usbInitToolType() in sync with
> the reset, we delete any duplicate tool type setting.
>
> Signed-off-by: Egbert Eich <e...@suse.com>

The majority of this patch should be merged with the prior two patches.

> ---
>  src/wcmUSB.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 919aec8..d69f291 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -1078,6 +1078,16 @@ static int usbIdToType(int id)
>         return type;
>  }
>
> +/**
> + * Find the tool type (STYLUS_ID, etc.) based on the device_id or the
> + *  current tool serial number if the device_id is unknown (0).
> + *
> + * Protocol 5 devices report different IDs for different styli and pucks,
> + * Protocol 4 devices simply report STYLUS_DEVICE_ID, etc.
> + *
> + * @device_id id of the device
> + * @return device type
> + */

This hunk belongs in the first patch.

>  static int usbFindDeviceTypeById(int device_id)
>  {
>         switch (device_id)
> @@ -1240,7 +1250,6 @@ static void usbParseAbsMTEvent(WacomCommonPtr common, 
> struct input_event *event)
>
>                 case ABS_MT_TRACKING_ID:
>                         ds->proximity = (event->value != -1);
> -                       ds->device_type = TOUCH_ID;

This is not correct. Touches are stored in multiple channels, not just
the single "current" channel whose device type is set when we call
`usbInitToolType` in `usbDispatchEvents`. For every ABS_MT_SLOT event
that we see, we update `private->wcmMTChannel` to point to it. This
secondary channel will only ever have its device type set by this
line. (We also have a dedicated channel for the PAD device, but its
device type is set statically when the driver is loaded so I don't
think it would have any problems)

My initial thought would be to have `usbChooseChannel` automatically
set `ds->device_type` if it has to return a new channel. This would
require a little thought though, since the "no device type?" fixup
done in `usbDispatchEvents` assumes that the device type hasn't been
overwritten yet...

>                         ds->device_id = TOUCH_DEVICE_ID;
>                         ds->sample = (int)GetTimeInMillis();
>                         break;
> @@ -1283,7 +1292,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                 case BTN_TOOL_PENCIL:
>                 case BTN_TOOL_BRUSH:
>                 case BTN_TOOL_AIRBRUSH:
> -                       ds->device_type = STYLUS_ID;
>                         /* V5 tools use ABS_MISC to report device_id */
>                         if (common->wcmProtocolLevel == WCM_PROTOCOL_4)
>                                 ds->device_id = STYLUS_DEVICE_ID;
> @@ -1294,7 +1302,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                         break;
>
>                 case BTN_TOOL_RUBBER:
> -                       ds->device_type = ERASER_ID;
>                         /* V5 tools use ABS_MISC to report device_id */
>                         if (common->wcmProtocolLevel == WCM_PROTOCOL_4)
>                                 ds->device_id = ERASER_DEVICE_ID;
> @@ -1311,7 +1318,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                         DBG(6, common,
>                             "USB mouse detected %x (value=%d)\n",
>                             event->code, event->value);
> -                       ds->device_type = CURSOR_ID;
>                         /* V5 tools use ABS_MISC to report device_id */
>                         if (common->wcmProtocolLevel == WCM_PROTOCOL_4)
>                                 ds->device_id = CURSOR_DEVICE_ID;
> @@ -1329,7 +1335,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                                         DBG(6, common,
>                                             "USB 1FG Touch detected %x 
> (value=%d)\n",
>                                             event->code, event->value);
> -                                       ds->device_type = TOUCH_ID;
>                                         ds->device_id = TOUCH_DEVICE_ID;
>                                         ds->proximity = event->value;
>                                 }
> @@ -1343,7 +1348,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                                 DBG(6, common,
>                                     "USB Pad detected %x (value=%d)\n",
>                                 event->code, event->value);
> -                               ds->device_type = PAD_ID;
>                                 ds->device_id = PAD_DEVICE_ID;
>                                 ds->proximity = (event->value != 0);
>                                 break;
> @@ -1354,7 +1358,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                         DBG(6, common,
>                             "USB Touch detected %x (value=%d)\n",
>                             event->code, event->value);
> -                       ds->device_type = TOUCH_ID;
>                         ds->device_id = TOUCH_DEVICE_ID;
>                         ds->proximity = event->value;
>                         /* time stamp for 2FGT gesture events */
> @@ -1367,7 +1370,6 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>                         DBG(6, common,
>                             "USB Touch second finger detected %x 
> (value=%d)\n",
>                             event->code, event->value);
> -                       ds->device_type = TOUCH_ID;
>                         ds->device_id = TOUCH_DEVICE_ID;
>                         ds->proximity = event->value;
>                         /* time stamp for 2GT gesture events */
> @@ -1492,7 +1494,7 @@ static int toolTypeToDeviceType(WacomCommonPtr common, 
> int type, int code, int v
>
>                         case BTN_TOOL_FINGER:
>                                 if ((common->wcmProtocolLevel != 
> WCM_PROTOCOL_GENERIC)
> -                                   && !private->wcmUseMT)
> +                                   && !private->wcmUseMT)  /* this isn't in 
> usbParseKeyEvent() */

This shouldn't be a problem, but I need to do more thinking to be sure.

>                                         return PAD_ID;
>                                 else
>                                         return TOUCH_ID;
> --
> 1.8.4.5
>

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to