From 1430fc208badb6837c53516d8d8fc583dfabc70c Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Sun, 11 Sep 2022 13:03:02 -0400 Subject: [PATCH 1/6] avoid repeated calls to rgblight_set() in corner case --- quantum/rgblight/rgblight.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/quantum/rgblight/rgblight.c b/quantum/rgblight/rgblight.c index e5d3a98bea5e..ffafae69819c 100644 --- a/quantum/rgblight/rgblight.c +++ b/quantum/rgblight/rgblight.c @@ -128,6 +128,7 @@ LED_TYPE led[RGBLED_NUM]; #ifdef RGBLIGHT_LAYERS rgblight_segment_t const *const *rgblight_layers = NULL; +bool deferred_rgblight_set_for_layers = false; #endif rgblight_ranges_t rgblight_ranges = {0, RGBLED_NUM, 0, RGBLED_NUM, RGBLED_NUM}; @@ -752,7 +753,11 @@ void rgblight_set_layer_state(uint8_t layer, bool enabled) { # ifdef RGBLIGHT_LAYERS_OVERRIDE_RGB_OFF // If not enabled, then nothing else will actually set the LEDs... if (!rgblight_config.enable) { - rgblight_set(); + // Doing rgblight_set() here could potentially cause timing + // issues when there are multiple successive calls to + // rgblight_set_layer_state(). Instead, set a flag and do the + // rgblight_set() the next time rgblight_task() runs. + deferred_rgblight_set_for_layers = true; } # endif } @@ -1150,8 +1155,15 @@ void rgblight_task(void) { } } -# ifdef RGBLIGHT_LAYER_BLINK +# ifdef RGBLIGHT_LAYERS +# ifdef RGBLIGHT_LAYER_BLINK rgblight_blink_layer_repeat_helper(); +# endif + + if (deferred_rgblight_set_for_layers) { + deferred_rgblight_set_for_layers = false; + rgblight_set(); + } # endif } From c3ceb75f56ea560c2567fa7d13fabfc6ebbe2e9c Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Sun, 11 Sep 2022 13:25:49 -0400 Subject: [PATCH 2/6] fix another case where rgblight_set() is called indirectly --- quantum/rgblight/rgblight.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/quantum/rgblight/rgblight.c b/quantum/rgblight/rgblight.c index ffafae69819c..451649404167 100644 --- a/quantum/rgblight/rgblight.c +++ b/quantum/rgblight/rgblight.c @@ -128,6 +128,7 @@ LED_TYPE led[RGBLED_NUM]; #ifdef RGBLIGHT_LAYERS rgblight_segment_t const *const *rgblight_layers = NULL; +bool deferred_rgblight_mode_for_layers = false; bool deferred_rgblight_set_for_layers = false; #endif @@ -745,18 +746,20 @@ void rgblight_set_layer_state(uint8_t layer, bool enabled) { rgblight_status.enabled_layer_mask &= ~mask; } RGBLIGHT_SPLIT_SET_CHANGE_LAYERS; + + // Calling rgblight_set() here (directly or indirectly) could + // potentially cause timing issues when there are multiple + // successive calls to rgblight_set_layer_state(). Instead, + // set a flag and do it the next time rgblight_task() runs. + // Static modes don't have a ticker running to update the LEDs if (rgblight_status.timer_enabled == false) { - rgblight_mode_noeeprom(rgblight_config.mode); + deferred_rgblight_mode_for_layers = true; } # ifdef RGBLIGHT_LAYERS_OVERRIDE_RGB_OFF // If not enabled, then nothing else will actually set the LEDs... if (!rgblight_config.enable) { - // Doing rgblight_set() here could potentially cause timing - // issues when there are multiple successive calls to - // rgblight_set_layer_state(). Instead, set a flag and do the - // rgblight_set() the next time rgblight_task() runs. deferred_rgblight_set_for_layers = true; } # endif @@ -1160,6 +1163,10 @@ void rgblight_task(void) { rgblight_blink_layer_repeat_helper(); # endif + if (deferred_rgblight_mode_for_layers) { + deferred_rgblight_mode_for_layers = false; + rgblight_mode_noeeprom(rgblight_config.mode); + } if (deferred_rgblight_set_for_layers) { deferred_rgblight_set_for_layers = false; rgblight_set(); From 81ab613637ab3ed80c6c87dc9bc91c70daaa8d54 Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Sun, 11 Sep 2022 14:49:05 -0400 Subject: [PATCH 3/6] simplify, and hopefully fix splits too --- quantum/rgblight/rgblight.c | 38 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/quantum/rgblight/rgblight.c b/quantum/rgblight/rgblight.c index 451649404167..cd27f85c52a0 100644 --- a/quantum/rgblight/rgblight.c +++ b/quantum/rgblight/rgblight.c @@ -128,8 +128,8 @@ LED_TYPE led[RGBLED_NUM]; #ifdef RGBLIGHT_LAYERS rgblight_segment_t const *const *rgblight_layers = NULL; -bool deferred_rgblight_mode_for_layers = false; -bool deferred_rgblight_set_for_layers = false; + +bool deferred_set_layer_state = false; #endif rgblight_ranges_t rgblight_ranges = {0, RGBLED_NUM, 0, RGBLED_NUM, RGBLED_NUM}; @@ -745,24 +745,14 @@ void rgblight_set_layer_state(uint8_t layer, bool enabled) { } else { rgblight_status.enabled_layer_mask &= ~mask; } - RGBLIGHT_SPLIT_SET_CHANGE_LAYERS; // Calling rgblight_set() here (directly or indirectly) could // potentially cause timing issues when there are multiple // successive calls to rgblight_set_layer_state(). Instead, // set a flag and do it the next time rgblight_task() runs. - // Static modes don't have a ticker running to update the LEDs - if (rgblight_status.timer_enabled == false) { - deferred_rgblight_mode_for_layers = true; - } + deferred_set_layer_state = true; -# ifdef RGBLIGHT_LAYERS_OVERRIDE_RGB_OFF - // If not enabled, then nothing else will actually set the LEDs... - if (!rgblight_config.enable) { - deferred_rgblight_set_for_layers = true; - } -# endif } bool rgblight_get_layer_state(uint8_t layer) { @@ -1163,13 +1153,21 @@ void rgblight_task(void) { rgblight_blink_layer_repeat_helper(); # endif - if (deferred_rgblight_mode_for_layers) { - deferred_rgblight_mode_for_layers = false; - rgblight_mode_noeeprom(rgblight_config.mode); - } - if (deferred_rgblight_set_for_layers) { - deferred_rgblight_set_for_layers = false; - rgblight_set(); + if (deferred_set_layer_state) { + deferred_set_layer_state = false; + RGBLIGHT_SPLIT_SET_CHANGE_LAYERS; + + // Static modes don't have a ticker running to update the LEDs + if (rgblight_status.timer_enabled == false) { + rgblight_mode_noeeprom(rgblight_config.mode); + } + +# ifdef RGBLIGHT_LAYERS_OVERRIDE_RGB_OFF + // If not enabled, then nothing else will actually set the LEDs... + if (!rgblight_config.enable) { + rgblight_set(); + } +# endif } # endif } From bcb20d15be3f25fbfaea99f16baa23297fc77780 Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Sun, 18 Sep 2022 12:38:52 -0400 Subject: [PATCH 4/6] don't change where we set split sync bits --- quantum/rgblight/rgblight.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quantum/rgblight/rgblight.c b/quantum/rgblight/rgblight.c index cd27f85c52a0..16ba008c2587 100644 --- a/quantum/rgblight/rgblight.c +++ b/quantum/rgblight/rgblight.c @@ -745,6 +745,7 @@ void rgblight_set_layer_state(uint8_t layer, bool enabled) { } else { rgblight_status.enabled_layer_mask &= ~mask; } + RGBLIGHT_SPLIT_SET_CHANGE_LAYERS; // Calling rgblight_set() here (directly or indirectly) could // potentially cause timing issues when there are multiple @@ -1155,7 +1156,6 @@ void rgblight_task(void) { if (deferred_set_layer_state) { deferred_set_layer_state = false; - RGBLIGHT_SPLIT_SET_CHANGE_LAYERS; // Static modes don't have a ticker running to update the LEDs if (rgblight_status.timer_enabled == false) { From 5700a52a813c197688c208a9ab658263191da8c5 Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Sun, 18 Sep 2022 15:44:43 -0400 Subject: [PATCH 5/6] format --- quantum/rgblight/rgblight.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/quantum/rgblight/rgblight.c b/quantum/rgblight/rgblight.c index 16ba008c2587..a8604a51b975 100644 --- a/quantum/rgblight/rgblight.c +++ b/quantum/rgblight/rgblight.c @@ -128,8 +128,8 @@ LED_TYPE led[RGBLED_NUM]; #ifdef RGBLIGHT_LAYERS rgblight_segment_t const *const *rgblight_layers = NULL; - -bool deferred_set_layer_state = false; + +bool deferred_set_layer_state = false; #endif rgblight_ranges_t rgblight_ranges = {0, RGBLED_NUM, 0, RGBLED_NUM, RGBLED_NUM}; @@ -747,13 +747,12 @@ void rgblight_set_layer_state(uint8_t layer, bool enabled) { } RGBLIGHT_SPLIT_SET_CHANGE_LAYERS; - // Calling rgblight_set() here (directly or indirectly) could - // potentially cause timing issues when there are multiple + // Calling rgblight_set() here (directly or indirectly) could + // potentially cause timing issues when there are multiple // successive calls to rgblight_set_layer_state(). Instead, // set a flag and do it the next time rgblight_task() runs. deferred_set_layer_state = true; - } bool rgblight_get_layer_state(uint8_t layer) { From aaaec4d6466200b7984ed4a76f515b39fad03c37 Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Sun, 18 Sep 2022 18:34:17 -0400 Subject: [PATCH 6/6] deferred_set_layer_state shouldn't be global Co-authored-by: Sergey Vlasov --- quantum/rgblight/rgblight.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quantum/rgblight/rgblight.c b/quantum/rgblight/rgblight.c index a8604a51b975..f4fc1fd34a1b 100644 --- a/quantum/rgblight/rgblight.c +++ b/quantum/rgblight/rgblight.c @@ -129,7 +129,7 @@ LED_TYPE led[RGBLED_NUM]; #ifdef RGBLIGHT_LAYERS rgblight_segment_t const *const *rgblight_layers = NULL; -bool deferred_set_layer_state = false; +static bool deferred_set_layer_state = false; #endif rgblight_ranges_t rgblight_ranges = {0, RGBLED_NUM, 0, RGBLED_NUM, RGBLED_NUM};