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

[ColorUtil] kelvinToXY: Extend Colour Temperature range 1000 K .. 10000 K #4429

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Oct 26, 2024

Extend the supported colour temperature range of the kelvinToXy() method from 2000 .. 6500 K to 1000 .. 10000 K

See #3891 (comment)

Please note that the inverse xyToKelvin() method (which is based on McCamy's approximation) is only accurate above 2000 K. This means that round trip conversions xyToKelvin(kelvinToXy()) are very inaccurate for colour temperatures below 2000 K. This is not perfect but it seems that most real bindings in OH are using a range of 2000 K to 6500 K .. where the accuracy is better than 1%..

Signed-off-by: AndrewFG [email protected]

@andrewfg andrewfg requested a review from a team as a code owner October 26, 2024 13:31
@andrewfg
Copy link
Contributor Author

=> @lolodomo for info

@andrewfg andrewfg changed the title [ColorUtil] extend supported colour temperature range [ColorUtil] Extend supported colour temperature range Oct 27, 2024
Copy link
Member

@holgerfriedrich holgerfriedrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andrewfg,
could we consider restricting the number of tests runs to a reasonable number? 270000 tests take quite a while to complete.
Mybe increasing allHSB::step to 20 and allRGBW::step to 25 could be a compromise.... WDYT?

@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 27, 2024

Yeah. :)

EDIT: done!

@andrewfg andrewfg changed the title [ColorUtil] Extend supported colour temperature range [ColorUtil] Extend colour temperature range 1000 K .. 10000 K Oct 28, 2024
@andrewfg andrewfg changed the title [ColorUtil] Extend colour temperature range 1000 K .. 10000 K [ColorUtil] Extend Colour Temperature range 1000 K .. 10000 K Oct 28, 2024
Copy link
Member

@holgerfriedrich holgerfriedrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@holgerfriedrich holgerfriedrich added the enhancement An enhancement or new feature of the Core label Oct 29, 2024
@holgerfriedrich holgerfriedrich merged commit 728c737 into openhab:main Oct 29, 2024
5 checks passed
@holgerfriedrich holgerfriedrich added this to the 4.3 milestone Oct 29, 2024
@holgerfriedrich holgerfriedrich changed the title [ColorUtil] Extend Colour Temperature range 1000 K .. 10000 K [ColorUtil] kelvinToXY: Extend Colour Temperature range 1000 K .. 10000 K Oct 29, 2024
@andrewfg andrewfg deleted the colorutil-increase-colour-temperature-range branch November 6, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants