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.

Sign up for GitHub

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

Jump to bottom

Fix FORCE_NKRO handling #7601

Merged
merged 1 commit into from
Dec 11, 2019
Merged

Fix FORCE_NKRO handling #7601

merged 1 commit into from
Dec 11, 2019

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Dec 10, 2019

Description

Fixed Issues:

  1. Fix force nkro overwriting on matrix scan

  2. Fix first magic keycode overwriting force nkro

    • existing issue
    • when NKRO_FORCE, tmk_core/common/keyboard.c sets nkro value, which when the first magic keycode triggers eeconfig_read_keymap(), the previous nkro value in memory is lost.
    • changed to persist nkro config
      • this should be safe as eeprom will only write if value is different?

Types of Changes

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

Checklist

  • My code follows the code style of this project.
  • 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).

@zvecr zvecr requested a review from a team December 10, 2019 21:03
yanfali
yanfali approved these changes Dec 10, 2019
View reviewed changes
Copy link
Contributor

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

looks reasonable. not about the process_magic changes, but if you're not sure it's equivalent I'm ok with it

@zvecr
Copy link
Member Author

zvecr commented Dec 10, 2019

Hide whitespace changes makes it look a lot less meh. Its mostly due to the indentation changes, due to the if pressed logic wrapping everything instead of early exit.

@yanfali
Copy link
Contributor

yanfali commented Dec 10, 2019

Hide whitespace changes makes it look a lot less meh. Its mostly due to the indentation changes, due to the if pressed logic wrapping everything instead of early exit.

Ah ok, w=1 does make a lot more sense

@drashna drashna merged commit efb21c0 into qmk:master Dec 11, 2019
patrl pushed a commit to patrl/qmk_firmware that referenced this pull request Dec 29, 2019
@zvecr @patrl
Fix FORCE_NKRO handling (qmk#7601)
e4cbd2a
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Jan 2, 2020
@zvecr @drashna
Fix FORCE_NKRO handling (qmk#7601)
ef253c3
@drashna drashna mentioned this pull request Jan 2, 2020
Merged
fdidron added a commit to zsa/qmk_firmware that referenced this pull request Jan 6, 2020
@drashna @yanfali
@zvecr @manna-harbour @fauxpark @fdidron
Big keycode cleanup (#230)
6396419 
* clean up quantum.c (qmk#7485)

* idea

* progress

* more stuff

* wip

* wip

* last couple of keycodes you can move safely

* Update quantum/quantum.c

Co-Authored-By: fauxpark <[email protected]>

* Put back RGB_MODE_BREATHE

* Compile out some keycode processing when features are disabled (qmk#7506)

* Add shift-to-invert to remaining directional RGB_* keycode pairs (qmk#7484)

* Add shift-to-invert to remaining directional RGB_* keycode pairs

RGB_MODE_FORWARD / RGB_MODE_REVERSE invert their functions when shift is held.
This change adds the same capabilities to the remaining directional RGB_*
keycode pairs.  This improves consistency and provides full RGB control in a
keymap containing only one keycode from each pair.

* remove redundant variable

* fix typo

* Fix more typos

Flyspell is on now I swear!

* Relocate magic keycode processing (qmk#7512)

* Move magic keycode processing to own file

* Save some bytes

* Update comments

* Update define to one thats not already used...

* Fix audio

* Fix breathing toggle when rgb is disabled (qmk#7550)

* Ifdef MAGIC_EE_HANDS until #178 drops

revert this commit once it does

* Add short aliases for Magic keycodes (qmk#7541)

* Add short alias for `MAGIC_TOGGLE_NKRO`

* Add aliases for the other Bootmagic keycodes

* Replace long form in default keymaps

* Fix FORCE_NKRO handling (qmk#7601)

* Add until #173 drops

* Relocate RGB keycode processing (qmk#7508)

* Move rgb keycode logic to process_keycode

* Fixes for rgb matrix

* Fixes for mxss

* Fix inc/dec logic, add comments

* Fix return RAINBOW_SWIRL logic

* stop external use of rgb helper functions

* merge fix

* Fix 'defined but not used' when all animations are disabled

Co-authored-by: Yan-Fa Li <[email protected]>
Co-authored-by: Joel Challis <[email protected]>
Co-authored-by: Manna Harbour <[email protected]>
Co-authored-by: fauxpark <[email protected]>
Co-authored-by: Florian Didron <[email protected]>
fdidron added a commit to zsa/qmk_firmware that referenced this pull request Jan 8, 2020
@drashna @yanfali
@zvecr @manna-harbour @fauxpark @fdidron
Big keycode cleanup (#230)
493c675 
* clean up quantum.c (qmk#7485)

* idea

* progress

* more stuff

* wip

* wip

* last couple of keycodes you can move safely

* Update quantum/quantum.c

Co-Authored-By: fauxpark <[email protected]>

* Put back RGB_MODE_BREATHE

* Compile out some keycode processing when features are disabled (qmk#7506)

* Add shift-to-invert to remaining directional RGB_* keycode pairs (qmk#7484)

* Add shift-to-invert to remaining directional RGB_* keycode pairs

RGB_MODE_FORWARD / RGB_MODE_REVERSE invert their functions when shift is held.
This change adds the same capabilities to the remaining directional RGB_*
keycode pairs.  This improves consistency and provides full RGB control in a
keymap containing only one keycode from each pair.

* remove redundant variable

* fix typo

* Fix more typos

Flyspell is on now I swear!

* Relocate magic keycode processing (qmk#7512)

* Move magic keycode processing to own file

* Save some bytes

* Update comments

* Update define to one thats not already used...

* Fix audio

* Fix breathing toggle when rgb is disabled (qmk#7550)

* Ifdef MAGIC_EE_HANDS until #178 drops

revert this commit once it does

* Add short aliases for Magic keycodes (qmk#7541)

* Add short alias for `MAGIC_TOGGLE_NKRO`

* Add aliases for the other Bootmagic keycodes

* Replace long form in default keymaps

* Fix FORCE_NKRO handling (qmk#7601)

* Add until #173 drops

* Relocate RGB keycode processing (qmk#7508)

* Move rgb keycode logic to process_keycode

* Fixes for rgb matrix

* Fixes for mxss

* Fix inc/dec logic, add comments

* Fix return RAINBOW_SWIRL logic

* stop external use of rgb helper functions

* merge fix

* Fix 'defined but not used' when all animations are disabled

Co-authored-by: Yan-Fa Li <[email protected]>
Co-authored-by: Joel Challis <[email protected]>
Co-authored-by: Manna Harbour <[email protected]>
Co-authored-by: fauxpark <[email protected]>
Co-authored-by: Florian Didron <[email protected]>
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
@zvecr @HokieGeek
Fix FORCE_NKRO handling (qmk#7601)
0ebfb77
@zvecr zvecr deleted the feature/fix_force_nkro branch April 28, 2020 01:04
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@zvecr @drashna
Fix FORCE_NKRO handling (qmk#7601)
c5a64dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanfali yanfali yanfali approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
bug core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zvecr @yanfali @drashna