ayushsharma82 / ElegantOTA

OTA updates made slick and simple for everyone!
https://elegantota.pro
GNU Affero General Public License v3.0
643 stars 119 forks source link

[v3] callbacks not properly usable on classes (to capture out of scope variables) #124

Closed mathieucarbou closed 1 year ago

mathieucarbou commented 1 year ago

The callback signature seems wrong: I cannot use any form of callback to capture the variables from an object which are out of scope of the closure.

I had to change:

    void onStart(std::function<void()> callable);
    void onProgress(std::function<void(size_t current, size_t final)> callable);
    void onEnd(std::function<void(bool success)> callable);
    std::function<void()> preUpdateCallback;
    std::function<void(size_t current, size_t final)> progressUpdateCallback;
    std::function<void(bool success)> postUpdateCallback;
void ElegantOTAClass::onStart(std::function<void()> callable){
    preUpdateCallback = callable;
}

void ElegantOTAClass::onProgress(std::function<void(size_t current, size_t final)> callable){
    progressUpdateCallback= callable;
}

void ElegantOTAClass::onEnd(std::function<void(bool success)> callable){
    postUpdateCallback = callable;
}

to be able to use it in my class:

bool Mycila::OTA::begin()
{
  if (_enabled)
    return true;

  if (_hostname.isEmpty())
    return false;

  if (_httpd)
  {
    _console->log(TAG, "[begin] GET /update");
    ElegantOTA.setAutoReboot(false);
    ElegantOTA.onEnd([this](bool success)
                     { if (_callback) _callback(success); });
    ElegantOTA.begin(_httpd->server, _username.c_str(), _password.c_str());
  }

  _enabled = true;
  return true;
}
ayushsharma82 commented 1 year ago

I chose that format thinking it would offer wider platform compatibility. Sad to know it didn't work inside class.

Fixed with https://github.com/ayushsharma82/ElegantOTA/commit/482fe173ec2eec6e8ea7f6047dd0ba83e719fce4

mathieucarbou commented 1 year ago

yeah callback inside classes are more complex because variables have to be captured ([this], [=], [$], etc).

BTW, you forgot #include <functional> in the header ;-)

ayushsharma82 commented 1 year ago

Still compiled, looking at CI haha. I'll add it just in case.

mathieucarbou commented 1 year ago

yeah, saw that! weird because in my fork I had to add it to make the project compile... Anyway ;-) thanks for the merge!