Closed Ben79543 closed 3 years ago
h2zero
commented
3 years ago Hi @Ben79543, looks like this recent commit might have broken your use case.
If you are calling BLEScan::setAdvertisedDeviceCallbacks with wantDuplicates set to true I think that will cause this issue. try setting that to false and test.
Ben79543
commented
3 years ago I have changed pBLEScan->setAdvertisedDeviceCallbacks(new MyAdvertisedDeviceCallbacks()); with pBLEScan->setAdvertisedDeviceCallbacks(new MyAdvertisedDeviceCallbacks(), false, true); Same result:
20:12:45.502 -> Devices found: 0
20:12:45.502 -> Scan done!
Yes you're correct. It seems the change was such that if you set a callback it would not insert the device into the scan results map. It should work if you don't set a callback and parse the results after.
Ben79543
commented
3 years ago I see. But this is really a problem : I need a callback to be able to count the detected device and stop the scan after a threshold. Otherwise the scan ends up crashing. I refer to this issue: #3559
So without the callback this will not work. Any chance this will be fixed?
h2zero
commented
3 years ago I'm not sure what you're code is doing but to avoid the crash issue that you linked you could call clearResults(); before each scan to avoid the memory use. As far as fixing this it might be something for @chegewara to look at or you could PR a fix.
Ben79543
commented
3 years ago Your are very right: when I remove the "setAdvertisedDeviceCallbacks" getCounts returns a value even in 1.0.5
In my case, I need to limit the nember of detected device in a SINGLE scan. A single scan was enough to make the crash happen so I have no time to call for clearResults. This is what happened to me in an environment with hundreds of ble beacons (the hall of a trade show).
my callback is only a counter that stops scan after a threshold:
class MyAdvertisedDeviceCallbacks: public BLEAdvertisedDeviceCallbacks {
void onResult(BLEAdvertisedDevice advertisedDevice) {
nbDetectedDevice++;
Serial.println(nbDetectedDevice);
if (nbDetectedDevice >= 50) {
advertisedDevice.getScan()->stop();
}
}
};Looking at BLESCAN.cpp the issue is right there. in 1.0.5 there is a condition (as you saw it) that says: " if has callback, no need to record to vector". This is what's causing my issue. This was probably inserted for a reason. But how can I achieve my goal without a callback (that is to interupt the scan after a threshold before the scan crashes because there are too many devices) @chegewara ? Wouldn't it make sense to include a "max_detected_device" parameter when running a scan, that will end the scan if that number is reached?
in 1.0.4:
if (!found) { // If we have previously seen this device, don't record it again.
m_scanResults.m_vectorAdvertisedDevices.insert(std::pair<std::string, BLEAdvertisedDevice*>(advertisedAddress.toString(), advertisedDevice));
}
if (m_pAdvertisedDeviceCallbacks) {
m_pAdvertisedDeviceCallbacks->onResult(*advertisedDevice);
}
if(found)
delete advertisedDevice;While in 1.0.5-rc4:
if (m_pAdvertisedDeviceCallbacks) { // if has callback, no need to record to vector
m_pAdvertisedDeviceCallbacks->onResult(*advertisedDevice);
} else if (!m_wantDuplicates && !found) { // if no callback and not want duplicate, and not already in vector, record it
m_scanResults.m_vectorAdvertisedDevices.insert(std::pair<std::string, BLEAdvertisedDevice*>(advertisedAddress.toString(), advertisedDevice));
shouldDelete = false;
}
if (shouldDelete) {
delete advertisedDevice;
}Looks like a workaround would be just to use the callback but create your own vector of devices and limit it in your application.
The suggestion to add a limiter on the number of devices is also a good idea.
adam-ah
commented
3 years ago The official code example is also broken as a result (Devices found: 0) even though the console is showing devices.
It might be a different issue, but the getName() now is always null as well.
Ben79543
commented
3 years ago So as a summary from 1.0.5, the code was changed so that if there is a callback set, the code supposes that the user will do all the device detection data collection in the call back. Therefore it does not create the array of detected devices.
Before 1.0.5 both were happening: callback+constructing the array.
It was changed in commit #3995 (actually the first commit was ok, but a additionnal change was brought and broke my usecase)
I tested a VERY easy fix: I just removed one "ELSE" in 1.0.5-rc6:
if (m_pAdvertisedDeviceCallbacks) { // if has callback, no need to record to vector
m_pAdvertisedDeviceCallbacks->onResult(*advertisedDevice);
} // else //REMOVING THIS ELSE IS THE FIX**
if (!m_wantDuplicates && !found) { // if no callback and not want duplicate, and not already in vector, record it
m_scanResults.m_vectorAdvertisedDevices.insert(std::pair<std::string, BLEAdvertisedDevice*>(advertisedAddress.toString(), advertisedDevice));
shouldDelete = false;
}I tested and it works fine again.
not sure what to do I will start with a comment on the problematic commit...
h2zero
commented
3 years ago Yes that seems to be the purpose of the commit. Good fix IMO.
I have recently implemented the ability to cap the number of scanned devices in the NimBLE library, inspired by your use case. If you don't need classic bluetooth, you might want to try NimBLE, it'll also save a lot of ram and flash.
Ben79543
commented
3 years ago I have recently implemented the ability to cap the number of scanned devices in the NimBLE library, inspired by your use case. If you don't need classic bluetooth, you might want to try NimBLE, it'll also save a lot of ram and flash.
Thanks, I might give it a try indeed
CLDiego
commented
3 years ago @h2zero @Ben79543 Has there been any updates regarding this issue? I recently updated to release 1.0.6 and now when scanning I get multiple duplicates of the same device and sometimes it even crashes the ESP32.
I tried playing around with the wantDuplicates variable, setting it to true or false makes no difference.
h2zero
commented
3 years ago I'm not aware of any changes that would affect this issue. If you don't need classic Bluetooth I suggest using NimBLE as it works the way you'd expect and probably only requires a header change in your code.
CLDiego
commented
3 years ago Thanks for the recommendation. I'll certainly will give it a go.
Ben79543
commented
3 years ago I have created a PR for this : #5241 My first PR hopefully done well...
stale[bot]
commented
3 years ago [STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.
stale[bot]
commented
3 years ago [STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.
gsapinto
commented
1 year ago I know this is old, but is there any chance someone made this example work with extended advertising? In this case the data comes in the scan response data instead of the advertising data, what do I need to change to detect that?
I have tried to run the standard supplied BLE_scan.ino example code - not a single change to it. It's running fine when I am on 1.0.4 - I get the list of detected devices, and it tells me "found devices: 6" for instance.
When I switch (via board manager) to 1.0.5-rc4:
Something broken with getCount()?
Hardware:
Board: ESP32 Dev Module Core Installation version: 1.0.4 and 1.0.5-rc4 IDE name: Arduino IDE 1.8.12 Flash Frequency: 80Mhz PSRAM enabled: no Upload Speed: 115200 Computer OS: Windows 10
Logs with 1.0.4 (working): (note I remove the middle lengthy part)
LOGS with 1.0.5-rc4 (note I remove the middle lengthy part)