esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
16.09k stars 13.33k forks source link

Callbacks of WiFi events (onX) are not called #9086

Closed srvmgr closed 8 months ago

srvmgr commented 9 months ago

https://github.com/esp8266/Arduino/blob/de1029ffe0f85b40465ecb64218ef7ab3643ffa7/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp#L253

I figured out that callbacks of events registered with methods onX... (like onStationModeGotIP) are not executed. Looking in code, in file ESP8266WiFiGeneric.cpp, method _eventCallback:

For events registered using predefined methods (which have mCanExpire = true), are removed and not executed, because of the next line:

if (handler->canExpire() && handler.unique()) {

But when they are registered using the old way (onEvent(....)), because mCanExpire = false, they are executed.

for(auto it = std::begin(sCbEventList); it != std::end(sCbEventList); ) {
        WiFiEventHandler &handler = *it;
        if (handler->canExpire() && handler.unique()) {
            it = sCbEventList.erase(it);
        }
        else { /** ELSE SHOULD EXISTS? or  (*handler)(event); should be called allways regardless of if (handler->canExpire()... ***/
            (*handler)(event);
            ++it;
        }
    }
mcspr commented 9 months ago

Are you adding event callback like this?

void setup() {
  WiFi.onStationModeGotIP([](auto&& event) {
    ...
  });
}

or

void somefunc() {
  auto foo = WiFi.onStationModeGotIP([](auto&& event) {
    ...
  });
}

If so, you cause the 'unique()' check above to succeed. Return value of onStationModeGotIP is a shared pointer object, which counts amount of references to it. If there is only one reference i.e. reference of the object in the sCbEventList and it was not registered through onEvent(...), it erases the callback as it deems it unused. This was done with the intent of allowing de-registering callbacks.

https://en.cppreference.com/w/cpp/memory/shared_ptr Shared pointer object must be copied to some place outside of the func, so reference counter stays >1

srvmgr commented 9 months ago

I register it one time only, like the first example.

WiFi.onStationModeGotIP([](const WiFiEventStationModeGotIP& event) {
    ...
});

Now I have to do 2 modifications in order to fix it. 1: Assign the return value (pointer) to a variable. 2: Make variable static when the life of de variable is limited in time, like in my case in which I register it inside a function,

So the result will be:

Inside function:

void somefunc() {
  static auto gotIpEventHandler = WiFi.onStationModeGotIP([](auto&& event) {
    ...
  });
}

Pointer declared in global scope:

WiFiEventHandler gotIpEventHandler;

void somefunc() {
  gotIpEventHandler = WiFi.onStationModeGotIP([](auto&& event) {
    ...
  });
}

I think this behavior should be more detailed in docs. With more explicit comments about cases in which it will not work.

Doc WiFiEvents.ino