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

fallback keymap #1304

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fallback keymap #1304

wants to merge 3 commits into from

Conversation

tlyu
Copy link
Collaborator

@tlyu tlyu commented Oct 30, 2022

Make EEPROMSettings fall back to a usable key map if it detects a possibly incompatible EEPROM layout. This forces keymap.onlyCustom 0 and settings.defaultLayer 0, but does not write them to EEPROM.

A new settings.acceptInvalid Focus command may be used to accept the current settings as valid, updating the checksum.

Make `EEPROMSettings` fall back to a usable key map if it detects
a possibly incompatible EEPROM layout. This forces
`keymap.onlyCustom 0` and `settings.defaultLayer 0`, but does
not write them to EEPROM.

A new `settings.acceptInvalid` Focus command may be used to
accept the current settings as valid, updating the checksum.

Signed-off-by: Taylor Yu <code@argon.blue>
@tlyu
Copy link
Collaborator Author

tlyu commented Oct 30, 2022

This should probably not get merged until Chrysalis gets modified to do something reasonable when it encounters a keyboard in the fallback state. At least #1217 should be fixed.

Also, Chrysalis should do display a useful warning if it finds invalid settings. (It should check for mismatched CRCs in settings.crc; settings.valid? is basically useless for older firmware.) Ideally, it would give an opportunity to review the key layout, and possibly use the new settings.acceptInvalid command if the user approves, or else offer a factory reset.

Adapt previous to fit on the Atreus example sketch. Notably, this
makes `EEPROMKeymap::max_layers()` private so that it can be inlined.
Therefore, sketches like the `EEPROM-Keymap-Programmer` example sketch
that used that method needed to be adjusted. That method seemed obsolete
anyway, because it doesn't do some error checking that
`EEPROMKeymap::setup()` does.

Also, it removes the `settings.acceptInvalid` command, replacing it
with an optional argument to `settings.valid?`. Unfortunately, this
means that Chrysalis has no direct way to detect the presence of this
capability.

Signed-off-by: Taylor Yu <code@argon.blue>
@tlyu tlyu marked this pull request as draft October 30, 2022 23:24
@tlyu
Copy link
Collaborator Author

tlyu commented Oct 30, 2022

OK, this unfortunately breaks the keymap toggle magic combo on the Model 100, because it unconditionally resets the keymap to EEPROMSettings::ignoreHardcodedLayers() on each cycle.

Signed-off-by: Taylor Yu <code@argon.blue>
@tlyu
Copy link
Collaborator Author

tlyu commented Oct 31, 2022

This now exactly fits into flash on the Atreus. I had to remove the redundant Layer.move(EEPROMSettings.default_layer()) from the Atreus example sketch. I think we need to do something to free up flash on the Atreus if we want to implement a proper EEPROM directory.

@tlyu tlyu marked this pull request as ready for review October 31, 2022 00:17
@algernon algernon self-requested a review November 27, 2022 05:12
@algernon algernon self-assigned this Nov 27, 2022
@algernon algernon added the enhancement New feature or request label Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Waiting for review
Development

Successfully merging this pull request may close these issues.

2 participants