[Bug] default_layer_state is 0 when the keyboard is powered up after flashing.

[gagarski]

default_layer_state is 0 when the keyboard is powered up after flashing.

Describe the Bug

this page says:

Keymap layer '0' is usually the default_layer, with other layers initially off after booting up the firmware, although this can configured differently in config.h. It is useful to change default_layer when you completely switch a key layout, for example, if you want to switch to Colemak instead of Qwerty.

Unfortunately there is nowhere in docs mentioned, how exactly I can do it. Also this text implies that default_layer_state is 0x1 by default (a mask that sets layer 0 as active).

Let's try this code:

#include QMK_KEYBOARD_H
#include <string.h>

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
    [0] = LAYOUT_ortho_2x4(
        KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS,
        KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS
    )
};

// Trash to check if `default_layer_state_set_user` was called on keyboard initalization
layer_state_t local_layer_state = 0xdeadbeef; 

layer_state_t default_layer_state_set_user(layer_state_t state) {
    local_layer_state = state;
    return local_layer_state;
}

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    uprintf("DLS %x\n", default_layer_state);
    uprintf("LS %x\n", layer_state);
    uprintf("LLS %x\n", local_layer_state);
    return true;
}

It prints:

DLS 0
LS 0
LLS 0

This can be fixed by adding the following eeconfig_init_user

void eeconfig_init_user(void) {
    set_single_persistent_default_layer(0);
}

There are also a few things that bother me when using set_single_persistent_default_layer:

set_single_persistent_default_layer does eeconfig_update_default_layer(1U << default_layer) which seems like writing a mask (but not a layer number) to EEPROM. Digging deeper reveals that eeconfig_update_default_layer calls eeprom_update_byte to store it. Which means that only first 8 layers can be set as default persistent layers. Seems like it was intended to store layer number, but not a mask here (according to "single layer" in the name of the function and a storage size for it). Seems like storing layer number here would solve the problem (default value for default_layer_state)

Is there a bug or I am missing something?

System Information

• Keyboard: Geekboards Macropad V2
• Operating system: Arch Linux, Windows 10 (as a keyboard users)
• ARM GCC version: arm-none-eabi-gcc (Arch Repository) 11.1.0
• QMK Firmware version: 0.13.5

[drashna]

although this can configured differently in config.h.

I don't know why it says that. The default is always 0, and there isn't a config.h setting to changed that.

However, it is always read from eeprom, so the set_single_persistent_default_layer function will set that.

[gagarski]

Yeah, but by default (if set_single_persistent_default_layer was never called after flashing) 0x0 value is read from EEPROM as a mask, which means no default layer is set at all (not default layer is 0). Does it make sense?

[drashna]

That makes sense, but the default is set when the eeprom is initialized.
https://github.com/qmk/qmk_firmware/blob/2538d341d828d04392898a9d3ce691bcd4a79e1f/tmk_core/common/eeconfig.c#L48

However, you are right that it only allows for the first 8 layers to be used in this manner. While not ideally, that should still be fine. Most people have 4 default layers at most, and the only person that has more than that ... is me, and that's at 8.

And yeah, it's stored as a bitmask rather than a number

[gagarski]

but the default is set when the eeprom is initialized

Yes. And as I said above, it is set to 0x0 which is a mask which means there is no default layer (again, "no default layer" vs "default layer is 0"). The keyboard still works fine (seems like scanning handles the situation when there is no default layer). However there still can be trouble if you are trying to read default_layer_state for some reason and use it. I ran into it when I tried to implement layer indication using LEDs. It took a lot of time to understand why after flashing the keyboard, no default layers are active, while docs say that layer 0 should be active.

So:

• after flashing there is no default layers enabled which makes no sense but still works :)
• set_single_persistent_default_layer allows you to set only one default layer (as the name would suggest), however the persistent storage actually supports multiple default layers (well, OK), but only among first 8 (which actually is nowhere documented)

It seems like storing a layer number here (of course it should be translated to mask when read into RAM) will make more sense (you actually would have a notion of single persistent default layer and support layers 8-31, despite the fact that no one uses that much layers). This change also won't break existing users' code (that is actually a big concern). Here is why:

• "Public API" is unchanged, set_single_persistent_default_layer will still accept a number and default_layer_state will still be a mask.
• Unless you (as a user) update firmware, mask set by old version of set_single_persistent_default_layer will be read by old init code which will treat it as a mask
• After you (as a user) update it, you will have new set_single_persistent_default_layer code and new version of keyboard init code. The values written by old version of set_single_persistent_default_layer are not in EEPROM anymore, since the flash it will always be a number.

However you (as a user) are still in trouble if:

• You (for some reason rely) on default_layer_mask being 0x0
• You use eeprom_update_byte and eeprom_read_byte to manipulate this state.

I actually can create a PR for that now (or are you still not convinced?:))

[gagarski]

Well, there is one place that relies on it being an 8-bit mask:

https://github.com/qmk/qmk_firmware/blob/89c01970e333e346c25202e9c568b75cad4c7be5/quantum/api.c#L75

Now I am not so sure that it would be a good idea to break it. Is it a good idea to return 4 bytes instead on 1 here?

[j-hap]

I ended up here, because I had trouble toggling between two default layers. Since this is maybe related, I'll just comment what I found to solve my problem.

My layer enum looks like this enum LAYER { DE_BASE_, EU_BASE_, DE_SYML_, DE_SYMR_, EU_SYML_, EU_SYMR_, NUM_, NAV_ };

DE_BASE and EU_BASE are the same except for some symbols and umlauts. I defined a combo to persistently toggle between base layers:

enum combo_events { SWP_BASE };

const uint16_t PROGMEM toggle_base_layer_combo[] = {KC_G, KC_M, COMBO_END};

combo_t key_combos[] = {[SWP_BASE] = COMBO_ACTION(toggle_base_layer_combo)};

void process_combo_event(uint16_t combo_index, bool pressed) {
    if (combo_index == SWP_BASE && pressed) {
        if (IS_LAYER_ON(DE_BASE_)) {
            set_single_persistent_default_layer(EU_BASE_);
        } else if (IS_LAYER_ON(EU_BASE_)) {
            set_single_persistent_default_layer(DE_BASE_);
        }
    }
}

This combo only works once, because layer_state != default_layer_state after boot. What I observed with the current qmk version is, that default_layer_state is indeed 0x1, but layer_state is 0x0 after boot. So when I check which layer is active to decide what the combo should do, the following happens: IS_LAYER_ON(DE_BASE_) is an alias for layer_state_is(DE_BASE_), which is an alias for layer_state_cmp(layer_state, DE_BASE_). layer_state_cmp however handles a first input of 0x0 different: If the first input is 0x0, it returns if the second argument is also 0x0. So in my case, since DE_BASE_ is the first entry in the enum and layer_state is 0x0, it returns true, therefore the swap from DEBASE to EU_BASE works. The next problem I ran into is that set_single_persistent_default_layer does not change the layer_state. Therefore, after the first swap, layer_state is still 0 and on the second combo press, the first case is still true.

So here's my solution:

fix the layer_state on boot:

void keyboard_post_init_user(void) {
    layer_state_set(default_layer_state);
}

and also do that whenever set_single_persistent_default_layer is called in the process_combo_event. Fortunately after flashing, the default_layer_state is not 0x0, but 0x1 (despite the initial issue, maybe that changed over the last 3 years), so this works.