-
Notifications
You must be signed in to change notification settings - Fork 618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HOG - ATT DB / hids_device_t #483
Comments
Hi Benjamin. Nice you're still interested. While trying to avoid complexity, we've limited HID over GATT a bit, that's correct. :) Haven't used HID in a while. If I remember correctly, you can add as many usages into a single report. Is this not possible for LE? Assuming you need more than one report per type:
In the first case, we had the vague idea to provide params to included gatt service definitions. Cheers |
Hi! Using just one report with
And finally some demo to show off its actions:
I feel sending separate reports for keyboard/mouse (and whatever input device) would be more natural for USB HID standard, although it works for me to USBfy and BLEfy Lenovo G505 keyboard with Pico W. PS. Last (6th) keyboard keycode is changed to be multimedia key (consume control) here. |
Dear @mringwal, dear @matsobdev , regarding the single report, identified by the report ID: normally you are right, what else is the report id used for :-). @matsobdev I'm sure your report map works on an Android device, these are the most compatible ones (similar to Linux). If you want to use iOS (mandatory for us) and Windows, there is a BIG difference on how the report map is parsed. ad PS: I'm really not sure how this works on all platforms, but I like your creative solution! THX for the hint, it would be option 1, we know the number of reports at compile time. Greetings, |
Hi @benjaminaigner To use two or more reports per type, we'd need to a) extend the database, as well as make the service implementation more flexible. For the GATT Compiler, I can image adding a mechanism to repeat one or more lines in the .gatt file controlled by a (global-)variable or even parameters to the #include call. We'll have some internal discussion and get back. |
I just compared mouse and keyboard hog examples and was optimistic (possibly prematurely) when saw:
in |
Dear @matsobdev , this is a special mode for using the BLE mouse&keyboard before the OS is loaded. AFAIK it is a basic mode, where the input report has a specific format. No HID report descriptor is used in this case. Personally, I've never saw any device using the boot mode. All major OS use the report mode (in your |
I was aware of that mode and wasn't sure but had an idea how it would work. Was too optimistic and missed that boot word, thanks for clarification. |
Victory!
And
And test code:
It is Android tho, but wasn't working before, so progress. Sorry, too fast, only one is working... 0x01 rep id not. |
And which one is working?
Nice, thanks for the hint, so an adaption of the .gatt file is necessary. |
BTW: I'm working with the RaspberryPi Pico W with Earle Philhower's arduino-core: https://github.com/earlephilhower/arduino-pico |
Yes, I'm on Pico W as well. I'm not gonna call it a success, but successfully used hardcoded output report (not boot) inside
So output report is now input
This time with proper combo descriptor:
I was searching throughout BTStack, maybe And |
We did a first analysis of the requirements and sketched out changes to the gatt compiler and hids_device.c. Did we miss anything here? We'll try to make the changes to the hid_device.c first as you can manually copy & paste & modify the current hids_device.gatt file and adapt it to your needs. |
That sounds exactly like what we need 👍
I don't think so.
Thank you very much, I think as well that the hid_device changes are important, the GATT file can be adapted manually. |
@matsobdev Thank you very much for the intensive experimenting! @mringwal Do you know how long it would take from your implementation to be included in the pico-sdk :-)? |
Couldn't it be the way it is now, like adding an extra input
this:
works as expected. |
This will take a while as they currently prepare their 1.5.1 what uses our latest v1.5.6.2 release. |
@benjaminaigner surprise! We've pushed a first update that supports multiple reports per type on the develop branch. |
Wow, thank you very much! I'll test it and report back (unfortunately so many other things to do, so it will be next week) |
It is a side question, but still HOG. Should in, out and feature (out and feature to be exact) had different properties? Like here https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=245140, table 2.1 for example feature:
without |
@matsobdev good catch! yes, they are different. I've updated the hids.gatt file on develop to match the spec. |
Noticed changes, but still output has extra not permitted Tricky matter. Chapter 2.2 of the same spec. document says:
and in
Proper keyboard will have output for LEDs, might have feature for configuration and input no doubt about it. Now, for combo devices everything will be times 2, 3 or more. It is one device, so one channel for configuration will be more the enough I guess. With keyboard-mice combo will be unused output, since there is no output in mice (standard descriptor, some custom might have one, but now there will be no way to get rid of that unused output for standard mice for example). Multiple input reports tied with descriptor are that For example, here: https://github.com/TexasInstruments/HOGP-BLE-HID-EXAMPLE/blob/main/source/ti/ble5stack/profiles/hid/hidkbdservice.h, they have:
Concluding, shouldn't flexibility be like adding one report characteristic at a time (instead of a full set) that is of a customisable type? PS.: Again table 2.1. Now examples for keyboard and mice shares the same |
Hi @matsobdev. Ah, only Input reports can notify. Noted and fixed. We then should also drop the send report for Feature and Output reports. It also looks like our implementation doesn't provide a way to get/set Input / Output reports either. I'll make a note about it. BTstack avoids dynamic memory, so we assume that an app knows which reports it needs. With the hids_device_init_with_storage() you can specify how many reports you'd like to have and need to provide a suitable .gatt file for that. You're free to select any combination or reports, incl. e.g. no output reports. You're correct that in boot mode for e.g. a keyboard, there should not be a mouse input report. With our architecture, this would require custom .gatt files. With the planned extension of the gatt compiler, we could model that. |
To be honest I was looking for something to |
Yupp. HID Device uses a GATT Service and the only way to send something is by sending a Notification. |
Thank you very much for your work on this issue! THX |
If tried to work with a fixed .gatt setup right now, simply because I'll try to build it dynamically when the other stuff is working :-).
When using this ATT DB, it is connecting and Wireshark shows properly the characteristics declaration. Do you have any hint for me? |
Hi @benjaminaigner In your .gatt file, the report ids are not unique, the first two use the same. While I don't know if these only need to be unique per report type, our code currently assumes that they are unique. If a ATT Request is not answered by a GATT Server, the transaction failed and the connection might get closed as it cannot be used anymore. The other thing, as mentioned in reply to @matsobdev, is that there's currently no support for Feature or Output write. Could you attache a .pklg file here? |
I'm using the same report ID for keyboard in & out as well as 0 for the feature report. This works with all tested OS. Should I remove the out & feature reports? The host sends an ATT read request to handle 0x001f, which is the first report characteristics handle. Here is a Wireshark capture of the traffic: |
Thanks. Ok, another look into the spec won't hurt here - although the USB HID Spec might have the final word. Two things here: If you like, please run your test code on a desktop and try to find out why BTstack doesn't send anything back. |
ad a) maybe use something similar to I could try to setup a Desktop test, but currently I'm working directly with the PicoW (and the arduino-pico core), so I need to boil it down to a HOG composite example for working without Arduino. |
GATT Server has a way to postpone a Read/Write operation, but I don't like it/would prefer to avoid it. Test code that's close to our examples makes debugging much easier. |
Dear @mringwal , I think I found a bug in In both functions, the Adding following at the end of the while loop:
Fixes the hangup and the device connects (still no success with composite devices :-)) |
Hi again @mringwal, I'm not sure, but I think I found another problem within Is it possible you need instead of
? Simply because:
|
Hi @benjaminaigner Thanks for pointing out the bugs. I've taken care of these and also cleaned up the API. Could you check the current version? I think sending input reports should be complete now. What's missing is: The current idea would be to emit events for a) and allow the application to set the data for each {report id/type} that is returned by GetReport. This would allow the app also to validate data from the HID Host, e.g. the host sets invalid data, but won't read it back as the app can ignore or sanitize it. |
Hi! Again with the same what's above:
Output and feature with the same PS. But still, a far I'm concerned for example pairs input/output, output/feature still shall have separate characteristics - they have different mandatory and excluded properties - table 2.4 of https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=245140 |
THX for the fixes, the lookup is working, the ATT DB is created in the correct location. One thing is still not working: hids_device.c:400: Replacing it with this one works:
With this fix, the mouse moves with a combined example, but I've still problems with keyboard & joystick. I've changed the report ID assignment for the BTStack example, but it is definitely working on the Bluedroid stack (ESP32). |
@benjaminaigner thanks for the feedback. What's your current .gatt file that causes the issue with the handle lookup? |
Dear @mringwal I used this gatt file, mouse is working with this one when applying my fix:
EDIT: I'm not quite sure how to implement the LED output report, because in my working HID report map, the output is integrated with the keyboard application and has no unique report ID (there is only one output report, therefore no ID necessary). This may mess up the keyboard interpretation. EDIT2/Update: The keyboard keys work as well, still missing or to be tested is the consumer report & the joystick. |
@benjaminaigner A generic keyboard HID descriptor uses a single report for the keys and the LEDs. In HOG, these results in two report characteristics with the same report id. As mentioned before, the read/write operations are not yet implemented, so LEDs cannot work. We'll get there, but I'll look at the reported issue with the handle lookup next/soon-ish. :) |
No hurry, LEDs are the least priority for us as well :-) |
Everything is working, PR: Thanks for your help, I think I can close this issue now. |
Thanks again for pointing out that the logic doesn't work for reports without notify. It's fixed on the develop branch. |
Perfect, THX! |
Dear @mringwal , thx for your help on this issue, I've now completed the implementation of the easy Arduino libraries (Mouse Keyboard Joystick) using BTStack on the RaspberryPi PicoW. There is one additional note regarding license/sourcecode within Earle Philhowers arduino-pico core: Because the official pico-sdk will take some time to implement the new functionality and Earle doesn't want to fiddle around with the upstream pico-sdk, we handled it by copying the att_db & hid_device source code files into the sdkoverride folder. And of course, there is no copyright removal or anything else, hope this is OK for you. Greetings |
@benjaminaigner So far I haven't used ESP32 and Pico W with Arduino. Where are these files in the arduino-pico repo? |
The files are here (currently an open PR): I just changed the guard:
(otherwise the header would be compiled, even if the BT stack is disabled in the menu). Greetings |
Thanks for the link. Ok, looks good. Please try to remove the duplicate files after the next Pico SDK release. |
Dear Matthias,
my last contact with BTStack is a long time ago, but I'm glad the RaspberryPi Foundation is using BTStack for the PicoW!
I would like to implement a flexible way to use either combination of Mouse, Keyboard and Joystick in Bluetooth as it is working
with USB-HID (TinyUSB), see earlephilhower/arduino-pico#1506 .
I've done this before with the ESP32 (https://github.com/asterics/esp32_mouse_keyboard), which was of course no fun, but it is working. The problem is: with Bluedroid one is much more flexible with the creation of the ATT DB, I'm currently thinking on how to do this with BTStack, without re-implementing most of the HID stuff.
Is your feature request related to a problem? Please describe.
Although HID would allow composite devices (e.g. keyboard+mouse+joystick),
BTStack is using a fixed ATT setup with 1 IN, 1 OUT & 1 FEATURE entry.
Describe the solution you'd like
Some flexibility what parts are in the hids_device struct and the possibility to get/set a handle to the
hids_device_send_input_report
function (at least more than 1 input entry)Describe alternatives you've considered
Maybe a flexible list of input,output&feature entries, including the handles & values.
Additional context
Thank you very much,
greetings,
Benjamin
The text was updated successfully, but these errors were encountered: