[Bug] RGBLIGHT_LAYERS can't use with RGBLIGHT_CUSTOM_DRIVER

[yulei]

Describe the Bug

If using RGBLIGHT_LAYERS with RGBLIGHT_CUSTOM_DRIVER, there will be a compilation error.

Compiling: quantum/rgblight.c                                                                      quantum/rgblight.c:638:13: error: 'rgblight_layers_write' defined but not used [-Werror=unused-function]
static void rgblight_layers_write(void) {
            ^
cc1.exe: all warnings being treated as errors

System Information

• Keyboard:
  • Revision (if applicable):
• Operating system:
• AVR GCC version:

• ARM GCC version:

• QMK Firmware version:

• Any keyboard related software installed?
  • AutoHotKey
  • Karabiner
  • Other:

Additional Context

possible fix:

static void rgblight_layers_write(void) {

remove the static from the function declaration.

[spidey3]

This turns out to be a little more complicated that one might expect.

Yes we could just make rgblight_layers_write() publicly accessible. But I don't think that's really the right thing to do.

Originally rgblight_set() had a simple contract: Set the physical LED state according to the led array. But since lighting layers were added, it now also has to account for the lighting layer state (by calling rgblight_layers_write()).

That's fine if you are using the default implementation, but keyboard builders who need to implement their own driver shouldn't need to worry about lighting layers in their code to deal with their unusual physical LED setups. Lighting layers are an abstraction that should be above all of this. We shouldn't require those who need to override the LED driving logic to remember to include the call to handle the lighting layers; it's extra work for them, and it is a potential source for bugs if it's not done right.

My proposal instead is to resolve this by splitting rgblight_set() into:

• rgblight_set_driver() – does only what rgblight_set() used to do before lighting layers were added. Can be overridden if necessary by defining RGBLIGHT_CUSTOM_DRIVER.
• rgblight_set() – calls rgblight_layers_write() and then calls rgblight_set_driver().

By doing this:

• Only the < 10 custom implementations of rgblight_set() need to be changed to instead be named rgblight_set_driver(); and there will be no need for them to call rgblight_layers_write().
• No change is needed to the > 60 places where rgblight_set() is called.
• Current functionality will be maintained, and perhaps a few keyboards where Lighting Layers didn't work will be fixed.

Stay tuned for a pull request soon...

[spidey3]

After discussion, I have cancelled Pull Request #8894, as @drashna is already working on #7720. That PR also refactors rgblight_set(), solving this issue, and a few others as well.

[yulei]

Thanks, hopping #7720 will be merged soon.

[spidey3]

Looks like #7720 was recently merged.

[yulei]

thanks