Relocate RGB keycode processing (#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

common_features.mk

@@ -109,6 +109,7 @@ ifeq ($(strip $(RGBLIGHT_ENABLE)), yes)
     SRC += $(QUANTUM_DIR)/rgblight.c
     CIE1931_CURVE = yes
     LED_BREATHING_TABLE = yes
+    RGB_KEYCODES_ENABLE = yes
     ifeq ($(strip $(RGBLIGHT_CUSTOM_DRIVER)), yes)
         OPT_DEFS += -DRGBLIGHT_CUSTOM_DRIVER
     else
@@ -147,6 +148,7 @@ endif
     SRC += $(QUANTUM_DIR)/rgb_matrix.c
     SRC += $(QUANTUM_DIR)/rgb_matrix_drivers.c
     CIE1931_CURVE = yes
+    RGB_KEYCODES_ENABLE = yes
 endif
 
 ifeq ($(strip $(RGB_MATRIX_ENABLE)), yes)
@@ -187,6 +189,10 @@ ifeq ($(strip $(RGB_MATRIX_CUSTOM_USER)), yes)
     OPT_DEFS += -DRGB_MATRIX_CUSTOM_USER
 endif
 
+ifeq ($(strip $(RGB_KEYCODES_ENABLE)), yes)
+    SRC += $(QUANTUM_DIR)/process_keycode/process_rgb.c
+endif
+
 ifeq ($(strip $(TAP_DANCE_ENABLE)), yes)
     OPT_DEFS += -DTAP_DANCE_ENABLE
     SRC += $(QUANTUM_DIR)/process_keycode/process_tap_dance.c

keyboards/mxss/rules.mk

@@ -32,6 +32,7 @@ FAUXCLICKY_ENABLE = no      # Use buzzer to emulate clicky switches
 
 # Remove the common RGB light code and use my iteration instead
 OPT_DEFS += -DRGBLIGHT_ENABLE
+SRC += $(QUANTUM_DIR)/process_keycode/process_rgb.c
 SRC += rgblight.c
 SRC += ws2812.c
 CIE1931_CURVE = yes

quantum/process_keycode/process_rgb.c

@@ -0,0 +1,141 @@
+/* Copyright 2019
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "process_rgb.h"
+#include "rgb.h"
+
+typedef void (*rgb_func_pointer)(void);
+
+/**
+ * Wrapper for inc/dec rgb keycode
+ * 
+ * noinline to optimise for firmware size not speed (not in hot path) 
+ */
+static void __attribute__((noinline)) handleKeycodeRGB(const uint8_t is_shifted, const rgb_func_pointer inc_func, const rgb_func_pointer dec_func) {
+    if (is_shifted) {
+        dec_func();
+    } else {
+        inc_func();
+    }
+}
+
+/**
+ * Wrapper for animation mode
+ *   - if not in animation family -> jump to that animation
+ *   - otherwise -> wrap round animation speed
+ * 
+ * noinline to optimise for firmware size not speed (not in hot path) 
+ */
+static void __attribute__((noinline,unused)) handleKeycodeRGBMode(const uint8_t start, const uint8_t end) {
+    if ((start <= rgblight_get_mode()) && (rgblight_get_mode() < end)) {
+        rgblight_step();
+    } else {
+        rgblight_mode(start);
+    }
+}
+
+/**
+ * Handle keycodes for both rgblight and rgbmatrix
+ */
+bool process_rgb(const uint16_t keycode, const keyrecord_t *record) {
+#ifndef SPLIT_KEYBOARD
+    if (record->event.pressed) {
+#else
+    // Split keyboards need to trigger on key-up for edge-case issue
+    if (!record->event.pressed) {
+#endif
+        uint8_t shifted = get_mods() & (MOD_BIT(KC_LSHIFT) | MOD_BIT(KC_RSHIFT));
+        switch (keycode) {
+            case RGB_TOG:
+                rgblight_toggle();
+                return false;
+            case RGB_MODE_FORWARD:
+                handleKeycodeRGB(shifted, rgblight_step, rgblight_step_reverse);
+                return false;
+            case RGB_MODE_REVERSE:
+                handleKeycodeRGB(shifted, rgblight_step_reverse, rgblight_step);
+                return false;
+            case RGB_HUI:
+                handleKeycodeRGB(shifted, rgblight_increase_hue, rgblight_decrease_hue);
+                return false;
+            case RGB_HUD:
+                handleKeycodeRGB(shifted, rgblight_decrease_hue, rgblight_increase_hue);
+                return false;
+            case RGB_SAI:
+                handleKeycodeRGB(shifted, rgblight_increase_sat, rgblight_decrease_sat);
+                return false;
+            case RGB_SAD:
+                handleKeycodeRGB(shifted, rgblight_decrease_sat, rgblight_increase_sat);
+                return false;
+            case RGB_VAI:
+                handleKeycodeRGB(shifted, rgblight_increase_val, rgblight_decrease_val);
+                return false;
+            case RGB_VAD:
+                handleKeycodeRGB(shifted, rgblight_decrease_val, rgblight_increase_val);
+                return false;
+            case RGB_SPI:
+                handleKeycodeRGB(shifted, rgblight_increase_speed, rgblight_decrease_speed);
+                return false;
+            case RGB_SPD:
+                handleKeycodeRGB(shifted, rgblight_decrease_speed, rgblight_increase_speed);
+                return false;
+            case RGB_MODE_PLAIN:
+                rgblight_mode(RGBLIGHT_MODE_STATIC_LIGHT);
+                return false;
+            case RGB_MODE_BREATHE:
+#ifdef RGBLIGHT_EFFECT_BREATHING
+                handleKeycodeRGBMode(RGBLIGHT_MODE_BREATHING, RGBLIGHT_MODE_BREATHING_end);
+#endif
+                return false;
+            case RGB_MODE_RAINBOW:
+#ifdef RGBLIGHT_EFFECT_RAINBOW_MOOD
+                handleKeycodeRGBMode(RGBLIGHT_MODE_RAINBOW_MOOD, RGBLIGHT_MODE_RAINBOW_MOOD_end);
+#endif
+                return false;
+            case RGB_MODE_SWIRL:
+#ifdef RGBLIGHT_EFFECT_RAINBOW_SWIRL
+                handleKeycodeRGBMode(RGBLIGHT_MODE_RAINBOW_SWIRL, RGBLIGHT_MODE_RAINBOW_SWIRL_end);
+#endif
+                return false;
+            case RGB_MODE_SNAKE:
+#ifdef RGBLIGHT_EFFECT_SNAKE
+                handleKeycodeRGBMode(RGBLIGHT_MODE_SNAKE, RGBLIGHT_MODE_SNAKE_end);
+#endif
+                return false;
+            case RGB_MODE_KNIGHT:
+#ifdef RGBLIGHT_EFFECT_KNIGHT
+                handleKeycodeRGBMode(RGBLIGHT_MODE_KNIGHT, RGBLIGHT_MODE_KNIGHT_end);
+#endif
+                return false;
+            case RGB_MODE_XMAS:
+#ifdef RGBLIGHT_EFFECT_CHRISTMAS
+                rgblight_mode(RGBLIGHT_MODE_CHRISTMAS);
+#endif
+                return false;
+            case RGB_MODE_GRADIENT:
+#ifdef RGBLIGHT_EFFECT_STATIC_GRADIENT
+                handleKeycodeRGBMode(RGBLIGHT_MODE_STATIC_GRADIENT, RGBLIGHT_MODE_STATIC_GRADIENT_end);
+#endif
+                return false;
+            case RGB_MODE_RGBTEST:
+#ifdef RGBLIGHT_EFFECT_RGB_TEST
+                rgblight_mode(RGBLIGHT_MODE_RGB_TEST);
+#endif
+                return false;
+        }
+    }
+
+    return true;
+}

quantum/process_keycode/process_rgb.h

@@ -0,0 +1,20 @@
+/* Copyright 2019
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#pragma once
+
+#include "quantum.h"
+
+bool process_rgb(const uint16_t keycode, const keyrecord_t *record);

quantum/quantum.c

@@ -16,10 +16,6 @@
 
 #include "quantum.h"
 
-#if !defined(RGBLIGHT_ENABLE) && !defined(RGB_MATRIX_ENABLE)
-#    include "rgb.h"
-#endif
-
 #ifdef PROTOCOL_LUFA
 #    include "outputselect.h"
 #endif
@@ -253,6 +249,9 @@ bool process_record_quantum(keyrecord_t *record) {
 #endif
 #ifdef MAGIC_KEYCODE_ENABLE
             process_magic(keycode, record) &&
+#endif
+#if defined(RGBLIGHT_ENABLE) || defined(RGB_MATRIX_ENABLE)
+            process_rgb(keycode, record) &&
 #endif
             true)) {
         return false;
@@ -293,176 +292,24 @@ bool process_record_quantum(keyrecord_t *record) {
                 return false;
 #endif
 #ifdef BLUETOOTH_ENABLE
-        case OUT_AUTO:
+            case OUT_AUTO:
                 set_output(OUTPUT_AUTO);
                 return false;
-        case OUT_USB:
+            case OUT_USB:
                 set_output(OUTPUT_USB);
                 return false;
-        case OUT_BT:
+            case OUT_BT:
                 set_output(OUTPUT_BLUETOOTH);
                 return false;
 #endif
 #if defined(BACKLIGHT_ENABLE) && defined(BACKLIGHT_BREATHING)
-        case BL_BRTG:
+            case BL_BRTG:
                 backlight_toggle_breathing();
                 return false;
 #endif
         }
     }
 
-#if defined(RGBLIGHT_ENABLE) || defined(RGB_MATRIX_ENABLE)
-#    ifndef SPLIT_KEYBOARD
-    if (record->event.pressed) {
-#    else
-    // Split keyboards need to trigger on key-up for edge-case issue
-    if (!record->event.pressed) {
-#    endif
-        uint8_t shifted = get_mods() & (MOD_BIT(KC_LSHIFT) | MOD_BIT(KC_RSHIFT));
-        switch (keycode) {
-            case RGB_TOG:
-                rgblight_toggle();
-                return false;
-            case RGB_MODE_FORWARD:
-                if (shifted) {
-                    rgblight_step_reverse();
-                } else {
-                    rgblight_step();
-                }
-                return false;
-            case RGB_MODE_REVERSE:
-                if (shifted) {
-                    rgblight_step();
-                } else {
-                    rgblight_step_reverse();
-                }
-                return false;
-            case RGB_HUI:
-                if (shifted) {
-                    rgblight_decrease_hue();
-                } else {
-                    rgblight_increase_hue();
-                }
-                return false;
-            case RGB_HUD:
-                if (shifted) {
-                    rgblight_increase_hue();
-                } else {
-                    rgblight_decrease_hue();
-                }
-                return false;
-            case RGB_SAI:
-                if (shifted) {
-                    rgblight_decrease_sat();
-                } else {
-                    rgblight_increase_sat();
-                }
-                return false;
-            case RGB_SAD:
-                if (shifted) {
-                    rgblight_increase_sat();
-                } else {
-                    rgblight_decrease_sat();
-                }
-                return false;
-            case RGB_VAI:
-                if (shifted) {
-                    rgblight_decrease_val();
-                } else {
-                    rgblight_increase_val();
-                }
-                return false;
-            case RGB_VAD:
-                if (shifted) {
-                    rgblight_increase_val();
-                } else {
-                    rgblight_decrease_val();
-                }
-                return false;
-            case RGB_SPI:
-                if (shifted) {
-                    rgblight_decrease_speed();
-                } else {
-                    rgblight_increase_speed();
-                }
-                return false;
-            case RGB_SPD:
-                if (shifted) {
-                    rgblight_increase_speed();
-                } else {
-                    rgblight_decrease_speed();
-                }
-                return false;
-            case RGB_MODE_PLAIN:
-                rgblight_mode(RGBLIGHT_MODE_STATIC_LIGHT);
-                return false;
-            case RGB_MODE_BREATHE:
-#    ifdef RGBLIGHT_EFFECT_BREATHING
-                if ((RGBLIGHT_MODE_BREATHING <= rgblight_get_mode()) && (rgblight_get_mode() < RGBLIGHT_MODE_BREATHING_end)) {
-                    rgblight_step();
-                } else {
-                    rgblight_mode(RGBLIGHT_MODE_BREATHING);
-                }
-#    endif
-                return false;
-        case RGB_MODE_RAINBOW:
-#    ifdef RGBLIGHT_EFFECT_RAINBOW_MOOD
-                if ((RGBLIGHT_MODE_RAINBOW_MOOD <= rgblight_get_mode()) && (rgblight_get_mode() < RGBLIGHT_MODE_RAINBOW_MOOD_end)) {
-                    rgblight_step();
-                } else {
-                    rgblight_mode(RGBLIGHT_MODE_RAINBOW_MOOD);
-                }
-#    endif
-            case RGB_MODE_SWIRL:
-#    ifdef RGBLIGHT_EFFECT_RAINBOW_SWIRL
-                if ((RGBLIGHT_MODE_RAINBOW_SWIRL <= rgblight_get_mode()) && (rgblight_get_mode() < RGBLIGHT_MODE_RAINBOW_SWIRL_end)) {
-                    rgblight_step();
-                } else {
-                    rgblight_mode(RGBLIGHT_MODE_RAINBOW_SWIRL);
-                }
-#    endif
-                return false;
-            case RGB_MODE_SNAKE:
-#    ifdef RGBLIGHT_EFFECT_SNAKE
-                if ((RGBLIGHT_MODE_SNAKE <= rgblight_get_mode()) && (rgblight_get_mode() < RGBLIGHT_MODE_SNAKE_end)) {
-                    rgblight_step();
-                } else {
-                    rgblight_mode(RGBLIGHT_MODE_SNAKE);
-                }
-#    endif
-                return false;
-            case RGB_MODE_KNIGHT:
-#    ifdef RGBLIGHT_EFFECT_KNIGHT
-                if ((RGBLIGHT_MODE_KNIGHT <= rgblight_get_mode()) && (rgblight_get_mode() < RGBLIGHT_MODE_KNIGHT_end)) {
-                    rgblight_step();
-                } else {
-                    rgblight_mode(RGBLIGHT_MODE_KNIGHT);
-                }
-#    endif
-                return false;
-            case RGB_MODE_XMAS:
-#    ifdef RGBLIGHT_EFFECT_CHRISTMAS
-                rgblight_mode(RGBLIGHT_MODE_CHRISTMAS);
-#    endif
-                return false;
-            case RGB_MODE_GRADIENT:
-#    ifdef RGBLIGHT_EFFECT_STATIC_GRADIENT
-                if ((RGBLIGHT_MODE_STATIC_GRADIENT <= rgblight_get_mode()) && (rgblight_get_mode() < RGBLIGHT_MODE_STATIC_GRADIENT_end)) {
-                    rgblight_step();
-                } else {
-                    rgblight_mode(RGBLIGHT_MODE_STATIC_GRADIENT);
-                }
-#    endif
-                return false;
-            case RGB_MODE_RGBTEST:
-#    ifdef RGBLIGHT_EFFECT_RGB_TEST
-                rgblight_mode(RGBLIGHT_MODE_RGB_TEST);
-#    endif
-                return false;
-        }
-    }
-#endif
-
     // keycodes that depend on both pressed and non-pressed state
     switch (keycode) {
         case GRAVE_ESC: {
@@ -513,7 +360,6 @@ bool process_record_quantum(keyrecord_t *record) {
             send_keyboard_report();
             return false;
         }
-
     }
 
     return process_action_kb(record);

quantum/quantum.h

@@ -137,6 +137,10 @@ extern layer_state_t layer_state;
 #    include "process_magic.h"
 #endif
 
+#if defined(RGBLIGHT_ENABLE) || defined(RGB_MATRIX_ENABLE)
+#    include "process_rgb.h"
+#endif
+
 #ifdef HD44780_ENABLE
 #    include "hd44780.h"
 #endif

quantum/rgb_matrix.h

@@ -128,26 +128,26 @@ void        rgb_matrix_sethsv(uint16_t hue, uint8_t sat, uint8_t val);
 void        rgb_matrix_sethsv_noeeprom(uint16_t hue, uint8_t sat, uint8_t val);
 
 #ifndef RGBLIGHT_ENABLE
-#    define rgblight_toggle() rgb_matrix_toggle()
-#    define rgblight_enable() rgb_matrix_enable()
-#    define rgblight_enable_noeeprom() rgb_matrix_enable_noeeprom()
-#    define rgblight_disable() rgb_matrix_disable()
-#    define rgblight_disable_noeeprom() rgb_matrix_disable_noeeprom()
-#    define rgblight_step() rgb_matrix_step()
-#    define rgblight_sethsv(hue, sat, val) rgb_matrix_sethsv(hue, sat, val)
-#    define rgblight_sethsv_noeeprom(hue, sat, val) rgb_matrix_sethsv_noeeprom(hue, sat, val)
-#    define rgblight_step_reverse() rgb_matrix_step_reverse()
-#    define rgblight_increase_hue() rgb_matrix_increase_hue()
-#    define rgblight_decrease_hue() rgb_matrix_decrease_hue()
-#    define rgblight_increase_sat() rgb_matrix_increase_sat()
-#    define rgblight_decrease_sat() rgb_matrix_decrease_sat()
-#    define rgblight_increase_val() rgb_matrix_increase_val()
-#    define rgblight_decrease_val() rgb_matrix_decrease_val()
-#    define rgblight_increase_speed() rgb_matrix_increase_speed()
-#    define rgblight_decrease_speed() rgb_matrix_decrease_speed()
-#    define rgblight_mode(mode) rgb_matrix_mode(mode)
-#    define rgblight_mode_noeeprom(mode) rgb_matrix_mode_noeeprom(mode)
-#    define rgblight_get_mode() rgb_matrix_get_mode()
+#    define rgblight_toggle rgb_matrix_toggle
+#    define rgblight_enable rgb_matrix_enable
+#    define rgblight_enable_noeeprom rgb_matrix_enable_noeeprom
+#    define rgblight_disable rgb_matrix_disable
+#    define rgblight_disable_noeeprom rgb_matrix_disable_noeeprom
+#    define rgblight_step rgb_matrix_step
+#    define rgblight_sethsv rgb_matrix_sethsv
+#    define rgblight_sethsv_noeeprom rgb_matrix_sethsv_noeeprom
+#    define rgblight_step_reverse rgb_matrix_step_reverse
+#    define rgblight_increase_hue rgb_matrix_increase_hue
+#    define rgblight_decrease_hue rgb_matrix_decrease_hue
+#    define rgblight_increase_sat rgb_matrix_increase_sat
+#    define rgblight_decrease_sat rgb_matrix_decrease_sat
+#    define rgblight_increase_val rgb_matrix_increase_val
+#    define rgblight_decrease_val rgb_matrix_decrease_val
+#    define rgblight_increase_speed rgb_matrix_increase_speed
+#    define rgblight_decrease_speed rgb_matrix_decrease_speed
+#    define rgblight_mode rgb_matrix_mode
+#    define rgblight_mode_noeeprom rgb_matrix_mode_noeeprom
+#    define rgblight_get_mode rgb_matrix_get_mode
 #endif
 
 typedef struct {