MO trigger is not remembered when pressing multiple triggers

[Thanatermesis]

Describe the Bug

I have a MO(1) key in each side of my ergodox keyboard, which is used for the programming keys

If I press the MO(1) key from the left keyboard (to type keys on the right keyboard), then I press the MO(1) key from the other keyboard (to type other keys on the other side), and then I release the first MO(1) key pressed, the layer is switched back to the default one (0), this is wrong because one of my fingers is still pressing the MO(1) modifier key, but seems like QMK only knows that one of these modifiers was released and so switching back to the default layer

In a case like mine (that's why the examples) it is extremely annoying since layer 1 is the most used one after the default one, so I switch to it all the time, sometimes pressing the MO(1) key from each side and so being affected by this issue

Keyboard Used

customized ergodox

Link to product page (if applicable)

No response

Operating System

Linux

qmk doctor Output

No response

Is AutoHotKey / Karabiner installed

• [ ] AutoHotKey (Windows)
• [ ] Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

No response

[elpekenin]

I dont think this is really a bug, but just a limitation on how the whole event-processing flow works.

If you take a look at the source you'll find:

#define MO(layer) (QK_MOMENTARY | ((layer)&0x1F))

i.e. MO is [identifier to say "this is a momentary layer key" + the layer it affects]

This will later be processed (somewhere i couldn't find) as "when pressed, turn layer ON, when released, OFF", and there is no way for QMK to "remember" you've pressed it twice unless you add custom code,

---

As a simple workaround you could add a dummy layer and swap one of the MO, eg:

- [0] = LAYOUT( some keys, MO(1), some keys, MO(1), some keys)
+ [0] = LAYOUT( some keys, MO(1), some keys, MO(2), some keys)

// with 2 containing exactly the same keycodes as 1

// higher layers (if any) would need to be incremented by one, of course

Note: For the sake of easily making this kind of changes, i highly suggest using names for the layers, instead of numbers, in this case it would be something like

enum layers {
    _BASE,
    _FN1
    _DUMMY,
    ...
};
// ^ if you add a new name in between, all the ones after it will get automatically "fixed"

[_BASE] = LAYOUT( ..., MO(_FN1), ..., MO(_DUMMY))

PS: The discord server is usually a faster way of getting this kind of help

[Thanatermesis]

Unfortunately the chipset im using seems to be very limited to the number of layers

I was more like thinking about a variable (or array?) like:

int layer_keys_pressed = 0

which the number is incremented each time a new key is pressed, and removed when the key is released, then if the value is 0, the layer is deactivated, it sounds like a simple solution, easy to integrate in the code and pretty small in footprint terms 🤔

what I don't know is where exactly I should put the increment / decrease value and if i should do that on the QMK code or in the userspace code (using the vial fork at the moment)

Do you think this simple improvement should be implemented by default in QMK?

[elpekenin]

limited to the number of layers

that's likely an EEPROM size, not flash size limitation -> ie you wouldn't have such problem if you get rid of VIA(L)

variable (or array) ... where exactly

a uint8_t is more than enough, you can use process_record_user to add the custom behaviour (read docs for further info)

❓ at this point (writing custom code), do you really feel like VIA(L) is still useful? i mean, its main purpose is not having to code/compile/flash

do you think this should be implemented?

i dont feel like adding extra code for such an edge-case situation (most people will trigger a different layer with each of their MO keys, not the same one) is worth... anyway, that decission is not up to me but to the core team

[Thanatermesis]

There's this piece of code proposed by @elpekenin which seems to be able to do what is needed to make it working:

..... keymap.c
@@ -205,8 +205,33 @@ const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
 };

+/*uint8_t counter = 0;*/
+uint8_t layer_keys_pressed[SPECIAL] = {0};
+uint8_t layer = 0;
+
 bool process_record_user(uint16_t keycode, keyrecord_t *record) {
      switch (keycode) {
+        case QK_MOMENTARY...QK_MOMENTARY_MAX: // any MO key
+           layer = QK_MOMENTARY_GET_LAYER(keycode); // check which layer this MO affects
+
+           // -- same logic as before --
+           // update the affected position
+           if (record->event.pressed) layer_keys_pressed[layer]++;
+           else layer_keys_pressed[layer]--;
+
+           // turn the layer on/off
+           if (layer_keys_pressed[layer]) layer_on(layer);
+           else layer_off(layer);
+
+           return false;
         case VRSN:
            SEND_STRING(QMK_KEYBOARD "/" QMK_KEYMAP " @ " QMK_VERSION);
            return true;

Unfortunately I have this compilation error and I don't see the QK_MOMENTARY_GET_LAYER code anywhere or I didn't find a similar one:

keymap.c:215:20: error: implicit declaration of function ‘QK_MOMENTARY_GET_LAYER’ [-Werror=implicit-function-declaration]
            layer = QK_MOMENTARY_GET_LAYER(keycode); // check which layer this MO affects
                    ^

Ideas?

[elpekenin]

You probably need some #include

Also worth checking if such function exists on your local copy of the code, you are working on a fork that could be out date + your copy of it could also be

You can always check QMK's source and re-inplement the macro.

Im on bed now, so i cant check stuff out.

Note: discord will likely get answers faster

[Thanatermesis]

Thank you!

This is the final code that works, I keep it here just in case QMK wants to implement it on the main project 🤔

+// backport this needed macro from updated QMK code
+#define QK_MOMENTARY_GET_LAYER(kc) ((kc)&0x1F)
+
+uint8_t layer_keys_pressed[SPECIAL] = {0}; // macro to know the amount of keys pressed for MO layer switching
+uint8_t layer = 0; // layer number
+
+
 bool process_record_user(uint16_t keycode, keyrecord_t *record) {
      switch (keycode) {
+        case QK_MOMENTARY...QK_MOMENTARY_MAX: // any MO key
+           layer = QK_MOMENTARY_GET_LAYER(keycode); // check which layer this MO affects
+
+           // increase / decrease the amount of keys pressed to set a layer
+           if (record->event.pressed) layer_keys_pressed[layer]++;
+           else layer_keys_pressed[layer]--;
+
+           // turn the layer on/off
+           if (layer_keys_pressed[layer]) layer_on(layer);
+           else layer_off(layer); // deactivate layer only if there's no more remaining keys pressed
+
+           return false;
         case VRSN:
            SEND_STRING(QMK_KEYBOARD "/" QMK_KEYMAP " @ " QMK_VERSION);
            return true;

Should this issue be closed?