-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Poll device when device does not support reporting. #2122
Conversation
@Koenkk as promised on #2108, here is the code I have now. It is working pretty well, although I think this probably more work. I have not written tests (not sure how?) and I think the code can be optimize some more by someone who has a better understanding of zigbee2mqtt and zigbee-herdsman. Edit: rebased because conflict with last commit in dev |
3e835b5
to
4011fca
Compare
nice pr! However I'm wondering for how many cases this feature can be used. For know the only use case I see is the combination of a hue dimmer with a hue bulb. If that is indeed the case we could try a much simpler and local solution (inside zigbee-shepherd-converters) which works without any additional configuration. Did you have any other use cases in mind for this? |
@Koenkk well it's not just the hue dimmer. Same for the tradfri remote. So I guess it is tied to the bulb, rather than the remote used? But yes, a sort of work around that applies to all hue bulbs without reporting, might be better. I only own hue and tradfri bulbs, so I am not sure there are other bulbs that do not support attReport for brightness/color. So it currently catching a lot of messages, as we work backwards. From whatever that triggers a change via group or binding to the bulb that is lacking attReport. Edit: maybe something is possible though to hook into the bind/group add+remove to keep the relation in the database? And then somehow mark the bulb(s) in zigbee-herdman-converters so they can request a sort of callback? I'm not familiar enough yet with zigbee-herdman/converters |
AFAIK the current code won't work for the TRADFRI remote control as it addresses groupID and not device ID. I think this code should be moved to The latest zigbee-herdsman keeps track of binds (https://github.com/Koenkk/zigbee-herdsman/blob/master/src/controller/model/endpoint.ts#L53). In pseudo code we need to do something like onStageChange(endpoint) {
for binds in endpoint.binds
if (typeof bind.target === group)
// Update all children of group with new state
else if (typeof bind.target === endpoint)
// update endpoint with new state
} |
I did test it with the trådfri remote and it was working fine? (As I also loop all group members if the message was directed at a group.) |
OK, I will try to experiment and review the code this weekend. |
I haven’t look at the hook suggestion directly in herdsman yet, got super sick this week so haven’t been doing much at all. There is definitely room for improvement |
don't worry, get well soon! |
Just started to try and use the groups feature and found that Hue bulbs aren't updating their state after a set (thus Hass thinks the individual room lights are still on/off vs. what they are). My current workaround is from the suggestion you gave #2108 (comment) where I query after the group update. I think the Will also be an issue with Scenes I suspect as well? Interesting how Hue solves this themselves, whether proactively querying every X, or proprietary status response back. |
I did notice the occasional 3-8% off in brightness because the bulb was bit slow to transition. Multiple clicks vs hold on the brightness buttons. But overal after a ~1 sec delay they seem to be mostly up to date. I guess for the hue remote in particular you could use the reported brightness from the remote instead of querying the bulb. But I’d rather see a more generic query after a bound device or a message to a group the bulb is a member trigger the get of the brightness and color attributes. Still not 100% so haven’t revisited this at all. |
I've checked again, but the brightness value will never be correct using this method. The brightness value of the hue dimmer is actually calculated by zigbee2mqtt and not by the dimmer itself. E.g. the brightness up command will result in a 32 increase (https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/converters/fromZigbee.js#L1582) however the bulb itself might use another value. I think we should go another direction by querying the brightness from the bulb itself. (basically #2122 (comment)). @sjorge what do you think? I can implement this fairly easily. |
The current hacky code in this Pr queries the bulb directly, it is... mostly up to date. |
Some device do not support attReport for some keys, this will emulate it. You can configure which keys that should be read when another device send a message and the configured device a bind target or in a group the message was send to. ```yaml devices: '0x0017880104259333': friendly_name: bedroom/desk_lamp retain: true debounce: 0.5 report_emulate: - brightness - color ``` Will have the brightness and color queried for example when a hue dimmer sends commands to the bulb.
4011fca
to
3211216
Compare
// Read the following attributes | ||
read: {cluster: 'genLevelCtrl', attributes: ['currentLevel']}, | ||
// When the bound devices have the following manufacturerID | ||
manufacturerID: ZigbeeHerdsman.Zcl.ManufacturerCode.Philips, |
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.
Do we want Geldopto too? IIRC some of them also lack or have not working attReporting?
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.
Yes we can easily add that later.
This PR is ready for merge, @sjorge can you do some final tests? It works nice in my setup. |
I'll give it some testing tomorrow once I get home from work. |
Oops, looks like the udev changes broke zigbee2mqtt on non linux systems!
Error makes sense, as there is no udevadm on illumos. Same error if I try a raspberrypi running FreeBSD, which also makes sense as there is no udevadm there either. Error is comming from herdsman actually, looks like SerialPort.list() is linux only :( It's probably fine to fallback to the user configure value if SerialPort.list() fails, if one is specified. That seems to be what zwave2mqtt does, OpenZWave/Zwave2Mqtt@7e247b2 |
@sjorge fixed. please try again. |
All looks good: Hue Dimmer:
IKEA Remote
Doesn't seem to do anything... so maybe a bug somewhere |
Also works fine with an IKEA remote via a group:
Apparently groupnames with a '/' cause issues again, it worked fine for a while, it's only the group, devices are fine and don't have to be renamed... I'll file a follow up issues for this after dinner, if I don't forget.
But your update is working fine with both the Hue Dimmer and IKEA remotes. |
Great! thanks for everything! |
After updating to dev, turning off my lamps before heading to bed... I did noticed this.
It looks like the off/on press also triggers the poll of brightness, but we do get on/off state via attReporting for those bulbs. Maybe we should only check when the Hue Dimmer does the brightness up/down buttons? For now I will bumb my debounce to 2 seconds, as a work around |
@sjorge added, will only poll when pressing brightness up or down. |
Thanks, seems to work fine now. 👍 |
@sjorge / @Koenkk - Could polling similiar to this be used to poll Hue bulbs also when they are powered on? Only thing that appears I can see when Hue bulb (https://www.zigbee2mqtt.io/devices/9290012573A.html) is turned on from mains is |
IIRC the few hue bulbs I tested did announce state off/on without any sort of polling. It was just brightless and color that is missing, although it would be cool to query color too I guess... as after a power outage the color of the bulb and what zigbee2mqtt retained are different :( |
I mean state update after total power loss (=power off from mains). I do have some bulbs that are getting switched off from physical switch every now and then and it would be nice to have current info of their state when they are powered back on. Sometimes bulbs are on even though Z2M and HA thinks that they are off after being turned off/on again. Only thing I see when I switch on mains is forementioned This is the entry for bulb in question from .db |
Ah yeah, might be worth fetching state, brightness and color when the device comes online from a cold power off. I just checked, from a cold off -> on it indeed doesn't report the state. |
Would be nice if it is possible to get state queried when device announcement is received. Sorry for my terrible explanation of the issue :) I thought that this issue somewhat would fit in here since it it's near "not reporting" -category which this PR was addressing. |
I think that will already be done when using the availability feature, can you check? |
expect(zigbeeHerdsman.devices.bulb_color_2.getEndpoint(1).read).toHaveBeenCalledTimes(2); | ||
|
||
// Should only call Hue bulb, not e.g. tradfri | ||
expect(zigbeeHerdsman.devices.bulb.getEndpoint(1).read).toHaveBeenCalledTimes(0); |
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.
@sjorge shouldn't this be devices.bulb_2
instead of devices.bulb
?
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.
It's been a long time, but yes... looking at the code, it should be bulb_2 I think. It just happens to work as is because bulb is never involved so it doesn't get called either.
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.
Ok, I can fix this as part of my PR!
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.
Done: #5139
Following up on Koenkk#2122 (comment)
…codes (#5139) * Poll lamps with Philips or Atmel manufacturer code * Fixes incorrect test Following up on #2122 (comment) * Adds GLEDOPTO_CO_LTD and MUELLER_LICHT_INT to lamp polling
Some device do not support attReport for some keys, this will emulate
it.
You can configure which keys that should be read when another device
send a message and the configured device a bind target or in a group the
message was send to.
Will have the brightness and color queried for example when a hue dimmer
sends commands to the bulb.