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

Update how default layers are stored/retrieved from EEPROM #22359

Closed
wants to merge 3 commits into from

Conversation

infinityis
Copy link
Contributor

Description

Currently QMK saves the default layer persistently in EEPROM as an 8-bit wide bit field. Only one byte is allocated for storing the defautl layer, and the current method allows for multiple default layers to be enabled simultaneously, but only for layers 0 through 7.

Pull request #21881 intends to allow default layers to range from 0 to 31, but that is currently not possible with the existing implementation. Functions like set_single_persistent_default_layer suggest that the more common use case is a single default layer. Indeed, in the entire QMK repo, only one user/keymap (datagrok) actually implements capability to save multiple default layers.

This PR attempts to unify these approaches, changing the default/presumed paradigm to a single default layer. However, the original method of storing and retrieving default layers as bit fields is still available conditionally with #define DEFAULT_LAYER_BITMASK_ENABLE

  • All bespoke implementations that save default layers by calling eeconfig_update_default_layer (only to set a single layer) have been replaced with calls to set_single_persistent_default_layer
  • DEFAULT_LAYER_BITMASK_ENABLE has been added to datagrok's files
  • EEPROM version number has been decremented

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).

@github-actions github-actions bot added core keyboard keymap documentation via Adds via keymap and/or updates keyboard for via support labels Oct 29, 2023
Copy link

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 bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Dec 14, 2023
Copy link

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]

@github-actions github-actions bot closed this Jan 14, 2024
@infinityis
Copy link
Contributor Author

infinityis commented Jan 17, 2024

Requesting reopen and review, rebased the PR'd branch on develop

@tzarc
Copy link
Member

tzarc commented Jan 17, 2024

Requesting reopen and review, rebased the PR'd branch on develop

You'll need to re-raise.

Screenshot 2024-01-17 at 7 56 33 pm 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core documentation keyboard keymap stale Issues or pull requests that have become inactive without resolution. via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants