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

Change led to led_matrix in rgb_matrix_drivers #9412

Merged
merged 3 commits into from
Jun 20, 2020
Merged

Conversation

drashna
Copy link
Member

@drashna drashna commented Jun 14, 2020

Is a minor change that only affects the driver file.

However, this will allow somebody to run rgblight along side rgb matrix
using the ws2812 driver, as well. Specifically, so you can use the
custom driver for rgblight to set a different pin (barring a change to
the ws2812_setleds function).

Courtesy of discord conversion:
https://discordapp.com/channels/440868230475677696/568161140534935572/721555623191248906

Specifically, the issue is that both rgblight and rgb matrix use an array named led to hold the LED array before sending it to the hardware. This creates a conflict. Renaming the led array in rgb matrix allows for coexistence.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization

Checklist

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

Is a minor change that only affects the driver file. 

However, this will allow somebody to run rgblight along side rgb matrix
using the ws2812 driver, as well.  Specifically, so you can use the
custom driver for rgblight to set a different pin (barring a change to
the `ws2812_setleds` function).  

Courtesy of discord conversion:
https://discordapp.com/channels/440868230475677696/568161140534935572/721555623191248906
@skullydazed
Copy link
Member

We already have an led matrix feature, and I fear this could create confusion. Is there a different name that could be used here?

@drashna
Copy link
Member Author

drashna commented Jun 15, 2020

We already have an led matrix feature, and I fear this could create confusion. Is there a different name that could be used here?

Good catch.

I've updated the name to be ... well, super specific, so that there shouldn't be any issues with name collisions.

@sigprof
Copy link
Contributor

sigprof commented Jun 15, 2020

The ws2812_led_array name does not look “super specific” — it could equally well apply to the usage in the rgblight code (although that part of rgblight code is not really specific to ws2812). Maybe something like rgbmatrix_ws2812_led would be better?

@drashna drashna changed the base branch from master to develop June 15, 2020 10:31
@drashna drashna changed the base branch from develop to master June 15, 2020 10:31
@drashna
Copy link
Member Author

drashna commented Jun 15, 2020

ws2812_rgb_matrix_array sounds better to me.

Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

lgtm

@zvecr zvecr merged commit 30cdf93 into master Jun 20, 2020
@drashna drashna deleted the fix/rgb_matrix_led_array branch July 2, 2020 00:13
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
)

* Change `led` to `led_matrix` in rgb_matrix_drivers

Is a minor change that only affects the driver file. 

However, this will allow somebody to run rgblight along side rgb matrix
using the ws2812 driver, as well.  Specifically, so you can use the
custom driver for rgblight to set a different pin (barring a change to
the `ws2812_setleds` function).  

Courtesy of discord conversion:
https://discordapp.com/channels/440868230475677696/568161140534935572/721555623191248906

* Change name to be super specific

* Update rgb_matrix_drivers.c
drashna added a commit to zsa/qmk_firmware that referenced this pull request Aug 9, 2020
)

* Change `led` to `led_matrix` in rgb_matrix_drivers

Is a minor change that only affects the driver file. 

However, this will allow somebody to run rgblight along side rgb matrix
using the ws2812 driver, as well.  Specifically, so you can use the
custom driver for rgblight to set a different pin (barring a change to
the `ws2812_setleds` function).  

Courtesy of discord conversion:
https://discordapp.com/channels/440868230475677696/568161140534935572/721555623191248906

* Change name to be super specific

* Update rgb_matrix_drivers.c
@drashna drashna mentioned this pull request Aug 21, 2020
8 tasks
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
)

* Change `led` to `led_matrix` in rgb_matrix_drivers

Is a minor change that only affects the driver file. 

However, this will allow somebody to run rgblight along side rgb matrix
using the ws2812 driver, as well.  Specifically, so you can use the
custom driver for rgblight to set a different pin (barring a change to
the `ws2812_setleds` function).  

Courtesy of discord conversion:
https://discordapp.com/channels/440868230475677696/568161140534935572/721555623191248906

* Change name to be super specific

* Update rgb_matrix_drivers.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants