-
-
Notifications
You must be signed in to change notification settings - Fork 38.9k
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
[Core] suspend: suppress wake up keypress #23389
base: develop
Are you sure you want to change the base?
Conversation
platforms/suspend.c
Outdated
void process_wakeup_matrix(uint8_t row, uint8_t col, bool pressed) { | ||
if (!pressed) { | ||
wakeup_matrix[row] &= ~((matrix_row_t)1 << col); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the wakeup flag gets cleared only when the main loop had seen both the press and the release event for the wakeup key. However, if USB_SUSPEND_WAKEUP_DELAY
is defined to a large enough value (and some boards have it defined to 5000), the key will probably be already released when the main loop gets a chance to run again after the wakeup delay; depending on the debouncing algorithm, the wakeup key may not be reported as pressed at that time, therefore the next real press and release events for that key would be eaten by the wakeup key filter.
Not sure what to do about that, apart from moving the filtering and clearing to the matrix_get_row()
call in matrix_task()
(but then it would run every time instead of only after detecting a change).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I have added a update_wakeup_matrix
function in 36f3da61f36252f3818114d8bd2e84e355400d97 which is called after the wake up delay and resets only the keys which have been released during the suspend. This should mitigate the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sigprof friendly ping 🙂
73cdf41
to
36f3da6
Compare
|
36f3da6
to
db8e91c
Compare
Thanks for pointing this out. Is this behavior observed with this PR as well? The last time I captured the wake up sequence of multiple non-qmk keyboards (some Cherry) non of them sent a keyboard report after the remote wake-up, so these keyboards should show the same problem in theory.
This is done by the USB peripheral automatically as long as there is data ready to be sent in the output FIFOs. But I agree the
The buffering of the reports was implemented in #21656. |
Thank you for your contribution! |
FWIW, I've been using this for a while, though predominantly on a Mac system, without any issues. |
Waking the host from suspend is done by pressing any key on the keyboard, the regular key codes assigned to the keys are not important and must not be sent - otherwise they usually end up in password prompts as ghost characters that have to be deleted again. This commit adds suppression for all keys pressed at the time of wake up. Once a key is released it functions as a regular key again. Signed-off-by: Stefan Kerkmann <karlk90@pm.me>
If USB_SUSPEND_WAKEUP_DELAY is set, the keyboard sleeps during wake up - which can be up to multiple seconds. To not miss any key releases the wake up matrix is updated with the current state of the matrix - only resetting the keys that have been released. Signed-off-by: Stefan Kerkmann <karlk90@pm.me>
db8e91c
to
6807d84
Compare
Description
Waking the host from suspend is done by pressing any key on the keyboard, the regular key codes assigned to the keys are not important and must not be sent - otherwise they usually end up in password prompts as ghost characters that have to be deleted again. This PR adds suppression for all keys pressed at the time of wake up. Once a key is released it functions as a regular key again.
Types of Changes
Issues Fixed or Closed by This PR
Checklist