Skip to content

Fix ghost events for sensors - #8189

Merged
manup merged 1 commit into
dresden-elektronik:masterfrom
manup:fix_sensor_ghost_events
Apr 28, 2025
Merged

Fix ghost events for sensors#8189
manup merged 1 commit into
dresden-elektronik:masterfrom
manup:fix_sensor_ghost_events

Conversation

@manup

@manup manup commented Apr 26, 2025

Copy link
Copy Markdown
Member

After the refactor of sensorToMap() when an event arrived in handleSensorEvent() not only the respective parent object like attr/, state/, config/, etc. was emitted on Websocket but sometimes other event objects too.

The PR prevents this to filter out any item that doesn't match the events item suffix.

Related forum posts:

https://forum.phoscon.de/t/ghost-button-events-with-deconz-2-29-5-and-aqara-smart-button/6300/14

https://forum.phoscon.de/t/lumi-vibration-aq1-sensor-with-phantom-triggers-since-deconz-2-29-5/6305/6

After the refactor of sensorToMap() when an event arrived in handleSensorEvent() not only respective parent object like attr, state, config, etc. was emitted on websocket but sometimes other event objects too.

The PR prevents this to filter out any item that doesn't match the events item suffix.

Related forum posts:

https://forum.phoscon.de/t/ghost-button-events-with-deconz-2-29-5-and-aqara-smart-button/6300/14

https://forum.phoscon.de/t/lumi-vibration-aq1-sensor-with-phantom-triggers-since-deconz-2-29-5/6305/6
@manup manup added the Fix label Apr 26, 2025
@manup manup added this to the v2.30.0-beta milestone Apr 26, 2025
@ebaauw

ebaauw commented Apr 27, 2025

Copy link
Copy Markdown
Collaborator

While I'm happy with these changes, I fail to understand in full how they're addressing the mentioned issues. Setting websocketnotifyall to false would probably have the same effect (except maybe during deCONZ startup). Also, as lightToMap() uses exactly the same logic as sensorToMap(), the same issue would manifest itself there.

I don't understand how superfluous web socket notifications would lead to ghost events. For a client to "see" a button event, it would need to check whether state/lastupdated has changed value. Just taking the fact that you've received an event notification for the state seems wrong (and would probably, even with this PR, continue to generate one ghost button press per wireless switch after deCONZ startup). Likewise, to conclude motion, vibration, or open, a client would need to check that the corresponding state item has been set to true, not just the receipt of a state notification.

@Smanar

Smanar commented Apr 27, 2025

Copy link
Copy Markdown
Collaborator

It I m right the event
"e": "changed",

changed - resource attributes have changed;

Is for a change, it mean there is a change, what is the utility to send websocket notification with useless information ? For me if deconz send me a "buttonevent" it's because there is a buttonevent, no need to check the "lastupdated". No need to send periodic report with a previous button state.

@manup

manup commented Apr 28, 2025

Copy link
Copy Markdown
Member Author

Yes lightToMap() needs also to be looked at. There I assume it doesn't cause much notable side effects since lights state usually isn't used as trigger for automations (sans the lightoids).

This PR only aims to restore the messaging of earlier versions, while keeping the improvements and cleanups from the refactor PR.

What concerns me more is that some items for which ghost events were seen must have had push flags set. For example from the issue in the forum the hourly Xiaomi report from a switch also caused to emit state/buttonevent so there seems to be some left overs in a state/* item which triggered these to be send to Websocket as well when the other events were received. Note the hourly special report on this device only set attr/* and config/*.

I need to debug this further to see what's going on, so this PR is only a first workaround.

@manup
manup merged commit 52c8e03 into dresden-elektronik:master Apr 28, 2025
@manup
manup deleted the fix_sensor_ghost_events branch April 28, 2025 11:15
@ebaauw

ebaauw commented Apr 28, 2025

Copy link
Copy Markdown
Collaborator

What concerns me more is that some items for which ghost events were seen must have had push flags set

Indeed. With websocketnotifyall set to false, I tend to see full web socket notifications incl. all items when the first Zigbee message is received from the device, after deCONZ startup. I suspect the push flags are set on startup and only cleared when the event notification is delivered. Hence my remark that this PR might not prevent all superfluous notifications.

For me if deconz send me a "buttonevent" it's because there is a buttonevent, no need to check the "lastupdated".

My API client code originates from before the websocket notifications were implemented. It polls the deCONZ gateway, checking for changed values. The notifications are used as optional accelerator; my code continues to work when the notifications fail (or the websocket connection cannot be established). The same piece of code handles the poll response and the websocket payload, comparing the received values to the cached previous values. With this setup, I'm immune to this issue and to the fact that still not all values changes are notified over the websocket.

Note that deCONZ rules also check for changed values, and don't just rely on notification events.

manup added a commit to manup/deconz-rest-plugin that referenced this pull request Apr 28, 2025
Similar treatment as in dresden-elektronik#8189

This PR will only emit Websocket events for the objects attr, state, config, etc. if the internal event for it originates such an item. It prevents extra events like config object when just an attr object should be send via Websocket.

Clearing the push flags happens after lightToMap() was called to clear only items which fit the internal event suffix (and I prefer to let lightToMap() to not have this logic). The check group event is also emitted outside lightToMap() now.
manup added a commit that referenced this pull request Apr 29, 2025
Similar treatment as in #8189

This PR will only emit Websocket events for the objects attr, state, config, etc. if the internal event for it originates such an item. It prevents extra events like config object when just an attr object should be send via Websocket.

Clearing the push flags happens after lightToMap() was called to clear only items which fit the internal event suffix (and I prefer to let lightToMap() to not have this logic). The check group event is also emitted outside lightToMap() now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants