Skip to content

Migrate zwave_js light tests to use Kelvin #133034

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.

Sign up for GitHub

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

Merged
frenck merged 1 commit into from
Dec 12, 2024
Merged

Migrate zwave_js light tests to use Kelvin #133034

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions tests/components/zwave_js/test_light.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from homeassistant.components.light import (
ATTR_BRIGHTNESS,
ATTR_COLOR_MODE,
ATTR_COLOR_TEMP,
ATTR_COLOR_TEMP_KELVIN,
ATTR_HS_COLOR,
ATTR_MAX_MIREDS,
ATTR_MIN_MIREDS,
ATTR_MAX_COLOR_TEMP_KELVIN,
ATTR_MIN_COLOR_TEMP_KELVIN,
ATTR_RGB_COLOR,
ATTR_RGBW_COLOR,
ATTR_SUPPORTED_COLOR_MODES,
Expand Down Expand Up @@ -51,8 +51,8 @@ async def test_light(

assert state
assert state.state == STATE_OFF
assert state.attributes[ATTR_MIN_MIREDS] == 153
assert state.attributes[ATTR_MAX_MIREDS] == 370
assert state.attributes[ATTR_MAX_COLOR_TEMP_KELVIN] == 6500
assert state.attributes[ATTR_MIN_COLOR_TEMP_KELVIN] == 2700
assert state.attributes[ATTR_SUPPORTED_FEATURES] == LightEntityFeature.TRANSITION
assert state.attributes[ATTR_SUPPORTED_COLOR_MODES] == ["color_temp", "hs"]

Expand Down Expand Up @@ -130,7 +130,7 @@ async def test_light(
assert state.state == STATE_ON
assert state.attributes[ATTR_COLOR_MODE] == "color_temp"
assert state.attributes[ATTR_BRIGHTNESS] == 255
assert state.attributes[ATTR_COLOR_TEMP] == 370
assert state.attributes[ATTR_COLOR_TEMP_KELVIN] == 2702
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Dec 12, 2024

Choose a reason for hiding this comment

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

Why is it 2702 instead of 2700 as above?

Copy link
Copy Markdown
Contributor Author

@epenet epenet Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

2700 above is the "min kelvin" which gets set once

core/homeassistant/components/zwave_js/light.py

Lines 108 to 109 in 72cc1f4

_attr_min_color_temp_kelvin = 2700 # 370 mireds as a safe default
_attr_max_color_temp_kelvin = 6500 # 153 mireds as a safe default

2702 is the current value from the integration (looks like a % ratio converted to mireds)

core/homeassistant/components/zwave_js/light.py

Lines 498 to 500 in 72cc1f4

self._color_temp = color_util.color_temperature_mired_to_kelvin(
MAX_MIREDS - ((cold_white / 255) * (MAX_MIREDS - MIN_MIREDS))
)

I see three options:

  • the current code is kept with slight rounding discrepencies
  • we can change the min/max from 2700/6500 to 2702/6535 to more closely match the previous defaults
  • the formula is changed so the % ratio is based on Kelvin and not Mireds

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Dec 13, 2024

Choose a reason for hiding this comment

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

I'd use numbers in the tests that make sense when using Kelvin. It looks weird at the moment.

assert state.attributes[ATTR_RGB_COLOR] is not None

# Test turning on with same brightness
Expand Down Expand Up @@ -256,7 +256,7 @@ async def test_light(
assert state.attributes[ATTR_COLOR_MODE] == "hs"
assert state.attributes[ATTR_BRIGHTNESS] == 255
assert state.attributes[ATTR_RGB_COLOR] == (255, 76, 255)
assert state.attributes[ATTR_COLOR_TEMP] is None
assert state.attributes[ATTR_COLOR_TEMP_KELVIN] is None

client.async_send_command.reset_mock()

Expand Down Expand Up @@ -293,7 +293,7 @@ async def test_light(
await hass.services.async_call(
"light",
"turn_on",
{"entity_id": BULB_6_MULTI_COLOR_LIGHT_ENTITY, ATTR_COLOR_TEMP: 170},
{"entity_id": BULB_6_MULTI_COLOR_LIGHT_ENTITY, ATTR_COLOR_TEMP_KELVIN: 5881},
blocking=True,
)

Expand Down Expand Up @@ -358,14 +358,14 @@ async def test_light(
assert state.state == STATE_ON
assert state.attributes[ATTR_COLOR_MODE] == "color_temp"
assert state.attributes[ATTR_BRIGHTNESS] == 255
assert state.attributes[ATTR_COLOR_TEMP] == 170
assert state.attributes[ATTR_COLOR_TEMP_KELVIN] == 5881
assert ATTR_RGB_COLOR in state.attributes

# Test turning on with same color temp
await hass.services.async_call(
"light",
"turn_on",
{"entity_id": BULB_6_MULTI_COLOR_LIGHT_ENTITY, ATTR_COLOR_TEMP: 170},
{"entity_id": BULB_6_MULTI_COLOR_LIGHT_ENTITY, ATTR_COLOR_TEMP_KELVIN: 5881},
blocking=True,
)

Expand All @@ -379,7 +379,7 @@ async def test_light(
"turn_on",
{
"entity_id": BULB_6_MULTI_COLOR_LIGHT_ENTITY,
ATTR_COLOR_TEMP: 170,
ATTR_COLOR_TEMP_KELVIN: 5881,
ATTR_TRANSITION: 35,
},
blocking=True,
Expand Down