feat(behaviors): Allow mod-morph to swallow mods

[vrinek]

This PR is based on https://github.com/aumuell/zmk/commit/fa61c6a3c6e7cbfcb5b3d9168b3454bf95c and most, if not all, of the credits should go to @aumuell. I have only contributed some git-fu to get their work rebased on top of main.

Related issues

Fixes https://github.com/zmkfirmware/zmk/issues/683.

Feature description

This PR adds a new option to mod-morph: masked_mods. This option complements the mods option, and allows the user to configure which of the mods will be swallowed when the morph triggers.

Behavior without masked_mods

behaviors {
    mmlt: grave_escape {
        compatible = "zmk,behavior-mod-morph";
        label = "LPAR_LT";
        #binding-cells = <0>;
        bindings = <&kp LPAR>, <&kp COMMA>;
        mods = <(MOD_LSFT|MOD_RSFT)>;
    };
};

• Pressing &mmlt results in (
• Pressing L-shift + &mmlt results in L-shift + ,, which is interpreted as <
• Pressing R-shift + &mmlt results in R-shift + ,, which is interpreted as <

Behavior with masked_mods

 behaviors {
     mmlt: grave_escape {
         compatible = "zmk,behavior-mod-morph";
         label = "LPAR_LT";
         #binding-cells = <0>;
         bindings = <&kp LPAR>, <&kp COMMA>;
         mods = <(MOD_LSFT|MOD_RSFT)>;
+        masked_mods = <(MOD_LSFT)>;
     };
 };

• Pressing &mmlt results in (
• Pressing L-shift + &mmlt results in , (the L-shift is swallowed)
• Pressing R-shift + &mmlt results in R-shift + ,, which is interpreted as <

Manual testing

Sadly, the only keyboard I have at hand that runs ZMK is a Corne-ish Zen which (for the time being) depends on a fork of this repo (https://github.com/LOWPROKB/zmk). As such I have performed no testing of this branch and would welcome any testers to lend a hand.

What I have tested is https://github.com/vrinek/zmk/tree/masked-mod-morphs-corne-ish-zen, which is the same changes but on top of the branch that works for my keyboard:

1. defined the behavior as described at "Behavior with masked_mods"
2. tried pressing the key I bound :heavy_check_mark:
3. with L-shift :heavy_check_mark:
4. with R-shift :heavy_check_mark:
5. with other mods :heavy_check_mark:

Testers that are willing to help are more than welcome.

TODO

• [ ] write some tests

[vrinek]

Successful manual test by @alex-popov-tech on a "jorne nrfmicro_13": https://github.com/zmkfirmware/zmk/issues/683#issuecomment-1030247663

[trankillity]

Have just tested this PR on a Sweep (Cradio shield) using nice!nano v2 boards. The tested over-ride was for <(MOD_LSFT|MOD_RSFT)> on bindings = <&kp COMMA>, <&kp SEMI>;.

Can confirm that it works as expected.

[okke-formsma]

Before we can merge this, one or more tests will have to be written to verify the behavior.

I don't know why the tests are not being run for this MR, but I think some of them would fail (e.g., app/tests/modifiers/implicit/kp-mod1-dn-mod2-dn-mod2-up-mod1-up)

[vrinek]

Before we can merge this, one or more tests will have to be written to verify the behavior.

I agree, and it's on my tasklist for this PR to fix the (I think only one) failing test as well as writting at least one more to validate behavior.

Life has gotten in the way though and I am making very slow progress. :sweat:

[MatCyg]

FYI: Tested this on top of macro branch, and it seems to be broken. I used the example provided in first post. Not sure if that should be fixed in this PR, but worth keeping that in mind.

PS. I am glad someone picked this up, I am looking forward to use it!

[infused-kim]

I have rebased it onto the main branch before the zephyr-3 update (but after the macro addition) and it's working perfectly for me.

Thank you for this PR and adding this amazing feature.

Here are a few of my use case ideas that might be useful for others:

    /*
     * Shifted Backspace deletes + Layer Tap
     *
     * Usage: &mm_bspc_del_layer
     * Tap: Backspace
     * Shift-Tap: Delete
     * Hold: Switch layer
     */
    mm_bspc_del_layer: bspc_del_layer {
        compatible = "zmk,behavior-mod-morph";
        label = "BACKSPACE_DELETE_LAYER";
        #binding-cells = <0>;
        bindings = <&lt SYM BSPC>, <&kp DEL>;
        mods = <(MOD_LSFT|MOD_RSFT)>;

        // Requires forked zmk that supports this feature
        // https://github.com/zmkfirmware/zmk/pull/1114
        //
        // Ensures that when pressing Shift-Delete only delete is sent
        // and not Shift-Delete
        masked_mods = <(MOD_LSFT|MOD_RSFT)>;
    };

    /*
     * Shifted Space underscores + Layer Tap
     *
     * Usage: &mm_spc_unds_layer
     * Tap: Space
     * LShift-Tap: -
     * RShift-Tap: _
     * Hold: Switch layer
     */
    mm_spc_unds_layer: mod_morph_underscore {
        compatible = "zmk,behavior-mod-morph";
        label = "MM_UNDERSCORE";
        #binding-cells = <0>;
        bindings = <&lt NAV SPACE>, <&kp MINUS>;
        mods = <(MOD_LSFT|MOD_RSFT)>;

        // Requires forked zmk that supports this feature
        // https://github.com/zmkfirmware/zmk/pull/1114
        masked_mods = <(MOD_LSFT)>;
    };

    /*
     * Grave Escape + GUI
     *
     * Usage: &grescm_gui
     * Tap: Escape
     * LShift-Tap: `
     * RShift-Tap: ~
     * Hold: GUI
     */
     grescm_gui: grave_escape_masked_gui {
         compatible = "zmk,behavior-mod-morph";
         label = "GRESC_GUI";
         #binding-cells = <0>;
         bindings = <&hm LGUI ESCAPE>, <&kp GRAVE>;
         mods = <(MOD_LSFT|MOD_RSFT)>;

        // Requires forked zmk that supports this feature
        // https://github.com/zmkfirmware/zmk/pull/1114
         masked_mods = <(MOD_LSFT)>;
     };

[dxmh]

I also used this masked_mods functionality for a while – it worked great to get macOS-like text navigation on Linux:

https://github.com/dxmh/zmk-config/blob/6561ae7d9485a9e373dc22aeff00c2cd3b3b0e7d/config/mod_morphs.dtsi

I particularly liked that I could have multiple mod-morphs per key, for example:

1. right = right
2. gui + right = end
3. alt + right = ctrl + right (word right)

[trankillity]

Some great example use cases in here. I especially love the shift + space for underscore! Definitely stealing that.

[infused-kim]

Some great example use cases in here. I especially love the shift + space for underscore! Definitely stealing that.

Thanks. Credit goes to Jon on discord. He is the one who came up with the idea. I just added the masking mode so that you can type minus too.

All of these work really well with home row mods. It’s a pleasure to press shift with the pointing finger of a hand and then get a underscore or dash.

Perhaps it would make sense to add some of these examples to the documentation so that new users have some inspiration for what’s possible.

[vrinek]

Perhaps it would make sense to add some of these examples to the documentation so that new users have some inspiration for what’s possible.

Great idea. Once I find the time and get the tests passing, I'll add some real world examples in the docs.

[tap0119]

I was testing this and found that keycodes for symbols that normally use shift like DOLLAR, PERCENT and QUESTION won't send properly and will send the normal unshifted key if you ignore LSFT. DOLLAR sends 4, PERCENT sends 5, QUESTION sends / and so on. If you only ignore RSFT they will send properly.

[trankillity]

I was testing this and found that keycodes for symbols that normally use shift like DOLLAR, PERCENT and QUESTION won't send properly and will send the normal unshifted key if you ignore LSFT. DOLLAR sends 4, PERCENT sends 5, QUESTION sends / and so on. If you only ignore RSFT they will send properly.

Well... I mean... yeah. But just use normal mod-morph without the swallowing for this. That's what I did for ./:

[tap0119]

I was testing this and found that keycodes for symbols that normally use shift like DOLLAR, PERCENT and QUESTION won't send properly and will send the normal unshifted key if you ignore LSFT. DOLLAR sends 4, PERCENT sends 5, QUESTION sends / and so on. If you only ignore RSFT they will send properly.

Well... I mean... yeah. But just use normal mod-morph without the swallowing for this. That's what I did for ./:

I was using shift plus a key to momentarily move to a layer and wanted to use those keys on that layer but not have shift held down for arrow keys or numbers on that layer. You can do that as long as you use right shift.

[alex-popov-tech]

hey guys, any update on this?

[vrinek]

hey guys, any update on this?

No real updates. The task list right now is:

• [ ] update patch with latest main
• [ ] fix broken tests
• [ ] add new tests to cover new functionality

I have not been able to make any progress, cause life priorities.

[urob]

A quick update on this. I cherry-picked 241f82e into the latest main and successfully build against it. So far, all my tests did go well and masked_mods seem to do exactly what they set out to do. In case someone else wants to test without messing with a (minor) merge conflict, this is the commit: e40b9a7.

[alex-popov-tech]

@urob i am willing to test, can you please advice on how i can do that?

[urob]

@urob i am willing to test, can you please advice on how i can do that?

If you don't have a local copy of ZMK, you can use the beta testing feature to select an arbitrary remote branch of ZMK to be used with Github Actions.

If you want masked mods with the latest ZMK, you can build against this branch. ~~I didn't create a dedicated branch as there is already this PR, but this branch will give you the latest ZMK (as of 06/21/2022) + masked mods.~~ EDIT: I made a dedicated branch so that it won't be affected by other things I change in my personal branch. You can use it with Github Actions by changing your west.yml to something like this:

manifest:
  remotes:
    - name: zmkfirmware
      url-base: https://github.com/zmkfirmware
    - name: urob
      url-base: https://github.com/urob
  projects:
    - name: zmk
      remote: urob
      revision: masked-mods
      import: app/west.yml
  self:
    path: config

[alex-popov-tech]

@urob i usually build things locally with devcontainers

so i've pulled your repo, checkout to your branch, reopened it in container, did all instructions, then tried to build and received following error:

est build -p -s ./app -d build/jian_vu_keys/left -b nrfmicro_13 -- -DSHIELD=jian_left -DZMK_CONFIG="/workspaces/zmk-config/jian/config"
-- west build: making build dir /workspaces/zmk_fork/build/jian_vu_keys/left pristine
-- west build: generating a build system
Including boilerplate (Zephyr base): /workspaces/zmk_fork/zephyr/cmake/app/boilerplate.cmake
-- Application: /workspaces/zmk_fork/app
-- Adding /workspaces/zmk_fork/app/boards/shields/jian
-- ZMK Config directory: /workspaces/zmk-config/jian/config
-- ZMK Config Kconfig: /workspaces/zmk-config/jian/config/jian.conf
-- Using keymap file: /workspaces/zmk-config/jian/config/jian.keymap
-- Using keymap file: /workspaces/zmk-config/jian/config/jian.keymap
-- Zephyr version: 3.0.0 (/workspaces/zmk_fork/zephyr)
-- Found Python3: /usr/bin/python3.8 (found suitable exact version "3.8.10") found components: Interpreter 
-- Found west (found suitable version "0.13.0", minimum required is "0.7.1")
-- Board: nrfmicro_13, Shield(s): jian_left
-- Cache files will be written to: /root/.cache/zephyr
-- ZEPHYR_TOOLCHAIN_VARIANT not set, trying to locate Zephyr SDK
-- Found host-tools: zephyr 0.13.2 (/opt/zephyr-sdk-0.13.2)
-- Found dtc: /opt/zephyr-sdk-0.13.2/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.6.0", minimum required is "1.4.6")
-- Found toolchain: zephyr 0.13.2 (/opt/zephyr-sdk-0.13.2)
-- Found BOARD.dts: /workspaces/zmk_fork/app/boards/arm/nrfmicro/nrfmicro_13.dts
-- Found devicetree overlay: /workspaces/zmk_fork/app/boards/shields/jian/jian_left.overlay
-- Found devicetree overlay: /workspaces/zmk-config/jian/config/jian.keymap
'debounce-period' is marked as deprecated in 'properties:' in /workspaces/zmk_fork/app/drivers/zephyr/dts/bindings/kscan/zmk,kscan-gpio-matrix.yaml for node /kscan.
-- Generated zephyr.dts: /workspaces/zmk_fork/build/jian_vu_keys/left/zephyr/zephyr.dts
-- Generated devicetree_unfixed.h: /workspaces/zmk_fork/build/jian_vu_keys/left/zephyr/include/generated/devicetree_unfixed.h
-- Generated device_extern.h: /workspaces/zmk_fork/build/jian_vu_keys/left/zephyr/include/generated/device_extern.h
-- Including generated dts.cmake file: /workspaces/zmk_fork/build/jian_vu_keys/left/zephyr/dts.cmake
Parsing /workspaces/zmk_fork/app/Kconfig
/workspaces/zmk_fork/zephyr/scripts/kconfig/kconfig.py: /workspaces/zmk_fork/app/zephyr/Kconfig:8: recursive 'source' of 'Kconfig.zephyr' detected. Check that environment variables are set correctly.
Include path:
/workspaces/zmk_fork/app/Kconfig:473
Kconfig.zephyr:33
modules/Kconfig:6
/workspaces/zmk_fork/build/jian_vu_keys/left/Kconfig/Kconfig.modules:2
/workspaces/zmk_fork/app/zephyr/Kconfig:8
CMake Error at /workspaces/zmk_fork/zephyr/cmake/kconfig.cmake:272 (message):
  command failed with return code: 1
Call Stack (most recent call first):
  /workspaces/zmk_fork/zephyr/cmake/app/boilerplate.cmake:543 (include)
  /workspaces/zmk_fork/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:24 (include)
  /workspaces/zmk_fork/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:35 (include_boilerplate)
  CMakeLists.txt:15 (find_package)


-- Configuring incomplete, errors occurred!
FATAL ERROR: command exited with status 1: /usr/local/bin/cmake -DWEST_PYTHON=/usr/bin/python3 -B/workspaces/zmk_fork/build/jian_vu_keys/left -S/workspaces/zmk_fork/app -GNinja -DBOARD=nrfmicro_13 -DSHIELD=jian_left -DZMK_CONFIG=/workspaces/zmk-config/jian/config
make: *** [Makefile:11: jian_vu_keys] Error 1

here is my config - https://github.com/alex-popov-tech/zmk-config/tree/master/jian/config

[urob]

@alex-popov-tech Not sure what's causing this or whether this is related to this PR. The ZMK discord https://discord.com/channels/719497620560543766/719909884769992755 might be able to provide better feedback on this

[alex-popov-tech]

just tested your branch @urob , it all working perfectly fine, even tho i have macros, combos, modmorph plain, modmorph with masked mods, and alltogether it works perfectly fine! is it possible to apply patch to this pr maybe?

[vrinek]

just tested your branch @urob , it all working perfectly fine, even tho i have macros, combos, modmorph plain, modmorph with masked mods, and alltogether it works perfectly fine! is it possible to apply patch to this pr maybe?

I'll try to make the time to get this done today.

[vrinek]

I just merged the latest main into this PR's branch. @urob, this is the only diff I see between this branch and yours:

diff --git a/app/src/hid.c b/app/src/hid.c
index 798801cd..1c903ca8 100644
--- a/app/src/hid.c
+++ b/app/src/hid.c
@@ -161,29 +161,29 @@ static inline int check_keyboard_usage(zmk_key_t usage) {
     }

 int zmk_hid_implicit_modifiers_press(zmk_mod_flags_t new_implicit_modifiers) {
-    zmk_mod_flags_t current = GET_MODIFIERS;
     implicit_modifiers = new_implicit_modifiers;
+    zmk_mod_flags_t current = GET_MODIFIERS;
     SET_MODIFIERS(explicit_modifiers);
     return current == GET_MODIFIERS ? 0 : 1;
 }

 int zmk_hid_implicit_modifiers_release() {
-    zmk_mod_flags_t current = GET_MODIFIERS;
     implicit_modifiers = 0;
+    zmk_mod_flags_t current = GET_MODIFIERS;
     SET_MODIFIERS(explicit_modifiers);
     return current == GET_MODIFIERS ? 0 : 1;
 }

 int zmk_hid_masked_modifiers_set(zmk_mod_flags_t new_masked_modifiers) {
-    zmk_mod_flags_t current = GET_MODIFIERS;
     masked_modifiers = new_masked_modifiers;
+    zmk_mod_flags_t current = GET_MODIFIERS;
     SET_MODIFIERS(explicit_modifiers);
     return current == GET_MODIFIERS ? 0 : 1;
 }

 int zmk_hid_masked_modifiers_clear() {
-    zmk_mod_flags_t current = GET_MODIFIERS;
     masked_modifiers = 0;
+    zmk_mod_flags_t current = GET_MODIFIERS;
     SET_MODIFIERS(explicit_modifiers);
     return current == GET_MODIFIERS ? 0 : 1;
 }

I do not think the above affects anything, but I am not familiar with C or this codebase much. Do you see something I do not?

[urob]

I just merged the latest main into this PR's branch. @urob, this is the only diff I see between this branch and yours. [...] Do you see something I do not?

Nope, the order shouldn't matter here. There is a minor formatting issue in app/include/zmk/hid.h. We both did resolve the merge conflict in that file by deleting the blank line that was added by @okke-formsma (57fca34dc0d2a8106678d311dd06559642dcb443) in main since @aumuell has written the fix. Obviously, this is not of any substance, but it probably makes sense to keep it for clarity (after the two new modifier-related lines).

[gaoDean]

Sorry for the inconvenience, but I’ve noticed that key-repeat doesn’t work with the swallowed key. Specifically, shift+key works, but then keyrepeat produces the uppercase version. Everything else works extremely well, thank you for this!

[vrinek]

Sorry for the inconvenience, but I’ve noticed that key-repeat doesn’t work with the swallowed key. Specifically, shift+key works, but then keyrepeat produces the uppercase version. Everything else works extremely well, thank you for this!

@gaoDean I am not sure I follow. Can you break this down to "keydown"/"keyup" events for me?

[gaoDean]

@vrinek

edit: got some things wrong edit: I think I have my shift on sticky-key-quick, don’t remember if this effect happens when holding shift

x = key (mod morph) ^ = keyup _ = key down

( the shift I am using below in the keydown/keyup is merely the key shift as bound to, not the actual modifier being pressed ) _shift, _x, ^x, ^shft -> key ( no shift ) skq version: _shift, ^shift, _x, ^x -> key ( no shift) _keyrepeat, ^keyrepeat -> shift + key

say I have mod morph with normal being dot, morph being slash. Shift (as the mod) + dot produces slash. After that, I press key repeat and it outputs question mark

[urob]

@vrinek @aumuell

A few quick comments on the PR.

1. I submitted a small "PR to the PR" that excludes implicit modifiers from getting masked. It enables users to re-include masked out mods in the mod-morph binding, which is (1) convenient (see @slashfoo 's review comment) and (2) enables binding macros that at some point (but not the entire time) use the masked-mods. As far as I see, there is no real downside given that LS(&my_mod_morph) is not a thing
2. Does it make sense to default masked_mods to mods if unspecified? It seems this would be the expected behavior of most mod-morph use cases, and users can overwrite it if wanted (and with the proposed change to implicit mods, there would also no unexpected implications when binding mods to the mod-morph)
3. I did a bit more exploring of @slashfoo 's question about the unused mod-trigger definition and recompiled excluding it. So far everything seems fine, raising the question whether it can indeed be deleted?
4. This PR seems now be used & tested by various people. What's needed at this point to push it forward?

[vrinek]

@urob

1. I submitted a small "PR to the PR" that excludes implicit modifiers from getting masked. It enables users to re-include masked out mods in the mod-morph binding, which is (1) convenient (see @slashfoo 's review comment) and (2) enables binding macros that at some point (but not the entire time) use the masked-mods. As far as I see, there is no real downside given that LS(&my_mod_morph) is not a thing

I am on vacation at the moment and can't test this on a physical device until late next week.

2. Does it make sense to default masked_mods to mods if unspecified? It seems this would be the expected behavior of most mod-morph use cases, and users can overwrite it if wanted (and with the proposed change to implicit mods, there would also no unexpected implications when binding mods to the mod-morph)

This sounds like a breaking change, no?

3. I did a bit more exploring of @slashfoo 's question about the unused mod-trigger definition and recompiled excluding it. So far everything seems fine, raising the question whether it can indeed be deleted?

Can you point me to the mod-trigger definition? https://github.com/vrinek/zmk/search?q=mod-trigger&type=code brings up no results.

4. This PR seems now be used & tested by various people. What's needed at this point to push it forward?

From what I know, test coverage is the only piece missing. See https://github.com/zmkfirmware/zmk/pull/1114#issuecomment-1066181448.

[urob]

This sounds like a breaking change, no?

Technically, yes, it does change the behavior of mod-morph. But I would argue that in the majority of cases what the uses wants is to indeed swallow the mods (see https://github.com/zmkfirmware/zmk/issues/683). Hence, I'd view it more a bug fix than a breaking change.

Can you point me to the mod-trigger definition? https://github.com/vrinek/zmk/search?q=mod-trigger&type=code brings up no results.

Sorry, I meant trigger_mods: https://github.com/vrinek/zmk/blob/89dac4c54bf0f679d79f1da27c069d30cc102581/app/src/behaviors/behavior_mod_morph.c#L49