Skip to content

Commit 0e2ca51

Browse files
committed
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`.
1 parent b9395f4 commit 0e2ca51

File tree

5 files changed

+54
-18
lines changed

5 files changed

+54
-18
lines changed

src/main/java/org/altbeacon/beacon/service/BeaconService.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import android.os.Messenger;
4343
import android.support.annotation.MainThread;
4444
import android.support.annotation.NonNull;
45+
import android.support.annotation.WorkerThread;
4546

4647
import org.altbeacon.beacon.Beacon;
4748
import org.altbeacon.beacon.BeaconManager;
@@ -391,6 +392,7 @@ public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean back
391392
}
392393

393394
protected final CycledLeScanCallback mCycledLeScanCallback = new CycledLeScanCallback() {
395+
@MainThread
394396
@TargetApi(Build.VERSION_CODES.HONEYCOMB)
395397
@Override
396398
public void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord) {
@@ -405,6 +407,7 @@ public void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord) {
405407
}
406408
}
407409

410+
@MainThread
408411
@Override
409412
public void onCycleEnd() {
410413
mDistinctPacketDetector.clearDetections();
@@ -418,6 +421,9 @@ public void onCycleEnd() {
418421

419422
if (0 != (getApplicationInfo().flags &= ApplicationInfo.FLAG_DEBUGGABLE)) {
420423
for (Beacon beacon : simulatedScanData) {
424+
// This is an expensive call and we do not want to block the main thread.
425+
// But here we are in debug/test mode so we allow it on the main thread.
426+
//noinspection WrongThread
421427
processBeaconFromScan(beacon);
422428
}
423429
} else {
@@ -430,6 +436,9 @@ public void onCycleEnd() {
430436
if (BeaconManager.getBeaconSimulator().getBeacons() != null) {
431437
if (0 != (getApplicationInfo().flags &= ApplicationInfo.FLAG_DEBUGGABLE)) {
432438
for (Beacon beacon : BeaconManager.getBeaconSimulator().getBeacons()) {
439+
// This is an expensive call and we do not want to block the main thread.
440+
// But here we are in debug/test mode so we allow it on the main thread.
441+
//noinspection WrongThread
433442
processBeaconFromScan(beacon);
434443
}
435444
} else {
@@ -452,7 +461,15 @@ private void processRangeData() {
452461
}
453462
}
454463

455-
private void processBeaconFromScan(Beacon beacon) {
464+
/**
465+
* Helper for processing BLE beacons. This has been extracted from {@link ScanProcessor} to
466+
* support simulated scan data for test and debug environments.
467+
* <p>
468+
* Processing beacons is a frequent and expensive operation. It should not be run on the main
469+
* thread to avoid UI contention.
470+
*/
471+
@WorkerThread
472+
private void processBeaconFromScan(@NonNull Beacon beacon) {
456473
if (Stats.getInstance().isEnabled()) {
457474
Stats.getInstance().log(beacon);
458475
}
@@ -520,6 +537,7 @@ public ScanProcessor(NonBeaconLeScanCallback nonBeaconLeScanCallback) {
520537
mNonBeaconLeScanCallback = nonBeaconLeScanCallback;
521538
}
522539

540+
@WorkerThread
523541
@Override
524542
protected Void doInBackground(ScanData... params) {
525543
ScanData scanData = params[0];
@@ -554,18 +572,6 @@ protected Void doInBackground(ScanData... params) {
554572
}
555573
return null;
556574
}
557-
558-
@Override
559-
protected void onPostExecute(Void result) {
560-
}
561-
562-
@Override
563-
protected void onPreExecute() {
564-
}
565-
566-
@Override
567-
protected void onProgressUpdate(Void... values) {
568-
}
569575
}
570576

571577
private List<Region> matchingRegions(Beacon beacon, Collection<Region> regions) {
Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
package org.altbeacon.beacon.service.scanner;
22

33
import android.bluetooth.BluetoothDevice;
4+
import android.support.annotation.MainThread;
45

56
/**
7+
* Android API agnostic Bluetooth scan callback wrapper.
8+
* <p>
9+
* Since Android bluetooth scan callbacks occur on the main thread it is expected that these
10+
* callbacks will also occur on the main thread.
11+
*
612
* Created by dyoung on 10/6/14.
713
*/
14+
@MainThread
815
public interface CycledLeScanCallback {
9-
public void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord);
10-
public void onCycleEnd();
16+
void onLeScan(BluetoothDevice device, int rssi, byte[] scanRecord);
17+
void onCycleEnd();
1118
}

src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import android.os.HandlerThread;
1515
import android.os.Looper;
1616
import android.os.SystemClock;
17+
import android.support.annotation.AnyThread;
1718
import android.support.annotation.MainThread;
1819
import android.support.annotation.NonNull;
1920
import android.support.annotation.WorkerThread;
@@ -22,6 +23,7 @@
2223
import org.altbeacon.beacon.logging.LogManager;
2324
import org.altbeacon.beacon.startup.StartupBroadcastReceiver;
2425
import org.altbeacon.bluetooth.BluetoothCrashResolver;
26+
2527
import java.util.Date;
2628

2729
@TargetApi(18)
@@ -75,7 +77,22 @@ public abstract class CycledLeScanner {
7577
protected boolean mBackgroundFlag = false;
7678
protected boolean mRestartNeeded = false;
7779

78-
private boolean mDistinctPacketsDetectedPerScan = false;
80+
/**
81+
* Flag indicating device hardware supports detecting multiple identical packets per scan.
82+
* <p>
83+
* Restarting scanning (stopping and immediately restarting) is necessary on many older Android
84+
* devices like the Nexus 4 and Moto G because once they detect a distinct BLE packet in a scan,
85+
* subsequent detections do not get a scan callback. Stopping scanning and restarting clears
86+
* this out, allowing subsequent detection of identical advertisements. On most newer device,
87+
* this is not necessary, and multiple callbacks are given for identical packets detected in
88+
* a single scan.
89+
* <p>
90+
* This is declared {@code volatile} because it may be set by a background scan thread while
91+
* we are in a method on the main thread which will end up checking it. Using this modifier
92+
* ensures that when we read the flag we'll always see the most recently written value. This is
93+
* also true for background scan threads which may be running concurrently.
94+
*/
95+
private volatile boolean mDistinctPacketsDetectedPerScan = false;
7996
private static final long ANDROID_N_MIN_SCAN_CYCLE_MILLIS = 6000l;
8097

8198
protected CycledLeScanner(Context context, long scanPeriod, long betweenScanPeriod, boolean backgroundFlag, CycledLeScanCallback cycledLeScanCallback, BluetoothCrashResolver crashResolver) {
@@ -188,10 +205,12 @@ public void stop() {
188205
}
189206
}
190207

208+
@AnyThread
191209
public boolean getDistinctPacketsDetectedPerScan() {
192210
return mDistinctPacketsDetectedPerScan;
193211
}
194212

213+
@AnyThread
195214
public void setDistinctPacketsDetectedPerScan(boolean detected) {
196215
mDistinctPacketsDetectedPerScan = detected;
197216
}
@@ -331,7 +350,7 @@ private void finishScanCycle() {
331350
// so it is best avoided. If we know the device has detected to distinct
332351
// packets in the same cycle, we will not restart scanning and just keep it
333352
// going.
334-
if (!getDistinctPacketsDetectedPerScan() || mBetweenScanPeriod != 0) {
353+
if (!mDistinctPacketsDetectedPerScan || mBetweenScanPeriod != 0) {
335354
long now = SystemClock.elapsedRealtime();
336355
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.N &&
337356
mBetweenScanPeriod+mScanPeriod < ANDROID_N_MIN_SCAN_CYCLE_MILLIS &&

src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScannerForLollipop.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ private BluetoothLeScanner getScanner() {
280280
private ScanCallback getNewLeScanCallback() {
281281
if (leScanCallback == null) {
282282
leScanCallback = new ScanCallback() {
283-
283+
@MainThread
284284
@Override
285285
public void onScanResult(int callbackType, ScanResult scanResult) {
286286
if (LogManager.isVerboseLoggingEnabled()) {
@@ -299,6 +299,7 @@ public void onScanResult(int callbackType, ScanResult scanResult) {
299299
}
300300
}
301301

302+
@MainThread
302303
@Override
303304
public void onBatchScanResults(List<ScanResult> results) {
304305
LogManager.d(TAG, "got batch records");
@@ -311,6 +312,7 @@ public void onBatchScanResults(List<ScanResult> results) {
311312
}
312313
}
313314

315+
@MainThread
314316
@Override
315317
public void onScanFailed(int errorCode) {
316318
switch (errorCode) {

src/main/java/org/altbeacon/beacon/service/scanner/NonBeaconLeScanCallback.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.altbeacon.beacon.service.scanner;
22

33
import android.bluetooth.BluetoothDevice;
4+
import android.support.annotation.WorkerThread;
45

56
/**
67
* Allows an implementation to see non-Beacon BLE devices as they are scanned.
@@ -23,6 +24,7 @@
2324
* }
2425
* </code></pre>
2526
*/
27+
@WorkerThread
2628
public interface NonBeaconLeScanCallback {
2729
/**
2830
* NOTE: This method is NOT called on the main UI thread.

0 commit comments

Comments
 (0)