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
Use ExecuteIfOff on color cluster for supported bulbs with ZHA #84874
Merged
dmulcahey
merged 9 commits into
home-assistant:dev
from
TheJulianJES:tjj/zha_light_colors_options
Jan 23, 2023
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
dfc4eb4
Add options and execute_if_off_supported properties to Color channel
TheJulianJES 5cfedf8
Initialize "options" attribute on Color channel (allowing cache)
TheJulianJES a16cce9
Implement execute_if_off_supported for ZHA lights
TheJulianJES a909c64
Make sure that color_channel exists, before checking execute_if_off_s…
TheJulianJES 10ed5d6
Replace "color_channel is not None" check with simplified "if color_c…
TheJulianJES a5d9182
Make "test_number" test expect "options" for init attribute
TheJulianJES 7d1f720
Add test_on_with_off_color test to test old and new behavior
TheJulianJES 7019f71
Experimental code to also support "execute_if_off" for groups if all …
TheJulianJES 8aecd53
Remove support for groups for now
TheJulianJES File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ | |
get_zha_gateway, | ||
patch_zha_config, | ||
send_attributes_report, | ||
update_attribute_cache, | ||
) | ||
from .conftest import SIG_EP_INPUT, SIG_EP_OUTPUT, SIG_EP_PROFILE, SIG_EP_TYPE | ||
|
||
|
@@ -1199,6 +1200,151 @@ async def test_transitions( | |
assert eWeLink_state.attributes["max_mireds"] == 500 | ||
|
||
|
||
@patch( | ||
"zigpy.zcl.clusters.lighting.Color.request", | ||
new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]), | ||
) | ||
@patch( | ||
"zigpy.zcl.clusters.general.LevelControl.request", | ||
new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]), | ||
) | ||
@patch( | ||
"zigpy.zcl.clusters.general.OnOff.request", | ||
new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]), | ||
) | ||
async def test_on_with_off_color(hass, device_light_1): | ||
"""Test turning on the light and sending color commands before on/level commands for supporting lights.""" | ||
|
||
device_1_entity_id = await find_entity_id(Platform.LIGHT, device_light_1, hass) | ||
dev1_cluster_on_off = device_light_1.device.endpoints[1].on_off | ||
dev1_cluster_level = device_light_1.device.endpoints[1].level | ||
dev1_cluster_color = device_light_1.device.endpoints[1].light_color | ||
|
||
# Execute_if_off will override the "enhanced turn on from an off-state" config option that's enabled here | ||
dev1_cluster_color.PLUGGED_ATTR_READS = { | ||
"options": lighting.Color.Options.Execute_if_off | ||
} | ||
update_attribute_cache(dev1_cluster_color) | ||
|
||
# turn on via UI | ||
dev1_cluster_on_off.request.reset_mock() | ||
dev1_cluster_level.request.reset_mock() | ||
dev1_cluster_color.request.reset_mock() | ||
|
||
await hass.services.async_call( | ||
LIGHT_DOMAIN, | ||
"turn_on", | ||
{ | ||
"entity_id": device_1_entity_id, | ||
"color_temp": 235, | ||
}, | ||
blocking=True, | ||
) | ||
|
||
assert dev1_cluster_on_off.request.call_count == 1 | ||
assert dev1_cluster_on_off.request.await_count == 1 | ||
assert dev1_cluster_color.request.call_count == 1 | ||
assert dev1_cluster_color.request.await_count == 1 | ||
assert dev1_cluster_level.request.call_count == 0 | ||
assert dev1_cluster_level.request.await_count == 0 | ||
|
||
assert dev1_cluster_on_off.request.call_args_list[0] == call( | ||
False, | ||
dev1_cluster_on_off.commands_by_name["on"].id, | ||
dev1_cluster_on_off.commands_by_name["on"].schema, | ||
expect_reply=True, | ||
manufacturer=None, | ||
tries=1, | ||
tsn=None, | ||
) | ||
assert dev1_cluster_color.request.call_args == call( | ||
False, | ||
dev1_cluster_color.commands_by_name["move_to_color_temp"].id, | ||
dev1_cluster_color.commands_by_name["move_to_color_temp"].schema, | ||
color_temp_mireds=235, | ||
transition_time=0, | ||
expect_reply=True, | ||
manufacturer=None, | ||
tries=1, | ||
tsn=None, | ||
) | ||
Comment on lines
+1251
to
+1270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question regarding tests: It's not too important here, but how would I check the message order? (messages are sent on different clusters) |
||
|
||
light1_state = hass.states.get(device_1_entity_id) | ||
assert light1_state.state == STATE_ON | ||
assert light1_state.attributes["color_temp"] == 235 | ||
assert light1_state.attributes["color_mode"] == ColorMode.COLOR_TEMP | ||
|
||
# now let's turn off the Execute_if_off option and see if the old behavior is restored | ||
dev1_cluster_color.PLUGGED_ATTR_READS = {"options": 0} | ||
update_attribute_cache(dev1_cluster_color) | ||
|
||
# turn off via UI, so the old "enhanced turn on from an off-state" behavior can do something | ||
await async_test_off_from_hass(hass, dev1_cluster_on_off, device_1_entity_id) | ||
|
||
# turn on via UI (with a different color temp, so the "enhanced turn on" does something) | ||
dev1_cluster_on_off.request.reset_mock() | ||
dev1_cluster_level.request.reset_mock() | ||
dev1_cluster_color.request.reset_mock() | ||
|
||
await hass.services.async_call( | ||
LIGHT_DOMAIN, | ||
"turn_on", | ||
{ | ||
"entity_id": device_1_entity_id, | ||
"color_temp": 240, | ||
}, | ||
blocking=True, | ||
) | ||
|
||
assert dev1_cluster_on_off.request.call_count == 0 | ||
assert dev1_cluster_on_off.request.await_count == 0 | ||
assert dev1_cluster_color.request.call_count == 1 | ||
assert dev1_cluster_color.request.await_count == 1 | ||
assert dev1_cluster_level.request.call_count == 2 | ||
assert dev1_cluster_level.request.await_count == 2 | ||
|
||
# first it comes on with no transition at 2 brightness | ||
assert dev1_cluster_level.request.call_args_list[0] == call( | ||
False, | ||
dev1_cluster_level.commands_by_name["move_to_level_with_on_off"].id, | ||
dev1_cluster_level.commands_by_name["move_to_level_with_on_off"].schema, | ||
level=2, | ||
transition_time=0, | ||
expect_reply=True, | ||
manufacturer=None, | ||
tries=1, | ||
tsn=None, | ||
) | ||
assert dev1_cluster_color.request.call_args == call( | ||
False, | ||
dev1_cluster_color.commands_by_name["move_to_color_temp"].id, | ||
dev1_cluster_color.commands_by_name["move_to_color_temp"].schema, | ||
color_temp_mireds=240, | ||
transition_time=0, | ||
expect_reply=True, | ||
manufacturer=None, | ||
tries=1, | ||
tsn=None, | ||
) | ||
assert dev1_cluster_level.request.call_args_list[1] == call( | ||
False, | ||
dev1_cluster_level.commands_by_name["move_to_level"].id, | ||
dev1_cluster_level.commands_by_name["move_to_level"].schema, | ||
level=254, | ||
transition_time=0, | ||
expect_reply=True, | ||
manufacturer=None, | ||
tries=1, | ||
tsn=None, | ||
) | ||
|
||
light1_state = hass.states.get(device_1_entity_id) | ||
assert light1_state.state == STATE_ON | ||
assert light1_state.attributes["brightness"] == 254 | ||
assert light1_state.attributes["color_temp"] == 240 | ||
assert light1_state.attributes["color_mode"] == ColorMode.COLOR_TEMP | ||
|
||
|
||
async def async_test_on_off_from_light(hass, cluster, entity_id): | ||
"""Test on off functionality from the light.""" | ||
# turn on at light | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explanation/Note:
Technically initializing this attribute isn't really needed at the moment, as the
options
parameter needs to be set manually anyway (for now) and theoptions
property defaults to0
if there isn't a cached attribute.In the future, when a configuration entity is added for this attribute, it would be nice if that attribute was already read/initialized (so the configuration entity is created appropriately). (Hence why I added it here (cached, as it won't ever update on its own))