On Tue, Mar 4, 2014 at 5:16 AM, Egbert Eich <e...@freedesktop.org> wrote:
> From: Egbert Eich <e...@suse.com>
>
> usbInitToolType() tries to find the device type of a tool.
> Unlike usbFindDeviceType() it doesn't take into account the device_id
> which may exist in the event stream.
> As a result the device type may be taken from the last known type.
> This is generally a bad idea if the type has changed.
> This will happen for example when pressing a key on the Cintiq 21UX menu
> strips after removing a pen from the tablet.
>
> Signed-off-by: Egbert Eich <e...@suse.com>
> ---
> src/wcmUSB.c | 50 ++++++++++++++++++++++++++------------------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 14fff1b..f32c591 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -1078,6 +1078,26 @@ static int usbIdToType(int id)
> return type;
> }
>
> +static int usbFindDeviceTypeById(int device_id)
Move the documentation from patch 5/6 here.
> +{
> + switch (device_id)
> + {
> + case STYLUS_DEVICE_ID:
> + return STYLUS_ID;
> + case ERASER_DEVICE_ID:
> + return ERASER_ID;
> + case CURSOR_DEVICE_ID:
> + return CURSOR_ID;
> + case TOUCH_DEVICE_ID:
> + return TOUCH_ID;
> + case PAD_DEVICE_ID:
> + return PAD_ID;
> + default: /* protocol 5 */
> + return usbIdToType(device_id);
> + }
> + return 0;
> +}
> +
> /**
> * 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).
> @@ -1096,27 +1116,7 @@ static int usbFindDeviceType(const WacomCommonPtr
> common,
>
> if (!ds->device_id) return 0;
>
> - switch (ds->device_id)
> - {
> - case STYLUS_DEVICE_ID:
> - device_type = STYLUS_ID;
> - break;
> - case ERASER_DEVICE_ID:
> - device_type = ERASER_ID;
> - break;
> - case CURSOR_DEVICE_ID:
> - device_type = CURSOR_ID;
> - break;
> - case TOUCH_DEVICE_ID:
> - device_type = TOUCH_ID;
> - break;
> - case PAD_DEVICE_ID:
> - device_type = PAD_ID;
> - break;
> - default: /* protocol 5 */
> - device_type = usbIdToType(ds->device_id);
> - }
> -
> + device_type = usbFindDeviceTypeById(ds->device_id);
> return device_type;
> }
>
> @@ -1474,7 +1474,7 @@ static void usbParseBTNEvent(WacomCommonPtr common,
> * @param[in] code Linux input tool code (e.g. BTN_STYLUS_PEN)
> * @return Wacom device ID (e.g. STYLUS_ID) or 0 if no match.
> */
> -static int toolTypeToDeviceType(WacomCommonPtr common, int type, int code)
> +static int toolTypeToDeviceType(WacomCommonPtr common, int type, int code,
> int value)
The new 'value' parameter needs documenting.
Also, this function seems slightly ill-named, especially now that
we're examining more than just the event code. It should probably be
renamed to something like deviceTypeFromEvent (and the docs should
reflect that it translates an event not just a tool code...)
> {
> wcmUSBData* private = common->private;
>
> @@ -1506,6 +1506,8 @@ static int toolTypeToDeviceType(WacomCommonPtr common,
> int type, int code)
> case ABS_MT_SLOT:
> case ABS_MT_TRACKING_ID:
> return TOUCH_ID;
> + case ABS_MISC:
> + return usbFindDeviceTypeById(value);
> }
> }
>
> @@ -1535,7 +1537,7 @@ static int refreshDeviceType(WacomCommonPtr common)
> for (i = 0; i < KEY_MAX; i++)
> {
> if (ISBITSET(keys, i))
> - device_type = toolTypeToDeviceType(common, EV_KEY, i);
> + device_type = toolTypeToDeviceType(common, EV_KEY, i,
> 0);
> if (device_type)
> return device_type;
> }
> @@ -1565,7 +1567,7 @@ static int usbInitToolType(WacomCommonPtr common, const
> struct input_event *even
>
> for (i = 0; (i < nevents) && !device_type; ++i, event_ptr++)
> {
> - device_type = toolTypeToDeviceType(common, event_ptr->type,
> event_ptr->code);
> + device_type = toolTypeToDeviceType(common, event_ptr->type,
> event_ptr->code, event_ptr->value);
This smells like it may cause issues. If a protocol 5 tool goes out of
prox, we'll request the device type for code=ABS_MISC and value=0.
That's not a problem in and of itself, but the `usbIdToType` function
that is eventually called will incorrectly return a non-zero value if
passed zero in its argument. This could potentially result in the tool
type apparently changing.
A patch making usbIdToType return zero if provided zero should appear
prior to this patch (with a message along the lines of "zero is not a
valid ID, so no type can be inferred").
Relatedly, it looks like usbIdToType will actually return CUSOR_ID for
unknown IDs, when it should be returning STYLUS_ID (what's with the
'!' ???)
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....
> }
>
> if (!device_type)
> --
> 1.8.4.5
>
>
> ------------------------------------------------------------------------------
> Subversion Kills Productivity. Get off Subversion & Make the Move to Perforce.
> With Perforce, you get hassle-free workflows. Merge that actually works.
> Faster operations. Version large binaries. Built-in WAN optimization and the
> freedom to use Git, Perforce or both. Make the move to Perforce.
> http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk
> _______________________________________________
> Linuxwacom-devel mailing list
> Linuxwacom-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
------------------------------------------------------------------------------
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
