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

Power management doesn't take color/temp correction into account #697

Open
kriegsman opened this Issue Dec 12, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@kriegsman
Copy link
Contributor

kriegsman commented Dec 12, 2018

The power limiting functions don't take the color correction and color temperature adjustments into account. So, worst case, if you set color correction to 255,0,0, e.g. green and blue are 100% off no matter what, and you try to show a strip of pixels that are all green, plus one red one, the power calculation assumes that the green pixels are all lit and drawing power, and thus (may) dim down the red one -- even though there is no power being drawn by the 'green' pixels.

Either calculate_max_brightness_for_power_mW should take this into account, OR CFastLED::show should take it into account, but not both.

Another stray thought: it might be nice if CFastLED::show somehow stored the last brightness or power consumption value, so that code could see what just happened, without having to re-calculate the power draw. It'd cost us a couple of bytes of SRAM, which might or might not be worth it, so I'm still just thinking about this. I suspect that since most people wouldn't use it, the burden of 're-calculating' the power draw should be on the few people who do want that reading, without a 2-byte tax on literally everyone.

@kriegsman kriegsman self-assigned this Dec 12, 2018

@chrisparton1991

This comment has been minimized.

Copy link

chrisparton1991 commented Dec 12, 2018

Adding a stray thought of my own, could the behaviour you describe be enabled by a macro so you can opt in? Comes at the cost of more complexity and another macro for people to know about though.

@kriegsman

This comment has been minimized.

Copy link
Contributor

kriegsman commented Dec 12, 2018

Not a bad thought. I’m more tempted right now to just do the big fix now, and queue up the enhancement for the next time we do a rev to the library (and docs). But thank you for weighing in... always good to hear what sounds useful to other people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment