Skip to content

Use Kelvin as the preferred color temperature unit#79591

Merged
emontnemery merged 3 commits into
devfrom
light_color_temp_kelvin
Oct 6, 2022
Merged

Use Kelvin as the preferred color temperature unit#79591
emontnemery merged 3 commits into
devfrom
light_color_temp_kelvin

Conversation

@emontnemery
Copy link
Copy Markdown
Contributor

@emontnemery emontnemery commented Oct 4, 2022 •
edited
Loading

Proposed change

Use Kelvin as the preferred color temperature unit

WTH: https://community.home-assistant.io/t/wth-is-light-temperature-not-in-kelvin/467449

Implements the proposal from home-assistant/architecture#564 (comment):

  • Add ATTR_COLOR_TEMP_KELVIN, which can be used in service calls instead of ATTR_KELVIN
  • We currently have ATTR_KELVIN as an alternative way to provide ATTR_COLOR_TEMP. Reverse this relationship, making ATTR_COLOR_TEMP_KELVIN the primary attribute that light platforms receive in their turn_on handler.
    • During a deprecation period, lights will receive both ATTR_COLOR_TEMP and ATTR_COLOR_TEMP_KELVIN
  • Introduce a LightEntity.color_temp_kelvin property for the color temperature of a light. Keep support for lights implementing LightEntity.color_temp during a deprecation period, just like hs_color auto-converts to rgb_color.
  • Introduce LightEntity.min_color_temp_kelvin / LightEntity.max_color_temp_kelvin attributes. Keep support for lights implementing LightEntity.min_mireds / LightEntity.max_mireds during a deprecation period.
  • Introduce state and capability attributes ATTR_COLOR_TEMP_KELVIN, ATTR_MIN_COLOR_TEMP_KELVIN and ATTR_MAX_COLOR_TEMP_KELVIN
    • During a deprecation period, capability attributes ATTR_COLOR_TEMP, ATTR_MIN_MIREDS and ATTR_MAX_MIREDS are also set.
  • Deprecate constants ATTR_KELVIN and ATTR_COLOR_TEMP from the light.turn_on service call
  • Deprecate state and capability attributes ATTR_COLOR_TEMP, ATTR_MIN_MIREDS and ATTR_MAX_MIREDS
  • Deprecate properties LightEntity.color_temp, LightEntity.min_mireds and LightEntity.max_mireds

To be followed up with:

  • Change the UI to use a kelvin scale.
  • Update color temp selector to use K
  • Update UI to filter the new attributes
  • Update color profiles to support ATTR_COLOR_TEMP_KELVIN
  • Update core integrations
  • Update color utilitites

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@emontnemery emontnemery requested a review from a team as a code owner October 4, 2022 11:43
@homeassistant
Copy link
Copy Markdown
Contributor

homeassistant commented Oct 4, 2022

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (light) you are listed as a code owner for? Thanks!

@emontnemery emontnemery marked this pull request as draft October 4, 2022 11:43
@frenck frenck added the WTH Issues & PRs generated from the "Month of What the Heck?" label Oct 4, 2022
@emontnemery emontnemery force-pushed the light_color_temp_kelvin branch from 5a56593 to 9e121c1 Compare October 4, 2022 12:48
emontnemery
emontnemery commented Oct 4, 2022
View reviewed changes
Comment thread tests/util/test_color.py
assert color_util.color_rgb_to_rgbww(64, 64, 64, 2702, 6493) == (0, 14, 25, 64, 64)
assert color_util.color_rgb_to_rgbww(32, 64, 16, 2702, 6493) == (9, 64, 0, 38, 38)
assert color_util.color_rgb_to_rgbww(0, 0, 0, 2702, 6493) == (0, 0, 0, 0, 0)
assert color_util.color_rgb_to_rgbww(0, 0, 0, 10000, 1000000) == (0, 0, 0, 0, 0)
Copy link
Copy Markdown
Contributor Author

@emontnemery emontnemery Oct 4, 2022

Choose a reason for hiding this comment

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

Note that min_mired=0 in the test was not realistic, it means an infinitely high color temperature

Comment thread tests/util/test_color.py
Comment on lines -438 to -448
assert color_util.color_rgbww_to_rgb(255, 255, 255, 0, 0, 0, 0) == (255, 255, 255)
assert color_util.color_rgbww_to_rgb(255, 255, 255, 255, 255, 0, 0) == (
255,
161,
128,
)
assert color_util.color_rgbww_to_rgb(255, 255, 255, 255, 255, 0, 370) == (
255,
245,
237,
)
Copy link
Copy Markdown
Contributor Author

@emontnemery emontnemery Oct 4, 2022

Choose a reason for hiding this comment

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

Removed because these tests used invalid min/max mireds

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Oct 4, 2022

Screen Shot 2022-10-04 at 9 23 36 AM

All seems to be working as expected. The mireds conversion makes the rounding a bit obvious, but thats expected

@emontnemery emontnemery marked this pull request as ready for review October 6, 2022 06:07
bdraco
bdraco reviewed Oct 6, 2022
View reviewed changes
Comment thread homeassistant/util/color.py
@emontnemery emontnemery merged commit 47d0598 into dev Oct 6, 2022
@emontnemery emontnemery deleted the light_color_temp_kelvin branch October 6, 2022 10:22
@Djelibeybi Djelibeybi mentioned this pull request Oct 7, 2022
Merged
23 tasks
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 7, 2022
@frenck frenck added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bdraco bdraco bdraco approved these changes

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

@Jc2k Jc2k Awaiting requested review from Jc2k Jc2k is a code owner

Assignees

No one assigned

Labels

cla-signed core Hacktoberfest has-tests integration: light noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) WTH Issues & PRs generated from the "Month of What the Heck?"

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@emontnemery @homeassistant @bdraco @MartinHjelmare @frenck