keyboardio / Chrysalis

Graphical configurator for Kaleidoscope-powered keyboards
https://github.com/keyboardio/Chrysalis#chrysalis
GNU General Public License v3.0
493 stars 65 forks source link

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

Closed parke closed 3 months ago

parke commented 3 months ago

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

obra commented 3 months ago

Hi! Can you confirm what keyboard you're using and which firmware version you're running on that keyboard?

On Sun, Apr 21, 2024 at 6:56 PM parke @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/keyboardio/Chrysalis/issues/1338, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2B6TFX2EMU6MEH75JTY6RU3LAVCNFSM6AAAAABGR4GBGGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TKNBSGI4TCNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

parke commented 3 months ago

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 commented 3 months ago

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

obra commented 3 months ago

If you make the same change, is the "wrong" number always the same? This may be a key identifier database bug. Also, is it only that one key that's showing wrong? If so, it may be worth trying to save it and seeing if it works - is the issue a display issue in Chrysalis or is it setting the wrong keycode for kaleidoscope?

A factory reset probably won't affect this, but it is surprising that making the same change in Chrysalis has non-deterministic behavior even without saving to the keyboard.

On Sun, Apr 21, 2024 at 7:11 PM parke @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/keyboardio/Chrysalis/issues/1338#issuecomment-2068361457, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2HG4OPCMLLTF5LZOALY6RWWPAVCNFSM6AAAAABGR4GBGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYGM3DCNBVG4 . You are receiving this because you commented.Message ID: @.***>

parke commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.