AltBeacon / android-beacon-library

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

Feature Request: Variable scan cycle time in CycledLeScanner #1156

Closed juliankotrba closed 8 months ago

juliankotrba commented 9 months ago

Android Beacon Library version

2.19

Request

Hello! I would like to ask something about the implementation of the library, which could potentially lead to a feature request depending on your answer. However, if this is the wrong place for my question, feel free to just close the issue!

In our app we start ranging for beacons with your library after the user clicks a button. So after clicking, the CycledLeScanner starts and stops scanning with the interval defined in ANDROID_N_MIN_SCAN_CYCLE_MILLIS in order to avoid the limits introduced after Android 7.

In addition, by clicking a second button, the user can start scanning and connecting to other BLE devices. As you may have guessed, this often leads to our beloved silent error App 'xxx' is scanning too frequently when we start the second scan. We already implemented some logic to postpone the second start of our scan, however, since the CycledLeScanner is already starting 5 times in 30 seconds, we have a hard time avoiding the error.

I am thinking now about increasing the time of ANDROID_N_MIN_SCAN_CYCLE_MILLIS in order to fit another scan start in the 30 second interval.

My question now is whether you see any problems with increasing the scan cycle time in the CycledLeScanner and if not, could this feature (variable cycle time) be interesting for a public release?

Thanks!

juliankotrba commented 8 months ago

@davidgyoung just FYI, I added the AltBeacon code locally to my project and updated the ANDROID_N_MIN_SCAN_CYCLE_MILLIS to 10 seconds. With this change, the CycledLeScanner starts only 3 times in the limited 30 second interval and we can therefore also start 2 additional scans in this interval. It seems to be working pretty good and we do not receive any App 'xxx' is scanning too frequently anymore.

Since I have not received an answer yet I assume it is not planned to make this interval configurable. If that is the case feel free to just close this issue. Thanks and best regards Julian

davidgyoung commented 8 months ago

Apologies for the delayed response. I think your solution is fine, but a simpler solution may be to to disable the scan off/on behavior entirely.

The default foreground scan intervals are 1100ms on / 0ms off. The only reason the library turns off the scan momentarily between cycles in this case is because the first generation BLE Android devices from 2012 (Nexus 4, Moto G 1st Gen) would only allow be detection per device per scan. This problem has been solved for nearly 10 years now and very few affected devices still exist.

there may be an easy way to turn this off here:

https://github.com/AltBeacon/android-beacon-library/blob/0488d78527055ca9701facb158c37affb908f3fd/lib/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java#L405

Do you think this would solve your problem?

juliankotrba commented 8 months ago

@davidgyoung thank you for your answer and your advice!

This would be an even better solution since it would simplify our logic with the delay to avoid the scanning too frequently. I just did a quick check and saw, that the flag mDistinctPacketsDetectedPerScan is set to true on all of our test devices. However, the condition in the if statement also consists of the Android N timeout (mustStopScanToPreventAndroidNScanTimeout()) and the mBetweenScanPeriod != 0 check. When you say turn it, do you mean to falsify the whole condition (if(false))?

davidgyoung commented 8 months ago

The method mustStopScanToPreventAndroidNScanTimeout() just checks to see if a scan has been going on for 30 minutes, and if so returns true. This logic exists because Android N+ stops such long running scans automatically, requiring a stop and restart to avoid the OS cancelling. In short, I don’t think this part of the logic is a problem for you — it is something you want.

If you have mDistinctPacketsDetectedPerScan set to true, this is enough for your use case to work, yes?

The only trouble I can see is the his flag does not get set to true immediately after app start up. The library must first detect 2+ beacon packets in one scan cycle. And until this happens, the library will be cycling scans on and off at the max rates allowed, possibly interfering with your app’s other scans.

So my proposal is to offer a quicker way to set this flag to true. As far as I know, it should be true for all modern Android devices. Unfortunately there is no way to easily detect if an Android device is modern, but there are proxies, such as the ability to run a more recent Android OS version.

davidgyoung commented 8 months ago

One idea would be to set the flag to true by default for all Android 6+ devices (the Nexus 4, one of the devices which needs to be false, never received Android 6). However there could in theory be some folks running old Nexus 4s on Lineage OS based on Android 11… I am aware of at least two cases where companies have bought up lots of old older phones for use with this library as dedicated beacon scanners.

Obviously this would be unusual. But the fix needed is also for a somewhat usb usual use case. I am just trying to figure out a fix to support a relatively unusual use case that doesn’t cause new problems on other unusual use cases.

juliankotrba commented 8 months ago

@davidgyoung I just tried to set the flag mDistinctPacketsDetectedPerScan immediately (hardcoded) to true however this doesn't change anything because the field mBetweenScanPeriod always has the value 50, therefore the condition mBetweenScanPeriod != 0 is true which makes the whole condition true.

Assuming that the flag mDistinctPacketsDetectedPerScan works as expected, could it maybe work to just remove the condition mBetweenScanPeriod != 0? If I am totally wrong on this please let me know if I am missing something. Thanks!

davidgyoung commented 8 months ago

Ok, yeah, what you say is true if you set a custom between scan period to 50ms. Must you do this? Why not leave it at 0ms, which is the default.

juliankotrba commented 8 months ago

@davidgyoung I wasn't aware we made this change, sorry and thanks for pointing it out!

As you suggest I will update the period to 0 and set the flag hardcoded to true. Thank you very much for detailed help, I appreciate a lot!

Since the logic should already work as expected and therefore no change for the library is required I will now close this GH issue.

BR Julian