feat: Poll manuSpecificPhilips2 for Hue lights

[burmistrzak]

As mentioned in Koenkk/zigbee-herdsman-converters#12256 and #31960, Z2M automatically reads (i.e. polls) specific attributes (brightness, state, etc.) when a device is being controlled via Zigbee Bindings.

Instead of reading only a limited amount of standard attributes, the Philips Hue-exclusive manuSpecificPhilips2 cluster can be used to reduce airtime and improve responsiveness by fetching the complete device state with a single command.

To my knowledge, the only Hue device that doesn't feature this cluster is the Hue Smart Plug, but it is capable of reporting anyways.

cc. @andrei-lazarov

[andrei-lazarov]

Cool! Can you explain the logic behind polling a little bit, to make sure I understand it?

Is it like this?
When Z2M sees "turn on group x" message from a remote, it reads the onOff state of each group member, just in case they don't support reporting. And we make the distinction by manufacturer?

[burmistrzak]

@andrei-lazarov Correct. Same thing for device-to-device bindings. 😊

[Koenkk]

LGTM! Could you fix the CI?

[burmistrzak]

LGTM! Could you fix the CI?

@Koenkk I was about to do that, but wouldn't it be better to confirm before polling that the manuSpecificPhilips is actually supported by the device?

[burmistrzak]

I've been thinking: Why not use the state of the Hue Native Control device option to dynamically override the poll target?
Because as it stands, the override would automatically take effect for every supported Hue light, regardless of user preferences.

@Nerivec Would that be doable with existing APIs and also actually be testable?

[Nerivec]

Not sure how this was implemented. I assume there's a meta value or something in the device's ZH layer that's being stored by the converter?
You should be able to access anything in ZH that ZHC stored using device.zh.xyz.
e.g.:

zigbee2mqtt/lib/extension/configure.ts

Line 70 in cfc7ab8

if (data.device.zh.meta.configured !== undefined) {

[burmistrzak]

@Nerivec Would you mind sanity checking my test harness? 😊

I think the issue was that hue_native_control is an endpoint-level meta option... Duh!

[burmistrzak]

@andrei-lazarov Would you be able to test this PR on your end?

[Nerivec]

Actually, looking at the philips branch in the context of the whole poll fn, is this the right place to do this?
It would end up triggering for every polls, seems weird?
I'm not familiar with any of the hue stuff, so, could be wrong, but my thinking was, it should be done as an if/else directly around for (const poll of polls) { (i.e. if philips, send to custom cluster, else, enter for polls loop).
Also doesn't fit very well with the POLL_ON_MESSAGE.indexOf(poll) logic bit.
Seems technically you even could go higher, since the only requirement is the toPoll logic, rest is not used in the philips branch. And it might actually end up being more of a problem than anything (checks like !endpoint.supportsInputCluster(poll.read.cluster) don't make sense if sending to custom cluster).

[burmistrzak]

@Nerivec Ok, how about a separate branch incl. debounce for manuSpecificPhilips2 right after the (const poll of polls) { block?

Edit: Actually, triggering the Hue branch for every poll is correct. We're simply swapping out the cluster/attribute used during polling to provide more accurate/efficient state updates for supported Hue lights with hue_native_control enabled.

[burmistrzak]

I think we're switching to the Hue-specific cluster as soon as possible without having to refactor the entire poll method.

Also, lots of typos in my commit messages today, huh? 😅

[Nerivec]

I thought the intent was to make one request instead of many? This ends up same as before, just a different cluster used, doesn't it? Technically less attributes (bit smaller payload), but same number of requests.

Why revert the block change?

@Koenkk isn't there a way to keep this custom stuff in ZHC? Poll logic override like custom time read or something?
As soon as we start introducing ZHC stuff in ZH/Z2M we end up having to type-assert, workaround & whatnot 🥵

[burmistrzak]

I thought the intent was to make one request instead of many? This ends up same as before, just a different cluster used, doesn't it? Technically less attributes (bit smaller payload), but same number of requests.

Yes, just a different cluster (for now).
However, I'm currently cooking up a patch that produces only one manuSpecificPhilips2.state request per endpoint.

Why revert the block change?

See above. 😅

@Koenkk isn't there a way to keep this custom stuff in ZHC? Poll logic override like custom time read or something? As soon as we start introducing ZHC stuff in ZH/Z2M we end up having to type-assert, workaround & whatnot 🥵

AFAIK no... But would be a much clearer approach tho.

[burmistrzak]

@Nerivec Let's put this PR on hold until we have a more maintainable way of implementing such overrides from ZHC. Cramming manufacturer-specific code into ZH/Z2M isn't the way to go. 😊