USBHID update rate is really slow

[AlessandroAU]

Coming from an OpenTX platform I found the USBHID to be unflyable. Feels like the stick update rate is really slow. I filmed some slow-mo footage on a 100hz monitor of me spinning a stick in a circle on both a QX7 and a T8SG to illustrate the difference.
QX7:

Devo:

[eried]

It is noticeable without slow footage? I do not have another controller to test but it seems smooth on my t8sg v2 with devo

[AlessandroAU]

Yes very noticeable in velocidrone, you can even hear the difference. It's what lead me to investigate.

[AlessandroAU]

Sharp turns in the sim sounds like D-term oscillations on a real quad and swift stick deflections are really jerky. Obviously hard to convey in a video but here it is anyway. First half is T8SG v2 (with hall gimabls) and second half is QX7. https://www.youtube.com/watch?v=oLlId8_gTKM&feature=youtu.be

[eried]

Oh I see. Can you record with slow motion the stick monitor screen and the physical stick moving together to full left and then full right? For both controllers? To calculate the lag with an objective target. I can take a look on the USBhid code

[eried]

deviation-t8sg_v2_plus-v5.0.0-32d5597.zip

Can you test this one in the video? @AlessandroAU

[AlessandroAU]

Sorry no video yet. Latency feels a bit better (I might just be getting used to the controller) but update rate feels the same.

[eried]

Can you test this version now:
deviation-t8sg_v2_plus-v5.0.0-32d5597.zip

I did some checkings against a normal game controller, and the previous version was quite slow. However this one is on par with the commercial controller:

If you can do the video on slow motion to confirm I will push this change to the repo.

I am not sure what would be the minimum latency vs the best performance, but I do not really use my controller for games so I never noticed this issue.

[somewhatlurker]

Hi there @eried, it seems you did indeed find the issue if my understanding is correct. (based on the branch you created)

Assuming that the usbhid_cb return value is in microseconds, the original rate was one update per 50ms (only 20Hz).
This is indeed low enough to cause visual jerkiness to gamers and decreasing it should be an improvement in every way.

The standard minimum refresh rate for gaming has been 60Hz (~16.7ms) for a long time, but even most non-gaming input devices update at 125Hz (8ms). Gaming devices typically also feature 250Hz (4ms), 500Hz (2ms), or even 1000Hz (1ms) rates.

I think targeting 125Hz (8ms) or 250Hz (4ms) updates would satisfy most if not all users, provided the transmitter hardware can handle it. Those rates seem to be similar to what most real RC protocols run at so should provide a decent experience.

[somewhatlurker]

bInterval in the HID endpoint descriptor (hid_endpoint in src/target/drivers/usb/devo_hid.c) may also need changing to ensure proper operation.

For low/full speed USB devices, this should be set to the desired update/polling interval in milliseconds. For high speed USB 2 devices, it should be set as the number of 125 microsecond frames.

[eried]

I did not get the second change but I will take a look on those files home. I have not made a PR with this because "if is not broken..." type of thought, and it might change the experience of the joystick for people that actually use the USBHID (i.e.: "it is too sensitive now!")

[somewhatlurker]

Yeah, I suppose that's a valid concern. But I personally don't see why many people would complain about a move to at least the standard 125Hz.

Perhaps adding rate adjustment as an option like in sbus or sumd would be feasible (though I'm not sure how to account for that in the HID endpoint).

[somewhatlurker]

I guess making hid_endpoint not const and creating a function to change the interval (eg. HID_SetInterval) called before HID_Enable() might work, but I don't have a build environment set up for deviation nor a radio to test on yet.

[somewhatlurker]

Would you mind checking this for compiler errors/warnings, and if it's all good also testing that it seems to use the correct period length on real hardware? @eried
https://github.com/somewhatlurker/deviation/tree/usbhid-improvements

If it's not working (there's a decent chance), I'll have to look into this all properly after my radio arrives.
Otherwise if it does work, I'm 90% sure it safely resolves the issue with no regression other than a config change for anyone who wants to keep the slow update rate.

[TheRealMoeder]

Perhaps adding rate adjustment as an option like in sbus or sumd would be feasible (though I'm not sure how to account for that in the HID endpoint).

Setting update rate as a protocol option seems like the obvious thing to do. I'll try to spare time to test your code changes.

I created a PR to pull your changes to my development environment and also run Travis CI.

[TheRealMoeder]

Would you mind checking this for compiler errors/warnings, and if it's all good also testing that it seems to use the correct period length on real hardware? @eried
https://github.com/somewhatlurker/deviation/tree/usbhid-improvements

You need to add void HID_SetInterval(u8 interval) to target.h

[somewhatlurker]

okay, I fixed that up
I would've thought it's best to wait until after the code is known to be working before opening a PR though

[eried]

Nice! I have not tested it but it looks clean :D

Maybe you can take a look on how to optimize how the sounds are handled. I have a solution I like on my repo (using a queue only for the "system sounds")

[somewhatlurker]

Yeah, after USB HID changes are tested (possibly also including looking into what can be done about #855 -- perhaps just increasing the number of buttons sent if that's not too hard) I might have a bit of a look into that.
I guess #960 and #961 are the appropriate discussion points for that.