AltBeacon / android-beacon-library

Allows Android apps to interact with BLE beacons
Apache License 2.0
2.84k stars 836 forks source link

Status Preservation causes problems with modified beacon regions #1075

Closed davidgyoung closed 8 months ago

davidgyoung commented 2 years ago

The library by default persists the beacon Regions being monitored. If using the default Scheduled Job scan strategy, it also persists the beacon Regions being ranged. (The discussion below is about the monitoring persistence problem, but the same applies to ranging when using Scheduled Jobs).

This persistence is necessary because Android can kill and restart the app in the background, requiring such preservation for the library to continue from where it left off. But this presents a number of problems:

  1. If you call startMonitoring(Region("my region 1", UUID1, major1, minor1)), and run your app, then later edit this line to be startMonitoring(Region("my region 2", UUID2, major1, minor1)) and run your app again, now TWO regions will be monitored. This continues forever and the orphaned "my region 1" will never stop being monitored. This can cause unexpected behavior and bugs that are hard to track down. To stop monitoring the orphaned region, the user has to add code and run it at least once to call stopMonitoring(Region("my region 1", UUID1, major1, minor1)).

  2. Because the first parameter of the Region is used as a unique key, changing the identifiers (the last three parameters) without changing the unique key parameter has no effect. The original region definition applies forever despite the fact that the user thinks they made a change. See example case of this here

Two things should be done to fix the above problems:

  1. If you call startMonitoring/startRanging with the same unique key but a different set of identifiers, it should replace the region definition with a new one. This would solve the second problem above.
  2. Somehow, the orphaned regions should be removed. I'm not sure how to do this because there is no clear way to know when an old call to startMonitoring/startRanging for a specific region is no longer applicable. It is certainly possible for a region definition to be set up on a phone once, then it is run for years. I am open to ideas on how to solve this problem.

For the second problem described above, iOS addresses this by special handling in the AppDelegate's didFinishLaunching method. That method is special in that it outlines a transaction. Any beacon regions that are not re-created (by calling startMonitoring) by the time that method ends, are deemed to be expired, and they are removed. We could perhaps implement the equivalent on Android in the Application onCreate method, clearing out any stale regions at that time. But this would be a breaking change for apps that do not start monitoring in that method. It would also be difficult to explain because Application onCreate is not as commonly used by Android developers as the iOS AppDelegate's didFinishLaunching.

davidgyoung commented 2 years ago

One idea: look for a change on packageInfo.lastUpdateTime per here https://developer.android.com/reference/android/content/pm/PackageInfo.html#lastUpdateTime

if the value changes from one run to the next then we you know the app code has changed and it is a new execution. This would be a safe time to clear all persistence

davidgyoung commented 2 years ago

I can find no working mechanism to detective the package has updated.

davidgyoung commented 2 years ago

It seems that the problem of changing the Region definition with the same identifier does not exist for monitoring, but does for ranging. This is because MonitoringStatus.java already has code to detect a region identifier change and applies the change accordingly. But I need to add the equivalent change for Ranging region updates to ScanState.java like this:

                // In case the user has changed the definition, update it.
                Region existingRegion = existingRangedRegions.get(existingMonitoredRegions.indexOf(newRangedRegion));
                if (newRangedRegion.hasSameIdentifiers(existingRegion)) {
                    mRangedRegionState.remove(existingRegion);
                    mRangedRegionState.put(newRangedRegion, new RangeState(new Callback(mContext.getPackageName())));
                }

Curiously, I noticed that ScanState.java (which only persists ranged regions) does not have the problem of orphaned regions, because it removes any that the BeaconManager does not already know about:

        for (Region existingRangedRegion: existingRangedRegions) {
            if (!newRangedRegions.contains(existingRangedRegion)) {
                LogManager.d(TAG, "Stopping ranging region: "+existingRangedRegion);
                mRangedRegionState.remove(existingRangedRegion);
            }
        }

Is there really a reason we can't do this with MonitoringStatus? Seems like it would be reasonable to expect that if you make one change to start/stop monitoring a region you would continue to set all the rest during the same app run. The issue is that we also retain the last known region status e.g. (inside vs. outside) and we don't want to inadvertently clear that by removing the region if it is about to be re-added.

davidgyoung commented 2 years ago

I think I found a way to do this. We can purge the inactive monitored regions at the end of the scan cycle. Any monitored regions that have not been re-registered by then will be purged.

davidgyoung commented 8 months ago

Fixed in #1089