Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Ensure consistent BLE scan behavior #2718

Merged
merged 2 commits into from Dec 13, 2023
Merged

Ensure consistent BLE scan behavior #2718

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db42285
Add a BLE scan option to allow duplicate results
XuGuohui Dec 13, 2023
9881648
[wiring] fixes BLE scanning cache
XuGuohui Dec 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 0 additions & 34 deletions hal/src/nRF52840/ble_hal.cpp
View file
Open in desktop
Expand Up @@ -374,9 +374,6 @@ class BleObject::Observer {
int processAdvReportEventFromThread(const ble_evt_t* event);

private:
bool isCachedDevice(const hal_ble_addr_t& address) const;
int addCachedDevice(const hal_ble_addr_t& address);
void clearCachedDevice();
hal_ble_scan_result_evt_t* getPendingResult(const hal_ble_addr_t& address);
int addPendingResult(const hal_ble_scan_result_evt_t& resultEvt);
void removePendingResult(const hal_ble_addr_t& address);
Expand All @@ -396,7 +393,6 @@ class BleObject::Observer {
hal_ble_on_scan_result_cb_t scanResultCallback_; /**< Callback function on scan result. */
void* context_; /**< Context of the scan result callback function. */
os_timer_t scanGuardTimer_; /**< Timer to guard the scanning procedure is terminated successfully. */
Vector<hal_ble_addr_t> cachedDevices_;
Vector<hal_ble_scan_result_evt_t> pendingResults_;
};

Expand Down Expand Up @@ -1424,7 +1420,6 @@ void BleObject::Observer::onScanGuardTimerExpired(os_timer_t timer) {
int BleObject::Observer::startScanning(hal_ble_on_scan_result_cb_t callback, void* context) {
CHECK_FALSE(isScanning_, SYSTEM_ERROR_INVALID_STATE);
SCOPE_GUARD ({
clearCachedDevice();
clearPendingResult();
});
ble_gap_scan_params_t bleGapScanParams = toPlatformScanParams();
Expand Down Expand Up @@ -1493,24 +1488,6 @@ ble_gap_scan_params_t BleObject::Observer::toPlatformScanParams() const {
return params;
}

bool BleObject::Observer::isCachedDevice(const hal_ble_addr_t& address) const {
for (const auto& addr : cachedDevices_) {
if (addressEqual(addr, address)) {
return true;
}
}
return false;
}

int BleObject::Observer::addCachedDevice(const hal_ble_addr_t& address) {
CHECK_TRUE(cachedDevices_.append(address), SYSTEM_ERROR_NO_MEMORY);
return SYSTEM_ERROR_NONE;
}

void BleObject::Observer::clearCachedDevice() {
cachedDevices_.clear();
}

hal_ble_scan_result_evt_t* BleObject::Observer::getPendingResult(const hal_ble_addr_t& address) {
for (auto& result : pendingResults_) {
if (addressEqual(result.peer_addr, address)) {
Expand Down Expand Up @@ -1590,17 +1567,11 @@ int BleObject::Observer::processAdvReportEventFromThread(const ble_evt_t* event)
}
const ble_gap_evt_adv_report_t& advReport = event->evt.gap_evt.params.adv_report;
hal_ble_addr_t newAddr = toHalAddress(advReport.peer_addr);
if (isCachedDevice(newAddr)) {
// This has been checked in the ISR. Check it here just for sure.
// Free the allocated RAM for the advertising data.
goto free;
}
if ((!scanParams_.active || !advReport.type.scannable) && !advReport.type.scan_response) {
// No scan response data is expected.
hal_ble_scan_result_evt_t result = {};
constructObserverEvent(result, advReport);
notifyScanResultEvent(result);
addCachedDevice(newAddr);
goto continue_scanning;
}
if (!advReport.type.scan_response) {
Expand All @@ -1620,7 +1591,6 @@ int BleObject::Observer::processAdvReportEventFromThread(const ble_evt_t* event)
}
constructObserverEvent(*result, advReport);
notifyScanResultEvent(*result);
addCachedDevice(newAddr);
removePendingResult(newAddr);
}
goto continue_scanning;
Expand All @@ -1644,10 +1614,6 @@ void BleObject::Observer::processObserverEvents(const ble_evt_t* event, void* co
}
const ble_gap_evt_adv_report_t& report = event->evt.gap_evt.params.adv_report;
hal_ble_addr_t newAddr = toHalAddress(report.peer_addr);
if (observer->isCachedDevice(newAddr)) {
observer->continueScanning();
break;
}
if (observer->scanParams_.active && report.type.scannable && !report.type.scan_response) {
// Advertising data packet, scan response data is expected.
if (observer->getPendingResult(newAddr) != nullptr) {
Expand Down
16 changes: 13 additions & 3 deletions user/tests/app/ble/scanner/application.cpp
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@

#include "Particle.h"

#define SCAN_RESULT_COUNT 30
#define SCAN_RESULT_COUNT 100
#define BLE_ADV_DATA_MAX 31

SYSTEM_MODE(MANUAL);
Expand All @@ -31,13 +31,23 @@ void setup() {

}

bool flag = false;

void loop() {
int count = BLE.scan(results, SCAN_RESULT_COUNT);
int count;
flag = !flag;
if (flag) {
Log.info(">>>> start scanning, filter duplicates");
count = BLE.scan(results, SCAN_RESULT_COUNT);
} else {
Log.info(">>>> start scanning, allow duplicates");
count = BLE.scanWithFilter(BleScanFilter().allowDuplicates(true), results, SCAN_RESULT_COUNT);
}

if (count > 0) {
Log.info("%d devices are found:", count);
for (int i = 0; i < count; i++) {
Log.info(" -------- MAC: %s | RSSI: %dBm --------", results[i].address().toString().c_str(), results[i].rssi());
Log.info(" -------- MAC: %s | RSSI: %d dBm --------", results[i].address().toString().c_str(), results[i].rssi());

String name = results[i].advertisingData().deviceName();
if (name.length() > 0) {
Expand Down
3 changes: 3 additions & 0 deletions user/tests/wiring/api/ble.cpp
View file
Open in desktop
Expand Up @@ -594,6 +594,9 @@ test(ble_scan_filter) {

API_COMPILE({ uint8_t buf[1]; BleScanFilter f = filter.customData(buf, 0); (void)f; });
API_COMPILE({ size_t len; const uint8_t* buf = filter.customData(&len); (void)len; (void)buf; });

API_COMPILE({ BleScanFilter f = filter.allowDuplicates(true); (void)f; });
API_COMPILE({ bool ret = filter.allowDuplicates(); (void)ret; });
}

test(ble_peer_device) {
Expand Down
13 changes: 12 additions & 1 deletion wiring/inc/spark_wiring_ble.h
View file
Open in desktop
Expand Up @@ -716,7 +716,8 @@ class BleScanFilter {
: minRssi_(BLE_RSSI_INVALID),
maxRssi_(BLE_RSSI_INVALID),
customData_(nullptr),
customDataLen_(0) {
customDataLen_(0),
allowDuplicates_(false) {
}
~BleScanFilter() = default;

Expand Down Expand Up @@ -813,6 +814,15 @@ class BleScanFilter {
return *this;
}

BleScanFilter& allowDuplicates(bool allow) {
allowDuplicates_ = allow;
return *this;
}

bool allowDuplicates() const {
return allowDuplicates_;
}

private:
Vector<String> deviceNames_;
Vector<BleUuid> serviceUuids_;
Expand All @@ -822,6 +832,7 @@ class BleScanFilter {
int8_t maxRssi_;
const uint8_t* customData_;
size_t customDataLen_;
bool allowDuplicates_;
};


Expand Down
16 changes: 16 additions & 0 deletions wiring/src/spark_wiring_ble.cpp
View file
Open in desktop
Expand Up @@ -2378,6 +2378,12 @@ class BleScanDelegator {
*/
static void onScanResultCallback(const hal_ble_scan_result_evt_t* event, void* context) {
BleScanDelegator* delegator = static_cast<BleScanDelegator*>(context);

if (!delegator->filter_.allowDuplicates() && delegator->isCachedDevice(event->peer_addr)) {
return;
}
delegator->cachedDevices_.append(event->peer_addr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be populated in case of allowDuplicates() to avoid uncontrollable growth of the vector, as otherwise it will continue to fill-up with duplicates.

Same can potentially happen normally depending on scan duration, although highly unlikely, but should potentially be taken care of somehow as well.


BleScanResult result = {};
result.address(event->peer_addr).rssi(event->rssi)
.scanResponse(event->sr_data, event->sr_data_len)
Expand Down Expand Up @@ -2554,13 +2560,23 @@ class BleScanDelegator {
return true;
}

bool isCachedDevice(const BleAddress& address) const {
for (const auto& addr : cachedDevices_) {
if (address == addr) {
return true;
}
}
return false;
}

Vector<BleScanResult> resultsVector_;
BleScanResult* resultsPtr_;
size_t targetCount_;
size_t foundCount_;
std::function<void(const BleScanResult*)> scanResultCallback_;
BleOnScanResultStdFunction scanResultCallbackRef_;
BleScanFilter filter_;
Vector<BleAddress> cachedDevices_;
};

int BleLocalDevice::setScanTimeout(uint16_t timeout) const {
Expand Down