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

refactor: move default RGB/LED matrix #defines #21938

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

lesshonor
Copy link
Contributor

Description

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to leverage these defaults without requiring #ifdef guards (and often repeating said fallback definitions).

...also corrected a typo in the console output for led_matrix_set_flags_eeprom_helper()/rgb_matrix_set_flags_eeprom_helper().


Generated SHAs for default and via keymaps using qmk mass-compile -f 'exists(led_matrix.driver)' -e SKIP_VERSION=yes and qmk mass-compile -f 'exists(rgb_matrix.driver)' -e SKIP_VERSION=yes (...plus teleport/native's perfmode keymaps) to verify this would not change existing behavior.

Out of 1396, there were 9 mismatches:

  1. junco/rev1:default, keebio/convolution/rev1:default, keychron/v3/iso:default,keychron/v3/iso:via, moonlander:default, moonlander:via all have console enabled, so were impacted by the typo fix.
  2. kbdfans/kbd67/mkiirgb/v3:default, kbdfans/kbd67/mkiirgb/v3:via and zykrah/fuyu:via all manipulate a caps lock indicator based on RGB_MATRIX_MAXIMUM_BRIGHTNESS/RGB_MATRIX_VAL_STEP.
    The way the existing code was written, the indicators ended up with a value step of 8 while the rest of the lights defaulted to 16. It seems clear that the intent was to scale the indicator with everything else, so the affected keymaps/keyboard were modified accordingly. @zykrah @moyi4681

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

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and
RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to
leverage these defaults without requiring #ifdef guards (and often
repeating said fallback definitions).
@github-actions github-actions bot added core keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Sep 6, 2023
quantum/led_matrix/led_matrix.h Outdated Show resolved Hide resolved
quantum/rgb_matrix/rgb_matrix.h Outdated Show resolved Hide resolved
and remove redundant UINT8_MAX checks on maximum brightness

Co-authored-by: Joel Challis <[email protected]>
Copy link
Contributor

@dexter93 dexter93 left a comment

Choose a reason for hiding this comment

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

this has been a long time coming. Yes please

@drashna drashna requested review from fauxpark and a team September 14, 2023 06:46
@zvecr zvecr merged commit 346b06d into qmk:develop Sep 28, 2023
@lesshonor lesshonor deleted the refactor/led_rgb_matrix_macro_defs branch September 29, 2023 11:16
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Oct 25, 2023
* refactor: move default RGB/LED matrix #defines

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and
RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to
leverage these defaults without requiring #ifdef guards (and often
repeating said fallback definitions).

* style: use if(n)def for consistency

and remove redundant UINT8_MAX checks on maximum brightness

Co-authored-by: Joel Challis <[email protected]>

* refactor: remove INDICATOR_MAX_BRIGHTNESS macro

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
christrotter pushed a commit to christrotter/qmk_firmware that referenced this pull request Nov 28, 2023
* refactor: move default RGB/LED matrix #defines

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and
RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to
leverage these defaults without requiring #ifdef guards (and often
repeating said fallback definitions).

* style: use if(n)def for consistency

and remove redundant UINT8_MAX checks on maximum brightness

Co-authored-by: Joel Challis <[email protected]>

* refactor: remove INDICATOR_MAX_BRIGHTNESS macro

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
zgagnon pushed a commit to zgagnon/qmk_firmware_waterfowl that referenced this pull request Dec 15, 2023
* refactor: move default RGB/LED matrix #defines

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and
RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to
leverage these defaults without requiring #ifdef guards (and often
repeating said fallback definitions).

* style: use if(n)def for consistency

and remove redundant UINT8_MAX checks on maximum brightness

Co-authored-by: Joel Challis <[email protected]>

* refactor: remove INDICATOR_MAX_BRIGHTNESS macro

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
future-figs pushed a commit to future-figs/qmk_firmware that referenced this pull request Dec 27, 2023
* refactor: move default RGB/LED matrix #defines

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and
RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to
leverage these defaults without requiring #ifdef guards (and often
repeating said fallback definitions).

* style: use if(n)def for consistency

and remove redundant UINT8_MAX checks on maximum brightness

Co-authored-by: Joel Challis <[email protected]>

* refactor: remove INDICATOR_MAX_BRIGHTNESS macro

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
mute-civilian pushed a commit to mute-civilian/qmk_firmware that referenced this pull request Feb 17, 2024
* refactor: move default RGB/LED matrix #defines

Moving the fallback definitions of macros like LED_MATRIX_VAL_STEP and
RGB_MATRIX_MAXIMUM_BRIGHTNESS to header files allows keyboards to
leverage these defaults without requiring #ifdef guards (and often
repeating said fallback definitions).

* style: use if(n)def for consistency

and remove redundant UINT8_MAX checks on maximum brightness

Co-authored-by: Joel Challis <[email protected]>

* refactor: remove INDICATOR_MAX_BRIGHTNESS macro

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants