Skip to content

Conversation

@h2zero
Copy link
Owner

@h2zero h2zero commented Nov 18, 2024

  • General code cleanup
  • NimBLEScan::start will no longer clear cache or results if scanning is already in progress.
  • NimBLEScan::clearResults will now reset the vector capacity to 0.
  • NimBLEScan::stop will no longer call the onScanEnd callback as the caller should know its been stopped when this is called.
  • NimBLEScan::clearDuplicateCache has been removed as it was problematic and only for the esp32. Stop and start the scanner for the same effect.
  • NimBLEScan::start takes a new bool parameter restart, default true, that will restart an already in progress scan and clear the duplicate filter so all devices will be discovered again.
  • Scan response data that is received without advertisement first will now create the device and send a callback.
  • Added new method: NimBLEAdvertisedDevice::isScannable() that returns true if the device is scannable.
  • Added default callbacks for NimBLEScanCallbacks
  • NimBLEScanCallbacks function signatures updated:
    • onDiscovered now takes a const NimBLEAdvertisedDevice*
    • onResult now takes a const NimBLEAdvertisedDevice*
    • onScanEnd now takes a const NimBLEScanResults& and int reason
  • Added new erase overload: NimBLEScan::erase(const NimBLEAdvertisedDevice* device)
  • NimBLEScanResults::getDevice methods now return const NimBLEAdvertisedDevice*
  • NimBLEScanResults iterators are now const_iterator

Fixes #73

@thekurtovic
Copy link
Collaborator

NimBLEScan::stop will no longer call the onScanEnd callback as the caller should know its been stopped when this is called.

There are some spots where NimBLEScan::stop is called internally, I think it would be good to still call the onScanEnd callback in such cases. What about adding a parameter which defaults to false, else will call onScanEnd?
I don't stop the scan explicitly before connecting as I liked how the NimBLE Async Client handles it.

@h2zero
Copy link
Owner Author

h2zero commented Nov 18, 2024

The reason for the change is so the application can make use of the callback to attempt to restart it. When connecting it will always be stopped first and that's about the only time that it is internally called other than when the app specifies to reset.

@h2zero
Copy link
Owner Author

h2zero commented Nov 18, 2024

Hmm, now that I think about it, a reason code parameter should be added to the callback so the application can decide what to do from there.

@thekurtovic
Copy link
Collaborator

thekurtovic commented Nov 18, 2024

I don't understand how can the application use the callback if it's never called? Do you mean the proper usage would be to always have the application stop the scan explicitly, and not use the convenience of the connect function?

@h2zero
Copy link
Owner Author

h2zero commented Nov 19, 2024

The purpose of the callback is to let the application know when the scan stopped due to a timeout or error. The stack itself doesn't generate an event when the scan is stopped manually, likely for the same reason.

@h2zero h2zero force-pushed the refactor-scan branch 6 times, most recently from 943df90 to 322dd8e Compare November 21, 2024 20:57
* General code cleanup
* `NimBLEScan::start` will no longer clear cache or results if scanning is already in progress.
* `NimBLEScan::clearResults` will now reset the vector capacity to 0.
* `NimBLEScan::stop` will no longer call the `onScanEnd` callback as the caller should know its been stopped when this is called.
* `NimBLEScan::clearDuplicateCache` has been removed as it was problematic and only for the esp32. Stop and start the scanner for the same effect.
* `NimBLEScan::start` takes a new bool parameter `restart`, default `true`, that will restart an already in progress scan and clear the duplicate filter so all devices will be discovered again.
* Scan response data that is received without advertisement first will now create the device and send a callback.
* Added new method: `NimBLEAdvertisedDevice::isScannable()` that returns true if the device is scannable.
* Added default callbacks for `NimBLEScanCallbacks`
* `NimBLEScanCallbacks` function signatures updated:
* - `onDiscovered` now takes a `const NimBLEAdvertisedDevice*`
* - `onResult` now takes a `const NimBLEAdvertisedDevice*`
* - `onScanEnd` now takes a `const NimBLEScanResults&` and `int reason`
* Added new erase overload: `NimBLEScan::erase(const NimBLEAdvertisedDevice* device)`
* `NimBLEScanResults::getDevice` methods now return `const NimBLEAdvertisedDevice*`
* `NimBLEScanResults` iterators are now `const_iterator`
@h2zero h2zero merged commit 2151386 into master Nov 21, 2024
58 checks passed
@h2zero h2zero deleted the refactor-scan branch November 21, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NimBLEScan::clearResults doesn't actually release memory space

3 participants