MiSTer-devel / Linux-Kernel_MiSTer

Other
12 stars 17 forks source link

Linux merges dual HID joysticks over Bluetooth (BLE). #8

Closed bootsector closed 2 years ago

bootsector commented 2 years ago

I suspect this is related to how the Linux kernel and/or Bluez handle HID over GATT:

https://github.com/bootsector/ControllaBLE/issues/2

The kernel quirks for the VID/PID used by ControllaBLE seem to be ignored when used through HoG/BLE and are not applied - basically we get two virtual pads merged as one, wth second player buttons getting stripped off.

If possible, I would love to see ControllaBLE fully supported on MiSTer (currently player 2 doesn't work).

sorgelig commented 2 years ago

Well, it's you made that device, so you need to debug and find where is exactly problem. Probably you can monitor BT packets, or hidraw, or something between them. I can help with specific things. General report "it doesn't work" doesn't help. I don't have your device.

I suggest to study HID/HoG/BLE protocol to design device properly, so it will represent independent devices not requiring quirks. For example Arduino project for multiple controllers -> USB adapter designed correctly and kernel driver doesn't try to merge them. While many Chinese manufacturers design their adapters so driver cannot distinguish it as multiple devices. So it depends on device firmware.

sorgelig commented 2 years ago

https://github.com/MiSTer-devel/Retro-Controllers-USB-MiSTer/blob/master/PaddleTwoControllersUSB/PaddleTwoControllersUSB.ino It uses PluggableUSB library to insert 2 HID devices. May be it can be translated to HID library you use.

Speaking in general, BLE module should be per device, so it can be integrated inside the case of controller. Otherwise you suggest to use wires between players which ruins the original idea. For wired connection there is Arduino project - it works well.

bootsector commented 2 years ago

Hey, thanks for replying!

Yeah, I've experience with USB devices, and this HID report is the very same one I've been using in another USB based dual joystick adapter. Problem seems to be somewhere between bluez and the kernel HID drivers where quirks are being lost and not getting applied.

As per the VID/PID I'm using, this should be the HID driver that should kick-in:

https://github.com/MiSTer-devel/Linux-Kernel_MiSTer/blob/80f1520f282e08dbeb8c811d23e7d16f483fe6af/drivers/hid/hid-pl.c#L183

However, it looks like a generic HID driver is taking place and not applying the quirks defined here:

https://github.com/MiSTer-devel/Linux-Kernel_MiSTer/blob/80f1520f282e08dbeb8c811d23e7d16f483fe6af/drivers/hid/hid-quirks.c#L139

I will keep looking, but any help is appreciated.

bootsector commented 2 years ago

In time, although it's off-topic: windows 10 sees the BLE device as a dual joystick adapter, just like the HID descriptor suggests. And it works correctly! πŸ€”

sorgelig commented 2 years ago

Windows developers may have their own views. Probably Windows fails in other situations where these 2 virtual joysticks should be a single device. If hid descriptor would be definitive reason to split devices, then i believe Linux would do it without creating so long quirk list. I believe there is a proper rule to describe 2 separate devices. I think by driver rules single descriptor should be handled by single driver which should properly handle concurrent access to those virtual devices. Input read-only device is just a special case when you don't need to write, so logically it can be 2 or more "independent" devices. But it's just a special case. If you need to write to device, then it won't work. I guess quirked devices have limited access through hidraw interface - it's obviously cannot be created per virtual device from single descriptor. With separate descriptor per virtual device for example in USB a separate end point pipe is used, so such device can use 2 independent drivers reading/writing at the same time (using lower level arbiter).

For BLE devices from my observation, linux uses uhid (user space hid?). You can compare classic BT device sysfs path, and any BLE devices sysfs path.

I believe the difference (from Ubuntu) lays somewhere in higher (HID) device layer somewhere in kernel. You can try even older MiSTer kernel 4.19 - it should work with current linux image at some extents (as long as your BT dongle is supported)

sorgelig commented 2 years ago

However, it looks like a generic HID driver is taking place and not applying the quirks defined here:

Using some generic/hijacked VID:PID may trigger wrong(original vendor) driver which you don't expect. List of supported devices vary from kernel to kernel. So 5.x may include new driver using your VID:PID.

bootsector commented 2 years ago

I was reading this documentation while I was writing the firmware for the dual BLE adapter:

https://github.com/jpbrucker/BLE_HID/blob/master/doc/HIDService.md#ble-hid-service

It says:

Report Map: the HID Report descriptor, defining the possible format for Input/Output/Feature reports.
Report: a characteristic used as a vehicle for HID reports. Unlike USB, where the ID is sent as a prefix when there is more than one report per type, the ID is stored in a characteristic descriptor. This means that there will be one characteristic per report described in the Report Map.

My Report Map indeed defines the format for both report ids:

Report ID 1: https://github.com/bootsector/ControllaBLE/blob/faa2bfd1b9ab9be0d74851c2a8a817769df19922/src/hid_report.h#L25

Report ID 2: https://github.com/bootsector/ControllaBLE/blob/faa2bfd1b9ab9be0d74851c2a8a817769df19922/src/hid_report.h#L74

In a regular USB device, I would send both HID reports through, say, endpoint 1 (IN) by prefixing the ReportID number with the payload, as seen here:

https://i.imgur.com/PbrDvN8.png

However, the above documentation says:

Unlike USB, where the ID is sent as a prefix when there is more than one report per type, the ID is stored in a characteristic descriptor. This means that there will be one characteristic per report described in the Report Map.

So, unlike USB, and observing the BLE specs above, I don't send the report id as the prefix:

https://github.com/bootsector/ControllaBLE/blob/faa2bfd1b9ab9be0d74851c2a8a817769df19922/src/main.cpp#L39

and I just separate every report into their separate input characteristics as pointed:

https://github.com/bootsector/ControllaBLE/blob/faa2bfd1b9ab9be0d74851c2a8a817769df19922/src/main.cpp#L114

The results, on Linux, if I use the current VID:PID or any other is exactly the same.

So it really looks like I'm adhering to the standards, but there's something at Linux side that doesn't behave like Windows does.

Anyways, I think I might just follow your idea and make this a single player adapter. The ESP32 module is pretty cheap anyways. But it's a shame that so many spare GPIOs won't get used in this case!

Cheers and thanks for all the input you gave me!

sorgelig commented 2 years ago

You gave up too fast, i think :) At least you could find out why MiSTer behaves differently than Ubuntu, so at least reserve quirk way.

Actually i'm thinking about such adapter (single version). I have several unique wired gamepads which i would like to make wireless. I think it will be enough space to place esp32 inside, but i have no idea about battery. It should be some battery booster with charging support i guess.

bootsector commented 2 years ago

LOL! I gave up, for now. I'm just too tired to keep researching about this for this weekend, but I will surely revisit this later.

I've made a single version firmware on this branch for latency tests purposes: https://github.com/bootsector/ControllaBLE/tree/singlejoy

Latency results are pretty much the same if compared to the 2P version. Feel free to use any of this code if you think it's worth it. I've put some effort to make it very clean and concise and this is a very good example about how to use NimBLE-Arduino's NimBLEHIDDevice class (the official docs don't have any examples for it!).

This is the prototype I've put in place:

https://i.imgur.com/HusRtGu.jpg

I'm currently using an USB power bank, so not very portable yet! πŸ˜„

bootsector commented 2 years ago

So I've just confirmed that Linux Kernel's USB HID system relies on the HID_QUIRK_MULTI_INPUT quirk for implementing dual joystick/input devices.

Anyone can confirm this by flashing the attached STM32F103 firmwares (instructions and pin mapping inside the zip) onto a cheap Blue Pill dev board. These firmwares, that I've created, implement a clone of the Pantherlord dual device, which uses standard HID report descriptor with multiple report ids. This is the same HID report descriptor I'm using on ControllaBLE. The official Pantherlord firmware/device uses the 0x0810:0x0001 VID:PID pair, which forces Linux to load the "pantherlord" HID driver which implements the HID_QUIRK_MULTI_INPUT quirk:

[ 3435.879706] usb 1-1.1: new full-speed USB device number 8 using dwc2
[ 3435.969967] usb 1-1.1: New USB device found, idVendor=0810, idProduct=0001, bcdDevice= 1.06
[ 3435.969987] usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[ 3435.970007] usb 1-1.1: Product: Twin USB Joystick 
[ 3435.970016] usb 1-1.1: Manufacturer: Pantherlord  
[ 3435.976513] HID usage: 0x00010004, original interval: 4
[ 3435.976538] JS: endpoint->bInterval=4, interval=4
[ 3435.976545] HID usage: 0x00010004, applied interval: 4
[ 3435.976854] input: Pantherlord   Twin USB Joystick  as /devices/platform/soc/ffb40000.usb/usb1/1-1/1-1.1/1-1.1:1.0/0003:0810:0001.0005/input/input7
[ 3435.979284] input: Pantherlord   Twin USB Joystick  as /devices/platform/soc/ffb40000.usb/usb1/1-1/1-1.1/1-1.1:1.0/0003:0810:0001.0005/input/input8
[ 3435.981248] **pantherlord** 0003:0810:0001.0005: input,hidraw0: USB HID v1.10 Joystick [Pantherlord   Twin USB Joystick ] on usb-ffb40000.usb-1.1/input0

As you can see, two different inputs/joypads were created.

If I flash the STM32 with the STM32_non-pantherlord.bin, which uses a different VID, Linux's pantherlord driver won't be loaded, hence, not applying the HID_QUIRK_MULTI_INPUT quirk:

[ 3212.135958] usb 1-1.1: new full-speed USB device number 7 using dwc2
[ 3212.226213] usb 1-1.1: New USB device found, idVendor=0820, idProduct=0001, bcdDevice= 1.06
[ 3212.226236] usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[ 3212.226255] usb 1-1.1: Product: Twin USB Joystick 
[ 3212.226264] usb 1-1.1: Manufacturer: Pantherlord  
[ 3212.233664] HID usage: 0x00010004, original interval: 4
[ 3212.233689] JS: endpoint->bInterval=4, interval=4
[ 3212.233697] HID usage: 0x00010004, applied interval: 4
[ 3212.234029] input: Pantherlord   Twin USB Joystick  as /devices/platform/soc/ffb40000.usb/usb1/1-1/1-1.1/1-1.1:1.0/0003:0820:0001.0004/input/input6
[ 3212.236751] **hid-generic** 0003:0820:0001.0004: input,hidraw0: USB HID v1.10 Joystick [Pantherlord   Twin USB Joystick ] on usb-ffb40000.usb-1.1/input0

As you can see, the hid-generic kernel driver was used, which doesn't know anything about creating multiple inputs/virtual joysticks. Hence you get a single device created.

Going back to the ControllaBLE device, as we can see, hid-generic is also being used by the kernel, which doesn't know anything about creating multiple input/joypads:

[   19.121087] input: ControllaBLE as /devices/virtual/misc/uhid/0005:0810:0001.0001/input/input1
[   19.124316] **hid-generic** 0005:0810:0001.0001: input,hidraw0: BLUETOOTH HID v10.02 Joystick [ControllaBLE] on 00:1a:7d:da:71:11

I wonder if there's a way to force a BT HID device to use a specific HID driver instead of hid-generic would just do it? This is the Pantherlord dual joystick device driver: https://github.com/MiSTer-devel/Linux-Kernel_MiSTer/blob/38ff362a59c80f3d9036c91f39677f633e8f6fe3/drivers/hid/hid-pl.c#L216

I wonder if simply changing a line from the kernel at https://github.com/MiSTer-devel/Linux-Kernel_MiSTer/blob/38ff362a59c80f3d9036c91f39677f633e8f6fe3/drivers/hid/hid-quirks.c#L139 and modify from "{ HID_USB_DEVICE(USB_VENDOR_ID_PANTHERLORD,..." to "{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_PANTHERLORD,..." would force a BT HID device to use a given kernel HID driver?

If that's the case, and it actually works, we could just copy the pantherlord hid driver and correlate it with a different VID:PID that could then be used by ControllaBLE in order to get properly broken up into a dual input device?

STM32F103_Pantherlord_Dual_Stick_clone.zip

bootsector commented 2 years ago

To make things worse, I've just discovered this device implements the same VID:PID as Pantherlord :man_facepalming:

https://github.com/MiSTer-devel/Linux-Kernel_MiSTer/blob/38ff362a59c80f3d9036c91f39677f633e8f6fe3/drivers/hid/hid-quirks.c#L551

We should just probably try to clone the hid-pl.c driver so it kicks in for another VID:PID and add it as a HID_BLUETOOTH_DEVICE inside hid-quirks.c and move away from this Pantherlord VID:PID hacking inside the kernel.

bootsector commented 2 years ago

Tried to make some changes to the kernel (patch file attached) by following the steps here:

https://github.com/MiSTer-devel/Main_MiSTer/wiki/Compiling-the-Linux-kernel-for-MiSTer

Everything went fine, but after kernel is replaced, MiSTer boots but both WiFI and BT are disabled yet the devices show up in lsusb.

@sorgelig any clues?

Cheers! ControllaBLE.patch.zip

sorgelig commented 2 years ago

Because you need to compile modules and then put them into /lib/modules/.... Since you've used different gcc version, modules may become incompatible.

here is the kernel with your patch should be compatible with existing modules: zImage_dtb.zip

bootsector commented 2 years ago

Thanks, @sorgelig!

Yes, we've got some progress:

[   19.730979] input: ControllaBLE as /devices/virtual/misc/uhid/0005:1209:FACA.0001/input/input1
[   19.735082] input: ControllaBLE as /devices/virtual/misc/uhid/0005:1209:FACA.0001/input/input2
[   19.741153] pantherlord 0005:1209:FACA.0001: input,hidraw0: BLUETOOTH HID v10.02 Joystick [ControllaBLE] on 00:1a:7d:da:71:11

I've got two joystick devices created on /dev/input: js0 and js1 and they both report separately with jstest, which is the expected behavior!

However, on the MiSTer side, we get both controllers working as player 1! πŸ€”

I think we are in the right track!

We just need to figure out how to report those two separate js devices to MiSTer now! πŸ˜„

sorgelig commented 2 years ago

try this Main: MiSTer.zip

bootsector commented 2 years ago

Holy shit! You've nailed it! 2P support over BT is working perfectly on ControllaBLE now! πŸŽ‰

sorgelig commented 2 years ago

I still think that BT mod should be per device. Otherwise it's like bluetooth earphones with wires :)

bootsector commented 2 years ago

Wired BT headphones are still useful! 😁

Thanks for looking at this @sorgelig!

Do you want me to open a PR for the Kernel portion or do you have it covered already?

Thanks a lot!

sorgelig commented 2 years ago

since i've already applied it, i will simply commit it and push then

sorgelig commented 2 years ago

I think if you will use the same VID:PID for different controllers then button map will be messed. Probably it will require distinguish then by name like arduino/teensy projects.

bootsector commented 2 years ago

That’s easily circumvented with a per core mapping. People tend to use a specific pad with a specific core, so that should do it, until we figure something out.

sorgelig commented 2 years ago

correction: arduino/teensy use unique field to differentiate the projects. For BLE this field is used for MAC. also.. what is pantherlord? Is it some random input device you mimic? Why not generic hid?

bootsector commented 2 years ago

Generic hid knows nothing about multi input devices. I’ve cloned a cheap dual ps2 controller adapter to USB and, on Linux, they use the pantherlord driver, which does the dual input magic.