arkhipenko / TaskScheduler

Cooperative multitasking for Arduino, ESPx, STM32, nRF and other microcontrollers
http://playground.arduino.cc/Code/TaskScheduler
BSD 3-Clause "New" or "Revised" License
1.22k stars 224 forks source link

Cannot enable scheduled task by pointer #82

Closed JamoDevNich closed 4 years ago

JamoDevNich commented 4 years ago

System

OS: Windows, IDE: Atom/PlatformIO, Board: ESP8266EX

Please see the two affected files below, which are referenced in the explanation. I know pretty much nothing about C++ (Python and C# background)

Problem

I've got a task with an anonymous function to disconnect and shut off the ESP8266's WiFi. This task is added to the scheduler during setup().

I'm able to call enableDelayed() on this task within setup(), and as expected, the task runs after the delay.

However, when task->enableDelayed() is called by pointer, the task is enabled (scheduler->timeUntilNextIteration(*task) reports the correct delay on the "/shutdown" http page, which is within the same scope the task is enabled) then destroyed, as the pointer scheduler->timeUntilNextIteration(*task) reports -1 on the "/" http page afterwards.

I've tried a bunch of different ways to restructure the code (and even tested passing a boolean through, which was not destroyed, and was updated as expected on both http pages) so I'm not too sure what I'm missing here.

Main.cpp

#include "TaskScheduler.h"

ESP8266WebServer httpServer(80);
LoopOperationHandlers loopOperationHandlers;
Initialisation initialisation;
Scheduler scheduler;
Task shutdownTask(2000, 0, [](){ WiFi.disconnect(true); });

void setup() {
    Metrics::millisBoot = millis();
    pinMode(PIN_LED_BLUE, OUTPUT);

    scheduler.init();
    scheduler.addTask(shutdownTask);

    initialisation.wifiSetup(loopOperationHandlers);
    initialisation.httpServerSetup(&httpServer, &scheduler, &shutdownTask);  // Passing the mem addr/pointer for task 'shutdownTask'
}

void loop() {
    scheduler.execute();
    loopOperationHandlers.wifiConnectedStatus();
    httpServer.handleClient();
}

Initialisation.cpp

#include "TaskSchedulerDeclarations.h"

class Initialisation {
    public:
        void wifiSetup(LoopOperationHandlers loopOperationHandlers){
            loopOperationHandlers.wifiConnectedStatus();
            WiFi.mode(WIFI_STA);
            WiFi.setAutoConnect(true);
            WiFi.setAutoReconnect(true);
            WiFi.config(
                IPAddress(192,168,3,DEVICE_ID),
                IPAddress(192,168,3,1),
                IPAddress(255,255,255,0)
            );
            WiFi.begin(WLAN_SSID, WLAN_PASSWORD);
        }

        void httpServerSetup(ESP8266WebServer* httpServer, Scheduler* scheduler, Task* shutdownTask){
            httpServer->on("/", [=]() mutable {
                httpServer->send(200, "text/plain",
                    "Uptime: " + String(millis() - Metrics::millisBoot)
                    + "ms\nWiFi Handshake: " + String(Metrics::millisWifiConnected)
                    + "ms\nSignal Strength: " + String(WiFi.RSSI())
                    + "dB\nFree Memory: " + String(ESP.getFreeHeap())
                    + "\nNext Shutdown: " + String(scheduler->timeUntilNextIteration(*shutdownTask))
                );
            });
            httpServer->on("/shutdown", [=]() mutable {
                shutdownTask->enableDelayed();                     // enableDelayed called by pointer here
                httpServer->send(200, "text/plain",
                "Powering off...\nenabled=" + String(shutdownTask->isEnabled())
                + "\ninterval=" + String(shutdownTask->getInterval())
                + "\niterations=" + String(shutdownTask->getIterations())
                + "\nfirstRun=" + String(shutdownTask->isFirstIteration())
                + "\nlastRun=" + String(shutdownTask->isLastIteration())
                + "\nnextRun=" + String(scheduler->timeUntilNextIteration(*shutdownTask)));
            });
            httpServer->begin();
        }
};
arkhipenko commented 4 years ago

Hi,

Shouldn't ShutdownTask be defined with 1 iteration, not 0? Task shutdownTask(2000, TASK_ONCE, [](){ WiFi.disconnect(true); });

Also, I am not using the lambda methods since they are not supported by Arduino IDE for most of the boards (AFAIK only ESP toolchain supports them).

Why can't you use static methods instead of the "initialization" class? Plus all of the parameters that you are passing are already static global - so why not just use them?

Something like:

#include "TaskScheduler.h"

ESP8266WebServer httpServer(80);
LoopOperationHandlers loopOperationHandlers;
Scheduler scheduler;
Task shutdownTask(2000, TASK_ONCE, &shutdownCB);

void setup() {
    Metrics::millisBoot = millis();
    pinMode(PIN_LED_BLUE, OUTPUT);

    scheduler.init();
    scheduler.addTask(shutdownTask);

    wifiSetup();
    httpServerSetup(); 
}

void loop() {
    scheduler.execute();
    loopOperationHandlers.wifiConnectedStatus();
    httpServer.handleClient();
}

        void wifiSetup() {
            loopOperationHandlers.wifiConnectedStatus();
            WiFi.mode(WIFI_STA);
            WiFi.setAutoConnect(true);
            WiFi.setAutoReconnect(true);
            WiFi.config(
                IPAddress(192,168,3,DEVICE_ID),
                IPAddress(192,168,3,1),
                IPAddress(255,255,255,0)
            );
            WiFi.begin(WLAN_SSID, WLAN_PASSWORD);
        }

        void httpServerSetup() {
            httpServer.on("/", [=]() mutable {
                httpServer.send(200, "text/plain",
                    "Uptime: " + String(millis() - Metrics::millisBoot)
                    + "ms\nWiFi Handshake: " + String(Metrics::millisWifiConnected)
                    + "ms\nSignal Strength: " + String(WiFi.RSSI())
                    + "dB\nFree Memory: " + String(ESP.getFreeHeap())
                    + "\nNext Shutdown: " + String(scheduler->timeUntilNextIteration(&shutdownTask))
                );
            });
            httpServer.on("/shutdown", [=]() mutable {
                shutdownTask.enableDelayed();               
                httpServer.send(200, "text/plain",
                "Powering off...\nenabled=" + String(shutdownTask->isEnabled())
                + "\ninterval=" + String(shutdownTask->getInterval())
                + "\niterations=" + String(shutdownTask->getIterations())
                + "\nfirstRun=" + String(shutdownTask->isFirstIteration())
                + "\nlastRun=" + String(shutdownTask->isLastIteration())
                + "\nnextRun=" + String(scheduler->timeUntilNextIteration(*shutdownTask)));
            });
            httpServer.begin();
        }

void shutdownCB() {
    WiFi.disconnect(true);
}
JamoDevNich commented 4 years ago

Ideally I wanted to separate the sketch across several files to make it easily repurposeable... given the advice above though I think refactoring the sketch into a single file would be the best way forward, especially as I'm inexperienced with C++.

Thanks

arkhipenko commented 4 years ago

You can still use separate files - just not the lambda functions and static methods instead of class objects.