qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.04k stars 38.77k forks source link

RGBLIGHT_LIMIT_VAL not respected by many functions e.g. rgblight_setrgb(r, g, b) #6061

Closed rickard-von-essen closed 2 years ago

rickard-von-essen commented 5 years ago

In the documentation RGBLIGHT_LIMIT_VAL is described with:

The maximum brightness level

But reading the code this only limits the brightness when using sethsv not when using the setrgb (or any of the functions calling it).

Feature Request Type

Description

I suggest that this check is moved to the setrgb function (which is called by sethsv). This would make RGBLIGHT_LIMIT_VAL an overall limit of the RGB brightness which I think was the intention.

If you agree I can create a PR to address this.

drashna commented 5 years ago

That's a good point, and yeah, should be addressed in some way.

@XScorpion2 comments, ideas?

XScorpion2 commented 5 years ago

RGBLIGHT_LIMIT_VAL is HSV only, so it makes sense the setrgb functions to do not respect that define. I do agree having a global "brightness limit" is definitely a good idea, though how to go about doing that correctly will be interesting as it has to play nice with hsv > rgb conversion, linear lighting table, and raw rgb values uniformly.

Thinking about it, I think you would probably want to do the following steps:

That should keep the colors correct with how they are viewed today and keep things consistent across rgblight & rgbmatrix.

danielo515 commented 5 years ago

So the only way to control the rgb bright is by setting it to lower values?

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

tzarc commented 2 years ago

Closing due to inactivity.