Add UUID filter to Beacon Monitor

[chatziko]

Summary

The recently added Beacon Monitor is hugely useful, however the fact that it reports all beacons with no exception can lead to excessive traffic (and battery usage). It's quite common to see dozens of beacons throughout a day, of which just a couple might be relevant.

This PR implements a UUID filter to report only selected beacons. I tried to achieve a balance between features and ease-of-use:

• UUIDs are selected from a list containing all currently visible beacons (and those selected in the past, even if they are not currently visible). So one needs to be in proximity to add a beacon to the filter (but on the upside there is no need to manually enter UUIDs, which sucks).

• Only the UUID is matched, not the major/minor. Otherwise beacons that change major/minor would be a problem. Also there is no wildcard support (tricky to implement a nice UI for).

• There is a boolean flag to turn the filter from include (only report selected beacons) to exclude (report all but the selected).

I also fixed a couple of edge-case bugs I discovered:

• If no updates were made, the skippedUpdated counter was always being incremented even if a beacon was visible (it would reset to 0 only in case requireUpdate == true). So the counter could exceed MAX_SKIPPED_UPDATED causing the beacon to be removed the very first time it went missing.
• In the edge case when an update was present every single time, MAX_SKIPPED_UPDATED would never be enforced.

Finally the PR contains a tiny bit of polishing and small performance tweaks (given that updates run every second). Still, the overall PR is relatively simple.

I would be happy to modify the code, of course, if someone has a better idea about this feature.

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#876

[jpelgrom]

I'm not in a position to really test if these changes work as explained, but do have a few suggestions to make the code more Kotlin-y :)

[chatziko]

I'm not in a position to really test if these changes work as explained, but do have a few suggestions to make the code more Kotlin-y :)

Thanks for the feedback, I implemeted the suggestions (first time writing in kotlin, I think I like it 😄)

[jpelgrom]

Code looks good to me now.

@dshokouhi I think you've used the beacon monitor with multiple beacons in the past, would you be able to test this?

[dshokouhi]

@dshokouhi I think you've used the beacon monitor with multiple beacons in the past, would you be able to test this?

I haven't had a chance to test any code PRs lately, I'll see if I can later today or tomorrow

[dshokouhi]

Spoke too soon, functionally works as described. I am able to get a dynamic list of currently available UUIDs and can either include/exclude them.

I do wonder if we should disable the "Exclude selected UUIDs" toggle when the UUID filter is empty because it doesn't change anything functionally when the list is empty and its toggled. In fact that option really only applies when there is a UUID filter list correct?

[chatziko]

I do wonder if we should disable the "Exclude selected UUIDs" toggle when the UUID filter is empty because it doesn't change anything functionally when the list is empty and its toggled. In fact that option really only applies when there is a UUID filter list correct?

Indeed, it has no effect on an empty list.

I thought about disabling it, but I didn't know whether it's best from a UI perpective (will users understand why it's disabled?). I can surely add it.

[dshokouhi]

I thought about disabling it, but I didn't know whether it's best from a UI perpective (will users understand why it's disabled?). I can surely add it.

We do disable settings for high accuracy mode as the options do not apply until it is enabled. We could also just mention in the sensor description that the option has no effect when the UUID list is empty to avoid more code changes :)

[chatziko]

Ok I implemented disabling the option. This involved creating a CoroutineScope so have a quick look that I'm doing this properly.

[dshokouhi]

Looks good to me, tested the debug APK and it functions as desired. The exclude option is also disabled properly when it cannot be used.

[JBassett]

Need to fix merge conflicts, then looks good.

[chatziko]

Need to fix merge conflicts, then looks good.

I fixed the conflicts (they were caused by the recent db63a38)