Achordion fix 2345 #74
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
SiriusStarr
requested changes
Oct 1, 2024
| @@ -67,8 +68,10 @@ static uint8_t achordion_state = STATE_RELEASED; | |||
| static void update_streak_timer(uint16_t keycode, keyrecord_t* record) { | |||
| if (achordion_streak_continue(keycode)) { | |||
| // We use 0 to represent an unset timer, so `| 1` to force a nonzero value. | |||
There was a problem hiding this comment.
Should probably ditch this unnecessary comment and leave one explaining your change, so we know what's going on next time upstream achordion changes.
| @@ -201,19 +200,20 @@ bool process_record_kb(uint16_t keycode, keyrecord_t *record) { | |||
| in_mod_tap_layer = get_highest_layer(layer_state); | |||
| layer_state = layer_state & ~(1 << in_mod_tap_layer); | |||
|
|
|||
| action_exec(record->event); | |||
|
|
|||
| // action_exec(record->event); | |||
There was a problem hiding this comment.
I'd prefer to not leave code commented out with no explanation, because it makes my brain nervous wondering if it's meant to be that way. 😆 I say purge it or explain it.
| @@ -57,6 +57,7 @@ | |||
| extern "C" { | |||
| #endif | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Is this accidental or just missed from the upstream? Probably want to minimize the diff if it's unnecessary, unless your formatter is yelling that much.
This fixes a rare bug in the achordion timer, by simplifying the logic. I'm not sure where the math is wrong. Just that it is. Also make 2345=67 more compatible with achordion to close up the final bug I found in the whole thing.
ilc
force-pushed
the
achordion-fix-2345
branch
from
d79c8c4 to
65a8379
Compare
|
All comments are addressed. I'm not that worried about forgetting about streaking. I'll probably upstream the logic later. |
SiriusStarr
approved these changes
Oct 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates achordion to the latest version, fixes a logic error in achordion, and also fixes a bug with 2345=67
With this code my board is now stable.