On Mon, Mar 10, 2014 at 02:23:32PM -0700, Jason Gerecke wrote:
> >
> > +/**
> > + * 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.
Yup.
>
> > 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
Argh, yes of course! I didn't look closely enough: I missed the
usbChooseChannel() to set wcmMTChannel.
> 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...
With this patch (after removing the above) this "no device type?" fixup
is the only place where device_type for the single "current" channel:
Now device_type for the "current" channel is always set from wcmDeviceType.
The comment in this code:
/* tool was on tablet at startup, force type and proximity */
is really misleading - now even more than it was before.
Moving the device type setting to usbChooseChannel() could be done
later - for now I would like to keep the code in usbDispatchEvents()
and add a little test:
if (ds->device_type && ds->device_type != private->wcmDeviceType)
THIS_IS_A_BUG();
which will should cover a lot of what might go wrong with my changes
in place.
> > @@ -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.
>
Ok. I haven't looked much into multi touch devices, yet as I don't have one.
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
