-
Notifications
You must be signed in to change notification settings - Fork 748
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
Expose the 'multicolor' command on Hue lights #3664
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #3664 +/- ##
==========================================
+ Coverage 89.85% 90.49% +0.64%
==========================================
Files 322 323 +1
Lines 10379 10595 +216
==========================================
+ Hits 9326 9588 +262
+ Misses 1053 1007 -46 ☔ View full report in Codecov by Sentry. |
For an easier way to generate the command payloads, you can check out https://kjagiello.github.io/hue-gradient-command-wizard/. |
Fixes #2517. |
I also have some of these bulbs (Signify LTB003) and they support the candle effect in the Hue app. Should these (and other similar products) be included as well? I'm not entirely sure how to help test the changes but I could try to do so if that's helpful. |
Co-authored-by: BetaRavener <[email protected]>
@jtbandes I have HomeAssistant running in docker. To test the changes, I've logged in to the container running on remote server with VS Code as it's more convenient for text and folder editing, but you can also use classic If you installed HA directly without docker, I'm rather positive you can use the same approach, minus |
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.
I think we should combine most/all quirks into one file, e.g. gradient.py
, since it's essentially the "same quirk", just adding the PhilipsHueCluster
(and a friendly name).
Like, just have all QuirkBuilder
s in one file.
@BetaRavener cool, thanks for the info! Btw, are you still planning to pursue the UI work mentioned in #2517 (comment) to make the effects show up in the light's UI? I was able to confirm that by adding Actually it looks like in Z2M, if I’m reading this right, all Hue devices get the ![]() By the way, the Hue app gives me the following options for these bulbs: a few pre-configured candle effects. Curious if anyone knows how the "custom candle" would be accessed via this command's parameters? (Probably not a big deal as I can barely tell the difference between the 3 premade candle effects anyway 😅) |
Done! Though I have decided to put them under |
@jtbandes thanks for testing, we could include your bulb too then. I don't know what's best solution for dealing with all these models and there are apparently lot of them. With this PR, we are only making it possible to send command to the philips cluster. We could be implicit and enable the quirk for every philips device that has 0xFC03 cluster, but then we run risk that later philips might use the cluster for something else or otherwise break compatibility. Then we have to be explicit anyway and list all models. And for that we could either take something like z2m as source of truth, or add devices gradually as requested. In any case the list will have to be updated as Philips keeps creating new lights. Regarding the effects, I will use the hue go issue thread as not to spam here, but yes I'll be looking into the HA GUI part. To just answer rest - in z2m code you shared it does does indeed seem like all philips lights have the hueEffect enabled by default because of this line. The definition of light would have to contain so it's rather strange that z2m throws the "candle" effect into that mix. Maybe it's because any bulb that can change brightness (majority) could support "candle", but I'd still not do it that way. |
Co-authored-by: Jacob Bandes-Storch <[email protected]>
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.
And, do we wanna rename hue.py
to light.py
or hue_light.py
? I think that makes it a bit clearer that those quirks are for Hue lightning.
Otherwise, I think we can merge this. I really hope that friendly_name
doesn't cause us any issues. I don't think the implementation is ideal and we also don't implement model_id
in ZHA, but I guess we can still try it. There shouldn't be any blueprints looking for the previous names.
HA naming generally specifies that the names should be written like this: Hue ambiance spot
.
Looking at the US Hue page, this is also how Hue does it, so we should follow that:
"Hue Perifo light tube" should be fine, for example. This is also how Z2M does it: https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/devices/philips.ts
zhaquirks/philips/__init__.py
Outdated
@@ -340,3 +345,21 @@ class PhilipsRwlRemoteCluster(PhilipsRemoteCluster): | |||
3: Button("down", DIM_DOWN), | |||
4: Button("off", TURN_OFF), | |||
} | |||
|
|||
|
|||
class PhilipsHueCluster(CustomCluster): |
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.
Would renaming this to PhilipsLightCluster
or PhilipsHueLightCluster
make sense?
Here's some context behind my thought process when choosing the current naming. I went back and forth on the naming, but this Wiki article ultimately guided my choice. It describes Philips Hue as "a line of color-changing LED lamps and white bulbs". Using just "Hue" felt appropriate, as it directly ties to the product line, whereas "Hue lights" seemed a bit redundant. On the other hand, calling it simply "lights" felt too broad (far fetched scenario perhaps, but Philips could introduce a new line of lighting products). That said, I'm fully open to revisiting this if you feel a different name would be more appropriate. Let me know what you think!
Makes sense! I've just pushed a change that aligns the naming. |
Hmm, there are Philips Hue remotes/dimmers and their Hue Secure line with Zigbee door sensors and other stuff. I mean, we can always freely rename this in the future, but let's say you're working on a quirk for a new Hue dimmer and try to use and import the Hue cluster for the remotes, you might stumble upon |
Fair enough! Renamed everything now. @BetaRavener I think you will need to take this rename into account in zigpy/zha#355 |
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.
LGTM. Thanks!
Amazing this landed, I'll updated the other PR accordingly. 👍 |
Proposed change
I have sourced the Hue device models that support gradients from z2m and added a quirk for them, exposing Hue's "multicolor" command that can be used to control the gradient, i.e. set a custom gradient or activate a built-in gradient effect.
Below you will find more details on how this command can be used.
Static gradients
Here comes an example of how to set a custom gradient using an HA action:
To try out some other gradients, you can try one of the gradient scenes.
The payload structure is very cryptic, but, thankfully, the deconz and z2m communities have done a great job deciphering it. This PR does however not provide any handy interface for constructing the payload, since I was not sure how to do it properly.
If you want to create your own gradients, you can do it with the help of a simple wizard that I have put together: https://kjagiello.github.io/hue-gradient-command-wizard/..
Built-in effects
Using an HA action, you can activate, for example, the "opal" Hue effect:
You can find more effects here.
Additional info
This PR should, at least partially, resolve the following issues:
Checklist
pre-commit
checks pass / the code has been formatted using Black