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.

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

Define scanCompleteCB with using std::function #343

Conversation

asukiaaa
Copy link
Contributor

I want to use bind for scanCompleteCB like this.

class Controller {
  void startScan() {
    scanning = true;
    auto pScan = NimBLEDevice::getScan();
    pScan->setAdvertisedDeviceCallbacks(advDeviceCBs);
    pScan->setInterval(45);
    pScan->setWindow(15);
    Serial.println("Start scan");
    // pScan->start(scanTime, scanEndedCB);
    pScan->start(scanTime,
                 std::bind(&Core::scanEndedCB, this, std::placeholders::_1)); // <<<<<<<<<< ======== Here
  }

  void scanEndedCB(NimBLEScanResults results) {
    Serial.println("Scan Ended");
    scanning = false;
  }
};

Related issue: #342

@h2zero
Copy link
Owner

h2zero commented Feb 4, 2022

Sorry for the delay.

This is a good idea but we have a problem.

This will break the API for users that do not provide the is_continue parameter when they provide a callback that does not use std::bind (such as the examples). In this scenario the callback argument is converted to a bool(true) by the compiler and results in calling the wrong start() method, and the callback is not called.

In order to support this correctly another solution is needed. If we are going to break the API, I think the best way would be to have a setScanEndCallback method or a callback class added to NimBLEScan and remove the function parameter from the start calls. This would also require renaming the start() method to support the blocking and non-blocking calls with different naming.

I'm open to other suggestions, this is just my first thoughts.

Note: merging this will need to wait until the other breaking changes I have in mind are implemented with the upcoming 2.0 releases.

@asukiaaa
Copy link
Contributor Author

asukiaaa commented Feb 4, 2022

Thank you for the response.
I understand that this change breaks current code.

have a setScanEndCallback method or a callback class added to NimBLEScan

I agree passing a class instance for a function because we can support additional callback easily.

I have in mind are implemented with the upcoming 2.0 releases.

I see, thanks.

@h2zero
Copy link
Owner

h2zero commented Apr 30, 2022

@asukiaaa Please take a look at #389, it is part of a few breaking changes I have planned for a version 2 release and should suit your needs.

@asukiaaa
Copy link
Contributor Author

Thank you for updating but it seems renaming m_pAdvertisedDeviceCallbacks to m_pScanCallbacks and reuse it as m_scanCompleteCB.
Is it possible to distinguish the timing of finding some device and finishing scanning?

@h2zero
Copy link
Owner

h2zero commented Apr 30, 2022

Sorry, I need to update an examples to make it clearer. The advertised device callback class has been replaced with the scan callback class which contains the virtual function onResult and onScanEnd that you can define for your needs.

@asukiaaa
Copy link
Contributor Author

asukiaaa commented Apr 30, 2022

Thank you for the explanation.
I could use the updated NimBLE without using global(static?) functions and variables.
asukiaaa/arduino-XboxSeriesXControllerESP32@57e50c1

@h2zero
Copy link
Owner

h2zero commented Apr 30, 2022

You got it 😄

@asukiaaa
Copy link
Contributor Author

asukiaaa commented May 1, 2022

This request is satisfied by onScanEnd callback on #389.
Thank you.

@asukiaaa asukiaaa closed this May 1, 2022
@asukiaaa asukiaaa deleted the feature/define-scan-complete-cb-with-using-std-function branch May 1, 2022 05:03
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.

None yet

2 participants