agittins / bermuda

Bermuda Bluetooth/BLE Triangulation / Trilateration for HomeAssistant
MIT License
401 stars 10 forks source link

ibeacons falsely detected when mac address is shared #236

Open mogorman opened 1 month ago

mogorman commented 1 month ago

Version of the custom_component

0.6.7

Configuration

image

Describe the bug

I have a bluecharm beacon that sends out a different ibeacon address when motion is detected. I have both ibeacons in my devices list. Anytime either beacon broadcasts, both devices sensor data update. I assume this is happening as the bluecharm only transmits from a single mac address. I did watch with nrfconnect to confirm that the motion sensor was not triggering even when bermuda said it updated.

Debug log

Jul 11 00:12:58 bob homeassistant[2968776]: 2024-07-11 00:12:58.266 DEBUG (MainThread) [custom_components.bermuda] Device BCPro_774096 was in 'Garage', now in 'Magikarp'
Jul 11 00:12:58 bob homeassistant[2968776]: 2024-07-11 00:12:58.267 DEBUG (MainThread) [custom_components.bermuda] Device BCPro_774096 was in 'Garage', now in 'Magikarp'
mogorman commented 1 month ago

a long time later the in motion falls off and the config changes to this image Moving car makes pattern return image

Jul 11 00:13:05 bob homeassistant[2968776]: 2024-07-11 00:13:05.264 DEBUG (MainThread) [custom_components.bermuda] Device BCPro_774096 was in 'Garage', now in 'Magikarp'
Jul 11 00:13:05 bob homeassistant[2968776]: 2024-07-11 00:13:05.264 DEBUG (MainThread) [custom_components.bermuda] Device BCPro_774096 was in 'Garage', now in 'Magikarp'
agittins commented 1 month ago

Ha, yes I think you have nailed the cause here.

IIRC I think Bermuda is checking the payload of the advertisement to see if it's an iBeacon only in order to then create the ibeacon "meta-device". Thereafter, it is probably taking any advert received from any matching MAC as being signs of that meta-device still being present (which explains why both will then trigger).

The change in the config dialog is beacuse it can no longer match a MAC to the now absent ibeacon, so it just reflects the uuid/major/minor being saved.

It's a philosophical break from what I was originally doing (ie, I'm willing to accept any advert from a given "device" as proof that it's there), but this is a valid use-case given the implementation, I'll just need to think about how broadly I should or should not apply this change. I'm thinking that it will be only for iBeacons, since I think it's likely the only case where a single MAC address is going to be desired to present itself as more than a single entity - but I'll think on that a little more before applying a fix.

Thanks for the report, and your solid analysis of the issue!

jleinenbach commented 1 month ago

Maybe like this?

async def async_setup_entry(
    hass: HomeAssistant,
    entry: ConfigEntry,
    async_add_devices: AddEntitiesCallback,
) -> None:
    """Load Device Tracker entities for a config entry."""
    coordinator: BermudaDataUpdateCoordinator = hass.data[DOMAIN][entry.entry_id]

    created_devices = []  # list of devices we've already created entities for

    @callback
    def device_new(
        address: str, scanners: list[str]
    ) -> None:  # pylint: disable=unused-argument
        """Create entities for newly-found device

        Called from the data co-ordinator when it finds a new device that needs
        to have sensors created. Not called directly, but via the dispatch
        facility from HA.
        Make sure you have a full list of scanners ready before calling this.
        """
        if address not in created_devices:
            entities = []

            # Retrieve the device details from the coordinator
            device = coordinator.devices.get(address)
            if not device:
                return

            # Additional property checks specific to the device type
            if device.device_type == "iBeacon":
                if not (device.uuid and device.major and device.minor):
                    return

            entities.append(BermudaDeviceTracker(coordinator, entry, address))
            # We set update before add to False because we are being
            # call(back(ed)) from the update, so causing it to call another would be... bad.
            async_add_devices(entities, False)
            created_devices.append(address)
        else:
            # _LOGGER.debug(
            #     "Ignoring create request for existing dev_tracker %s", address
            # )
            pass
        # tell the co-ord we've done it.
        coordinator.device_tracker_created(address)

    # Connect device_new to a signal so the coordinator can call it
    entry.async_on_unload(async_dispatcher_connect(hass, SIGNAL_DEVICE_NEW, device_new))

    # Now we must tell the co-ord to do initial refresh, so that it will call our callback.
    await coordinator.async_config_entry_first_refresh()
agittins commented 1 month ago

Hi Jens, your contributions are extremely welcome, but could you present them as a diff instead of just the whole block of code? I don't have all three thousand lines committed to memory (in fact, I have zero) so it's impossible for me to visualise the change.

PRs in particular are welcome, but also just presenting a diff (perhaps with the ```python leader) is also a good way to present smaller changes.

jleinenbach commented 1 month ago

device_tracker_diff.txt I hope this helps.

mogorman commented 1 month ago

i made pr of their change so its easier to view / merge

agittins commented 1 month ago

Thanks @mogorman, I appreciate you taking the time to do that. Unfortunately I don't see how the change solves the issue at all.

@jleinenbach could you explain why this would work? Was this change created using an LLM like ChatGPT? I don't mean to sound unfriendly, but that's two changes you've submitted in the last few hours that are of dubious merit and have cost me time to review. I would like some assurance that these suggestions are being made in good faith and from an application of actual effort to learn and understand the codebase, rather than hoping to throw things via an LLM and see what sticks. If you have written these yourself then I am happy to help you learn as you go, but I would need to see some evidence that this is the case.

jleinenbach commented 1 month ago

@agittins I encountered a similar issue with my smartphone, which also identifies as an iBeacon. As an IT professional, I often use resources like ChatGPT to explore potential solutions. This suggestion seemed plausible after thorough discussion and had previously helped me resolve other issues. If I had been more certain of the solution, I would have directly submitted it as a pull request. However, my knowledge of the codebase is not enough to fully verify the solution, and I apologize for the time it cost you. I assure you that my suggestions are made with the intent of contributing constructively and learning through the process.

Meanwhile, I discovered a previously undocumented method to extract the IRK from the Windows Registry for my Android smartphone and my wife's iPhone, enabling successful addition as "Private BLE Devices". By starting regedit with psexec as a system user, the IRKs can be found under: HKLM\SYSTEM\CurrentControlSet\Services\BTHPORT\Parameters\Keys. I have proposed this as a documentation change on the Home Assistant website. Unfortunately, this method did not work for my Pixel Buds Pro - I guess it does not use an IRK.

Sorry again for the inconvenience, and thanks for your patience and understanding.

agittins commented 1 month ago

@mogorman on looking into this further I am wondering if the bluetooth backend might continue caching the adverts against the mac even after they've stopped sending.

If you have time, it would be great if you could:

The diags step might take several seconds, since it does a fair amount of work to scrub out the MAC addresses.

If you can send both files to me (you can just upload them here) I'll see what I can learn from that.

What I am looking for is to see if the old advertisement data is still persisted in the backend or not, in which case we will have a much harder time working out how to treat the "beacon" device as away despite the mac address still being active.

mogorman commented 1 month ago

1f6295d54f744c58a2d8cd83ca26bdf4 is the stationary beacon address 1f6295d54f744c58a2d8cd83ca26bdf5 is the moving beacon address

the captures were like you requested. moving, and then settled.

config_entry-bermuda-4779a3acd65447282319fa6f0c573205.json

config_entry-bermuda-4779a3acd65447282319fa6f0c573205(1).json

agittins commented 1 month ago

Great, thanks @mogorman. I've had some family stuff come up lately so haven't had a chance to go through this yet but what you've provided will hopefully help me get it sorted - just short on time currently.