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

Changing a "Layer shift when held" layer via the "Layers" tab often fails. #1338

Closed
parke opened this issue Apr 22, 2024 · 8 comments
Closed
Labels
bug Something isn't working

Comments

@parke
Copy link

parke commented Apr 22, 2024

Describe the bug
If a key has a secondary action of Layer shift, then attempting to change layer the via the LAYERS tab in Chrysalis often corrupts the configuration of the key.

To Reproduce

  1. Click on the KEYBOARD tab.
  2. Set a key to E (or to any other letter, it doesn't matter).
  3. Click on the MODIFIERS tab.
  4. Set the key's Secondary action to Layer shift and select a layer.
  5. Click on the LAYERS tab.
  6. Change the layer.
  7. The key's configuration will be corrupted. (If corruption does not happen on the first change, keep on changing the layer until corruption occurs. Sometime it takes several changes. As an example, a corrupted key will display something like: #51220.)

Expected behavior
The key's layer should be changed.

Desktop

  • OS: Linux, Chromium
  • Chrysalis Version: 2024.0318.1913
@parke parke added the bug Something isn't working label Apr 22, 2024
@obra
Copy link
Member

obra commented Apr 22, 2024 via email

@parke
Copy link
Author

parke commented Apr 22, 2024

My keyboard's firmware version is: 0.92.6+116.

Would you like me to do a factory reset and try to reproduce the issue?

Also, issue #1253 may be related.

@parke
Copy link
Author

parke commented Apr 22, 2024

Fyi, I haven't saved the corrupted configuration to the keyboard. I just see the corruption displayed in Chrysalis.

@obra
Copy link
Member

obra commented Apr 22, 2024 via email

@parke
Copy link
Author

parke commented Apr 22, 2024

Only the key I am trying to change is corrupted.

I've seen both #51220 and #51221. I'm assuming a bit mask is being manipulated incorrectly.

Also, once the key is corrupted, the options in the LAYERS tab change. The Type (of the layer change) is set to None. And the type cannot be changed back to Layer shift when held, because that option is disabled (as in #1253). Also, the letter assigned to the key is forgotten, so I have to reset it, too.

When I select layer 1, it corrupts to #51219.
When I select layer 2, it corrupts to #51219.
When I select layer 3, it corrupts to #51220.
When I select layer 4, it changes the key's letter from 'e' to 'a' (and does not change the layer).
When I select layer 5, it changes the key's letter from 'e' to 'b'.
When I select layer 6, it changes the key's letter from 'e' to 'c'.
When I select layer 7, it changes the key's letter from 'e' to 'd'.
When I select layer 8, it changes the key's letter back to 'e'.

Seems deterministic.

@TrueFalcon
Copy link

There is a bug here, but it is not what you think it is. First, I will tell you what the programmer intended on each tab.

The Keyboard tab should set as a primary action the letter you have selected, overwriting whatever is already programmed. This always works, even if a letter is already set with a secondary action. In this case it will wipe out the secondary action as well.

The Modifiers tab should add a secondary action to a programmed key. This works as long as the key is not set to Blocked or Transparent. Note that in this case, Secondary action is grayed out and unsettable. This is how it should and does work.

The Layers tab should set as a PRIMARY action whatever you select, OVERWRITING whatever is already programmed. This only 'appears' to work properly when the key does not have a secondary action or if it is Blocked or Transparent. In that case, it sets what you want as a PRIMARY action. Note if you now look at the Modifiers tab Secondary action is grayed out. Chrysalis is not clearing the key before attempting to set a new primary action in this tab, but sometimes it can get away with it. However if the key is a letter with secondary action, you get the unwanted results you have laid out.

There is a workaround here, when you go to the Layers tab, click Layer shift when held and then change the layer. Now you can properly set a Primary layer shifting action. Chrysalis should not allow you to attempt to modify a secondary action on this tab.

In summary, Modifiers sets a Secondary action, Layers sets a Primary action. Never the twain should meet ... but sometimes they do!

@obra obra closed this as completed in 6354fa9 May 1, 2024
@obra
Copy link
Member

obra commented May 1, 2024

I've just pushed up a fix to the "corruption" issue. It's not currently straightforward to set a key to be a layer shift key that is also a dual-use layer key because, iirc, the "which key do we shift to" data is going to end up in the same place in the keymap data structure.

@parke
Copy link
Author

parke commented May 3, 2024

Key corruption is still possible. To corrupt a key:

  1. Select a key with a secondary layer shift.
  2. Go to the Layers tab.
  3. Change the layer to any layer above layers 0-7.

Aside: Regarding:

It's not currently straightforward ...

Actually, I believe it is impossible, at least via Chrysalis, due to the design of the UI. And I believe it always has been impossible via Chrysalis.

... to set a key to be a layer shift key that is also a dual-use layer key because, iirc, the "which key do we shift to" data is going to end up in the same place in the keymap data structure.

I assume you mean: "which layer do we shift to"?

The storage locations may overlap, but I believe they are not identical, as I believe you can primary shift to layers 0-31, whereas you can secondary shift only to layers 0-7.

In any case, I have never wanted (nor even imagined) configuring a single key to perform multiple layer shifts. Also, I believe such a configuration is only logically possible if it combines: a one-shot-tap-shift to layer A with a hold-shift to layer B.

obra added a commit that referenced this issue May 15, 2024
- Added a `limits` object with `secondaryActionLayerLimit` to `constants` in `constants.js` to define the maximum layer limit for secondary actions due to technical constraints in Kaleidoscope.
- Updated `LayerKeys.js` and `SecondaryFunction.js` to use `secondaryActionLayerLimit` from `constants` for enforcing layer limits on dual-use keys, improving code maintainability and clarity.
- Disabled layer selection beyond the limit in the UI for better user guidance in `LayerKeys.js`.

Fixes #1338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants