nrf528xx immediatelly stop scan has sideeffects #403

Activity

1. gen2thomas commented on Jan 2, 2026

   

   gen2thomas

   on Jan 2, 2026

   ContributorAuthor

   More actions

   After some tests my previous assumption seem to be correct, because the callback is always used to stop the scan loop. This means "C.sd_ble_gap_scan_stop()" must not followed by "C.sd_ble_gap_scan_start(nil, &scanReportBufferInfo)", otherwise the latter returns the error "invalid state, operation disallowed in this state".

   There are some possibilities to fix it.
   V1: with existing code, return before calling "C.sd_ble_gap_scan_start(nil, &scanReportBufferInfo)", e.g. like this:

   // Wait for received scan reports.
   for a.scanning {
   	// Wait for the next advertisement packet to arrive.
   	// TODO: use some sort of condition variable once the scheduler supports
   	// them.
   	arm.Asm("wfe")
   	if gotScanReport.Get() == 0 {
   		// Spurious event. Continue waiting.
   		continue
   	}
   	gotScanReport.Set(0)
   
   	// Call the callback with the scan result.
   	callback(a, globalScanResult)
   
   	// break the loop if the callback has already stopped the scan, because start scan in this state is disallowed
   	if a.scanning {
   		break
   	}
         
   	// Restart the advertisement. This is needed, because advertisements are
   	// automatically stopped when the first packet arrives.
   	errCode := C.sd_ble_gap_scan_start(nil, &scanReportBufferInfo)
   	if errCode != 0 {
   		return Error(errCode)
   	}
   }

   V2: revert #398, but call the stop-function after the loop

   // Wait for received scan reports.
   for a.scanning {
   	// Wait for the next advertisement packet to arrive.
   	// TODO: use some sort of condition variable once the scheduler supports
   	// them.
   	arm.Asm("wfe")
   	if gotScanReport.Get() == 0 {
   		// Spurious event. Continue waiting.
   		continue
   	}
   	gotScanReport.Set(0)
   
   	// Call the callback with the scan result.
   	callback(a, globalScanResult)
   
   	// Restart the advertisement. This is needed, because advertisements are
   	// automatically stopped when the first packet arrives.
   	errCode := C.sd_ble_gap_scan_start(nil, &scanReportBufferInfo)
   	if errCode != 0 {
   		return Error(errCode)
   	}
   }
   
   // stop the device needs to be done, otherwise the next scan would fail
   errCode = C.sd_ble_gap_scan_stop()
   return makeError(errCode)

   IMO, the first version V1 is more clean regarding an error on calling "C.sd_ble_gap_scan_stop()", which is returned from a stop-function and not from a scan-function.

   Other optimizations/ideas/possibilities:

   • in addition to "V1", change for a.scanning { to for {
   • when using V1: if in addition to the "callback()" another go-routine is calling "StopScan()", e.g. responsible for watching the timeout, and this happens short before calling the "callback()", there needs to be an additional "if-not-scanning-break", otherwise 2 calls of "StopScan()" will exist, which would lead to an error "bluetooth: there is no scan in progress"
   • same as before, but "inside" the callback - IMO this leads to unpredictable problems -> V2 is safer
   • V3: Re-ordering the 3 blocks in the for-loop, so the callback is always after "C.sd_ble_gap_scan_start(nil, &scanReportBufferInfo)" - IMO the other problems described for "V1" still exists -> V2 is safer
2. gen2thomas commented on Jan 2, 2026

   

   gen2thomas

   on Jan 2, 2026

   ContributorAuthor

   More actions

   @deadprogram , @zopieux I have created an PR after some investigations and tests to fix/improve #398 .
3. deadprogram

   added

   next-release

   on Feb 5, 2026