Opened 14 years ago
Closed 11 years ago
#7354 closed enhancement (fixed)
usb_hid tablet support
Reported by: | lt_henry | Owned by: | mmlr |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | Drivers/USB | Version: | R1/Development |
Keywords: | usb_hid | Cc: | mdisreali@… |
Blocked By: | Blocking: | #6821 | |
Platform: | All |
Description
I've modified usb_hid to enable Tablet devices on Haiku. There are some bugs plugging/unplugging devices, but maybe someone can spot it faster than me. Tested with a pair of Trust digitizers (uc-logic and waltop chipsets), a nextwindow touchscreen and a Smart digital whiteboard.
Attachments (7)
Change History (36)
comment:1 by , 14 years ago
patch: | 0 → 1 |
---|
by , 14 years ago
Attachment: | tablet.patch added |
---|
comment:2 by , 14 years ago
Thanks for the patch!
My first feedback would be that you should look at the patch file you generated and revert any local changes until the patch no longer contains changes to the tracing output. There are some files currently in the patch which would not need to be changed at all (Driver.cpp, MouseDevice.cpp).
The patch for ProtocolHandler.cpp contains coding style violations with regards to the parenthesis style. Please refer to the coding style guide.
The TabletDevice code contains heaps of style violations with regard to parenthesis, when to use new lines and when not to, asterisk style, and spacing. Everything is inconsistent. Again please refer to the style guide. Also, the copyright is wrong, as Michael Lotz did not write these files.
TabletDevice::AddHandler():
if(x!=NULL && y!=NULL) { if(x->Relative() || y->Relative()) return NULL; }else return NULL; return new(std::nothrow) TabletDevice(report);
You can rewrite this to be more readable (also fixing the coding style):
if (x == NULL || y == NULL) return NULL; if (x->Relative() || y->Relative()) return NULL; return new(std::nothrow) TabletDevice(report);
It's both less code and much more obvious.
if(fBarrelSwitch!=NULL && fBarrelSwitch->Extract()==B_OK && fBarrelSwitch->Valid()) { info.buttons |= fBarrelSwitch->Data()<<1; }
Is the above code supposed to handle the eraser when you flip the pen? If so, it will not work in apps like WonderBrush. If you look at the Wacom tablet driver, you will see that the eraser is reported as primary mouse button with a boolean flag in the message which refers to it being the eraser. This is also documented in the BeBook to work this way. You probably need to add an extra flag to the tablet info structure.
Review of the input_server add-on in next comment.
comment:3 by , 14 years ago
The patch to the TabletDevice seems to contain only changes to device registration/unregistration, and you mention it is still buggy. Since your changes to the USB driver side do not concern themselves with this, I would assume tablet hot-plugging should work exactly as good as mouse hot-plugging on the usb_hid side of the fence. So the problems should be in the input_server add-on. A while ago, I have worked intensively on this subject and made both the mouse and keyboard input_server add-on behave well with hot-plugging. At the moment, I don't remember any of the details, but it would be my hope that by making the TabletDevice work exactly like the other two, the problems would be gone.
comment:4 by , 14 years ago
The way how different hardware types are supposed to be discerned is by not using the reports directly, but enumerating by the HID collections instead. The collections are logically grouped by their usage, have a usage id to identify them and can have sub-collections to expose different logical groups. The simple reason why I used the reports directly so far is that even with the lack of dedicated device handlers all devices with two axis were possibly usable as inputs and all devices wit buttons/keys would be seen as keyboard and could be used at least partially. With the addition of device class specific handlers like the tablet one here and the joystick one on the mailinglist, this direct use of the reports should be used only as a fallback if at all. Instead the AddHandlers function should now be extended to enumerate based on the HIDCollection (the root collection with its children beong available via HIDDevice::RootCollection or HIDParser::RootCollection, not sure anymore).
follow-up: 6 comment:5 by , 14 years ago
mmlr, Good work on usb_hid by the way. i've looked at the HIDCollection.cpp and HIDParser.cpp. In HIDParser.cpp when an ITEM_TYPE_LOCAL is detected and then the item->tag is an ITEM_TAG_LOCAL_USAGE should the code be as follows,
:)
value->u.s.usage_page = globalState.usage_page;
value->u.s.usage_id = data;
The code as it is now is
value->u.extended = data;
As far as i understand when an ITEM_TAG_LOCAL_USAGE is declared then it should have the usage page of the previously declared global usage page(Am i right ?). Also when a new HIDCollection is created when the item->tag is ITEM_TAG_MAIN_COLLECTION the localstate is always empty when it's created for the first time. This happens in the following situation,
USAGE_PAGE(Generic Desktop) type global
USAGE(GamePad) type local
Then when an item of ITEM_TAG_MAIN_COLLECTION is found for the first time, the localState.usage_stack is passed to HIDCollection but it's empty because the localState.usage_stack hasn't been initialized to point to usageStack.
I've modified the code as follows,
if (item->tag == ITEM_TAG_MAIN_COLLECTION) {
localState.usage_stack = usageStack;
localState.usage_stack_used = usageStackUsed;
then the code continues as normal.
By doing this modification the HIDCollection now at least has access to the correct USAGE_PAGE and USAGE_ID for the devices type we are trying to get.
comment:6 by , 14 years ago
Replying to caz_haiku:
In HIDParser.cpp when an ITEM_TYPE_LOCAL is detected and then the item->tag is an ITEM_TAG_LOCAL_USAGE should the code be as follows, :)
value->u.s.usage_page = globalState.usage_page;
value->u.s.usage_id = data;The code as it is now is
value->u.extended = data;
No this is not correct. The item can define a usage id only, or an extended usage (combined usage page and usage id). The is_extended flag holds that information and is initialized based on the item size. The transformation from usage id to extended usage is done prior to adding a main item, but I think I see where you're coming from (see below).
As far as i understand when an ITEM_TAG_LOCAL_USAGE is declared then it should have the usage page of the previously declared global usage page(Am i right ?).
The local usage can override the global usage page if it is an extended usage, see the HID 1.1 specs page 51.
USAGE_PAGE(Generic Desktop) type global
USAGE(GamePad) type local
Then when an item of ITEM_TAG_MAIN_COLLECTION is found for the first time, the localState.usage_stack is passed to HIDCollection but it's empty because the localState.usage_stack hasn't been initialized to point to usageStack.
You are right, and it is even worse: not only is the initialization of the usage stack missing, but also the whole other preprocessing like the potential conversion of usage ids to extended usages and the designator and string initialization. That is most probably also the reason why you thought what you wrote in the above case was necessary, as with the missing usage conversion you might've ended up with a collection that only gets a usage id istead of a full extended usage.
I've modified the code as follows, if (item->tag == ITEM_TAG_MAIN_COLLECTION) {
localState.usage_stack = usageStack;
localState.usage_stack_used = usageStackUsed;
then the code continues as normal.
By doing this modification the HIDCollection now at least has access to the correct USAGE_PAGE and USAGE_ID for the devices type we are trying to get.
Basically yes, but it should just do the same preprocessing of the local state as is done for adding a main item. I probably simply forgot about doing this processing also for collections, as back then I didn't use them for anything. I'll correct that in the driver now, thanks for the pointers!
A few remarks about this report: It would make it a lot easier to understand what you did by simply attaching a patch instead of describing the changes, as with the description you have to fully go through the code at hand first and then guess where exactly you might have put the changes. Also it'd be helpful if you stated how you arrived at the conclusions you did, for example if you based that on the code, on code of other projects or on the specs. The HID driver as it is now is completely "by the book" based on the specs. Adding code because it works instead of because one understood the intentions of the specs would be a very bad thing, as this can easily break things subtly which is later hard to track down.
comment:7 by , 14 years ago
See hrev40953 and hrev40954. As it turns out the HIDCollection already did the proper usage id and usage page combination, so only the initialization of the usage stack was really missing. If this doesn't work for your device then the report descriptor it has is likely to be non-conforming which'd require workarounds. Always overwriting the local usage page isn't really an option though. Please attach the report descriptor for your device so we can take a look. It is dumped to /tmp when the driver loads and can be inspected with hid_decode (can be built from the tree).
by , 14 years ago
Attachment: | HIDCollection.diff added |
---|
HIDCollection patch to add UsagePage and UsageID access
follow-up: 11 comment:8 by , 14 years ago
mmlr, Thanks for modifying usb_hid and i'm glad i could help a bit. I'm new to this way of contributing so i'll try to do things the right way from now on.
After your modifications the collection now has the correct usagepage and usageid(you know this already 'cause you did the code :-0), I want access to the UsagePage and UsageId in the collection so to check the device type, is this acceptable?. If this is not acceptable then we may have to have access to the vendor_id etc of the device.
if(collection->UsagePage() != B_HID_USAGE_PAGE_GENERIC_DESKTOP || collection->UsageID() != B_HID_UID_GD_GAMEPAD) return NULL;
by , 14 years ago
Attachment: | usb_hid_report_descriptor_0428_4001_0.bin added |
---|
Report Descriptor for Gravis Gamepad Pro USB
by , 14 years ago
Attachment: | GamepadDevice.zip added |
---|
Example of current GamepadDevice(not complete but does work with BJoystick)
comment:9 by , 14 years ago
Cc: | added |
---|---|
Version: | R1/alpha2 → R1/Development |
Should the gamepad related stuff go on ticket:4499 or possible a separate enhancement ticket?
comment:10 by , 14 years ago
I've updated usb hid patch. This version is cleaner, there are no unnecessary trace outputs, and there are some improvements regarding haiku code guideline. Still far from perfect, of course. Next day I will update input server side.
follow-up: 13 comment:11 by , 14 years ago
Replying to caz_haiku:
After your modifications the collection now has the correct usagepage and usageid
Good, so the other change you had in there first wasn't needed apparently and no need for any device workarounds.
I want access to the UsagePage and UsageId in the collection so to check the device type, is this acceptable?
if(collection->UsagePage() != B_HID_USAGE_PAGE_GENERIC_DESKTOP || collection->UsageID() != B_HID_UID_GD_GAMEPAD) return NULL;
Yes that's perfectly fine and how it was intended to be used. I just never added those getters because I didn't use them anywhere back then.
Replying to Disreali:
Should the gamepad related stuff go on ticket:4499 or possible a separate enhancement ticket?
I don't really mind either way. Both tablet and gamepad will need some common changes and there will be a device class driver for each. Shouldn't be a huge number of patches in the end, so I think we're fine consolidating both in this ticket.
Replying to lt_henry:
I've updated usb hid patch. This version is cleaner, there are no unnecessary trace outputs, and there are some improvements regarding haiku code guideline. Still far from perfect, of course. Next day I will update input server side.
Looks better already, but there are still serious coding style issues. The code itself looks fine as far as I see from looking over it. One thing I've noticed though: You extract the tip and barrel switch and store them in bit 0 and 1 of buttons, but then loop through the generic buttons and add them as well with "(button->UsageID() - 1)" which would overwrite the tip and barrel again if there are any buttons (as their usage is 1 based resulting in button 0 going to bit 0 as well). So I wonder, is that just an oversight or is the whole generic button handling just a leftover from the mouse driver? If so it should just be removed completely.
comment:12 by , 14 years ago
Like I said in an earlier comment, the tip and barrel handling does not work as intended by Be. It needs to end up as an independent boolean value in the tablet_info structure, and on the input_server side, it needs to be added as a boolean to the BMessage. Then it will work in apps like WonderBrush which follow the BeBook documentation.
comment:13 by , 14 years ago
Replying to mmlr:
Replying to caz_haiku:
After your modifications the collection now has the correct usagepage and usageid
Good, so the other change you had in there first wasn't needed apparently and no need for any device workarounds.
I want access to the UsagePage and UsageId in the collection so to check the device type, is this acceptable?
if(collection->UsagePage() != B_HID_USAGE_PAGE_GENERIC_DESKTOP || collection->UsageID() != B_HID_UID_GD_GAMEPAD) return NULL;Yes that's perfectly fine and how it was intended to be used. I just never added those getters because I didn't use them anywhere back then.
mmlr, before your modification the collection only had the UsageID, and the UsagePage wasn't included. Your mods now do basically what i was mumbling about :-). I think that for an XBOX 360 a seperate driver would be needed as the report descriptors are non comforming, alternatively the GamepadDevice would need access to the vendor id etc.
comment:14 by , 14 years ago
I have the following Waltop chipset based tablet. I Started a ticket to support this device some time ago. My next question is are there any utilitys with which to watch the input server? the tablet now seems to initialize and no longer cuases the mouse to blast off to the right lower corner of the screen, it however does not appear to do anything in regards to mouse movement.
I was assuming this had gotten fiarly far along and I did apply all the patchs.
172f:0034 /dev/bus/usb/3/1 "WALTOP International Corp." "Slim Tablet" ver. 1105
from listusb. I'd certainly like to help any testing etc that may come up for this ticket. I got pretty geeked about the idea of trying Wonderbrush with a tablet.
comment:15 by , 14 years ago
Blocking: | 6821 added |
---|
(In #6821) Replying to stargatefan:
Go ahead and close this ticket. Its covered by 7354. Some progress made there. No need for a redundant ticket.
Closing as per that comment.
comment:16 by , 14 years ago
Status: | new → in-progress |
---|
I've started a restructuring of the usb_hid internals to accommodate for the different enumeration (as described above). I'll probably have something ready and committed in the next few days.
comment:17 by , 13 years ago
Joystick/gamepad support is added in hrev41851. Tablet support will be added next.
follow-up: 19 comment:18 by , 13 years ago
Nice work :) My wingman now works as espekted (I know you know abut the hat thing ;) )
My Logitech GamePad don't work. What information do you need? By now working stick it won't move anything but shows a device. listusb gives me this. What else do you need?
046d:c21a /dev/bus/usb/1/0/1 "Logitech" "Logitech(R) Precision(TM) Gamepad" ver. 0004 8087:0020 /dev/bus/usb/1/0/hub "" "" ver. 0000 0000:0000 /dev/bus/usb/1/hub "HAIKU Inc." "EHCI RootHub" ver. 0200
comment:19 by , 13 years ago
Replying to modeenf:
Nice work :) My wingman now works as espekted (I know you know abut the hat thing ;) )
My Logitech GamePad don't work. What information do you need?
Please attach the report descriptors written to /tmp (/boot/common/cache/tmp). Attach the descriptors of both devices so I can take a look.
by , 13 years ago
Attachment: | usb_hid_report_descriptor.zip added |
---|
comment:20 by , 13 years ago
Interesting. If anything I would've expected the WingMan not to work (as it has a more complex structure than the pad). The pad is really about as simple as it gets, so I wouldn't see why it'd not work. Can you verify that the device actually works (using another OS for example)? Also by "doesn't work" you mean the whole device doesn't show up at all, doesn't trigger any action or only the axes don't work? Be as specific as you can.
comment:21 by , 13 years ago
I have the Gamepad working in windows7, snes9x. I also thought that it was broken but it works in windows 7. As you can se the Gamepad are found but what's not working are the buttons and pad. It's like no information are catch. But I has as resent as yesturday playing with it in windows. I know when i was working on USB things it was reported as another device not a gamedevice (it's more than 3 yers ago so I don't recal that much)
follow-up: 24 comment:22 by , 13 years ago
Hey modeenf and mmlr, I did a search on google and found this page http://www.everythingusb.com/forums/showthread.php?threadid=13443 it mentions that the Logitech Precision Gamepad does not work if the polling rate is above 125hz, whether this is the reason your Gamepad doesn't work who knows.
by , 13 years ago
Attachment: | usb_joystick_test.zip added |
---|
This can be used to find more info about a gamepad descriptor
comment:23 by , 13 years ago
I hacked the usb_joystick_test a while ago, so the code isn't great. To use the app type BeApp usb_hid_report_descriptor_046d_c21a_0.bin there is a makefile in there too if you have to rebuild and the binary will be in objects.x86-gccX-release folder.
comment:24 by , 13 years ago
Replying to caz_haiku:
Hey modeenf and mmlr, I did a search on google and found this page http://www.everythingusb.com/forums/showthread.php?threadid=13443 it mentions that the Logitech Precision Gamepad does not work if the polling rate is above 125hz, whether this is the reason your Gamepad doesn't work who knows.
Interesting. It's possible that the interrupt pipe is polled at a higher rate since it is connected to the EHCI root hub (probably via an internal transaction translator). The output of listusb -v should tell what poll rate the interrupt pipe specifies. Since we do not support Frame Span Traversal Nodes we always interact with low/full speed interrupt pipes at a 1 micro-frame level, which might just push the resulting full/low speed polling frequency high enough for this device to become problematic. Attaching this to a root port without a transaction translator (so that UHCI/OHCI get hold of the device) should work in that case.
That problem doesn't really belong into this ticket anymore though. Can you please open a new ticket modeenf? Attach the full listusb -v output and preferrably a syslog as well.
Replying to caz_haiku:
I hacked the usb_joystick_test a while ago, so the code isn't great.
Why not just build on the HID framework? Take a look at hid_decode browser:haiku/trunk/src/bin/hid_decode which uses it to generate a generic dump of descriptors.
On the original topic of this ticket: I've implemented generic support for absolute pointing devices through the tablet input_server device add-on (added to the image in hrev41950) and additions to the MouseProtocolHandler in hrev41951. I'll add the digitizer specific parts in a next step which should then complete this ticket.
comment:26 by , 13 years ago
Awesome progress on the tablet support!
I would like to point out that the Wacom tablet driver should be tossed out once the HID based support is ready. And of course if my assumption is correct that all Wacom tablets that the driver supports are indeed HID devices. Since the wacom kernel driver does the exact same initialization to put the tablets into absolute mode regardless of model, I would believe that to be the case, though.
The only thing that the Wacom add-on has over the tablet_device add-on is the filtering of the movement. A hand holding the pen is just too jittery for the precision of these devices. There needs to be some kind of low-pass filter applied. Ideally, if the user is holding the pen "still" hovering the tablet, there would be no movement messages generated and the pointer should not move on the screen. This obviously depends on the size of the tablet as well, which is something BTW, which the Wacom driver ignores completely. It only work well for the Intuos 2 A5 size. Since it probably also depends on the human holding the pen, it needs to be a setting in a preflet anyway. When the pressure changes (i.e. pen is down), it is important to generate messages, even when the movement filter keeps the position locked for the jitter range.
comment:27 by , 13 years ago
Can we close this ticket? I see absolute coordinates devices support was added in hrev41951 to usb_hid.
comment:28 by , 12 years ago
There's no support to Wacom Intuos 4 (PTK440) graphic tablet on Haiku R1 Alpha 4.1 yet. Below the messages from an Acer Aspire One D250-1080:
listusb 0000:0000 /dev/bus/usb/0/hub "HAIKU Inc." "UHCI RootHub" ver. 0110 056a:00b8 /dev/bus/usb/1/0 "Wacom Co., Ltd" "Intuos4 4x6" ver. 0104 0000:0000 /dev/bus/usb/1/hub "HAIKU Inc." "UHCI RootHub" ver. 0110 0000:0000 /dev/bus/usb/2/hub "HAIKU Inc." "UHCI RootHub" ver. 0110 0000:0000 /dev/bus/usb/3/hub "HAIKU Inc." "UHCI RootHub" ver. 0110 0c45:62c0 /dev/bus/usb/4/1 "Microdia" "Sonix USB 2.0 Camera" ver. 0100 0000:0000 /dev/bus/usb/4/hub "HAIKU Inc." "EHCI RootHub" ver. 0200
open /var/log/syslog (...) KERN: usb hub 22: port 2: new device connected KERN: usb ehci -1: fullspeed device connected, giving up port ownership KERN: usb hub 7: port 0: new device connected KERN: usb hub 22: port 2: device removed KERN: wacom: add_device() - wacom detected KERN: wacom: ... success! KERN: wacom: device_open() open: 2
The device is "seen" by the system, but nothing happens.
comment:29 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | in-progress → closed |
The parts of the patch that are useful have been included in hrev46015. What was added by mmlr in hrev41951 only implemented support for absolute X and Y positioning and no other tablet features such as pressure sensitivity. I've included theoretical support for that and even tilt, but cannot test it due to lack of hardware. The HID driver explicitly does not work with Wacom devices. Tablets following the HID spec should work, but regardless, Wacom devices are ignored by this driver. The Wacom USB driver claims ownership of any Wacom tablets attached, but the Wacom input_server add-on has support for a hard-coded number of devices only (explaining comment:28).
Please re-open if the driver does not work where you think it should work. Or if it breaks QEMU absolute mouse positioning.
input server addon