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

Fix/ble stop scanning #2796

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Fix/ble stop scanning #2796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04738da
[wiring] ble: add BLE.scanning() method
avtolstoy Jul 5, 2024
1a20fd8
[ble] fix scan/stopScanning behavior, add tests to wiring/no_fixture_ble
avtolstoy Jul 5, 2024
5877ac4
[gen4] ble: do not stop scanning during a central connect attempt
avtolstoy Jul 8, 2024
8ffeeba
[test] wiring/ble_centeral_peripheral: update tests with connect + scan
avtolstoy Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions hal/src/nRF52840/ble_hal.cpp
View file
Open in desktop
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() {
XuGuohui marked this conversation as resolved.
Show resolved Hide resolved
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());
XuGuohui marked this conversation as resolved.
Show resolved Hide resolved
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;
XuGuohui marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Open in desktop
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