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

Cap LM to 8 layers max in order to support right modifiers #13085

Closed
wants to merge 5 commits into from

Conversation

precondition
Copy link
Contributor

@precondition precondition commented Jun 2, 2021

Description

The current LM(layer, mod) is limited by memory constraints to max 16 layers (because the top 4 bits are used to store the layer) and only left modifiers (the remaining 4 bits are used for the GASC mod bits). Most people don't have more than 8 layers and would rather prefer to have the possibility to use right-hand modifiers in LM.

The amount of bits used to store the layer information is cut down to 3 bits and the freed bit is used as a LR flag for the modifiers (0 for left, 1 for right).

Akin to modtaps, only MOD_RSFT etc. are supported, not MOD_BIT(KC_RSFT) etc.

It's not possible to combine left and right modifiers in a single LM. If there is any right-handed modifier in the list, all the listed mods turn right-handed.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of this, but this ... rubs me the wrong way.

@precondition
Copy link
Contributor Author

I like the idea of this, but this ... rubs me the wrong way.

I understand where you're coming from, it does strike as a little odd to put a cap on max layers in order to "unlock" right hand modifiers in a quantum keycode.

Alternatively, we could consider making the 3 bits for layer and 5 bits for mods the new default for LM(layer, mod) and document the fact that it only supports up to layer 7.

I guess that would technically qualify as a breaking change in that case but I really believe that supporting right hand mods (especially for AltGr) is worth more than supporting up to layer 15.

@stale
Copy link

stale bot commented Jul 22, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale stale bot removed the awaiting changes label Jul 23, 2021
@precondition precondition changed the title Support for right mods in LM with 8bit layers Cap LM to 8 layers max in order to support right modifiers Mar 2, 2022
@precondition
Copy link
Contributor Author

@drashna I've reworked the PR to cap LM to 8 layers now as I have suggested, does this look better now? Now it means that the breaking changes label should be applied to my PR.

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, my biggest problem here is that it supports only 8 layers, and is a decrease from 16. Ideally, we should be targeting 16 layers, since that's what most of the other stuff like this targets. By limiting it to 8, it's creating an inconsistency of the limitations within the layer keycodes.

And this is made worse by the fact that 16 layers is the default max, right now.

From a technical perspective, it looks good. From a UX standpoint, I don't like it.

@drashna drashna requested a review from a team October 5, 2022 01:29
@sigprof
Copy link
Contributor

sigprof commented Oct 5, 2022

This change will also break compatibility with VIA (like #17989 did for TO(n) keycodes).

@sigprof sigprof mentioned this pull request Oct 8, 2022
17 tasks
@zvecr
Copy link
Member

zvecr commented Nov 5, 2022

Some of this is should be covered by the DD keycode migration. A slightly different implementation as we extended the range to align the mod usage with existing keycodes, while keeping the 16 layer cap.

Please let us know if we have missed anything in the process.

@zvecr zvecr closed this Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants