-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
refactor(Kconfig): Extracted designer defaults out into new files #2537
base: main
Are you sure you want to change the base?
Conversation
Moved the USB PID/VID stuff back to the standard Kconfig file. I realised that the |
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.
As a whole, happy with this, just needs some reformatting funkiness fixed.
1f047bb
to
aece6f0
Compare
Fixed formatting of comments as requested, did a pass to catch any that were previously misformatted (bunch of wrong spacings). While I was at it, I did a pass over the if blocks, moving lonely ones into I also got rid of the if blocks in the I left those last changes as a separate commit, to let it be removed easily if you'd prefer them not to change like this. Let me know if you want me to squash that or remove it. |
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.
One main item from reviewing the code. Will test locally a bit once that's fixed. Thanks!
b2e4db4
to
6dfe385
Compare
… to be set in device Kconfig.defconfig
6dfe385
to
38e9a65
Compare
38e9a65
to
d21eb6a
Compare
Did some local testing, ended up popping the if-refactor. Turns out they are not equivalent, Zephyr docs are wrong there. I built both halves of corne with n!nv2, nice60, and bt60_v2, and Currently it is being grouped under the RGB_UNDERGLOW section, with this change it gets moved under the advanced section alongside I2C etc. |
This is an alternative approach to #1886, with the same end goal. All of the "designer configurable" defaults are extracted out to separate files named
Kconfig.defaults
. The definitions stay in the sameKconfig
file, only the defaults move. The defaults are imported after the keyboard config options, allowing them to be overridden by designers.I used my own judgement to decide whether an option should be treated as "designer configurable". In general:
Specific additional options are made available to designers, such as USB vendor ID -- companies who have their own VID may want to override this one.There are also some options that are listed in the docs, which I don't think should be listed in the docs as I don't understand why either designers or users should have any business touching them. For example:CONFIG_ZMK_USB_INIT_PRIORITY
CONFIG_ZMK_BLE_INIT_PRIORITY
I have included such flags where I was uncertain as "designer configurable" out of caution.While at it, I added a name to the
EC11
trigger mode choice, and updated some values listed in the docs to be accurate.Hopefully this more explicit approach is more "palatable" than #1886 and can get merged easier...