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

[Bug] led_min/led_max issue with rgb matrix raindrops effect #17589

Closed
3 tasks
lokher opened this issue Jul 7, 2022 · 3 comments
Closed
3 tasks

[Bug] led_min/led_max issue with rgb matrix raindrops effect #17589

lokher opened this issue Jul 7, 2022 · 3 comments

Comments

@lokher
Copy link
Contributor

lokher commented Jul 7, 2022

Describe the Bug

led_min and led_max as parameter of rgb_matrix_indicators_advanced_kb()/rgb_matrix_indicators_advanced_user() are always same at each invoke when running raindrops effect. I dig into the code, look like Line 31~35 never have chance to run since this function always return false at Line 28, which lead to no increment of iter

bool RAINDROPS(effect_params_t* params) {
if (!params->init) {
// Change one LED every tick, make sure speed is not 0
if (scale16by8(g_rgb_timer, qadd8(rgb_matrix_config.speed, 16)) % 10 == 0) {
raindrops_set_color(rand() % DRIVER_LED_TOTAL, params);
}
return false;
}
RGB_MATRIX_USE_LIMITS(led_min, led_max);
for (int i = led_min; i < led_max; i++) {
raindrops_set_color(i, params);
}
return rgb_matrix_check_finished_leds(led_max);
}

In addition, RGB_MATRIX_INDICATOR_SET_COLOR can only set first 1/5~1/4 part of LEDs.

System Information

Keyboard:
Revision (if applicable):
Operating system:
qmk doctor output:

(Paste output here)

Any keyboard related software installed?

  • AutoHotKey (Windows)
  • Karabiner (macOS)
  • Other:

Additional Context

@filterpaper
Copy link
Contributor

Line 31~35 is ran once on init to light up every LED with random hues.

Line 23~29 will run on every cycle, and on interval tick, randomly change one LED to another random color.

Checked this effect again and it does change one LED on every tick. However the effect can be too subtle if user set saturation (in eeprom) is too high.

@lokher
Copy link
Contributor Author

lokher commented Jul 14, 2022

I think you didn't get my point, let me describe more details.
Rgb matrix doesn't render whole keyboard leds in a single loop , it renders RGB_MATRIX_LED_PROCESS_LIMIT number leds, which define in rgb_matrix.h:

#ifndef RGB_MATRIX_LED_PROCESS_LIMIT
# define RGB_MATRIX_LED_PROCESS_LIMIT (DRIVER_LED_TOTAL + 4) / 5
#endif

Generally, if there are 100 leds in the keyboard, rgb matrix renders led 0 to 20 in the 1s tloop, led_min is set to 0 and led_max is set to 20 , then increase params->iter , then renders led 21 to 40 in the 2nd loop, led_min is set to 21 and led_max is set to 40, then 3rd loop....

params->iter increase in the diffinition of RGB_MATRIX_USE_LIMITS

# define RGB_MATRIX_USE_LIMITS(min, max) \
uint8_t min = RGB_MATRIX_LED_PROCESS_LIMIT * params->iter; \
uint8_t max = min + RGB_MATRIX_LED_PROCESS_LIMIT; \
if (max > DRIVER_LED_TOTAL) max = DRIVER_LED_TOTAL;

RGB_MATRIX_USE_LIMITS(led_min, led_max) is run in other rgb matrix effect, but not in raindrop effect, so that led_min and led_max is always 0 and 20 on 100 leds keyboard.

If you use RGB_MATRIX_INDICATOR_SET_COLOR to set the led, it work only when target index is between led_min and led_max , setting led which index is greater than 20 won't work.

#define RGB_MATRIX_INDICATOR_SET_COLOR(i, r, g, b) \
if (i >= led_min && i <= led_max) { \
rgb_matrix_set_color(i, r, g, b); \
}

@filterpaper
Copy link
Contributor

I missed the part about breaking rgb_matrix_indicators_advanced_user(). Can you test the linked PR and confirm it is working?

This was referenced Jul 14, 2022
@zvecr zvecr closed this as completed Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants