bermuda icon indicating copy to clipboard operation
bermuda copied to clipboard

ibeacons falsely detected when mac address is shared

Open mogorman opened this issue 1 year ago • 12 comments

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 avatar Jul 11 '24 04:07 mogorman

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'

mogorman avatar Jul 11 '24 04:07 mogorman

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!

agittins avatar Jul 11 '24 13:07 agittins

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()

```

jleinenbach avatar Jul 11 '24 14:07 jleinenbach

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.

agittins avatar Jul 11 '24 14:07 agittins

device_tracker_diff.txt I hope this helps.

jleinenbach avatar Jul 11 '24 14:07 jleinenbach

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

mogorman avatar Jul 11 '24 15:07 mogorman

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.

agittins avatar Jul 11 '24 16:07 agittins

@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.

jleinenbach avatar Jul 11 '24 19:07 jleinenbach

@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:

  • update Bermuda to v0.6.8
  • click "Download Diagnostics" at a point when the beacon is moving, ie should be advertising two iBeacon ids
  • after the beacon is back to sending only one iBeacon id, (I'm guessing maybe 30 seconds?) do another "Download Diagnostics"

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.

agittins avatar Jul 16 '24 07:07 agittins

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

mogorman avatar Jul 18 '24 06:07 mogorman

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.

agittins avatar Jul 23 '24 21:07 agittins

Has there been anymore traction on this as of late? I also have some Bluecharm beacons I just received and was hoping to use this feature but I’m noticing the same behavior. Hoping a solution can be found or at the very least, if it shows as the same device in Bermuda, exposing major/minor and/or UUID as entities would suffice for my use case.

creemerica avatar Sep 13 '24 00:09 creemerica

Hi all - v0.8.0 should give you initial support for this aspect of the bluecharm beacons! If you add both iBeacon UUIDs as devices in the Bermuda configure dialog, they should now track independently. Note that the device tracker timeout still applies, so you'll typically see both of them active for a while after a change, but the triggering should only happen when the relevant beacon is received.

I'll leave this open for a bit but if you have a chance to try it out please let me know how it goes!

I can envisage this feature leading to a desire for a separate timeout value for the beacons, so that you could set the movement one to be quite short for good responsiveness while leaving the presence one long for reliable/stable presence sensing. It's one my to-do :-)

agittins avatar Apr 23 '25 09:04 agittins

I'm going to close this one out since as far as I know it's working now in v0.8.0. If anyone has any trouble with it please feel free to re-open or comment here so we can follow it up.

agittins avatar May 05 '25 13:05 agittins

Can't re-open but for tracking I want to add pointer to https://github.com/agittins/bermuda/discussions/675

It seems somewhere along the way things broke again. From what I can tell, the current code assigns the metadevice itself as a source to the metadevice which I believe is wrong (it should a device identified by MAC)

eigenphase avatar Oct 14 '25 04:10 eigenphase

Hi @mogorman , @creemerica (maybe @jleinenbach ?) , have you used this feature recently? Can you confirm it's working with the latest codebase?

I have some issues, it doesn't work at all for me and I suspect there's a regression that stopped it from working. Knowing if it's just me or not would already be very helpful. Thanks!

eigenphase avatar Oct 31 '25 08:10 eigenphase