search

Bluetooth-Devices / bthome-ble

Parser for BTHome BLE devices
https://bthome.io/
MIT License
67 stars 12 forks source link

fix: fix for repeated events #79

Closed Ernst79 closed 1 year ago

Ernst79 commented 1 year ago

fix: fix for repeated events (fix #75)

Ernst79 commented 1 year ago

Should fix https://github.com/home-assistant/core/issues/95892

@bdraco I'm not sure if this is the most clean way to fix this issue, possibly it should be fixed in sensor-state-data instead. What happens is that if no event is fired (e.g. only a battery update), it has the old event in memory (in self._events_updates) and it is fired on each subsequent sensor update here https://github.com/Bluetooth-Devices/sensor-state-data/blob/57d6ee07694c9a4f967b06af611227527db7fd88/src/sensor_state_data/data.py#L195C20-L195C20

I feel a bit uncomfortable changing code in sensor-state-data, so I leave that up to you to decide.

bdraco commented 1 year ago

How about https://github.com/Bluetooth-Devices/sensor-state-data/pull/53

Ernst79 commented 1 year ago

Yes, that is probably much better. I’ll update this PR after sensor-state-data is released. Thanks for the quick fix!

bdraco commented 1 year ago

Should be good to rebase now

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (74b38b7) 77.28% compared to head (54556a3) 77.28%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #79 +/- ## ======================================= Coverage 77.28% 77.28% ======================================= Files 6 6 Lines 471 471 Branches 71 71 ======================================= Hits 364 364 Misses 83 83 Partials 24 24 ``` | [Impacted Files](https://app.codecov.io/gh/Bluetooth-Devices/bthome-ble/pull/79?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices) | Coverage Δ | | |---|---|---| | [src/bthome\_ble/\_\_init\_\_.py](https://app.codecov.io/gh/Bluetooth-Devices/bthome-ble/pull/79?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices#diff-c3JjL2J0aG9tZV9ibGUvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Ernst79 commented 1 year ago

@bdraco It looks like sensor-state-data 2.16.1 does not fix the issue somehow. I'll have to look into it further tonight, but the last bthome test that is added in this PR is a test with a button event, followed by event = {}. But it still shows the old result on the second event, despite the fix. I'll look into it further tonight, but something seems still wrong.

Ernst79 commented 1 year ago

@bdraco I had a look why the event is still there after the update, and I found out that the update() function bthome_ble is using is defined in bluetooth_sensor_state_data. bluetooth_sensor_state_data is refering to SensorData from sensor_state_data, but both have an update() function. The one of bluetooth_sensor_state_data is being used in the end.

https://github.com/Bluetooth-Devices/bluetooth-sensor-state-data/blob/8bbfd5b2cf29b1dd2e6f546137ebf7d810b6a453/src/bluetooth_sensor_state_data/__init__.py#L69

That is why the clearing of the events in the same update() function in sensor_state_data is not used in my bthome test example. What would be the correct way to fix this? Adding the same self._events_updates.clear() in bluetooth_sensor_state_data?

bdraco commented 1 year ago

@bdraco I had a look why the event is still there after the update, and I found out that the update() function bthome_ble is using is defined in bluetooth_sensor_state_data. bluetooth_sensor_state_data is refering to SensorData from sensor_state_data, but both have an update() function. The one of bluetooth_sensor_state_data is being used in the end.

https://github.com/Bluetooth-Devices/bluetooth-sensor-state-data/blob/8bbfd5b2cf29b1dd2e6f546137ebf7d810b6a453/src/bluetooth_sensor_state_data/__init__.py#L69

That is why the clearing of the events in the same update() function in sensor_state_data is not used in my bthome test example. What would be the correct way to fix this? Adding the same self._events_updates.clear() in bluetooth_sensor_state_data?

Right. I forgot we override that for the Rssi sensor and we don't call super() because we need specific order

Best solution right now is the same exact change in bluetooth_sensor_state_data as you suggest above

Ernst79 commented 1 year ago

Thanks, that fixed it.