Re: [DIGImend-devel] PT-1001 digitizer

[Nikolai K. <sp...@gm...>]

Hi Yann,

Could you please keep the maillist in the CC? Thank you.

On 01/26/2015 01:01 AM, Yann Vernier wrote:
> On Sun, 25 Jan 2015 16:41:04 +0200
> Nikolai Kondrashov <sp...@gm...> wrote:
>> On 01/20/2015 03:46 PM, Yann Vernier wrote:
>>> Possible workarounds include patching in that extra byte or requesting more
>>> (both tested and work). A proper fix would probably include not trying to
>>> parse more descriptor data than the device actually returned.
>>
>> Well, such fix still wouldn't work with devices which would get a more
>> important part of their report descriptor lost. Then, making the parser handle
>> all the possible failures it introduces would make it too complicated.
>
> Yes, attempting to guess the missing data is a bad idea. The current behaviour
> of not even noticing that it is missing isn't really much better, however.
> However, if I just make sure to request a larger size I get the correct amount.

Hmm, from your description it seems to me that Linux notices it is missing and
actively drops the interface with a bad descriptor, doesn't it? Or did I
understand you wrong?

>>> I do suggest requesting more than the descriptor length by default, since
>>> this bug may be present in other devices (possibly as a misinterpretation of
>>> USB's use of a full frame to indicate more data is available). My own test
>>> was simply to add 1 to rsize and dev_rsize in kmalloc and
>>> hid_get_class_descriptor calls within drivers/hid/usbhid/hid-core.c, but
>>> that doesn't check how much was actually received.
>>
>> This would be one controversial solution. How much more would we need to
>> request? Would this encourage more sloppy HID descriptors in the future? When
>> should we stop requesting more and more?
>
> The descriptor isn't actually corrupt, only the transfer is incomplete.
> The descriptor is 317 bytes.
> Linux requests 317 bytes, and receives 316 bytes because of the device bug.
> Linux does not check the size received and therefore not only fails in
> parsing the descriptor but reports a different problem (failing to parse a
> byte that was never received in the first place).
> Windows requests 317+64 bytes, and receives 317 bytes.
> If I make Linux request 317+1 bytes, I get 317 bytes, forming the complete
> descriptor.
>
> The main reason I believe we could safely request a larger size from most
> HID devices is that Windows apparently already does. Then again, I would
> not be terribly surprised if a modern console used that behaviour to
> trigger weirdness (we've seen them abuse USB before). An alternative that
> could reasonably work is just attempting rsize+1 if we get an incomplete
> descriptor on the first try. The advantage is that it would only affect
> devices that were displaying bad behaviour in the first place.

Sorry, I might have misunderstood you. I'll need to read the Linux USB code to
talk about this.

>> I would say we simply replace the device descriptor with a rewritten
>> descriptor as is done for many other devices already. See hid-uclogic.c for
>> example.
>
> That would allow us to clean it up for better describing the device (such as
> it having a stylus), as you noted below. I'll set about doing so tomorrow,
> I think.

Great!

>>> That's not the end of the story, however. Even with proper parsing of the
>>> descriptors it seems the tablet only has one interface that reports a stylus,
>>> and it's not the one sending events. This causes the Xorg driver to deduce it
>>> is a touchpad, not a pen tablet, with all the resulting defects (relative
>>> mode, ignoring events when the nib isn't pressed). It also reports a rather
>>> mystifying set of buttons (from xinput list-props, but do come from kernel,
>>> as confirmed using input-events):
>>>       Button Labels (276):    "Button Left" (145), "Button Middle" (146),
>>>         "Button Right" (147), "Button Wheel Up" (148), "Button Wheel Down" (149),
>>>         "Button Horiz Wheel Left" (150), "Button Horiz Wheel Right" (151),
>>>         "Button Side" (265), "Button Extra" (266), "Button Forward" (714),
>>>         "Button Back" (715), "Button Task" (716), "Button Unknown" (264),
>>>         "Button Unknown" (264), "Button Unknown" (264), "Button Unknown" (264)
>>> The second pointer device reported, which does not produce events, has an
>>> equally useless button map:
>>>       "Button 0" (630), "Button Unknown" (264), "Button Unknown" (264),
>>>       "Button Wheel Up" (148), "Button Wheel Down" (149)
>>>
>>> The actual buttons (nib and two side buttons on the stylus) are those
>>> reported as Forward, Back and Task. Remapping them with xinput set-button-map
>>> and setting absolute mode makes it usable but with the serious issue of not
>>> moving the pointer if the nib isn't pressed (the pressure level works in
>>> inkscape). I did not find a way to tell Xorg's evdev driver that the device
>>> is a tablet.
>>
>> Windows report descriptor parsing works quite differently from Linux and such
>> mapping issues happen often.
>
> I suspect there's actually a parsing bug in Linux here, as the kernel values
> barely have any similarities to the descriptors themselves. The first five on
> interface 1 match (being the boot protocol mouse buttons), as do the axis,
> but none of the other buttons do - interface 2 doesn't even have five
> buttons (it has four). I might try to figure out what the kernel HID parser
> did later.

I think it is intentional, although presents a Windows-compatibility problem.
Basically, IIRC, if Linux encounters a field with usage which should be mapped
to the same event as a field previously seen in the same report descriptor, it
assigns this field a higher event number. This might be limited to some event
types. The HID_QUIRK_MULTI_INPUT quirk is used to make the driver create
separate event devices for different report IDs to avoid this.

>> Wow! This is the first tablet I see that is trying to actually convert
>> various working area events into something else in *hardware*, i.e. on the
>> device side. All others do this in the driver.
>
> I expect it's a firmware thing.

Sure.

> It does peculiar things including translating three virtual sliders into
> different types of events (mouse scroll by emulated mouse buttons, volume
> and zoom via keyboard keys) and plenty of regions send key combinations such
> as Ctrl+C rather than a HID keycode with assigned meaning.

Well, sending particular, Windows-oriented key combinations (at least for real
on-the-frame buttons) is common among graphics tablets.

> The PCB is marked 155-D, and has a Polostar controller marked TB92E and
> A941K0420G1 (in the one I opened). The only other IC was an opamp. It might
> have a command to report raw tablet data, like the Huion tablets, which would
> make sense of interface 2. Finding it would be trickier since there is no
> driver; it's clearly designed to operate with Windows' generic drivers.
> http://www.polostar-tech.com/ appears to be the correct company.

Thanks, I might take a look at them later. I think it's actually not a bad
thing of them to try to make it work without a driver.

>> This X.org heuristic is pretty hairy, yes, but we can still cater to it by
>> using appropriate report descriptor(s).
>>
>> So, I think the path to a solution here should be making a separate HID driver
>> for this tablet and having a new set of report descriptors written, replacing
>> the original ones. Perhaps with some bit tweaking of the input reports as
>> well.
>
> The formats described seem to work just fine; it just has a few more mystery
> reports than I've observed in use. I'll try overriding the button usage codes.

Alright.

>> You can take hid-uclogic.c for boilerplate, as the most basic HID driver at
>> the moment doing similar things. If you know how report descriptors work you
>> can try making one yourself. I can suggest using my hidrd-convert [1] tool for
>> authoring, although there are a few others. You can find some examples of
>> rewritten descriptor XML sources in our tablets repo [2] - search for
>> "fixed*.xml".
>
> The descriptors I attached to my first mail were decoded with hidrd-convert.

Ah, sorry, forgot to look at the attachments in a hurry.

> I should be able to alter them sensibly with the help of the public HID specs.

Good. Don't hesitate to write, if something is not obvious there. Perhaps I'll
be able to help.

Nick