zmk icon indicating copy to clipboard operation
zmk copied to clipboard

when using mod-morph to make shift-backspace work as delete, the shift key is not 'swallowed'

Open depadiernos opened this issue 4 years ago • 25 comments

I set up mod-morph for the backspace key so that shift-backspace sends the delete keycode. Most operating systems have a keyboard shortcut for delete (e.g. shift-delete ) which prevents the delete key to work on its own.

As a user, I'd like to be able configure mod-morph where the shift is swallowed so the morphed keycode can be sent by itself.

Here is an example keymap

depadiernos avatar Feb 18 '21 04:02 depadiernos

I think the easiest implementation would be to make a "negative" version of implicit_modifiers (keycode_state_changed.c, hid.c), which turns a modifier off until the next keypress.

okke-formsma avatar Feb 24 '21 21:02 okke-formsma

Just adding a voice to this. Would love to set up Shift + comma as semicolon, but obviously the shift applies to the semicolon, producing a colon.

trankillity avatar Sep 02 '21 04:09 trankillity

agreed on that, would be very cool to see!

alex-popov-tech avatar Sep 17 '21 19:09 alex-popov-tech

Adding my voice to this issue with a couple of examples:

Say I wanna override shift+space to be gui+tab, I’d then in reality get shift+gui+tab where ideally, in my mind, I’d get exactly gui+tab as the morphed output.

It’s also a problem in other scenarios, say I wanna override 2 and 3 on my numpad layer to be , and . together with shift. Then that would output < and > as that’s the shifted version of , and .

metheon avatar Jan 22 '22 07:01 metheon

I am having a similar issue to what others here have mentioned.

As far as I understand, taking the comment right above me as an example (shift+2 to ,):

What we have

  1. shift down
  2. 2 down morphed into , down
  3. shift+, is what the computer sees, interpreting it as <
  4. 2 up morphed into , up
  5. shift up

What we would like

  1. shift down
  2. 2 down morphed into shift up, followed by , down
  3. , is what the computer sees, interpreting it as ,
  4. 2 up morphed into , up, followed by shift down
  5. shift up

Please correct me if I got this wrong

I looked into the code that looks to be responible for mod-morph (https://github.com/zmkfirmware/zmk/blob/main/app/src/behaviors/behavior_mod_morph.c), and my premature understanding is that there might be a limitation that's stopping this from being an easy fix. The behavior_mod_morph_driver_api struct maps one key press/release to another key press/release. From what I understand, we need one key press to send sequentially a key release and press, then the release has to send a key release and a press sequentially.

I don't see an easy way to achieve this, but I am not familiar with the ZMK codebase so I may be missing something obvious.

I would love to pick this one up, and given some guidance, I might be able to.

vrinek avatar Feb 02 '22 20:02 vrinek

For a while, I was using @aumuell’s patched version of mod-morph which allows for the held modifiers to be optionally “masked” (or “swallowed”):

  • masked mods: https://github.com/aumuell/zmk/commit/fa61c6a3c6e7cbfcb5b3d9168b3454bf95c

Here’s an example, with this version, that sends only PAGE_UP when LGUI + UP is pressed:

mm_up: mm_up {
  compatible = "zmk,behavior-mod-morph";
  label = "mm_up";
  #binding-cells = <0>;
  bindings = <&kp UP>, <&kp PAGE_UP>;
  mods = <(MOD_LGUI)>;
  masked_mods = <(MOD_LGUI)>; // don't send held modifier
};

At the time of writing, this above commit no longer merges cleanly with the latest version of ZMK main, but I feel it’s certainly worth sharing in this thread.

dxmh avatar Feb 02 '22 20:02 dxmh

Good news

I started working on this at https://github.com/vrinek/zmk/tree/modmorph-rebased-reverted, and got masked mod-morph working on my Corne-ish Zen.

I do not have any other ZMK keyboard to try this on, and the Zen depends on a fork of ZMK (https://github.com/LOWPROKB/zmk) which is behind main by a few commits. If someone has a board that works with the main branch of zmk, please give https://github.com/vrinek/zmk/tree/masked-mod-morphs-untested a try and let me know how it goes.

~~Bad news~~

~~To get this to work I had to revert a conflicting commit (https://github.com/vrinek/zmk/commit/3f81e3f6c8fa111d71de4a6d6388fc8e0cf5eee5, https://github.com/zmkfirmware/zmk/pull/1038 by @petejohanson). I am not able to understand what this commit does, and would need help (hopefully by @petejohanson) in order to get this unreverted.~~

I managed to unrevert it (even though I do not fully understand what it's doing), so the https://github.com/vrinek/zmk/tree/masked-mod-morphs-untested branch should be ready to open a PR on. I'll look into the contribution guidelines and get this done.

vrinek avatar Feb 03 '22 18:02 vrinek

Sorry for being so slow on following the discussion here, but if it's still of any value I just force-pushed a version rebased onto zmk/main to https://github.com/aumuell/zmk/tree/modmorph. From what I read in @vrinek's message above, I guessed that it should be virtually identical, but apparently I couldn't figure out with which branch I should compare.

Unfortunately, it's not PR-ready at all:

  • failing basic tests such as code formatting
  • no tests

aumuell avatar Feb 03 '22 20:02 aumuell

@aumuell I was probably opening my PR at the same time as you were typing your comment :laughing:

Please head over and take a look to make sure I did not mangle anything up. I am not familiar with this codebase.

Additionally, everyone in this issue is welcome to give my PR a test and submit a test review. It will be very helpful to move this forward.

vrinek avatar Feb 03 '22 20:02 vrinek

hey guys! thank you for your efforts, super happy to see its moving! i have jian and jorne on nrfmicro, willing to help testing! the only problem is im not super familiar with config syntax things, so im getting syntax error when trying to use it :) so if someone will be so kind to point me on my error, i will test that PR on my both keyboards with pleasure :)

here is config chunk:

// modmorph.dtsi

#include <dt-bindings/zmk/keys.h>
#define str(s) #s

#define MOD_MORPH(NAME, KEY, MOD_KEY) \
  mm_##NAME: mm_##NAME { \
    compatible = "zmk,behavior-mod-morph"; \
    label = str(NAME); \
    #binding-cells = <0>; \
    bindings = <&kp KEY>, <&kp MOD_KEY>; \
    mods = <(MOD_LSFT|MOD_RSFT)>; \
    masked_mods =  <(MOD_LSFT|MOD_RSFT)>; 
  }

MOD_MORPH(dot_col, DOT, SEMI)
MOD_MORPH(comma_pipe, COMMA, BSLH)
MOD_MORPH(lbkt_excl, LBKT, N1)
MOD_MORPH(rbkt_at, RBKT, N2)
MOD_MORPH(lcbr_hash, LBRC, N3)
MOD_MORPH(rcbr_dlr, RBRC, N4)
MOD_MORPH(lrbr_amps, LS(N9), AMPS)
MOD_MORPH(rrbr_star, LS(N0), STAR)

#undef MOD_MORPH

error:

Error: nrfmicro_13.dts.pre.tmp:783.1-15 syntax error
FATAL ERROR: Unable to parse input tree

chunk of nrfmicro_13.dts.pre.tmp:

/ {
  behaviors {
mm_dot_col: mm_dot_col { compatible = "zmk,behavior-mod-morph"; label = "dot_col"; #binding-cells = <0>; bindings = <&kp (((((0x07) << 16) | (0x37))))>, <&kp (((((0x07) << 16) | (0x33))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_comma_pipe: mm_comma_pipe { compatible = "zmk,behavior-mod-morph"; label = "comma_pipe"; #binding-cells = <0>; bindings = <&kp ((((0x07) << 16) | (0x36)))>, <&kp (((((0x07) << 16) | (0x31))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_lbkt_excl: mm_lbkt_excl { compatible = "zmk,behavior-mod-morph"; label = "lbkt_excl"; #binding-cells = <0>; bindings = <&kp (((((0x07) << 16) | (0x2F))))>, <&kp (((((0x07) << 16) | (0x1E))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_rbkt_at: mm_rbkt_at { compatible = "zmk,behavior-mod-morph"; label = "rbkt_at"; #binding-cells = <0>; bindings = <&kp (((((0x07) << 16) | (0x30))))>, <&kp (((((0x07) << 16) | (0x1F))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_lcbr_hash: mm_lcbr_hash { compatible = "zmk,behavior-mod-morph"; label = "lcbr_hash"; #binding-cells = <0>; bindings = <&kp (((0x02 << 24 | (((0x07) << 16) | (0x2F)))))>, <&kp (((((0x07) << 16) | (0x20))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_rcbr_dlr: mm_rcbr_dlr { compatible = "zmk,behavior-mod-morph"; label = "rcbr_dlr"; #binding-cells = <0>; bindings = <&kp (((0x02 << 24 | (((0x07) << 16) | (0x30)))))>, <&kp (((((0x07) << 16) | (0x21))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_lrbr_amps: mm_lrbr_amps { compatible = "zmk,behavior-mod-morph"; label = "lrbr_amps"; #binding-cells = <0>; bindings = <&kp (0x02 << 24 | (((((0x07) << 16) | (0x26)))))>, <&kp (((0x02 << 24 | (((0x07) << 16) | (0x24)))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }
mm_rrbr_star: mm_rrbr_star { compatible = "zmk,behavior-mod-morph"; label = "rrbr_star"; #binding-cells = <0>; bindings = <&kp (0x02 << 24 | (((((0x07) << 16) | (0x27)))))>, <&kp (((0x02 << 24 | (((0x07) << 16) | (0x25)))))>; mods = <(0x02|0x20)>; masked_mods = <(0x02)>; }

alex-popov-tech avatar Feb 04 '22 08:02 alex-popov-tech

~~@alex-popov-tech I am not 100% sure but I think the problem is that this behavior needs to be called grave_escape. Please give the following a try:~~

 #define MOD_MORPH(NAME, KEY, MOD_KEY) \
-  mm_##NAME: mm_##NAME { \
+  mm_##NAME: grave_escape { \
     compatible = "zmk,behavior-mod-morph"; \
     label = str(NAME); \
     #binding-cells = <0>; \
     bindings = <&kp KEY>, <&kp MOD_KEY>; \
     mods = <(MOD_LSFT|MOD_RSFT)>; \
     masked_mods =  <(MOD_LSFT|MOD_RSFT)>; 
   }

Please disregard the above, I have verified that it is not necessary.

vrinek avatar Feb 04 '22 10:02 vrinek

tested on jorne nrfmicro_13 with following:

mm_test: mm_test { \
  compatible = "zmk,behavior-mod-morph"; \
  label = "test"; \
  bindings = <&kp DOT>, <&kp DOT>; \
  #binding-cells = <0>; \
  mods = <(MOD_LSFT|MOD_RSFT)>; \
  masked_mods = <(MOD_LSFT|MOD_RSFT)>; \
};

working as expected - without shift dot, with shift also dot

alex-popov-tech avatar Feb 04 '22 18:02 alex-popov-tech

This bug can be somewhat worked around with a macro that releases the shift.

Here's a working example that changes SHIFT + , into ;:

my_modmorph: my_modmorph {
  compatible = "zmk,behavior-mod-morph";
  label = "my_modmorph";
  #binding-cells = <0>;
  bindings = <&kp COMMA>, <&macro_semicolon>;
  mods = <(MOD_LSFT)>;
};
macro_semicolon: macro_semicolon {
  compatible = "zmk,behavior-macro";
  label = "macro_semicolon";
  #binding-cells = <0>;
  wait-ms = <30>;
  tap-ms = <40>;
  bindings = <&macro_release &kp LSHFT>,
    <&macro_tap &kp SEMICOLON>;
};

The main caveat is that shift is released even if you continue to physically hold it – meaning shift + repeated taps of , results in only a single semicolon followed by commas (e.g. ;,,,,,,).

dxmh avatar May 05 '22 20:05 dxmh

I found another workround that works as far as I can tell, also it uses a macro

This example changes shift + 0 into .:

     ZMK_MACRO(dot_dot,
        wait-ms = <0>;
        tap-ms = <0>;
        bindings
        = <&macro_release &kp LSHFT>
        , <&macro_tap &kp DOT>
        , <&macro_press &kp LSHFT>
        , <&macro_pause_for_release>;)

     odotw: zero_dotw {
          compatible = "zmk,behavior-mod-morph";
          label = "ZERO_DOTW";
          #binding-cells = <0>;
          bindings = <&kp N0>, <&dot_dot>;
          mods = <(MOD_LSFT)>;
     }; 

caveats: no hold repeat for morphed keys, sticky keys are locked.

AlaaSaadAbdo avatar May 05 '22 20:05 AlaaSaadAbdo

thank you guys, i copied your approach like this https://github.com/alex-popov-tech/zmk-config/blob/master/dactyl/config/boards/shields/manuform/modmorph.dtsi#L13-L28

alex-popov-tech avatar May 07 '22 19:05 alex-popov-tech

thank you guys, i copied your approach like this https://github.com/alex-popov-tech/zmk-config/blob/master/dactyl/config/boards/shields/manuform/modmorph.dtsi#L13-L28

I wanted to thank you for this macro. (Don't forget to grab Line 11). This works fine if you're using the standard shift key, but gets stuck with shift pressed if you use Sticky Keys since no 'release' event is sent afterward.

philipn7 avatar Jun 26 '22 20:06 philipn7

thank you guys, i copied your approach like this https://github.com/alex-popov-tech/zmk-config/blob/master/dactyl/config/boards/shields/manuform/modmorph.dtsi#L13-L28

I wanted to thank you for this macro. (Don't forget to grab Line 11). This works fine if you're using the standard shift key, but gets stuck with shift pressed if you use Sticky Keys since no 'release' event is sent afterward.

I have no issues with sticky keys using the masked-mods fix by @aumuell. If you don't want to deal with merging the PR into your own zmk branch/don't want to build locally, there is an easy way to use the PR with Github actions using the beta-testing feature.

urob avatar Jun 26 '22 20:06 urob

@urob Thank you so much for pointing me in that direction! It works great! This should definitely be included in ZMK.

I'm new to ZMK and Github Actions for that matter. If anyone else wants to try the masked-mods branch I'll add the steps I've taken here:

Change your config/west.yml file to include the repo from @urob :

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

Add masked_mods = <(MOD_LSFT)>; to your Mod Morph. I used macros for masked and unmasked mod morphs:

#define MOD_MORPH_MASKED(NAME, KEY, MOD_KEY) \
  mm_##NAME: mm_##NAME { \
    compatible = "zmk,behavior-mod-morph"; \
    label = #NAME; \
    #binding-cells = <0>; \
    bindings = <&kp KEY>, <&kp MOD_KEY>; \
    mods = <(MOD_LSFT|MOD_RSFT)>; \
    masked_mods = <(MOD_LSFT|MOD_RSFT)>; \
  };

#define MOD_MORPH(NAME, KEY, MOD_KEY) \
  mm_##NAME: mm_##NAME { \
    compatible = "zmk,behavior-mod-morph"; \
    label = #NAME; \
    #binding-cells = <0>; \
    bindings = <&kp KEY>, <&kp MOD_KEY>; \
    mods = <(MOD_LSFT|MOD_RSFT)>; \
  };

/ {
    behaviors {
      MOD_MORPH_MASKED(brackets, LEFT_BRACKET, RIGHT_BRACKET)
      MOD_MORPH(braces, LEFT_BRACE, RIGHT_BRACE)
      MOD_MORPH(lt_gt, LESS_THAN, GREATER_THAN)
      MOD_MORPH(parens, LEFT_PARENTHESIS, RIGHT_PARENTHESIS)
      MOD_MORPH_MASKED(slashes, SLASH, BACKSLASH)
      MOD_MORPH(minus_plus, MINUS, PLUS)
    };
};

philipn7 avatar Jun 26 '22 21:06 philipn7

I tried out @aumuell's masked-mods fix, and everything (including key repeat) seemed fine. Unfortunately, xev says that although each key press event isn't modified, each key release is. This didn't seem to effect typing anywhere, but when I tried to bind the key in a game, it still acted as though the modifier was being pressed. I can get it to send an unmodified key release by pressing the mod, pressing the masked-mods key, releasing the mod, then releasing the key.

Just wondering if anyone who's using the fix can reproduce this, or if I just happened to make a mistake somewhere.

CairnThePerson avatar Jul 10 '22 16:07 CairnThePerson

I tried out @aumuell's masked-mods fix, and everything (including key repeat) seemed fine. Unfortunately, xev says that although each key press event isn't modified, each key release is. This didn't seem to effect typing anywhere, but when I tried to bind the key in a game, it still acted as though the modifier was being pressed. I can get it to send an unmodified key release by pressing the mod, pressing the masked-mods key, releasing the mod, then releasing the key.

Just wondering if anyone who's using the fix can reproduce this, or if I just happened to make a mistake somewhere.

I think this is intended. masked-mods turns off the modifier for the duration of the triggered behavior but then re-applies it when it is released (if the mod is still held).

urob avatar Jul 17 '22 01:07 urob

I think this is intended. masked-mods turns off the modifier for the duration of the triggered behavior but then re-applies it when it is released (if the mod is still held).

Hm, alright. I think that makes sense for typing and text editing, but it's very unexpected behavior where key release matters. Would it interfere with anything if it were changed to also include key releases? Or could it at least be an option when configuring?

CairnThePerson avatar Jul 17 '22 02:07 CairnThePerson

I think this is intended. masked-mods turns off the modifier for the duration of the triggered behavior but then re-applies it when it is released (if the mod is still held).

Hm, alright. I think that makes sense for typing and text editing, but it's very unexpected behavior where key release matters. Would it interfere with anything if it were changed to also include key releases? Or could it at least be an option when configuring?

I assume you are suggesting we don't reapply the modifier at the end. I think this would interfere with the following scenario (assuming a masked shift+A -> B:

Action PR behavior suggested behavior
shift down shift down shift down
shift shift
A down shift up, B down shift up, B down
B B
A up B up, shift down B up
shift
C down C down C down
shift+C C

@CairnThePerson Does the above make sense? Is this what you are suggesting?

I have no idea if we should consider my scenario above as more or less of an edge case compared to the gaming scenario you brought forth (personally, I have a gaming-specific layer).

vrinek avatar Jul 22 '22 16:07 vrinek

I think this is intended. masked-mods turns off the modifier for the duration of the triggered behavior but then re-applies it when it is released (if the mod is still held).

Hm, alright. I think that makes sense for typing and text editing, but it's very unexpected behavior where key release matters. Would it interfere with anything if it were changed to also include key releases? Or could it at least be an option when configuring?

I assume you are suggesting we don't reapply the modifier at the end. I think this would interfere with the following scenario (assuming a masked shift+A -> B: Action PR behavior suggested behavior shift down shift down shift down shift shift A down shift up, B down shift up, B down B B A up B up, shift down B up shift C down C down C down shift+C C

@CairnThePerson Does the above make sense? Is this what you are suggesting?

I have no idea if we should consider my scenario above as more or less of an edge case compared to the gaming scenario you brought forth (personally, I have a gaming-specific layer).

Let me fill in some blanks from an exchange we had on discord. Using your example above, it seems that in some cases the order of B up and shift down upon behavior release is reversed (I can't reproduce that with my build). In either case, during normal typing flow this goes unnoticed, but apparently it matters for some applications.

I don't quite understand how that can be as it seems that the mod-morph binding is released before the mods are reinstated: https://github.com/vrinek/zmk/blob/89dac4c54bf0f679d79f1da27c069d30cc102581/app/src/behaviors/behavior_mod_morph.c#L69:L70

Hopefully, someone else with a better understanding of the code base can see through this.

urob avatar Jul 22 '22 16:07 urob

Yeah, as @urob explained, it's actually unintended behavior with my board. In your chart @vrinek, I was expecting my board to act like the PR behavior column. But instead of B up, shift down, I'm getting Shift down, B up.

On the Discord, context is in the "Masked mods key release" thread under the #development channel. I'm happy to try things out to get this resolved.

CairnThePerson avatar Jul 22 '22 16:07 CairnThePerson

Yeah, as @urob explained, it's actually unintended behavior with my board. In your chart @vrinek, I was expecting my board to act like the PR behavior column. But instead of B up, shift down, I'm getting Shift down, B up.

On the Discord, context is in the "Masked mods key release" thread under the #development channel. I'm happy to try things out to get this resolved.

I think I found the issue. This should fix it: https://github.com/urob/zmk/tree/fix-mod-morph.

urob avatar Jul 30 '22 21:07 urob