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
Use ExecuteIfOff on color cluster for supported bulbs with ZHA #84874
Conversation
Hey there @dmulcahey, @Adminiuga, @puddly, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
@@ -45,6 +45,7 @@ class ColorChannel(ZigbeeChannel): | |||
"color_capabilities": True, | |||
"color_loop_active": False, | |||
"start_up_color_temperature": True, | |||
"options": True, |
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 the options
property defaults to 0
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))
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, | ||
) |
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.
Question regarding tests:
It's not too important here, but how would I check the message order? (messages are sent on different clusters)
(That code doesn't care whether "on" or "move_to_color_temp" is sent first)
5195acf
to
c8fb8fa
Compare
EDIT: Removed group support from this PR now. I'll likely work on it in a separate PR. The old/standard behavior is still used for groups (even if all lights have "execute if off" enabled).
|
…members support it
c8fb8fa
to
7019f71
Compare
Group support will likely be added in a separate PR. For now, the old/standard behavior is used for groups.
This should be ready for a review now. I've removed the group stuff from this PR and will focus on that in a separate PR at some point. |
Proposed change
This implements the
options
->ExecuteIfOff
on the color cluster for supporting Zigbee 3.0 lights.This option allows the light to receive various color commands while it's still off. It won't turn on when just the color commands are received, but next time it turns on, it'll turn on with the new color.
options
andexecute_if_off_supported
property in the color channelturn_on
:ExecuteIfOff
(currently via settingoptions
from0
to1
through the clusters UI),The
transition
parameter for the variousmove_to_color
commands is apparently ignored by the light if it's off.This is why we can simply re-order how the calls are sent (when
ExecuteIfOff
is on) without needing to check the HA state for what transition we want to use when.(@dmulcahey previously implemented a similar behavior on some (now deleted) test branch)
Note: At the moment, this is only supported for single lights. Groups will always default to the old/standard behavior for now. I'll focus on adding group support for "execute if off" in a separate PR.
(The benefit of using that is that transitions to different colors from an off-state are done properly.)
Previous behavior
The behavior from this PR (disabled by default) is still used for older lights which don't support
ExecuteIfOff
(or when it's disabled):So the behavior only changes if the lights supports
ExecuteIfOff
and if it's also enabled.(Enabling it needs to be done via cluster settings right now. I'll likely add a config entity in a separate PR)
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: