Skip to content
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

Wait for USB HID ready before sending reports #93

Merged
merged 3 commits into from Aug 17, 2020
Merged

Wait for USB HID ready before sending reports #93

merged 3 commits into from Aug 17, 2020

Conversation

careyk007
Copy link
Contributor

@careyk007 careyk007 commented Aug 16, 2020

In Zephyr's hid-cdc example, they use a semaphore to signal when the host is ready for the next HID report. I haven't been able to test this with ZMK yet (still waiting for some hardware), but I had noticed similar behavior to #84 on my custom keyboard code and this fixed my issues.

The downside to this approach is that execution will halt until the host signals that it is ready for the next HID report. Some potential solutions would be to spawn a worker thread that will wait for the host before sending the HID report or to create a queue for the HID reports and a dedicated thread that sends queue'd reports as the host becomes ready for them.

@careyk007 careyk007 marked this pull request as ready for review Aug 16, 2020
@Nicell
Copy link
Member

Nicell commented Aug 16, 2020

I just tested this on my nice60, and it sure does work! Nothing logs as failed to send over USB, and my WPM on a typing test actually improved almost 20WPM, so real world it's working as well.

Thanks so much for this! I don't have any personal opinions on the worker queue system... It might be nice in the case that we get a failed USB send for another reason than USB being busy and we want to make sure it still goes through.

Copy link
Contributor

@petejohanson petejohanson left a comment

Rock on!

This is exactly the initial fix I had planned to explore.

I do think we may want to long term use a queue and background thread to process the queue and send the USB updates that way, but doing so actually requires more trickery than it looks at first blush.

I think this fix is a great start, with the one caveat that we could hit some edge cases with slow hosts causing problems.

Thanks for the fix.

@petejohanson petejohanson merged commit 8cd8933 into zmkfirmware:main Aug 17, 2020
1 check passed
petejohanson added a commit that referenced this pull request Aug 18, 2020
This reverts commit 8cd8933, reversing
changes made to 3f1dfba.
Nicell referenced this pull request in Nicell/zmk Sep 7, 2020
This reverts commit 8cd8933, reversing
changes made to 3f1dfba.
MangoIV referenced this pull request in MangoIV/zmk Dec 18, 2020
Wait for USB HID ready before sending reports
MangoIV referenced this pull request in MangoIV/zmk Dec 18, 2020
This reverts commit 8cd8933, reversing
changes made to 3f1dfba.
tyalie pushed a commit to tyalie/zmk that referenced this pull request Nov 15, 2022
This reverts commit f309f8a, reversing
changes made to 59aac49.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants