-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
(for v0.4) Add a way to jump to bootloader and/or reset settings on startup #3213
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
base: main
Are you sure you want to change the base?
Conversation
Moved the logic to select the type of reboot from behavior_reset.c to a new zmk_reset() function. This allows rebooting to bootloader from other code, and it gives us a starting point for future work to support other bootloaders aside from the Adafruit nrf52 bootloader.
For now, this just clears BLE bonds, but it can be updated in the future to handle clearing all settings.
This adds an optional feature to trigger an action if a specific key is held when the keyboard is powered on. It can be configured to jump to the bootloader and/or clear settings. This is inspired by QMK's "bootmagic lite" feature, and it is primarily intended as a way to recover a keyboard which doesn't have a physical reset button in case it is flashed with firmware that doesn't have a &bootloader key in its keymap. It can also be used to clear BLE bonds on the peripheral side of a split keyboard without needing to flash special firmware.
petejohanson
left a comment
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.
Thanks for working on this! A few thoughts on the implementation.
| jump-to-bootloader: | ||
| type: boolean | ||
| description: Reboots into the bootloader. | ||
| reset-settings: | ||
| type: boolean | ||
| description: Clears settings and reboots. |
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.
It would be really nice, now that we have ZMK Studio, if we could also add an option here for doing a "restore stock keymap", just in case someone mistakenly removes their &studio_unlock from their keymap when making changes.
On that node, this should probably just be one properly, that's an enum, e.g.:
| jump-to-bootloader: | |
| type: boolean | |
| description: Reboots into the bootloader. | |
| reset-settings: | |
| type: boolean | |
| description: Clears settings and reboots. | |
| action: | |
| type: string | |
| required: true | |
| enum: | |
| - "jump-to-bootloader" | |
| - "reset-settings" | |
| - "restore-studio-stock-keymap" |
since it only makes sense, IMHO, for a given boot magic key to perform one of the possible boot magic actions, and allows us to grow the enum list later.
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.
+1 on the ZMK Studio "restore default keymap" idea - worth logging on https://github.com/zmkfirmware/zmk-studio/issues ?
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 it would be even nicer if, instead of restoring the default keymap, we had an option to just unlock studio.
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 it would be even nicer if, instead of restoring the default keymap, we had an option to just unlock studio.
Yeah, like this much better.
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.
It's almost sounding like the action should just be a pointer to some ZMK behavior :)
|
|
||
| // Hardcoding a reasonable hardcoded value of peripheral addresses | ||
| // to clear so we properly clear a split central as well. | ||
| for (int i = 0; i < 8; i++) { |
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.
Given this and the loop above both iterate 8 times, lets unify this into one loop that deletes both kinds of settings.
| @@ -0,0 +1,94 @@ | |||
| /* | |||
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'm tempted to say this code should live in the fairly recently created app/src/boot/ directory at this point.
| @@ -0,0 +1,94 @@ | |||
| /* | |||
| * Copyright (c) 2023 The ZMK Contributors | |||
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.
| * Copyright (c) 2023 The ZMK Contributors | |
| * Copyright (c) 2026 The ZMK Contributors |
| zmk_reset_settings(); | ||
| } | ||
|
|
||
| if (config->jump_to_bootloader) { |
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.
Maybe I need a refresher here, or @joelspadin can weigh in... But do we really have a use case where you want to reset the settings and then jump to the bootloader? Having a hard time imaging a real need for that specific use case.
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'm sure I probably came up with a reason for that when I originally wrote this, but I can't think of one now.
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.
QMK does a reset then jump, but it's always bothered me that there's no way to do just a jump to bootloader. With QMK I've had some troubles with my saved changes causing conflicts when I make major changes to the hardware like changing the number of keys in the firmware, and a settings reset was necessary. I haven't stress tested ZMK as much, but if this isn't an issue here (Studio does seem very well architected), then I agree just one makes sense.
| @@ -0,0 +1,63 @@ | |||
| /* | |||
| * Copyright (c) 2023 The ZMK Contributors | |||
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.
| * Copyright (c) 2023 The ZMK Contributors | |
| * Copyright (c) 2026 The ZMK Contributors |
|
|
||
| :::caution | ||
|
|
||
| Currently this action _only_ clears BLE bonds. It will be updated to reset all settings in the future. |
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.
If we're really going to have this only do this, I'm wary about this changing out from under folks in the future. Can we name this current action "clear-ble-bonds" or something, and add a true "reset-settings" action later as an additional action option, instead of the current PR approach?
@joelspadin any concerns?
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.
IIRC, I originally wrote this when the settings_reset shield only cleared BLE bonds and didn't reset all settings. I'd suggest starting with a full settings reset option. I'm not sure if there are cases where you'd want to clear BLE bonds only instead of resetting everything, but if so, we could add a separate option for that too.
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.
The main reason I can think to maintain a "clear BLE only" is to preserve any studio changes when trying to fix pairing issues between halves of a split, so I think there's a use case for both actions being possible from boot keys.
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.
While we are thinking about the options, I think it might be nice to have separate options for "clear split bonds" vs. "clear BLE profiles" (except the split ones). Not necessarily in the first iteration.
| If you want a single boot magic key to perform multiple actions, simply add properties for each action to the same `zmk,boot-magic-key` node. The order of the properties does not matter. | ||
|
|
||
| For example, to make a key that resets settings and then reboots to the bootloader, add both `reset-settings` and `jump-to-bootloader`: |
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.
Still not sure we really need this. Open to being convinced otherwise though. Thanks.
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.
Clean slate when you've been tinkering with ZMK studio while getting the matrix right? Saves building and flashing a settings reset firmware too.
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 mean a dual function key that both resets settings and jumps into the bootloader.... Seems super niche, and I'm not sure I understand your described need for studio iteration... Can you explain in exact steps what use case you're considering?
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.
Possibly user ignorance or misunderstanding, but I've flashed new firware onto a board to find what I though were traces of past changes I'd made in ZMK Studio. An easy way to flash new firmware and wipe out any past Studio changes seems like a good thing. And wipe old Bluetooth pairings too?
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 can think of a scenario where a designer wants only a single bootmagic key, in which case you'd want it to do everything, including clearing all settings and jumping to bootloader.
| @@ -0,0 +1,23 @@ | |||
| # Copyright (c) 2023, The ZMK Contributors | |||
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.
| # Copyright (c) 2023, The ZMK Contributors | |
| # Copyright (c) 2026, The ZMK Contributors |
|
As I've thought on this more, I'm having some concerns about having "single key" magic keys.. unlike QMK, we are far more likely to have someone tapping a key to wake the device and run this early code, potentially with them tapping/accidentally tapping the 0th key to do so. If we keep this as is, I fear we'll get a lot of accidental triggers. What do folks think about having multiple positions assignable to a given boot magic key? Doesn't need to have fancy combo handling, just enough to track "are all positions pressed that are part of this boot magic key?" |
That's a valid concern but since we aren't enabling this by default, the users should be aware of the specific key position (or the keyboard designers should tell the users), right? I think it's good to have, rather than must. What would the effective combo term be? Is it a simple trigger where you want both keys pressed at the time of the check (so the effective term is the boot time) or do we implement logic to wait for keys? |
Still might have something bump the corner of your keeb and wake it and since the check is just "is this key pressed before the timeout" you'd suddenly potentially lose all your studio customization, etc. I think this functionality is super useful, but we also shoukd try to design this in a way that's "easy not to screw up". I also don't trust designers/vendors to consistently communicate this, nor users to always read what is communicated by their vendor.
I think just "are all keys held at once before the boot magic timeout" |
|
I wonder if "check on boot" is even the right thing to aim for, for wireless devices. I think a "check on usb insertion" might make more sense for those kind of devices. We would lose more function for any keyboards wanting to run hardware without usb support, but those sorts of devices need to be flashed with a programmer and are unlikely to have a bootloader/are more heavy enthusiast. |
I like where you're thinking! Why don't we use the hwinfo API to determine the reset reason, and ignore the boot magic unless it's one of the "valid" reasons (.e.g. reset pin or power-on-reset)? I think that might a good way to avoid the accidental triggers from a key press to wake, and allow simple one button boot magic. |
|
Adding support for combos probably wouldn't be too hard either. You could add a |
Yeah, my thoughts exactly. We don't need crazy "combo" logic here. |
|
I think it would be a good idea to predefine a bootloader boot magic key for our in-tree keyboards. Maybe top left key as a convention, mirrored for splits? |
I've built on #1757 to merge in the latest changes in ZMK. Most notably I've integrated the new boot retention mode settings and addressed the comments in the previous PR.
PR check-list