Skip to content

Commit

Permalink
Merge pull request #2796 from particle-iot/fix/ble-stop-scanning
Browse files Browse the repository at this point in the history 
Fix/ble stop scanning
  • Loading branch information
@avtolstoy
avtolstoy authored Aug 12, 2024
2 parents 1a8fc11 + 8ffeeba commit 615a136
Show file tree
Hide file tree
Showing 7 changed files with 714 additions and 19 deletions.
24 changes: 11 additions & 13 deletions hal/src/nRF52840/ble_hal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1418,8 +1418,11 @@ void BleObject::Observer::onScanGuardTimerExpired(os_timer_t timer) {
}

int BleObject::Observer::startScanning(hal_ble_on_scan_result_cb_t callback, void* context) {
BleLock lk;

CHECK_FALSE(isScanning_, SYSTEM_ERROR_INVALID_STATE);
SCOPE_GUARD ({
stopScanning();
clearPendingResult();
});
ble_gap_scan_params_t bleGapScanParams = toPlatformScanParams();
Expand All @@ -1439,6 +1442,7 @@ int BleObject::Observer::startScanning(hal_ble_on_scan_result_cb_t callback, voi
// We don't return here, as scanning may still timeout by Softdevice as expected.
}
}
lk.unlock();
if (os_semaphore_take(scanSemaphore_, CONCURRENT_WAIT_FOREVER, false)) {
SPARK_ASSERT(false);
return SYSTEM_ERROR_TIMEOUT;
Expand All @@ -1447,9 +1451,12 @@ int BleObject::Observer::startScanning(hal_ble_on_scan_result_cb_t callback, voi
}

int BleObject::Observer::stopScanning() {
BleLock lk;

if (!isScanning_) {
return SYSTEM_ERROR_NONE;
}

if (os_timer_is_active(scanGuardTimer_, nullptr)) {
os_timer_change(scanGuardTimer_, OS_TIMER_CHANGE_STOP, hal_interrupt_is_isr() ? true : false, 0, 0, nullptr);
}
Expand All @@ -1458,16 +1465,10 @@ int BleObject::Observer::stopScanning() {
if (sd_ble_gap_scan_stop() != NRF_SUCCESS) {
// LOG(ERROR, "Device is not in scanning state.");
}
bool give = false;
ATOMIC_BLOCK() {
if (isScanning_) {
isScanning_ = false;
give = true;
}
}
if (give) {
os_semaphore_give(scanSemaphore_, false);
}
isScanning_ = false;

// Just in case
os_semaphore_give(scanSemaphore_, false);
return SYSTEM_ERROR_NONE;
}

Expand Down Expand Up @@ -1783,8 +1784,6 @@ int BleObject::ConnectionsManager::connect(const hal_ble_conn_cfg_t* config, hal
CHECK_TRUE(connections_.size() < BLE_MAX_LINK_COUNT, SYSTEM_ERROR_LIMIT_EXCEEDED);
CHECK_TRUE(config, SYSTEM_ERROR_INVALID_ARGUMENT);
CHECK_TRUE(connHandle, SYSTEM_ERROR_INVALID_ARGUMENT);
// Stop scanning first to give the scanning semaphore if possible.
CHECK(BleObject::getInstance().observer()->stopScanning());
SCOPE_GUARD ({
connectingAddr_ = {};
});
Expand Down Expand Up @@ -4133,7 +4132,6 @@ int hal_ble_gap_get_scan_parameters(hal_ble_scan_params_t* scan_params, void* re
}

int hal_ble_gap_start_scan(hal_ble_on_scan_result_cb_t callback, void* context, void* reserved) {
BleLock lk;
LOG_DEBUG(TRACE, "hal_ble_gap_start_scan().");
CHECK_TRUE(BleObject::getInstance().initialized(), SYSTEM_ERROR_INVALID_STATE);
return BleObject::getInstance().observer()->startScanning(callback, context);
Expand Down
22 changes: 16 additions & 6 deletions hal/src/rtl872x/ble_hal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1675,21 +1675,31 @@ int BleGap::startScanning(hal_ble_on_scan_result_cb_t callback, void* context) {
resetStack = true;
return SYSTEM_ERROR_TIMEOUT;
}
// We unlock BLE lock here though so that other BLE APIs are accessible while we are performing a scan.
// Especially if it's a continuous scan.
lk.unlock();
// To be consistent with Gen3, the scan proceedure is blocked for now,
// so we can simply wait for the semaphore to be given without introducing a dedicated timer.
if (scanParams_.timeout == BLE_SCAN_TIMEOUT_UNLIMITED) {
lk.unlock();
}
os_semaphore_take(scanSemaphore_, (scanParams_.timeout == BLE_SCAN_TIMEOUT_UNLIMITED) ? CONCURRENT_WAIT_FOREVER : (scanParams_.timeout * 10), false);
return SYSTEM_ERROR_NONE;
}

int BleGap::stopScanning(bool resetStack) {
BleLock lk;

if (!isScanning_) {
return SYSTEM_ERROR_NONE;
}

BleLock lk;
if (BleEventDispatcher::getInstance().isThreadCurrent()) {
// Can't stop scanning properly from event processing thread
// Unblock the caller of scan() and perform stopScanning()
// in there.
// XXX: It's not safe to call le_scan_stop() especially
// from inside the ble scan result callback!
os_semaphore_give(scanSemaphore_, false);
return 0;
}

const int LE_SCAN_STOP_RETRIES = 1;
for (int i = 0; i < LE_SCAN_STOP_RETRIES; i++) {
Expand Down Expand Up @@ -1728,7 +1738,9 @@ int BleGap::stopScanning(bool resetStack) {
clearPendingResult();
}

// Just in case
os_semaphore_give(scanSemaphore_, false);

return SYSTEM_ERROR_NONE;
}

Expand Down Expand Up @@ -1886,8 +1898,6 @@ int BleGap::connect(const hal_ble_conn_cfg_t* config, hal_ble_conn_handle_t* con
// Make sure that event dispatching is started
CHECK(start());

// Stop scanning first to give the scanning semaphore if possible.
CHECK(stopScanning());
SCOPE_GUARD ({
connectingAddr_ = {};
connecting_ = false;
Expand Down
90 changes: 90 additions & 0 deletions user/tests/wiring/ble_central_peripheral/ble_central/central.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ using namespace particle::test;
constexpr uint16_t LOCAL_DESIRED_ATT_MTU = 100;
constexpr uint16_t PEER_DESIRED_ATT_MTU = 123;

Thread* scanThread = nullptr;
volatile unsigned scanResults = 0;

test(BLE_000_Central_Cloud_Connect) {
subscribeEvents(BLE_ROLE_PERIPHERAL);
Particle.connect();
Expand Down Expand Up @@ -579,5 +582,92 @@ test(BLE_32_Att_Mtu_Exchange) {
}
}

test(BLE_33_Central_Can_Connect_While_Scanning) {
BleScanParams params = {};
params.size = sizeof(BleScanParams);
params.timeout = 0;
params.interval = 8000; // *0.625ms = 5s
params.window = 8000; // *0.625 = 5s
params.active = true; // Send scan request
params.filter_policy = BLE_SCAN_FP_ACCEPT_ALL;
assertEqual(0, BLE.setScanParameters(&params));

scanThread = new Thread("test", [](void* param) -> os_thread_return_t {
BLE.scanWithFilter(BleScanFilter().allowDuplicates(true), +[](const BleScanResult *result, void *param) -> void {
auto scanResults = (volatile unsigned int*)param;
(*scanResults)++;
}, param);
}, (void*)&scanResults);

assertTrue(BLE.scanning());
assertFalse(BLE.connected());
peer = BLE.connect(peerAddr, false);
assertTrue(peer.connected());
assertTrue(BLE.scanning());
SCOPE_GUARD({
SCOPE_GUARD ({
delay(500);
assertEqual(BLE.disconnect(peer), (int)SYSTEM_ERROR_NONE);
assertFalse(BLE.connected());
delay(500);
});
});
scanResults = 0;
delay(2000);
#if !HAL_PLATFORM_NRF52840
assertMoreOrEqual((unsigned)scanResults, 1);
#endif
assertTrue(peer.connected());
}

test(BLE_34_Central_Can_Connect_While_Scanning_After_Disconnect) {
SCOPE_GUARD({
if (BLE.scanning() && scanThread) {
assertEqual(0, BLE.stopScanning());
scanThread->join();
delete scanThread;
}
});
assertTrue(BLE.scanning());
assertFalse(BLE.connected());
scanResults = 0;
delay(5000);
assertMoreOrEqual((unsigned)scanResults, 1);
}

test(BLE_35_Central_Can_Connect_While_Peripheral_Is_Scanning_Prepare) {
}

test(BLE_36_Central_Can_Connect_While_Peripheral_Is_Scanning) {
peer = BLE.connect(peerAddr, false);
assertTrue(peer.connected());
delay(5000);
{
SCOPE_GUARD ({
delay(500);
assertEqual(BLE.disconnect(peer), (int)SYSTEM_ERROR_NONE);
assertFalse(BLE.connected());
delay(500);
});
}
}

test(BLE_37_Central_Can_Connect_While_Peripheral_Is_Scanning_And_Stops_Scanning_Prepare) {
}

test(BLE_38_Central_Can_Connect_While_Peripheral_Is_Scanning_And_Stops_Scanning) {
peer = BLE.connect(peerAddr, false);
assertTrue(peer.connected());
delay(5000);
{
SCOPE_GUARD ({
delay(500);
assertEqual(BLE.disconnect(peer), (int)SYSTEM_ERROR_NONE);
assertFalse(BLE.connected());
delay(500);
});
}
}

#endif // #if Wiring_BLE == 1

116 changes: 116 additions & 0 deletions user/tests/wiring/ble_central_peripheral/ble_peripheral/peripheral.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ using namespace particle::test;
constexpr uint16_t LOCAL_DESIRED_ATT_MTU = 123;
constexpr uint16_t PEER_DESIRED_ATT_MTU = 100;

Thread* scanThread = nullptr;
volatile unsigned scanResults = 0;

test(BLE_000_Peripheral_Cloud_Connect) {
subscribeEvents(BLE_ROLE_PERIPHERAL);
Particle.connect();
Expand Down Expand Up @@ -487,5 +490,118 @@ test(BLE_32_Att_Mtu_Exchange) {
}
}

test(BLE_33_Central_Can_Connect_While_Scanning) {
assertTrue(waitFor(BLE.connected, 20000));
SCOPE_GUARD ({
assertTrue(waitFor([]{ return !BLE.connected(); }, 10000));
assertFalse(BLE.connected());
});
}

test(BLE_34_Central_Can_Connect_While_Scanning_After_Disconnect) {
}

test(BLE_35_Central_Can_Connect_While_Peripheral_Is_Scanning_Prepare) {
BleScanParams params = {};
params.size = sizeof(BleScanParams);
params.timeout = 0;
params.interval = 8000; // *0.625ms = 5s
params.window = 8000; // *0.625 = 5s
params.active = true; // Send scan request
params.filter_policy = BLE_SCAN_FP_ACCEPT_ALL;
assertEqual(0, BLE.setScanParameters(&params));

scanThread = new Thread("test", [](void* param) -> os_thread_return_t {
BLE.scanWithFilter(BleScanFilter().allowDuplicates(true), +[](const BleScanResult *result, void *context) -> void {
scanResults++;
}, nullptr);
}, nullptr);
assertTrue((bool)scanThread);
}

test(BLE_36_Central_Can_Connect_While_Peripheral_Is_Scanning) {
SCOPE_GUARD({
if (BLE.scanning() && scanThread) {
BLE.stopScanning();
scanThread->join();
delete scanThread;
scanThread = nullptr;
}
});
assertTrue(BLE.scanning());
assertTrue(waitFor(BLE.connected, 60000));
scanResults = 0;
delay(2000);
#if !HAL_PLATFORM_NRF52840
assertMoreOrEqual((unsigned)scanResults, 1);
#endif
SCOPE_GUARD ({
assertTrue(waitFor([]{ return !BLE.connected(); }, 10000));
assertFalse(BLE.connected());

scanResults = 0;
delay(2000);
#if !HAL_PLATFORM_NRF52840
assertMoreOrEqual((unsigned)scanResults, 1);
#endif

assertEqual(0, BLE.stopScanning());
assertFalse(BLE.scanning());
scanThread->join();
delete scanThread;
scanThread = nullptr;
});
}

test(BLE_37_Central_Can_Connect_While_Peripheral_Is_Scanning_And_Stops_Scanning_Prepare) {
BleScanParams params = {};
params.size = sizeof(BleScanParams);
params.timeout = 0;
params.interval = 8000; // *0.625ms = 5s
params.window = 8000; // *0.625 = 5s
params.active = true; // Send scan request
params.filter_policy = BLE_SCAN_FP_ACCEPT_ALL;
assertEqual(0, BLE.setScanParameters(&params));

scanThread = new Thread("test", [](void* param) -> os_thread_return_t {
BLE.scanWithFilter(BleScanFilter().allowDuplicates(true), +[](const BleScanResult *result, void *context) -> void {
scanResults++;
}, nullptr);
}, nullptr);
assertTrue((bool)scanThread);
}

test(BLE_38_Central_Can_Connect_While_Peripheral_Is_Scanning_And_Stops_Scanning) {
SCOPE_GUARD({
if (BLE.scanning() && scanThread) {
BLE.stopScanning();
scanThread->join();
delete scanThread;
scanThread = nullptr;
}
});
assertTrue(BLE.scanning());
assertTrue(waitFor(BLE.connected, 60000));
scanResults = 0;
delay(2000);
#if !HAL_PLATFORM_NRF52840
assertMoreOrEqual((unsigned)scanResults, 1);
#endif

assertEqual(0, BLE.stopScanning());
assertFalse(BLE.scanning());
scanThread->join();
delete scanThread;
scanThread = nullptr;

assertTrue(BLE.connected());

SCOPE_GUARD ({
assertTrue(waitFor([]{ return !BLE.connected(); }, 10000));
assertFalse(BLE.connected());
});
}


#endif // #if Wiring_BLE == 1

Loading

0 comments on commit 615a136

Please sign in to comment.