Skip to content
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 a way to jump to bootloader and/or reset settings on startup #1757

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

joelspadin
Copy link
Collaborator

This adds an optional feature to reboot into the bootloader if a specific key is held when the keyboard is powered on.

This is similar to 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.

@joelspadin
Copy link
Collaborator Author

This theoretically works on the peripheral (right) side of a split, but I don't own a split keyboard, so if someone wants to test it, that would be appreciated!

@caksoylar
Copy link
Contributor

I tested it on a split keyboard with caksoylar@5bc195b and it works as expected. I'll add a suggestion to docs emphasizing that the key positions are affected by matrix transforms.


:::info

Key positions are numbered like the keys in your keymap, starting at 0. So, if the first key in your keymap is `Q`, this key is in position `0`. The next key (possibly `W`) will have position 1, etcetera.
Copy link
Contributor

@caksoylar caksoylar Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Key positions are numbered like the keys in your keymap, starting at 0. So, if the first key in your keymap is `Q`, this key is in position `0`. The next key (possibly `W`) will have position 1, etcetera.
Key positions are numbered like the keys in your keymap, starting at 0. So, if the first key in your keymap is `Q`, this key is in position `0`. The next key (possibly `W`) will have position 1, etcetera. Note that the key position is affected by the chosen [matrix transform](../config/kscan#matrix-transform), so make sure to select an index that would be available in all of them if you define multiple matrix transforms.

Might be good to add a note like this one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a direct copy from the combos documentation. Do you think this should be added there too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, the first part applies there as well.

For the second part, combos are different because they are defined by the user in the keymap, where the transform is known. Here this is directed towards users writing keyboard definitions, where they don't know what matrix transform will be chosen by the user.

@joelspadin
Copy link
Collaborator Author

As discussed starting at https://discord.com/channels/719497620560543766/719565084208398406/1097298953650847824, I'm going to tweak this to be a "boot magic key" which performs one or more actions (jump to bootloader, reset settings, etc.) instead of only supporting jump to bootloader.

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.
@joelspadin joelspadin changed the title Add a way to jump to bootloader on startup Add a way to jump to bootloader and/or reset settings on startup Apr 30, 2023
@joelspadin
Copy link
Collaborator Author

Added options to jump to bootloader and/or reset settings (currently just runs the same function to clear BLE bonds as the settings_reset firmware does, but we can improve that later).

Renamed the feature to "boot magic key" since it is now, just like QMK's feature, a way to perform some magic function if a key is held at boot.

@caksoylar caksoylar added enhancement New feature or request core Core functionality/behavior of ZMK labels Jun 10, 2023
@@ -38,6 +38,7 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);

#if IS_ENABLED(CONFIG_ZMK_BLE_PASSKEY_ENTRY)
#include <zmk/events/keycode_state_changed.h>
#include "ble.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this included by mistake?

Triggers one or more actions if a specific key is held while the keyboard boots.
This is typically used for recovering a keyboard in cases such as &bootloader
being missing from the keymap or a split peripheral which isn't connected to
the central, and therefore can't process th ekeymap.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

th ekeymap

@caksoylar
Copy link
Contributor

caksoylar commented Oct 5, 2023

FYI this now has some minor merge conflicts, I rebased it (and seems to be functional) at caksoylar/zmk@f72433e...94e2dbc.

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I truly love this. A couple minor notes on implementation. Would love to get this rebased and in.

Comment on lines +526 to +529
DT_COMPAT_ZMK_BOOT_MAGIC_KEY := zmk,boot-magic-key
config ZMK_BOOT_MAGIC_KEY
bool "Enable actions when keys are held at boot"
default $(dt_compat_enabled,$(DT_COMPAT_ZMK_BOOT_MAGIC_KEY))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proper new way to do this is:

Suggested change
DT_COMPAT_ZMK_BOOT_MAGIC_KEY := zmk,boot-magic-key
config ZMK_BOOT_MAGIC_KEY
bool "Enable actions when keys are held at boot"
default $(dt_compat_enabled,$(DT_COMPAT_ZMK_BOOT_MAGIC_KEY))
config ZMK_BOOT_MAGIC_KEY
bool "Enable actions when keys are held at boot"
default y
depends on DT_HAS_ZMK_BOOT_MAGIC_KEY_ENABLED

LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);

struct boot_key_config {
int key_position;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to use an explicit uint16_t here?

Suggested change
int key_position;
uint16_t key_position;

Comment on lines +105 to +147
For split keyboards, you can define multiple boot magic keys and then only enable the correct key(s) for each side. For example, if key 0 is the top-left key on the left side and key 11 is the top-right key on the right side, you could use:

**shield.dtsi**

```c
/ {
...
bootloader_key_left: bootloader_key_left {
compatible = "zmk,boot-magic-key";
key-position = <0>;
jump-to-bootloader;
status = "disabled";
};

bootloader_key_right: bootloader_key_right {
compatible = "zmk,boot-magic-key";
key-position = <11>;
jump-to-bootloader;
status = "disabled";
};
...
};
```

**shield_left.overlay**

```c
#include "shield.dtsi"

&bootloader_key_left {
status = "okay";
};
```

**shield_right.overlay**

```c
#include "shield.dtsi"

&bootloader_key_right {
status = "okay";
};
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just define them locally in each left and right file? This seems maybe more complicated than needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should certainly try it again, but that didn't work the same for me the first time I tried it: https://discord.com/channels/719497620560543766/719544230497878116/1097338556655075499

@caksoylar
Copy link
Contributor

For posterity, I ran this for a few months, until I recently switched to a Zephyr 3.5 based branch. I haven't tried the settings reset functionality but the bootloader has been working well (rebased commits here).

@joelspadin
Copy link
Collaborator Author

I plan to first finish a PR that makes settings reset actually reset all settings, which will add a function to call for that, and then I'll update this PR to use that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionality/behavior of ZMK enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants