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

Check if the focus command is supported when doing backup #132

Conversation

HomerSp
Copy link

@HomerSp HomerSp commented Nov 24, 2020

As per #128, the firmware update can fail with "because the settings could not be saved" when you are on an older firmware. This is because the old firmwares didn't support certain command (specifically macros.map) and due to the way the focus api works it would only return an empty string causing errorFlag to be set to true.
This PR fixes that by using the very handy "help" command to check what commands are supported before doing the update, and skipping the ones it can't find.

I've tested the fix by downgrading to the firmware in the release/0.1.2 branch and tried to upgrade to latest. Without this fix it will throw the "settings could not be saved" error.

As an expansion of this PR, I feel like the Focus class should probably get the supported commands in the constructor, and do all the checking in there instead and possibly add a specific exception just for that (or a return value you can check for). This PR however is just a simple fix to get around the issue.

…ware update - this fixes Dygmalab#128.

Signed-off-by: Mathias Tillman <master.homer@gmail.com>
@AlexDygma AlexDygma merged commit d8eda6c into Dygmalab:development Nov 25, 2020
@AlexDygma AlexDygma linked an issue Nov 25, 2020 that may be closed by this pull request
@AlexDygma AlexDygma self-requested a review November 25, 2020 10:20
@AlexDygma AlexDygma added this to the 0.2.5 milestone Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard Firmware Update Error
2 participants