Skip to content
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

Sensor reports multiple times #1264

Closed
stoinov opened this issue Mar 16, 2019 · 19 comments
Closed

Sensor reports multiple times #1264

stoinov opened this issue Mar 16, 2019 · 19 comments
Milestone

Comments

@stoinov
Copy link
Contributor

stoinov commented Mar 16, 2019

I am using 1.2.1 with 20190223 and I have three WSDCGQ11LM Aqara sensors. When they report, I see update for each of their measured values consequently for under a second like this:

zigbee2mqtt:info 3/16/2019, 2:40:18 PM MQTT publish: topic 'topic/device/sensor', payload '{"temperature":21.94,"humidity":35.56,"pressure":945,"linkquality":126,"battery":100,"voltage":3005,"last_seen":1552747218411}'
zigbee2mqtt:info 3/16/2019, 2:40:18 PM MQTT publish: topic 'topic/device/sensor', payload '{"temperature":21.94,"humidity":34.69,"pressure":945,"linkquality":126,"battery":100,"voltage":3005,"last_seen":1552747218416}'
zigbee2mqtt:info 3/16/2019, 2:40:18 PM MQTT publish: topic 'topic/device/sensor', payload '{"temperature":21.94,"humidity":34.69,"pressure":946,"linkquality":126,"battery":100,"voltage":3005,"last_seen":1552747218420}'

as you can see the temperature is updated first, then the humidity and finally the pressure. I've noticed similar behavior in the report for #684.
When I press the button though, I get only one report with all three values updated.

Is this expected behavior?
Is the device actually sending three separate packets in the span of a second with each of the values and you just send a full report using previous data?
Is this limitation of the device, the protocol or the Zigbee2mqtt?
Can this be somehow fixed and all three reports be combined into one?

@Koenkk
Copy link
Owner

Koenkk commented Mar 16, 2019

This is expected behaviour, they come in separate zigbee messages.

@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

And you add the last known value for each of the missing parameters?
Also if I press the button they all come in one message?

@Koenkk
Copy link
Owner

Koenkk commented Mar 16, 2019

Yes, last know values are added

When pressing the button, or on a interval of 1 hour, the values are reported in one message trough the genBasic cluster, see https://github.com/Koenkk/zigbee-shepherd-converters/blob/master/converters/fromZigbee.js#L331

@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

I see. So the device definition and all of its properties include the genBasic cluster and this is basically a separate way of the device report that includes all three values.

Does the device send always all three values one after another or only sometimes? Because I see that it always returned three for me, even if they are not updated. If that's the case, then it makes sense to either combine them in one package and post a single mqtt payload, or post each one the way it comes without adding other last known values.

The reason behind it is that if you are storing the reports in a database for future use, those three values in short amount of time skew the results when you try to do calculations.

I've added logic in my Node-Red to start a window of 5 seconds, and import any new data from the same sensor as overwrite of the initial payload. This way I basically keep only the last packet.

similar logic can be added I hope in the report function for the devices - combine_reports for instance - and it will be taking seconds or milliseconds as value. It will just delay the report of a packet and combine all incoming properties into one packet during that period.

@Koenkk
Copy link
Owner

Koenkk commented Mar 16, 2019

That would be good, are you comfortable with implementing such a mechanism?

@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

I am not familiar with node.js programming - I just write pure JS functions for my NodeRed needs. That being said - I was looking through the files to see if I can do it myself, but I'd appreciate if you point me where exactly should I start the testing - is it in publishEntityState?

@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

Actually there should be a thread somewhere that starts a timer whenever a new report from a device with this parameter comes. This initial report and all incoming reports during this timer should only be overwriting the state, and not be published. Once the timer expires, the state for the device should be published

@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

There is setTimeout() function in Node that can be used like this for example.
We'll have to keep a variable with the state of the timer in order to know whether to update state or start a new timer. The variable should probably include the device ID so we can keep multiple separate timers.

If you can point me where would be the best place for implementing this, I can give it a try.

@Koenkk
Copy link
Owner

Koenkk commented Mar 16, 2019

I expect that only changes in zigbee-shepherd-conveters are required.

@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

I understand what you want me to do, but I don't understand how the linked code works and more precisely - which part is handling the publishing and the updating of the store.

As I said I am not really familiar with the latest JS code structs so I might be missing how those section works. My understanding is that they only handle the precision manipulation, but then return it for posting to another function.

@Koenkk
Copy link
Owner

Koenkk commented Mar 16, 2019

Thinking about this again, your combine_report idea sounds like a better solution to me. I've implemented this. The option is called debounce.

Documentation: https://github.com/Koenkk/zigbee2mqtt.io/blob/dev/configuration/device_specific_configuration.md

You can test this feature in the dev branch.

@Koenkk Koenkk added this to the 1.3 milestone Mar 16, 2019
@stoinov
Copy link
Contributor Author

stoinov commented Mar 16, 2019

Confirming that it works as expected - with debounce: 1 I no longer see multiple reports.

@david-kracht
Copy link

  • For xiaomi_pressure, xiaomi_humidity, xiaomi_temperature store the values in the store and emit them after a certain timeout (e.g. 1 second).

What about debouncing PIR sensors, too ?! I observe the following ...

Apr 27 14:07:20 raspberrypi3 npm[490]: zigbee2mqtt:info 2019-4-27 14:07:20 MQTT publish: topic 'home/zigbee/pir/office', payload '{"illuminance":238,"linkquality":54}'
Apr 27 14:07:20 raspberrypi3 npm[490]: zigbee2mqtt:info 2019-4-27 14:07:20 MQTT publish: topic 'home/zigbee/pir/office', payload '{"occupancy":true,"linkquality":54}'

Getting the change of illuminance first, when being triggered by a movement is somehow wired. I am almost sure, that the movement is the basis for the state-change.

@Koenkk
Copy link
Owner

Koenkk commented Apr 28, 2019

@davekr8 that is already possible using the current implementation (it's not device type specific)

@david-kracht
Copy link

david-kracht commented Apr 28, 2019

I have set debounce: 1 in the yaml, but the result is a double message.

@david-kracht
Copy link

Just to emphasis the problem (here such a pair from the my logs):
Mai 02 10:09:15 raspberrypi3 npm[3620]: zigbee2mqtt:info 2019-5-2 10:09:15 MQTT publish: topic 'home/zigbee/pir/office', payload '{"illuminance":351,"linkquality":80,"occupancy":false,"battery":100,"voltage":3035}'
Mai 02 10:09:15 raspberrypi3 npm[3620]: zigbee2mqtt:info 2019-5-2 10:09:15 MQTT publish: topic 'home/zigbee/pir/office', payload '{"illuminance":351,"linkquality":80,"occupancy":true,"battery":100,"voltage":3035}'
`
I have set debounce to 1.

We have the situation, that triggered by a movement, the first msg contains the following pair of states (illuminance x, occupancy:false), the second (bounce) message switches (occupancy:false -> true). From my point of view the intermediate state occupancy:false is misleading, or even wrong !

@stoinov
Copy link
Contributor Author

stoinov commented May 2, 2019

I can tell you this should work - I have the same config and debounce works for motion sensors:

  '0x00158d00000000000':
    friendly_name: living/zig06/motion
    retain: false
    debounce: 1

@david-kracht
Copy link

david-kracht commented May 3, 2019

Ah ... I see the difference in my (wrong) config item:
device_options: debounce: 1

Edit:
Now it's working, but would a generalized (type specific defaults) debounce flag not be helpful?

@thanhdotien278
Copy link

I've wasted many time for this problem untill see your comment. I'm going to complain to @Koenkk but I saw your comment @stoinov . You saved my day! Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants