Get bootmagic working with shared sn32f24xb matrix #356
Conversation
|
Currently keeping as draft as I want to test it compiles with all the relevant sonix boards currently in sn32_develop. |
|
Tested:
|
| @@ -548,6 +548,26 @@ void sn32f24xb_set_color_all(uint8_t r, uint8_t g, uint8_t b) { | |||
| } | |||
| } | |||
|
|
|||
| #if defined(SHARED_MATRIX) && defined(BOOTMAGIC_LITE) | |||
| void matrix_init_kb(void) { | |||
There was a problem hiding this comment.
using matrix_init_kb outside of a keyboard directory is non standard and probably bad practice. A more sane approach would be to hijack the bootmagic_lite declaration directly - this would probably be more reliable since bootmagic gets called before matrix functions. Code handling it might be a bit uglier however
There was a problem hiding this comment.
Ok, will try that approach. When making this I felt unsure what the agreed QMK convention for use of _kb, _custom methods are. Looking through the existing keyboard seems like many keyboards use matrix_init_kb as general purpose hardware initialization hook, not just initializing matrix. If that's a recommended usecase then it would indeed be wrong to use matrix_init_kb in the driver level.
I also tried using matrix_init_custom, but that didn't work because that's only called when CUSTOM_MATRIX is enabled. Currently sn32f24xb driver uses in my opinion somewhat hacky way of on hand being quite somewhat custom matrix, but instead of using CUSTOM_MATRIX=yes or lite, disabling parts of quantum/matrix.c with if !defined(SHARED_MATRIX) .
Maybe it would be cleaner if the SHARED sn32f24 matrix was handled same as other CUSTOM_MATRIX?
In longer term it might be worth reconsidering how matrix customization works. It doesn't feel like current architecture encourages reusable non standard key matrix. Maybe move it to similar driver pattern as other subystems?
There was a problem hiding this comment.
Currently, the shared implementation does indeed hack into the matrix routines. This was done in order to ease up on maintenance ( doing it full CUSTOM_MATRIX requires a carbon copy of matrix.c with the current code - lite is not much better). Upstream QMK did not feel at ease exposing matrix.c functions for us too hook up to, or allowing a CUSTOM_MATRIX=shared option, so we went with the current approach and added scan availability flags to be able to control it transparently.
Going forward, we should probably introduce a new platform variant, sn32f240b_shared to house all relevant shared configs/flags and/or a keyboard/sonixqmk/sn32f240b ( getting called from all shared matrix keyboards). We also need to get the driver specific configs in data driven format.
Keep in mind that while all that is doable in a couple different ways, a least intrusive approach must be taken so that upstream merges are clean-ish/plug and play in order to be able to keep up regularly with latest upstream and not have to rebase from scratch. We opt to be an extension of QMK to the sn32 world, not to butcher it up.
|
Thank you for your contribution! |
|
Thank you for your contribution! |
Description
Fix bootmagic lite when using sn32f24xb shared rgb/key matrix driver.
Bootmagic lite gets checked as early as possible -> after key matrix has been initialized but before other optional subystems like RGB are running. Even if bootmagic check happened after rgb is initialized the actual scanning code runs at it's own pace based on timer interrupts, which makes it's usage during early init somewhat problematic. As a result bootmagic_lite doesn't work probably for most sonix keyboards using shared matrix and the corresponding driver.
This PR adds single manual key only scan to ensure there is meaningful data when the default bootmagic implementation reads it.
Types of Changes
Checklist