Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Tri_Layer_State submodule to layers module #658

Closed
wants to merge 26 commits into from

Conversation

doesntfazer
Copy link
Contributor

TRI_LAYER_STATE

from kmk.modules.layers import Layers, Trilayerstate
keyboard.modules.append(Layers())
keyboard.modules.append(Trilayer())

#Keycodes

Key Description
KC.RAISE Switches to the default or defined Raise Layer.
KC.LOWER Switches to the default or defined Lower Layer.

Behavior

Tri-state layers is a feature that allows you to access a third layer by activating two other layers.

Changing Layers

By default, the default layer is 0, lower is 1, raise is 2, adjust is 3.
To change which layers are affected by the "lower raise adjust" add this just above your keymap.

Trilayer.place = [
    0, #default 'or the layer it goes to when you release all keys. for most it is 0'
    1, #lower 'or the 'KC.LOWER' key'
    2, #raise 'or the 'KC.LOWER' key'
    3, #adjust 'or the layer that is activated when lower or raise are pressed simultaneously'
    ]

Without Tri Layer State

For most usecases, the way you would create a "Tri_Layer_State would be to have KC.MO(x) KC.MO(y) and KC.MO(z) on opposing layer keys. This works out fine for most people. The issue you run into is if you press KC.MO(X), Then KC.MO(Y), to get to KC.MO(Z); then you release KC.MO(X), and continue holding KC.MO(Y), it stays in KC.MO(Z). To get back into KC.MO(Y) you need to release KC.MO(Y) and press it again.

With Tri Layer State

With Tri_Layer_state you can set opposing LOWER and RAISE keys. With the example above where you are not using tri layer state, when you release KC.MO(X) it will switch back to the state of KC.MO(Y). Each key acting independently.
With the way this is implimented in KMK (unlike in QMK) you can still access layer 3 independently.

Here is a Real world usecase:
You type a symbol on the raise layer, you have set enter to be on the lower layer. You quickly want to let go of raise and go to lower to press enter. You can do this as quickly as your finger releases raise with tri layer state. Without it, you could potentially hit lower before you let go of raise, causing you to be stuck on the adjust layer instead of lower.

Example code

import board

from kb import KMKKeyboard

from kmk.keys import KC, make_key
from kmk.modules.trilayerstate import Layers

keyboard = KMKKeyboard()

keyboard.modules.append(Layers())
keyboard.modules.append(Trilayer())

keyboard.keymap = [
    [ # QWERTY
KC.A,     KC.B,     KC.C,
KC.LOWER, KC.SPACE, KC.RAISE,

     ],
    [ # LOWER
KC.N1,    KC.N2,    KC.N3,
KC.LOWER, KC.SPACE, KC.RAISE,

     ],
    [ # RAISE
KC.EXLM,  KC.AT,    KC.HASH,
KC.LOWER, KC.SPACE, KC.RAISE,

     ],
     [ # ADJUST
KC.F1,    KC.F2,    KC.F3,
KC.LOWER, KC.SPACE, KC.RAISE

     ]
]

@doesntfazer
Copy link
Contributor Author

@xs5871 @DBendit I have made the code more efficient, I have also made it modular.

@xs5871
Copy link
Collaborator

xs5871 commented Dec 22, 2022

This version has the same issues as the one before:

  • hardcoded layer matching
  • single use, can't have multiple tri-layers
  • needs special keys for layer changes
  • doesn't integrate / breaks when combined with the existing layer module

@doesntfazer
Copy link
Contributor Author

doesntfazer commented Dec 27, 2022

  • hardcoded layer matching

This is not the case here. that was the point of Trilayer.place = [0, 1, 2, 3] in the keymap file.

  • doesn't integrate / breaks when combined with the existing layer module

This is not the case either. You can import the Trilayer module independently using from kmk.modules.layers import Layers, Trilayer and keyboard.modules.append(Trilayer())

  • single use, can't have multiple tri-layers
    needs special keys for layer changes

Wouldn't this be a bit more of a Want than a Need?

@doesntfazer
Copy link
Contributor Author

Debating on closing out this PR. I have been working on integrating this with the layer module, also doing the suggestion of @xs5871.

A global dictionary example:

trilayerstate = {
    (1, 2): 3,
    (1,3): 4,
}

Where the first 2 numbers behind the colon are the combo's and the last is the result of those combos.

I don't think asking for a Quadlayerstate is a reasonable ask. The logic to make this work is quite a bit more complicated than my minimal coding experience can handle.

A couple of question now,

  • if I can get a functional Multi Tri-layer-state working without the quad, what are the possibilities that it will be merged?
  • For the sake of futureproofing the name, since I'm sure quad layer states will be coming. Should we name this combo layer state, or even simply "Combo Layers" ?
  • I believe that it would be a good plan to make this feature have it's own arguement key. Something like KC.CL() for Combo Layer? I have everything in a "Mostly Working" state using the KC.MO() keys, but I am afraid that it could break other functions. It seems to make more sense to give it it's own argument keys. (As you said before though, @xs5871, you don't want it to have special keys for layer changes. But also you've said in the past, like in feat: add permissive_hold option to holdtap #653 "Feature parity is always good and welcome." So I'm not sure. You hold the keys to the castle. So please let me know the direction that I should be going.)

@xs5871
Copy link
Collaborator

xs5871 commented Dec 29, 2022

  • hardcoded layer matching

This is not the case here. that was the point of Trilayer.place = [0, 1, 2, 3] in the keymap file.

That wasn't what I meant, should have made it more clear. The part that checks
the active layer list if it should do something assumes that it's the only one
manipulating that list.
It'll only work if place[0] layer is always the base layer, for example. Or,
if any layer that is not in the place list gets activated, this module stops
working poperly.

  • doesn't integrate / breaks when combined with the existing layer module

This is not the case either. You can import the Trilayer module independently
using from kmk.modules.layers import Layers, Trilayer and
keyboard.modules.append(Trilayer())

The trilayer module uses a completely different heuristic to add and remove
active layers. It will interfere with the layer module, see previous point.

  • single use, can't have multiple tri-layers
    needs special keys for layer changes

Wouldn't this be a bit more of a Want than a Need?

Using the same keys and the same heuristic as the layer module is a need, not
a nice-to-have.
The ground work should be reasonably error and future proof. I don't want to
merge code that has obvious issues and will have to be replaced entirely if we
want to add features in the future.

@xs5871
Copy link
Collaborator

xs5871 commented Dec 29, 2022

Very quick answers:

  • if I can get a functional Multi Tri-layer-state working without the quad, what are the possibilities that it will be merged?

High, or at least not a merge blocker. Depends on the rest of the code, obviously, but that's something we can figure out together.

  • For the sake of futureproofing the name, since I'm sure quad layer states will be coming. Should we name this combo layer state, or even simply "Combo Layers" ?

"ComboLayers" or "LayerCombos", something like that. Both are more meaningfull than Trilayer or Multilayer.

  • I believe that it would be a good plan to make this feature have it's own arguement key.

No, here's why: Feature parity is not the same as copying APIs and naming schemes. Feature parity means you can achieve the same function/behaviour/etc. Doesn't mean it has to look the same.
I don't think that this specifically is a case of feature parity though. The idea behind using the same keys is, that it's less confusing for users which want to have both layers and combo layers, but don't understand how they are working under the hood.

@doesntfazer
Copy link
Contributor Author

Closing this PR and starting a new one for ComboLayers.

xs5871 added a commit that referenced this pull request Mar 10, 2023
sboysel pushed a commit to sboysel/kmk_firmware that referenced this pull request Apr 24, 2023
xs5871 added a commit that referenced this pull request Oct 17, 2023
hixan pushed a commit to hixan/kmk_firmware that referenced this pull request Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants