From 3acdfa0396b92155c89b9bed46f36c164306658d Mon Sep 17 00:00:00 2001 From: Aaron Kromer Date: Wed, 24 May 2017 12:54:40 -0400 Subject: [PATCH 1/5] Add some documentation for cycled scanner handlers This helps to try explaining how / when the various internal handlers should be used. This is important to help developers understand various threading flow orders, race conditions, and resource contention problems. As part of the documentation this adds `MainThread` and `WorkerThread` annotations to the deferred `Runnable` tasks' `run` methods. See Also: - https://developer.android.com/studio/write/annotations.html#thread-annotations --- .../service/scanner/CycledLeScanner.java | 26 +++++++++++++++++++ .../CycledLeScannerForJellyBeanMr2.java | 5 ++++ .../scanner/CycledLeScannerForLollipop.java | 5 ++++ 3 files changed, 36 insertions(+) diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java index 4b6782339..4c640cd6f 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java @@ -15,6 +15,8 @@ import android.os.HandlerThread; import android.os.Looper; import android.os.SystemClock; +import android.support.annotation.MainThread; +import android.support.annotation.NonNull; import org.altbeacon.beacon.BeaconManager; import org.altbeacon.beacon.logging.LogManager; @@ -41,8 +43,30 @@ public abstract class CycledLeScanner { protected long mBetweenScanPeriod; + /** + * Main thread handle for scheduling scan cycle tasks. + *

+ * Use this to schedule deferred tasks such as the following: + *

+ */ + @NonNull protected final Handler mHandler = new Handler(Looper.getMainLooper()); + + /** + * Handler to background thread for interacting with the low-level Android BLE scanner. + *

+ * Use this to queue any potentially long running BLE scanner actions such as starts and stops. + */ + @NonNull protected final Handler mScanHandler; + + /** + * Worker thread hosting the internal scanner message queue. + */ + @NonNull private final HandlerThread mScanThread; protected final BluetoothCrashResolver mBluetoothCrashResolver; @@ -177,6 +201,7 @@ public void destroy() { // So we stop the thread using the handler, so we make sure it happens after all other // waiting Runnables are finished. mHandler.post(new Runnable() { + @MainThread @Override public void run() { LogManager.d(TAG, "Quitting scan thread"); @@ -270,6 +295,7 @@ protected void scheduleScanCycleStop() { setWakeUpAlarm(); } mHandler.postDelayed(new Runnable() { + @MainThread @Override public void run() { scheduleScanCycleStop(); diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForJellyBeanMr2.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForJellyBeanMr2.java index b38f7281c..42c6d6cbd 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForJellyBeanMr2.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForJellyBeanMr2.java @@ -5,6 +5,8 @@ import android.bluetooth.BluetoothDevice; import android.content.Context; import android.os.SystemClock; +import android.support.annotation.MainThread; +import android.support.annotation.WorkerThread; import org.altbeacon.beacon.logging.LogManager; import org.altbeacon.bluetooth.BluetoothCrashResolver; @@ -36,6 +38,7 @@ protected boolean deferScanIfNeeded() { setWakeUpAlarm(); } mHandler.postDelayed(new Runnable() { + @MainThread @Override public void run() { scanLeDevice(true); @@ -65,6 +68,7 @@ private void postStartLeScan() { final BluetoothAdapter.LeScanCallback leScanCallback = getLeScanCallback(); mScanHandler.removeCallbacksAndMessages(null); mScanHandler.post(new Runnable() { + @WorkerThread @Override public void run() { try { @@ -85,6 +89,7 @@ private void postStopLeScan() { final BluetoothAdapter.LeScanCallback leScanCallback = getLeScanCallback(); mScanHandler.removeCallbacksAndMessages(null); mScanHandler.post(new Runnable() { + @WorkerThread @Override public void run() { try { diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java index 5a7ad075f..9787569b0 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java @@ -10,6 +10,8 @@ import android.content.Context; import android.os.ParcelUuid; import android.os.SystemClock; +import android.support.annotation.MainThread; +import android.support.annotation.WorkerThread; import org.altbeacon.beacon.BeaconManager; import org.altbeacon.beacon.logging.LogManager; @@ -134,6 +136,7 @@ protected boolean deferScanIfNeeded() { setWakeUpAlarm(); } mHandler.postDelayed(new Runnable() { + @MainThread @Override public void run() { scanLeDevice(true); @@ -190,6 +193,7 @@ private void postStartLeScan(final List filters, final ScanSettings final ScanCallback scanCallback = getNewLeScanCallback(); mScanHandler.removeCallbacksAndMessages(null); mScanHandler.post(new Runnable() { + @WorkerThread @Override public void run() { try { @@ -220,6 +224,7 @@ private void postStopLeScan() { final ScanCallback scanCallback = getNewLeScanCallback(); mScanHandler.removeCallbacksAndMessages(null); mScanHandler.post(new Runnable() { + @WorkerThread @Override public void run() { try { From 1be324b5ae49bbc12f177c2e3f048e4431993f60 Mon Sep 17 00:00:00 2001 From: Aaron Kromer Date: Wed, 24 May 2017 13:33:13 -0400 Subject: [PATCH 2/5] Start documenting thread usage for service objects This makes the current implicit assumption that the scan cycler is started/stopped on the main thread explicit. This helps quickly inform developers what the callback expectations are in regards to race conditions and state safety. In order to verify this we need to trace the `start`/`stop`/`destroy` calls back to the service. The service's core methods `onCreate` and `onDestroy` are called on the main thread (see links below). This means the service creates it's `mMessenger` on the main thread, which creates the `IncomingHandler` on the main thread which binds to the main looper. However, the `IncomingHandler` class itself leaves the looper unspecified relying on looper for the thread which creates it. As we did in #430, this modifies `IncomingHandler` to explicitly use the main looper as a future proofing mechanism. This way we can ensure this implicit expectation doesn't change or at least make it obvious when troubleshooting cases where someone expects it to have changed. Similarly, this adds the main thread annotation to it's `handleMessage` implementation. Working from here we can confirm that the only places where the beacon monitoring/ranging is updated is from the main thread through the `IncomingHandler`. As the `IncomingHandler` is the main communication channel for the service we transfer the main thread expectation to the associated ranging/monitoring methods. _If_ someone decides to call these methods directly on the service these annotations help the IDEs check/document that such calls are expected from the main thread. With all of that documented we can now confidently state that the scan cycler's `start`, `stop`, and `destroy` are also meant to be called from the main thread. As `start` and `stop` both call `scanLeDevice` we can start tracing any other threading expectations for it. We already know it's called from the main thread through deferred jobs. This leaves the `finishScanCycle` as the last call site. `finishScanCycle` is only called from `scheduleScanCycleStop`. As this method name implies it's called through a deferred job on the main thread. It is also called the "first" time in `scanLeDevice`. Thus we've shown that `scanLeDevice`, `finishScanCycle`, and `scheduleScanCycleStop` are expected to run on the main thread. See Also: - https://developer.android.com/reference/android/os/Handler.html#Handler%28%29 - https://developer.android.com/training/multiple-threads/communicate-ui.html - https://developer.android.com/reference/android/app/Service.html - https://developer.android.com/guide/components/services.html - https://github.com/AltBeacon/android-beacon-library/pull/430 --- .../altbeacon/beacon/service/BeaconService.java | 17 ++++++++++++++++- .../beacon/service/scanner/CycledLeScanner.java | 10 +++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/altbeacon/beacon/service/BeaconService.java b/src/main/java/org/altbeacon/beacon/service/BeaconService.java index 3a50855a7..29f0d9007 100644 --- a/src/main/java/org/altbeacon/beacon/service/BeaconService.java +++ b/src/main/java/org/altbeacon/beacon/service/BeaconService.java @@ -37,8 +37,10 @@ import android.os.Build; import android.os.Handler; import android.os.IBinder; +import android.os.Looper; import android.os.Message; import android.os.Messenger; +import android.support.annotation.MainThread; import android.support.annotation.NonNull; import org.altbeacon.beacon.Beacon; @@ -145,9 +147,16 @@ static class IncomingHandler extends Handler { private final WeakReference mService; IncomingHandler(BeaconService service) { + /* + * Explicitly state this uses the main thread. Without this we defer to where the + * service instance is initialized/created; which is usually the main thread anyways. + * But by being explicit we document our code design expectations for where things run. + */ + super(Looper.getMainLooper()); mService = new WeakReference(service); } + @MainThread @Override public void handleMessage(Message msg) { BeaconService service = mService.get(); @@ -206,7 +215,7 @@ else if (msg.what == MSG_SYNC_SETTINGS) { */ final Messenger mMessenger = new Messenger(new IncomingHandler(this)); - + @MainThread @Override public void onCreate() { bluetoothCrashResolver = new BluetoothCrashResolver(this); @@ -294,6 +303,7 @@ public boolean onUnbind(Intent intent) { return false; } + @MainThread @Override public void onDestroy() { LogManager.e(TAG, "onDestroy()"); @@ -330,6 +340,7 @@ private PendingIntent getRestartIntent() { /** * methods for clients */ + @MainThread public void startRangingBeaconsInRegion(Region region, Callback callback) { synchronized (rangedRegionState) { if (rangedRegionState.containsKey(region)) { @@ -342,6 +353,7 @@ public void startRangingBeaconsInRegion(Region region, Callback callback) { mCycledScanner.start(); } + @MainThread public void stopRangingBeaconsInRegion(Region region) { int rangedRegionCount; synchronized (rangedRegionState) { @@ -355,6 +367,7 @@ public void stopRangingBeaconsInRegion(Region region) { } } + @MainThread public void startMonitoringBeaconsInRegion(Region region, Callback callback) { LogManager.d(TAG, "startMonitoring called"); monitoringStatus.addRegion(region, callback); @@ -362,6 +375,7 @@ public void startMonitoringBeaconsInRegion(Region region, Callback callback) { mCycledScanner.start(); } + @MainThread public void stopMonitoringBeaconsInRegion(Region region) { LogManager.d(TAG, "stopMonitoring called"); monitoringStatus.removeRegion(region); @@ -371,6 +385,7 @@ public void stopMonitoringBeaconsInRegion(Region region) { } } + @MainThread public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean backgroundFlag) { mCycledScanner.setScanPeriods(scanPeriod, betweenScanPeriod, backgroundFlag); } diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java index 4c640cd6f..a176da53d 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java @@ -1,7 +1,6 @@ package org.altbeacon.beacon.service.scanner; import android.Manifest; -import android.annotation.SuppressLint; import android.annotation.TargetApi; import android.app.AlarmManager; import android.app.PendingIntent; @@ -125,6 +124,7 @@ public static CycledLeScanner createScanner(Context context, long scanPeriod, lo * between LOW_POWER_MODE vs. LOW_LATENCY_MODE * @param backgroundFlag */ + @MainThread public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean backgroundFlag) { LogManager.d(TAG, "Set scan periods called with %s, %s Background mode must have changed.", scanPeriod, betweenScanPeriod); @@ -165,6 +165,7 @@ public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean back } } + @MainThread public void start() { LogManager.d(TAG, "start called"); mScanningEnabled = true; @@ -175,7 +176,7 @@ public void start() { } } - @SuppressLint("NewApi") + @MainThread public void stop() { LogManager.d(TAG, "stop called"); mScanningEnabled = false; @@ -194,6 +195,7 @@ public void setDistinctPacketsDetectedPerScan(boolean detected) { mDistinctPacketsDetectedPerScan = detected; } + @MainThread public void destroy() { LogManager.d(TAG, "Destroying"); // We cannot quit the thread used by the handler until queued Runnables have been processed, @@ -218,7 +220,7 @@ public void run() { protected abstract void startScan(); - @SuppressLint("NewApi") + @MainThread protected void scanLeDevice(final Boolean enable) { try { mScanCyclerStarted = true; @@ -285,6 +287,7 @@ protected void scanLeDevice(final Boolean enable) { } } + @MainThread protected void scheduleScanCycleStop() { // Stops scanning after a pre-defined scan period. long millisecondsUntilStop = mScanCycleStopTime - SystemClock.elapsedRealtime(); @@ -308,6 +311,7 @@ public void run() { protected abstract void finishScan(); + @MainThread private void finishScanCycle() { LogManager.d(TAG, "Done with scan cycle"); try { From 5f98c55aeff9cac25c8db038f467497a0e82307d Mon Sep 17 00:00:00 2001 From: Aaron Kromer Date: Wed, 24 May 2017 14:09:32 -0400 Subject: [PATCH 3/5] Fix where scanner thread is quit. This fixes what I believe was a simply copy-paste error regarding how the scanner thread is quit. This uses the scanner thread handler, instead of the main thread handler, to schedule the `quit` operation. This way any queued scan jobs (such as a stop) complete before we quit the thread. This is what the comment states, but wasn't what the code implemented. The `mHandler.removeCallbacksAndMessages` is removed from the background job as `destroy` is already on the main thread. So we want to clear any other jobs still in the queue to run later now as we are in the process of destroying everything. --- .../beacon/service/scanner/CycledLeScanner.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java index a176da53d..fb8126ca8 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java @@ -16,6 +16,7 @@ import android.os.SystemClock; import android.support.annotation.MainThread; import android.support.annotation.NonNull; +import android.support.annotation.WorkerThread; import org.altbeacon.beacon.BeaconManager; import org.altbeacon.beacon.logging.LogManager; @@ -198,17 +199,19 @@ public void setDistinctPacketsDetectedPerScan(boolean detected) { @MainThread public void destroy() { LogManager.d(TAG, "Destroying"); + + // Remove any postDelayed Runnables queued for the next scan cycle + mHandler.removeCallbacksAndMessages(null); + // We cannot quit the thread used by the handler until queued Runnables have been processed, // because the handler is what stops scanning, and we do not want scanning left on. // So we stop the thread using the handler, so we make sure it happens after all other // waiting Runnables are finished. - mHandler.post(new Runnable() { - @MainThread + mScanHandler.post(new Runnable() { + @WorkerThread @Override public void run() { LogManager.d(TAG, "Quitting scan thread"); - // Remove any postDelayed Runnables queued for the next scan cycle - mHandler.removeCallbacksAndMessages(null); mScanThread.quit(); } }); From 0ab8bee9e960c80d56f8bdf043b32aed7fb9d6f2 Mon Sep 17 00:00:00 2001 From: Aaron Kromer Date: Wed, 24 May 2017 14:16:48 -0400 Subject: [PATCH 4/5] Future proof threading / scheduled task flows This adds a guard to ensure that we only attempt any further scans when the scan cycler is started. This helps ensure the cycler has terminated scanning for the case where all regions have been removed but the service is still bound. In this case only `stop` will be called (instead of both `stop` and `destroy`). When this happens there are two potential scheduled tasks still queued: - `scanLeDevice(true)` from `deferScanIfNeeded` - `scheduleScanCycleStop` Which potential task is queued is based on if we were in an active scan period or a between scan period when `stop` was called. In the case of a between scan period the `deferScanIfNeeded` _will_ cause a faux scanning cycle to start up again when it ends (scanning won't actually start, but all flags and internal state appear as if it is running). It will run for one more scan cycle before it terminates in `finishScanCycle`. Similarly, if we are in an active scan period the scheduled job for finishing the scan cycle will continue for the duration of the scan period. --- .../altbeacon/beacon/service/scanner/CycledLeScanner.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java index fb8126ca8..b224b0625 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java @@ -230,7 +230,7 @@ protected void scanLeDevice(final Boolean enable) { if (getBluetoothAdapter() == null) { LogManager.e(TAG, "No Bluetooth adapter. beaconService cannot scan."); } - if (enable) { + if (mScanningEnabled && enable) { if (deferScanIfNeeded()) { return; } @@ -283,6 +283,9 @@ protected void scanLeDevice(final Boolean enable) { mScanCyclerStarted = false; stopScan(); mLastScanCycleEndTime = SystemClock.elapsedRealtime(); + // Clear any queued schedule tasks as we're done scanning + mScanHandler.removeCallbacksAndMessages(null); + finishScanCycle(); } } catch (SecurityException e) { @@ -294,7 +297,7 @@ protected void scanLeDevice(final Boolean enable) { protected void scheduleScanCycleStop() { // Stops scanning after a pre-defined scan period. long millisecondsUntilStop = mScanCycleStopTime - SystemClock.elapsedRealtime(); - if (millisecondsUntilStop > 0) { + if (mScanningEnabled && millisecondsUntilStop > 0) { LogManager.d(TAG, "Waiting to stop scan cycle for another %s milliseconds", millisecondsUntilStop); if (mBackgroundFlag) { From 0f5e82358256c26d6659808cd628bd991a9e75c9 Mon Sep 17 00:00:00 2001 From: Aaron Kromer Date: Fri, 26 May 2017 12:35:12 -0400 Subject: [PATCH 5/5] Add thread annotations for `CycledLeScanCallback` Through testing it's been confirmed that the bluetooth scan callbacks are always run on the main thread. This chases that through the scanners and our callbacks so that this is properly documented / annotated. During this process the unused `AsyncTask` methods were removed as we don not use them. Additionally, the `mDistinctPacketsDetectedPerScan` flag has been declared `volatile` to ensure reads always see the most recently written value without having to rely on a heavy weight `synchronized` block. As this is a flag we only perform simple reads / writes. While we do perform a multi-step operation around reading/writing this value the work performed is too heavy weight and long to wrap it in a `synchronized` block. Using `volatile` gives multipel concurrent background threads a better chance of aborting this logic if another has changed the value. However, the downside of not having complete synchronization is that a background thread will try to check a few more packets and then re-set the flag to `true`. --- .../beacon/service/BeaconService.java | 32 +++++++++++-------- .../service/scanner/CycledLeScanCallback.java | 11 +++++-- .../service/scanner/CycledLeScanner.java | 23 +++++++++++-- .../scanner/CycledLeScannerForLollipop.java | 4 ++- .../scanner/NonBeaconLeScanCallback.java | 2 ++ 5 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/altbeacon/beacon/service/BeaconService.java b/src/main/java/org/altbeacon/beacon/service/BeaconService.java index 29f0d9007..d3ad8af19 100644 --- a/src/main/java/org/altbeacon/beacon/service/BeaconService.java +++ b/src/main/java/org/altbeacon/beacon/service/BeaconService.java @@ -42,6 +42,7 @@ import android.os.Messenger; import android.support.annotation.MainThread; import android.support.annotation.NonNull; +import android.support.annotation.WorkerThread; import org.altbeacon.beacon.Beacon; import org.altbeacon.beacon.BeaconManager; @@ -391,6 +392,7 @@ public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean back } protected final CycledLeScanCallback mCycledLeScanCallback = new CycledLeScanCallback() { + @MainThread @TargetApi(Build.VERSION_CODES.HONEYCOMB) @Override public void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord) { @@ -405,6 +407,7 @@ public void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord) { } } + @MainThread @Override public void onCycleEnd() { mDistinctPacketDetector.clearDetections(); @@ -418,6 +421,9 @@ public void onCycleEnd() { if (0 != (getApplicationInfo().flags &= ApplicationInfo.FLAG_DEBUGGABLE)) { for (Beacon beacon : simulatedScanData) { + // This is an expensive call and we do not want to block the main thread. + // But here we are in debug/test mode so we allow it on the main thread. + //noinspection WrongThread processBeaconFromScan(beacon); } } else { @@ -430,6 +436,9 @@ public void onCycleEnd() { if (BeaconManager.getBeaconSimulator().getBeacons() != null) { if (0 != (getApplicationInfo().flags &= ApplicationInfo.FLAG_DEBUGGABLE)) { for (Beacon beacon : BeaconManager.getBeaconSimulator().getBeacons()) { + // This is an expensive call and we do not want to block the main thread. + // But here we are in debug/test mode so we allow it on the main thread. + //noinspection WrongThread processBeaconFromScan(beacon); } } else { @@ -452,7 +461,15 @@ private void processRangeData() { } } - private void processBeaconFromScan(Beacon beacon) { + /** + * Helper for processing BLE beacons. This has been extracted from {@link ScanProcessor} to + * support simulated scan data for test and debug environments. + *

+ * Processing beacons is a frequent and expensive operation. It should not be run on the main + * thread to avoid UI contention. + */ + @WorkerThread + private void processBeaconFromScan(@NonNull Beacon beacon) { if (Stats.getInstance().isEnabled()) { Stats.getInstance().log(beacon); } @@ -520,6 +537,7 @@ public ScanProcessor(NonBeaconLeScanCallback nonBeaconLeScanCallback) { mNonBeaconLeScanCallback = nonBeaconLeScanCallback; } + @WorkerThread @Override protected Void doInBackground(ScanData... params) { ScanData scanData = params[0]; @@ -554,18 +572,6 @@ protected Void doInBackground(ScanData... params) { } return null; } - - @Override - protected void onPostExecute(Void result) { - } - - @Override - protected void onPreExecute() { - } - - @Override - protected void onProgressUpdate(Void... values) { - } } private List matchingRegions(Beacon beacon, Collection regions) { diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanCallback.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanCallback.java index 3fe8904c8..f8066130f 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanCallback.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanCallback.java @@ -1,11 +1,18 @@ package org.altbeacon.beacon.service.scanner; import android.bluetooth.BluetoothDevice; +import android.support.annotation.MainThread; /** + * Android API agnostic Bluetooth scan callback wrapper. + *

+ * Since Android bluetooth scan callbacks occur on the main thread it is expected that these + * callbacks will also occur on the main thread. + * * Created by dyoung on 10/6/14. */ +@MainThread public interface CycledLeScanCallback { - public void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord); - public void onCycleEnd(); + void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord); + void onCycleEnd(); } diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java index b224b0625..86f691d82 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java @@ -14,6 +14,7 @@ import android.os.HandlerThread; import android.os.Looper; import android.os.SystemClock; +import android.support.annotation.AnyThread; import android.support.annotation.MainThread; import android.support.annotation.NonNull; import android.support.annotation.WorkerThread; @@ -22,6 +23,7 @@ import org.altbeacon.beacon.logging.LogManager; import org.altbeacon.beacon.startup.StartupBroadcastReceiver; import org.altbeacon.bluetooth.BluetoothCrashResolver; + import java.util.Date; @TargetApi(18) @@ -75,7 +77,22 @@ public abstract class CycledLeScanner { protected boolean mBackgroundFlag = false; protected boolean mRestartNeeded = false; - private boolean mDistinctPacketsDetectedPerScan = false; + /** + * Flag indicating device hardware supports detecting multiple identical packets per scan. + *

+ * Restarting scanning (stopping and immediately restarting) is necessary on many older Android + * devices like the Nexus 4 and Moto G because once they detect a distinct BLE packet in a scan, + * subsequent detections do not get a scan callback. Stopping scanning and restarting clears + * this out, allowing subsequent detection of identical advertisements. On most newer device, + * this is not necessary, and multiple callbacks are given for identical packets detected in + * a single scan. + *

+ * This is declared {@code volatile} because it may be set by a background scan thread while + * we are in a method on the main thread which will end up checking it. Using this modifier + * ensures that when we read the flag we'll always see the most recently written value. This is + * also true for background scan threads which may be running concurrently. + */ + private volatile boolean mDistinctPacketsDetectedPerScan = false; private static final long ANDROID_N_MIN_SCAN_CYCLE_MILLIS = 6000l; protected CycledLeScanner(Context context, long scanPeriod, long betweenScanPeriod, boolean backgroundFlag, CycledLeScanCallback cycledLeScanCallback, BluetoothCrashResolver crashResolver) { @@ -188,10 +205,12 @@ public void stop() { } } + @AnyThread public boolean getDistinctPacketsDetectedPerScan() { return mDistinctPacketsDetectedPerScan; } + @AnyThread public void setDistinctPacketsDetectedPerScan(boolean detected) { mDistinctPacketsDetectedPerScan = detected; } @@ -334,7 +353,7 @@ private void finishScanCycle() { // so it is best avoided. If we know the device has detected to distinct // packets in the same cycle, we will not restart scanning and just keep it // going. - if (!getDistinctPacketsDetectedPerScan() || mBetweenScanPeriod != 0) { + if (!mDistinctPacketsDetectedPerScan || mBetweenScanPeriod != 0) { long now = SystemClock.elapsedRealtime(); if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && mBetweenScanPeriod+mScanPeriod < ANDROID_N_MIN_SCAN_CYCLE_MILLIS && diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java index 9787569b0..175c9400d 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java @@ -280,7 +280,7 @@ private BluetoothLeScanner getScanner() { private ScanCallback getNewLeScanCallback() { if (leScanCallback == null) { leScanCallback = new ScanCallback() { - + @MainThread @Override public void onScanResult(int callbackType, ScanResult scanResult) { if (LogManager.isVerboseLoggingEnabled()) { @@ -299,6 +299,7 @@ public void onScanResult(int callbackType, ScanResult scanResult) { } } + @MainThread @Override public void onBatchScanResults(List results) { LogManager.d(TAG, "got batch records"); @@ -311,6 +312,7 @@ public void onBatchScanResults(List results) { } } + @MainThread @Override public void onScanFailed(int errorCode) { switch (errorCode) { diff --git a/src/main/java/org/altbeacon/beacon/service/scanner/NonBeaconLeScanCallback.java b/src/main/java/org/altbeacon/beacon/service/scanner/NonBeaconLeScanCallback.java index f85139cb2..15aeda1ee 100644 --- a/src/main/java/org/altbeacon/beacon/service/scanner/NonBeaconLeScanCallback.java +++ b/src/main/java/org/altbeacon/beacon/service/scanner/NonBeaconLeScanCallback.java @@ -1,6 +1,7 @@ package org.altbeacon.beacon.service.scanner; import android.bluetooth.BluetoothDevice; +import android.support.annotation.WorkerThread; /** * Allows an implementation to see non-Beacon BLE devices as they are scanned. @@ -23,6 +24,7 @@ * } * */ +@WorkerThread public interface NonBeaconLeScanCallback { /** * NOTE: This method is NOT called on the main UI thread.