On Mon, Mar 10, 2014 at 02:03:23PM -0700, Jason Gerecke wrote:
> 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.
Will do.
>
> > +{
> > + switch (device_id)
> > + {
Missing: (see below)
case 0:
return 0;
> > + 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;
> > +}
> > +
[..]
> >
> > @@ -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...)
Will fix.
> > 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.
Yes, I noticed this today, also. This needs to be addressed in
usbFindDeviceTypeById() - see above.
>
> 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").
Right.
>
> Relatedly, it looks like usbIdToType will actually return CUSOR_ID for
> unknown IDs, when it should be returning STYLUS_ID (what's with the
> '!' ???)
I assume you are referring to the line marked below:
static int usbIdToType(int id)
{
int type = STYLUS_ID;
/* The existing tool ids have the following patten: all pucks, except
* one, have the third byte set to zero; all erasers have the fourth
* bit set. The rest are styli.
*/
if (id & ERASER_BIT)
type = ERASER_ID;
else if (!(id & PUCK_BITS) || (id == PUCK_EXCEPTION))
^^^^^^^^^^^
type = CURSOR_ID;
return type;
}
This is confusing at first but 'PUCK_BITS' seem to refer to bits which
are set on all devices but pucks.
we need to make sure though that the case where id = 0 is handled correctly
as this will obviously not have these bits set as well.
Cheers,
Egbert.
------------------------------------------------------------------------------
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
