Modifiers that are only on the base layer can get stuck

[pianohacker]

(This is on the ErgoDox, but I believe this is a tmk issue.)

So, I have a key which is LCTRL on layer 0 and BACKSPACE on layer 1, and another that is bound to ACTION_LAYER_MOMENTARY(1). If I hit the layer 1 key, then backspace, everything is kosher. However, if I press LCTRL, then layer 1, then release then release LCTRL, then layer 1, the LCTRL modifier bit stays stuck in the keyboard report. This makes sense, as del_mod_bit never gets called, as that keycode doesn't exist in that layer.

Is there some way to work around this? I'd rather not use tap modifiers unless I have to.

[tmk]

It is by design but I couldn't foresee your situation honestly. I can't come up with simple workaround unfortunately. This is indeed a limit of the design. I'll have to think about this for next design and look into how Ben's firmware handle this.

[BenBergman]

It has been a while since I looked at the source, so I'm not sure how easy this would be to implement, but could you somehow store the pressed key (LCTRL in this case) when a button is pressed and use this stored value to generate the release action instead of using the current layer map?

[pianohacker]

I have a workaround as a function key that is in the same place on both layers; it's not pretty but it works:

    case LCTRL_BSPACE:
        if (event->event.pressed) {
            if (layer_state & 2) {
                add_key(KC_BSPACE);
            } else {
                add_mods(MOD_BIT(KC_LCTRL));
            }
        } else {
            del_key(KC_BSPACE);
            del_mods(MOD_BIT(KC_LCTRL));
        }
        send_keyboard_report();
        break;

[fisofo]

I have this same issue, but a bit different: I have a momentary layer key that I hit with my thumb, then CTRL and other modifier keys I hit with my fingers, and if I am not careful and lift my thumb first, the modifier keys get stuck.

I unfortunately only know this code at a rudimentary layer; @pianohacker, would you be able to post a bit more context to your code so that I can give it a shot? It looks like it would do what I want.

[fisofo]

My apologies, I think I see how to do this after digging into the documentation a bit more. I'll post back if I can't get it sorted.

[fisofo]

@pianohacker: actually, if you'd be so kind to post your fork or something, that'd be helpful as I haven't been able to get your code working yet. I'm missing the definitions of layer_state, and the add/delete_key/mods functions.

thanks!

[pianohacker]

Let me clean it up a bit and I'll post it. It's a bit of a hack at the moment, it requires including several headers in the keymap.h and adding an extern to get at layer_state.

[fisofo]

That'd be awesome, thanks!
On May 4, 2014 11:35 PM, "Jesse Weaver" notifications@github.com wrote:

Let me clean it up a bit and I'll post it. It's a bit of a hack at the
moment, it requires including several headers in the keymap.h and adding an
extern to get at layer_state.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/105#issuecomment-42158636
.

[fisofo]

Soooo.... how's it going? :-)

[fisofo]

Thank you @pianohacker! For anyone else looking, I've updated my code with this, and it works quite well: https://github.com/fisofo/tmk_keyboard/blob/cub_layout/keyboard/ergodox/keymap_fisofo.h

Offtopic: @pianohacker: You may want to look at adjusting your debounce and CPU/clock though, as you can get much better performance by doing so! Here is what I am using:
https://github.com/fisofo/tmk_keyboard/blob/cub_layout/keyboard/ergodox/config.h
https://github.com/fisofo/tmk_keyboard/blob/cub_layout/keyboard/ergodox/twimaster.c

[pianohacker]

Dear lord, man. It's a night and day difference (now that I know to change DEBOUNCE in config.h). Thank you so much! I'll try to convince cubuanic to upstream these changes.

[fisofo]

Haha, you're welcome! :D

[cub-uanic]

as Hasu said, root of original problem is firmware design, and as for me - best solution will be to not assign different roles (usual key like Backspace and modifier like Control) to same key on different layers.

@pianohacker and @fisofo - could you please point me what exact parts you'd like to merge into my fork? last commit in @pianohacker repo is too big and have bunch of changes. will be better if you'll create pull request (to my repo) with only needed changes, and not including your layout files and so on.

[fisofo]

@cub-uanic I think actually your fork already has the modifications I was using.
@pianohacker I think you may have pulled the tmk branch directly, and missed the cub-layout, fyi!

[kejadlen]

@fisofo Thanks for the debounce example. Finally got around to applying it on my branch, and it's a far, far better experience.

[DreymaR]

My thought on this topic is: When do we need modifier states preserved upon a layer change?

I would think that we don't! When I change layers, at least temporarily, I could live with a clean modifier slate.

Therefore, making an ACTION_LAYER_MOMENTARY_CLEARMODS() or ACTION_LAYER_MOMENTARY_CM() action that activates a layer upon press and upon release not only deactivates the layer but also all mod states seems like a good solution to me. No more stuck modifiers, at a low cost.

[DreymaR]

A more fancy and robust way:

Add ACTION_LAYER_MOMENTARY_SRM() which is like the non-SRM version except it performs MOD_STORE on key down and MOD_RESTORE on key up (like the macro actions).

This way, we're guaranteed the same mod state when we leave our temp layer as we had before. This solution would be fully backwards compatible.

Alternatively, unless anyone can think of a counter-example to this being useful behavior, you might just add the SM/RM functionality to the old ACTION_LAYER_MOMENTARY(). More dramatic but simpler.

One caveat may be if something like a macro or other key wants to do SM/RM in that layer? There might have to be a special mod store/restore buffer for this layer switch, to avoid cannibalizing other buffers.

Modifiers may get stuck using other layer switching too, of course. But it seems to me that the ACTION_LAYER_MOMENTARY() switch is the most vulnerable and relevant in practice.

[tmk]

Fixed at ba2883f