Drakulix / googlehome

Google Home Integration for Home-Assistant
Apache License 2.0
29 stars 3 forks source link

Overlapping scanning devices #12

Open Edo78 opened 4 years ago

Edo78 commented 4 years ago

I'm not 100% sure and it's hard to debug but it seems that a device_tracker is updated with the information from the latest scan.

If a device is in range of multiple google home the RSSI and the ghname seems to be the one reported from the last google home to respond while they should be from the one with the better RSSI.

Can you confirm this behaviour?

Drakulix commented 4 years ago

Yes, that is correct.

Unfortunately there is no easy way to fix this, because that is how the legacy device_tracker api works. I cannot set a new 'last_seen'-time without overriding the previous values (e.g. RSSI and ghname).

This should be fixable by migrating to the new api. A good chunk of code needs to be refactored to do this though. So while this is on my TODO list, it might take some time, if nobody wants to set in and help. I am happy to mentor anyone willing to give this a shot.

Edo78 commented 4 years ago

I'm a developer but the last and only time I used python was more than 15 years ago to write a blender script but this may be the right time to give python a try. This evening I'm looking at the code to try to figure how it works and if I'm feeling confident enought I'll be back to talk about a refactor.

Drakulix commented 4 years ago

The home-assistant plugin system is an interesting one. Let me give you a quick rundown of how this works, before you take a look at it. Especially if you are not too familiar with python anymore.

Initialization of the component happens here

Per config_entry (that is per linked google account in the home-assistant UI, usually one, but may be more) this function is called and forwards setup to the sensor-platform (for timers and alarms) and the device_tracker-platform (bluetooth monitoring).

So initialization for the device-tracker component happens here.

This creates one DeviceTrackerMonitor per call, meaning again per config_entry. The Monitor shares some common code with the sensor component to discover chromecast devices, which is why it inherits the ChromecastMonitor class. But that really is not important for the functionality of the device_tracker.

Setup continues, when said helper class calls setup per discovered chromecast, that has bluetooth capabilities, which in turn creates a GoogleHomeDeviceScanner.

And that is the one, that inherits the "old" DeviceTracker class from home-assistant.

What should be done instead is creating an entity per discovered bluetooth device, that inherits the TrackerEntity-class and add those via the provided async_add_entities-function.

Then those devices can be updated on incoming scans (instead of overriding all values), but additionally we need to implement logic to remove devices that were not seen for a given time again. I believe, that the mentioned DeviceTracker legacy-api is built on top this new api and has logic to track this. Internally it even has it's own device class. So that code might be used as a starting point.

Additionally all components already using the new api can of course be used to study this. I have not yet found an up-to-date documentation on the subject, but once you get familiar enough with the home-assistant codebase, the existing components really provide a nice set of examples.

Let me know in case there is anything else, I can help you with.