-
Notifications
You must be signed in to change notification settings - Fork 837
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't restart scanning if device detects duplicate advertisements per scan #491
Merged
davidgyoung
merged 6 commits into
master
from
no-scanning-restarts-on-duplicate-detection-devices
Apr 21, 2017
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
15a723f
Track distinct packet detections per cycle and don't stop scanning if…
davidgyoung 746e319
Merge branch 'master' into no-scanning-restarts-on-duplicate-detectio…
davidgyoung 1628a56
Add changelog entry
davidgyoung 44592ad
Whitespace cleanup
davidgyoung c1dacc9
Fix PR number in changelog
davidgyoung 1e49cd6
Merge branch 'master' into no-scanning-restarts-on-duplicate-detectio…
davidgyoung File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,6 @@ | |
import org.altbeacon.beacon.logging.LogManager; | ||
import org.altbeacon.beacon.startup.StartupBroadcastReceiver; | ||
import org.altbeacon.bluetooth.BluetoothCrashResolver; | ||
|
||
import java.util.Date; | ||
|
||
@TargetApi(18) | ||
|
@@ -52,6 +51,7 @@ public abstract class CycledLeScanner { | |
protected boolean mBackgroundFlag = false; | ||
protected boolean mRestartNeeded = false; | ||
|
||
private 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) { | ||
|
@@ -162,6 +162,14 @@ public void stop() { | |
} | ||
} | ||
|
||
public boolean getDistinctPacketsDetectedPerScan() { | ||
return mDistinctPacketsDetectedPerScan; | ||
} | ||
|
||
public void setDistinctPacketsDetectedPerScan(boolean detected) { | ||
mDistinctPacketsDetectedPerScan = detected; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we drop these accessors? This is an internal flag that's only needed for this class. Exposing it means long term support for this flag and limits the ability of this class to be refactored / modified without breaking API changes. |
||
|
||
public void destroy() { | ||
mScanThread.quit(); | ||
} | ||
|
@@ -268,25 +276,39 @@ private void finishScanCycle() { | |
if (mScanning) { | ||
if (getBluetoothAdapter() != null) { | ||
if (getBluetoothAdapter().isEnabled()) { | ||
long now = SystemClock.elapsedRealtime(); | ||
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && | ||
mBetweenScanPeriod+mScanPeriod < ANDROID_N_MIN_SCAN_CYCLE_MILLIS && | ||
now-mLastScanCycleStartTime < ANDROID_N_MIN_SCAN_CYCLE_MILLIS) { | ||
// As of Android N, only 5 scans may be started in a 30 second period (6 | ||
// seconds per cycle) otherwise they are blocked. So we check here to see | ||
// if the scan period is 6 seconds or less, and if we last stopped scanning | ||
// fewer than 6 seconds ag and if so, we simply do not stop scanning | ||
LogManager.d(TAG, "Not stopping scan because this is Android N and we" + | ||
" keep scanning for a minimum of 6 seconds at a time. "+ | ||
"We will stop in "+(ANDROID_N_MIN_SCAN_CYCLE_MILLIS-(now-mLastScanCycleStartTime))+" millisconds."); | ||
// Determine if we need to restart scanning. Restarting scanning is only | ||
// needed on devices incapable of detecting multiple distinct BLE advertising | ||
// packets in a single cycle, typically older Android devices (e.g. Nexus 4) | ||
// On such devices, it is necessary to stop scanning and restart to detect | ||
// multiple beacon packets in the same scan, allowing collection of multiple | ||
// rssi measurements. Restarting however, causes brief detection dropouts | ||
// 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()) { | ||
long now = SystemClock.elapsedRealtime(); | ||
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && | ||
mBetweenScanPeriod+mScanPeriod < ANDROID_N_MIN_SCAN_CYCLE_MILLIS && | ||
now-mLastScanCycleStartTime < ANDROID_N_MIN_SCAN_CYCLE_MILLIS) { | ||
// As of Android N, only 5 scans may be started in a 30 second period (6 | ||
// seconds per cycle) otherwise they are blocked. So we check here to see | ||
// if the scan period is 6 seconds or less, and if we last stopped scanning | ||
// fewer than 6 seconds ag and if so, we simply do not stop scanning | ||
LogManager.d(TAG, "Not stopping scan because this is Android N and we" + | ||
" keep scanning for a minimum of 6 seconds at a time. "+ | ||
"We will stop in "+(ANDROID_N_MIN_SCAN_CYCLE_MILLIS-(now-mLastScanCycleStartTime))+" millisconds."); | ||
} | ||
else { | ||
try { | ||
LogManager.d(TAG, "stopping bluetooth le scan"); | ||
finishScan(); | ||
} catch (Exception e) { | ||
LogManager.w(e, TAG, "Internal Android exception scanning for beacons"); | ||
} | ||
} | ||
} | ||
else { | ||
try { | ||
LogManager.d(TAG, "stopping bluetooth le scan"); | ||
finishScan(); | ||
} catch (Exception e) { | ||
LogManager.w(e, TAG, "Internal Android exception scanning for beacons"); | ||
} | ||
LogManager.d(TAG, "Not stopping scanning. Device capable of multiple indistinct detections per scan."); | ||
} | ||
|
||
mLastScanCycleEndTime = SystemClock.elapsedRealtime(); | ||
|
43 changes: 43 additions & 0 deletions
43
src/main/java/org/altbeacon/beacon/service/scanner/DistinctPacketDetector.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package org.altbeacon.beacon.service.scanner; | ||
|
||
import android.util.Log; | ||
|
||
import java.nio.ByteBuffer; | ||
import java.util.HashSet; | ||
import java.util.Set; | ||
|
||
/** | ||
* Created by dyoung on 4/8/17. | ||
* | ||
* This class tracks whether multiple distinct BLE packets have been seen, with the purpose of | ||
* determining if the Android device supports detecting multiple distinct packets in a single scan. | ||
* Some older devices are not capable of this (e.g. Nexus 4, Moto G1), so detecting multiple packets | ||
* requires stopping and restarting scanning on these devices. This allows detecting if that is | ||
* neessary | ||
*/ | ||
public class DistinctPacketDetector { | ||
// Sanity limit for the number of packets to track, so we don't use too much memory | ||
private static final int MAX_PACKETS_TO_TRACK = 1000; | ||
protected Set<ByteBuffer> mDistinctPacketsDetected = new HashSet<ByteBuffer>(); | ||
|
||
public void clearDetections() { | ||
mDistinctPacketsDetected.clear(); | ||
} | ||
|
||
public boolean isPacketDistinct(String originMacAddress, byte[] scanRecord) { | ||
byte[] macBytes = originMacAddress.getBytes(); | ||
ByteBuffer buffer = ByteBuffer.allocate(macBytes.length+scanRecord.length); | ||
buffer.put(macBytes); | ||
buffer.put(scanRecord); | ||
buffer.rewind(); // rewind puts position back to beginning so .equals and .hashCode work | ||
|
||
boolean distinct = !mDistinctPacketsDetected.contains(buffer); | ||
if (mDistinctPacketsDetected.size() == MAX_PACKETS_TO_TRACK) { | ||
return mDistinctPacketsDetected.contains(buffer); | ||
} | ||
else { | ||
return mDistinctPacketsDetected.add(buffer); | ||
} | ||
} | ||
|
||
} |
57 changes: 57 additions & 0 deletions
57
src/test/java/org/altbeacon/beacon/service/scanner/DistinctPacketDetectorTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package org.altbeacon.beacon.service.scanner; | ||
import org.junit.AfterClass; | ||
import org.junit.BeforeClass; | ||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.robolectric.RobolectricTestRunner; | ||
import org.robolectric.annotation.Config; | ||
import static org.junit.Assert.assertFalse; | ||
import static org.junit.Assert.assertTrue; | ||
|
||
@Config(sdk = 18) | ||
|
||
@RunWith(RobolectricTestRunner.class) | ||
public class DistinctPacketDetectorTest { | ||
@BeforeClass | ||
public static void testSetup() { | ||
} | ||
|
||
@AfterClass | ||
public static void testCleanup() { | ||
|
||
} | ||
|
||
@Test | ||
public void testSecondDuplicatePacketIsNotDistinct() throws Exception { | ||
DistinctPacketDetector dpd = new DistinctPacketDetector(); | ||
dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x01, 0x02}); | ||
boolean secondResult = dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x01, 0x02}); | ||
assertFalse("second call with same packet should not be distinct", secondResult); | ||
} | ||
|
||
@Test | ||
public void testSecondNonDuplicatePacketIsDistinct() throws Exception { | ||
DistinctPacketDetector dpd = new DistinctPacketDetector(); | ||
dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x01, 0x02}); | ||
boolean secondResult = dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x03, 0x04}); | ||
assertTrue("second call with different packet should be distinct", secondResult); | ||
} | ||
|
||
@Test | ||
public void testSamePacketForDifferentMacIsDistinct() throws Exception { | ||
DistinctPacketDetector dpd = new DistinctPacketDetector(); | ||
dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x01, 0x02}); | ||
boolean secondResult = dpd.isPacketDistinct("01:01:01:01:01:01", new byte[] {0x01, 0x02}); | ||
assertTrue("second packet with different mac should be distinct", secondResult); | ||
} | ||
|
||
@Test | ||
public void clearingDetectionsPreventsDistinctDetection() throws Exception { | ||
DistinctPacketDetector dpd = new DistinctPacketDetector(); | ||
dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x01, 0x02}); | ||
dpd.clearDetections(); | ||
boolean secondResult = dpd.isPacketDistinct("01:02:03:04:05:06", new byte[] {0x01, 0x02}); | ||
assertTrue("second call with same packet after clear should be distinct", secondResult); | ||
} | ||
|
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to put this in the service and not the scanner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be on the service because the service class is where an
AsyncTask is kicked to do scan processing on another thread. This processing needs to be done there and not on the main thread as it is it is non-trivial. This also causes the accessors to be needed on CycledLeScanner