-
-
Notifications
You must be signed in to change notification settings - Fork 39.2k
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
Add hotfix for chibios keyboards not wake #10088
Conversation
Tested this PR against my MacBook Pro once I'd worked out the timing when the KB actually enters suspend. |
A stupid idea: if I call the |
Sadly not. Tapping something twice is definitely better than locking up the keyboard, though. |
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.
I think I'd prefer to get this in as-is, and treat the double-tap requirement for wakeup as a separate issue.
I have a feeling the solution to that will be to completely revert this and use a different approach. I especially am not fond of the 1.5 second blocking wait here. |
Neither; however a temporary solution that doesn't result in a lockup seemed like it was a step in the right direction for now. |
@fauxpark I found that even if I disable the 1.5 seconds blocking wait, the keyboard can back to work without any problem, so most likely the current configuration for STM32F401 or the Chibios implementation has some problem. @tzarc can you try to disable the 1.5 seconds blocking and see if you need a second press to wake the computer? I can wake the computer in just one keypress with/without the 1.5 seconds wait, so I can not test the problem you have, thank you, guys! |
It seems removing the |
Maybe, but periodically, we (ZSA) have had reports of this, and it creates a very bad user experience. A fix that mostly works is better than none at all. And yeah, I'd rather a fully flushed out solution, but ... if it works now...
I'd almost be inclined to say make it a define, so it can be adjusted/disabled. |
@drashna the thing is, I'm not seeing this issue on my Proton-C... so I think your problem lies elsewhere. |
Agreed -- I've had cases where things like the #if defined(USB_SUSPEND_RESTART_DELAY) && (USB_SUSPEND_RESTART_DELAY > 0)
wait_ms(USB_SUSPEND_RESTART_DELAY);
#endif |
Pretty sure this specific trigger scenario is limited to OTG-based STM32's... I'm not sure if it solves F303 or any other non-OTG MCU, as we don't have any specific reproduction scenarios for that case. It definitely helps with OTG devices, though. |
ok, so maybe I use a name like |
I wasn't seeing any issues with this implementation on non-OTG devices, so I'd rather keep it in on the off chance that it does indeed fix the sleep issue people were having on non-OTG devices too. |
3a8e1b8
to
a1b96ba
Compare
Applied the latest version and it's working perfectly. |
a1b96ba
to
3430907
Compare
This fix seems to have had the completely opposite effect on Ergodox Infinity (i.e. keyboard is now dead after it wakes the host, but works fine if I comment out the call to |
My opinion is still that I would rather this be reverted completely and the actual root cause identified and fixed. I'm not keen on having ifdefs like that in code that is supposed to Just Work for all ChibiOS boards, unless absolutely necessary. |
@fauxpark I agree this is just a hotfix, as the title already said, is there any good suggestion for debugging this? Also, if it is the case as you said, that is, this behavior only happens on STM32F4 boards and not for every board, then I think in the end we will add an option to turn on/off this behavior for specific boards. |
It seems to _cause_ the issue it intended to solve there.
It seems to _cause_ the issue it intended to solve there.
It seems to _cause_ the issue it intended to solve there.
It seems to _cause_ the issue it intended to solve there.
It seems to _cause_ the issue it intended to solve there.
Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well...
…2870) * Remove the #10088 hotfix for K20x MCU:s. It seems to _cause_ the issue it intended to solve there. * Cleaner way of removing #10088 hotfix. Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well... * Fix comment regarding `IC_TEENSY_3_1` for all keyboards using it.
…mk#12870) * Remove the qmk#10088 hotfix for K20x MCU:s. It seems to _cause_ the issue it intended to solve there. * Cleaner way of removing qmk#10088 hotfix. Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well... * Fix comment regarding `IC_TEENSY_3_1` for all keyboards using it.
…mk#12870) * Remove the qmk#10088 hotfix for K20x MCU:s. It seems to _cause_ the issue it intended to solve there. * Cleaner way of removing qmk#10088 hotfix. Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well... * Fix comment regarding `IC_TEENSY_3_1` for all keyboards using it.
* Branch point for 2020 November 28 Breaking Change * Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183) * Add support for soft serial to ATmega32U2 (qmk#10204) * Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940) * Add ability to build a subset of all keyboards based on platform. * Actually use eeprom_driver_init(). * Make bootloader_jump weak for ChibiOS. (qmk#10417) * Joystick 16-bit support (qmk#10439) * Per-encoder resolutions (qmk#10259) * Share button state from mousekey to pointing_device (qmk#10179) * Add hotfix for chibios keyboards not wake (qmk#10088) * Add advanced/efficient RGB Matrix Indicators (qmk#8564) * Naming change. * Support for STM32 GPIOF,G,H,I,J,K (qmk#10206) * Add milc as a dependency and remove the installed milc (qmk#10563) * ChibiOS upgrade: early init conversions (qmk#10214) * ChibiOS upgrade: configuration file migrator (qmk#9952) * Haptic and solenoid cleanup (qmk#9700) * XD75 cleanup (qmk#10524) * OLED display update interval support (qmk#10388) * Add definition based on currently-selected serial driver. (qmk#10716) * New feature: Retro Tapping per key (qmk#10622) * Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638) * Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530) * Rescale both ChibiOS and AVR backlighting. * Reduce Helix keyboard build variation (qmk#8669) * Minor change to behavior allowing display updates to continue between task ticks (qmk#10750) * Some GPIO manipulations in matrix.c change to atomic. (qmk#10491) * qmk cformat (qmk#10767) * [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657) * Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274) * [quantum] combine repeated lines of code (qmk#10837) * Add step sequencer feature (qmk#9703) * aeboards/ext65 refactor (qmk#10820) * Refactor xelus/dawn60 for Rev2 later (qmk#10584) * add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824) * [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549) * update chibios os usb for the otg driver (qmk#8893) * Remove HD44780 References, Part 4 (qmk#10735) * [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512) * Fix cursor position bug in oled_write_raw functions (qmk#10800) * Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972) * Allow for certain code in the codebase assuming length of string. (qmk#10974) * Add AT90USB support for serial.c (qmk#10706) * Auto shift: support repeats and early registration (qmk#9826) * Rename ledmatrix.h to match .c file (qmk#7949) * Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231) * Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840) * Merge point for 2020 Nov 28 Breaking Change
Reconnect the USB if users wake up a computer from the keyboard to restore the USB state.
Description
Keyboards using STM32 controllers with Chibios are suffering from keyboards that are frozen due to the USB state detection problem when waking up the computer from that keyboard. The hotfix forces the keyboard to reconnect to the computer.
Types of Changes
Issues Fixed or Closed by This PR
Checklist