diff --git a/src/ESPNetwork.cpp b/src/ESPNetwork.cpp index 6a51e58..d9df416 100644 --- a/src/ESPNetwork.cpp +++ b/src/ESPNetwork.cpp @@ -1,14 +1,8 @@ #include #include #include -#include +#include "TaskWdt.h" #include "esp_log.h" - -// Only reset the WDT if the current task is actually subscribed. -// setConnected() is called from the WiFi event task which is not registered. -static inline void safe_wdt_reset() { - if (esp_task_wdt_status(NULL) == ESP_OK) esp_task_wdt_reset(); -} #include "ConfigSettings.h" #include "ESPNetwork.h" #include "entryPoints/Web.h" @@ -157,7 +151,7 @@ void ESPNetwork::loop() { this->emitSockets(); this->lastEmit = millis(); } - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. } sockEmit.loop(); @@ -169,7 +163,7 @@ void ESPNetwork::loop() { else if(!settings.ssdpBroadcast && SSDP.isStarted) SSDP.end(); } bool ESPNetwork::changeAP(const uint8_t *bssid, const int32_t channel) { - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. if(SSDP.isStarted) SSDP.end(); mqtt.disconnect(); //sockEmit.end(); @@ -239,7 +233,7 @@ void ESPNetwork::emitSockets(uint8_t num) { if(num != 255) this->emitHeap(num); } void ESPNetwork::setConnected(conn_types_t connType) { - safe_wdt_reset(); + resetTaskWdt(); this->connType = connType; this->connectTime = millis(); connectRetries = 0; @@ -270,7 +264,7 @@ void ESPNetwork::setConnected(conn_types_t connType) { #endif // HAS_ETHERNET // NET: Begin this in the startup. //sockEmit.begin(); - safe_wdt_reset(); + resetTaskWdt(); if(this->connectAttempts == 1) { if(this->connType == conn_types_t::wifi) { @@ -293,7 +287,7 @@ void ESPNetwork::setConnected(conn_types_t connType) { settings.IP.dns1 = ETH.dnsIP(0); settings.IP.dns2 = ETH.dnsIP(1); } - safe_wdt_reset(); + resetTaskWdt(); JsonSockEvent *json = sockEmit.beginEmit("ethernet"); json->beginObject(); json->addElem("connected", this->connected()); @@ -301,7 +295,7 @@ void ESPNetwork::setConnected(conn_types_t connType) { json->addElem("fullduplex", ETH.fullDuplex()); json->endObject(); sockEmit.endEmit(); - safe_wdt_reset(); + resetTaskWdt(); } #endif } @@ -335,7 +329,7 @@ void ESPNetwork::setConnected(conn_types_t connType) { SSDP.setManufacturerURL(0, "https://github.com/rstrouse"); SSDP.setURL(0, "/"); SSDP.setActive(0, true); - safe_wdt_reset(); + resetTaskWdt(); if(MDNS.begin(settings.hostname)) { ESP_LOGI(TAG, "MDNS Responder Started: serverId=%s", settings.serverId); MDNS.addService("http", "tcp", 80); @@ -348,11 +342,11 @@ void ESPNetwork::setConnected(conn_types_t connType) { MDNS.addServiceTxt("espsomfy_rts", "tcp", "version", String(settings.fwVersion.name)); } if(settings.ssdpBroadcast) { - safe_wdt_reset(); + resetTaskWdt(); SSDP.begin(); } else if(SSDP.isStarted) SSDP.end(); - safe_wdt_reset(); + resetTaskWdt(); this->emitSockets(); settings.printAvailHeap(); this->needsBroadcast = true; @@ -516,7 +510,7 @@ bool ESPNetwork::connectWiFi(const uint8_t *bssid, const int32_t channel) { return true; } bool ESPNetwork::connect(conn_types_t ctype) { - esp_task_wdt_reset(); + resetTaskWdt(); if(this->connecting()) return true; if(this->disconnectTime == 0) this->disconnectTime = millis(); #if HAS_ETHERNET @@ -557,10 +551,10 @@ bool ESPNetwork::getStrongestAP(const char *ssid, uint8_t *bssid, int32_t *chann int32_t strength = -127; int32_t chan = -1; memset(bssid, 0x00, 6); - esp_task_wdt_delete(NULL); + const bool wdtPaused = pauseTaskWdt(); int16_t n = WiFi.scanComplete(); //int16_t n = this->connected() ? WiFi.scanComplete() : WiFi.scanNetworks(false, false, false, 300, 0, ssid); - esp_task_wdt_add(NULL); + resumeTaskWdt(wdtPaused); for(int16_t i = 0; i < n; i++) { if(WiFi.SSID(i).compareTo(ssid) == 0) { // Only on the roaming scans, so the log shows whether a better AP is @@ -587,7 +581,7 @@ bool ESPNetwork::openSoftAP() { if(this->connected()) WiFi.disconnect(false); this->openingSoftAP = true; ESP_LOGI(TAG, "Turning the HotSpot On"); - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. WiFi.softAP(strlen(settings.hostname) > 0 ? settings.hostname : "ESPSomfy RTS", ""); delay(200); return true; diff --git a/src/GitOTA.cpp b/src/GitOTA.cpp index 2c09b9f..9bdf70e 100644 --- a/src/GitOTA.cpp +++ b/src/GitOTA.cpp @@ -2,7 +2,7 @@ #include #include #include -#include +#include "TaskWdt.h" #include "esp_log.h" #include "ConfigSettings.h" #include "GitOTA.h" @@ -96,7 +96,7 @@ int16_t GitRepo::getReleases(uint8_t num) { HTTPClient https; https.setReuse(false); if(https.begin(sclient, url)) { - esp_task_wdt_reset(); + resetTaskWdt(); int httpCode = https.GET(); ESP_LOGD(TAG, "[HTTPS] GET... code: %d", httpCode); if(httpCode > 0) { @@ -117,7 +117,7 @@ int16_t GitRepo::getReleases(uint8_t num) { while(https.connected() && (len > 0 || len == -1) && ndx < count) { size_t size = stream->available(); if(size) { - esp_task_wdt_reset(); + resetTaskWdt(); int c = stream->readBytes(buff, ((size > sizeof(buff)) ? sizeof(buff) : size)); //Serial.write(buff, c); if(len > 0) len -= c; @@ -308,15 +308,15 @@ int GitUpdater::checkInternet() { WiFiClientSecure sclient; sclient.setInsecure(); sclient.setHandshakeTimeout(3); - esp_task_wdt_reset(); + resetTaskWdt(); HTTPClient https; https.setReuse(false); if(https.begin(sclient, "https://github.com/cjkas/ESPSomfy-RTS")) { https.setFollowRedirects(HTTPC_FORCE_FOLLOW_REDIRECTS); https.setTimeout(3000); - esp_task_wdt_reset(); + resetTaskWdt(); int httpCode = https.sendRequest("HEAD"); - esp_task_wdt_reset(); + resetTaskWdt(); if (httpCode == HTTP_CODE_OK || httpCode == HTTP_CODE_MOVED_PERMANENTLY || httpCode == HTTP_CODE_FOUND) { err = 0; ESP_LOGI(TAG, "Internet is Available: %ldms", millis() - t); @@ -330,7 +330,7 @@ int GitUpdater::checkInternet() { https.end(); sclient.stop(); } - esp_task_wdt_reset(); + resetTaskWdt(); return err; } void GitUpdater::emitDownloadProgress(size_t total, size_t loaded, const char *evt) { this->emitDownloadProgress(255, total, loaded, evt); } @@ -413,7 +413,7 @@ int8_t GitUpdater::downloadFile() { char url[196]; sprintf(url, "%s%s", this->baseUrl, this->currentFile); ESP_LOGD(TAG, "%s", url); - esp_task_wdt_reset(); + resetTaskWdt(); if(https.begin(sclient, url)) { https.setFollowRedirects(HTTPC_FORCE_FOLLOW_REDIRECTS); ESP_LOGD(TAG, "[HTTPS] GET..."); @@ -436,7 +436,7 @@ int8_t GitUpdater::downloadFile() { int timeouts = 0; while(https.connected() && (len > 0 || len == -1) && total < len) { size_t size = stream->available(); - esp_task_wdt_reset(); + resetTaskWdt(); if(size) { timeouts = 0; if(this->cancelled && !this->lockFS) { @@ -515,6 +515,6 @@ int8_t GitUpdater::downloadFile() { sclient.stop(); ESP_LOGI(TAG, "End update %s", this->currentFile); } - esp_task_wdt_reset(); + resetTaskWdt(); return 0; } diff --git a/src/MqttConnection.cpp b/src/MqttConnection.cpp index cb34f6b..3b2c4f9 100644 --- a/src/MqttConnection.cpp +++ b/src/MqttConnection.cpp @@ -1,7 +1,7 @@ #include #include #include -#include +#include "TaskWdt.h" #include "esp_log.h" #include "ConfigSettings.h" #include "MqttConnection.h" @@ -42,15 +42,15 @@ void MqttConnection::reset() { } bool MqttConnection::loop() { if(settings.MQTT.enabled && !rebootDelay.reboot && !this->suspended && !mqttClient.connected()) { - esp_task_wdt_reset(); + resetTaskWdt(); if(!this->connected() && net.connected()) this->connect(); } - esp_task_wdt_reset(); + resetTaskWdt(); if(settings.MQTT.enabled) mqttClient.loop(); return true; } void MqttConnection::receive(const char *topic, byte*payload, uint32_t length) { - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. ESP_LOGD(TAG, "MQTT Topic:%s payload:%.*s", topic, (int)length, (const char *)payload); // This already runs on loop(), but going through the loop queue keeps MQTT // and web requests in order and holds them while the radio is full. @@ -62,7 +62,7 @@ void MqttConnection::receive(const char *topic, byte*payload, uint32_t length) { loopQueue.submit(item); } bool MqttConnection::connect() { - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. if(mqttClient.connected()) { if(!settings.MQTT.enabled || this->suspended) return this->disconnect(); @@ -78,7 +78,7 @@ bool MqttConnection::connect() { char lwtTopic[128] = "status"; if(strlen(settings.MQTT.rootTopic) > 0) snprintf(lwtTopic, sizeof(lwtTopic), "%s/status", settings.MQTT.rootTopic); - esp_task_wdt_reset(); + resetTaskWdt(); if(mqttClient.connect(this->clientId, settings.MQTT.username, settings.MQTT.password, lwtTopic, 0, true, "offline")) { ESP_LOGI(TAG, "Successfully connected MQTT client %s", this->clientId); this->publish("status", "online", true); @@ -91,7 +91,7 @@ bool MqttConnection::connect() { for(const char *topic : MQTT_COMMAND_TOPICS) this->subscribe(topic); mqttClient.setCallback(MqttConnection::receive); ESP_LOGI(TAG, "MQTT Startup Completed"); - esp_task_wdt_reset(); + resetTaskWdt(); this->lastConnect = millis(); return true; } @@ -127,7 +127,7 @@ bool MqttConnection::unsubscribe(const char *topic) { } bool MqttConnection::subscribe(const char *topic) { if(mqttClient.connected()) { - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. char top[128]; if(strlen(settings.MQTT.rootTopic) > 0) snprintf(top, sizeof(top), "%s/%s", settings.MQTT.rootTopic, topic); @@ -145,7 +145,7 @@ bool MqttConnection::publish(const char *topic, const char *payload, bool retain snprintf(top, sizeof(top), "%s/%s", settings.MQTT.rootTopic, topic); else strlcpy(top, topic, sizeof(top)); - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. mqttClient.publish(top, payload, retain); return true; } @@ -162,7 +162,7 @@ bool MqttConnection::unpublish(const char *topic) { snprintf(top, sizeof(top), "%s/%s", settings.MQTT.rootTopic, topic); else strlcpy(top, topic, sizeof(top)); - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. mqttClient.publish(top, (const uint8_t *)"", 0, true); return true; } @@ -177,7 +177,7 @@ bool MqttConnection::publishBuffer(const char *topic, uint8_t *data, uint16_t le uint16_t offset = 0; uint16_t to_write = len; uint16_t buff_len; - esp_task_wdt_reset(); // Make sure we do not reboot here. + resetTaskWdt(); // Make sure we do not reboot here. mqttClient.beginPublish(topic, len, retain); do { buff_len = to_write; diff --git a/src/ShadeController.cpp b/src/ShadeController.cpp index 0179832..9d0e676 100644 --- a/src/ShadeController.cpp +++ b/src/ShadeController.cpp @@ -1,5 +1,5 @@ #include -#include +#include "TaskWdt.h" #include "esp_log.h" #include "ConfigSettings.h" #include "Somfy.h" @@ -78,7 +78,7 @@ bool SomfyShadeController::begin() { } void SomfyShadeController::commit() { if(git.lockFS) return; - esp_task_wdt_reset(); // Make sure we don't reset inadvertently. + resetTaskWdt(); // Make sure we don't reset inadvertently. ShadeConfigFile file; file.begin(); file.save(this); @@ -88,7 +88,7 @@ void SomfyShadeController::commit() { } void SomfyShadeController::writeBackup() { if(git.lockFS) return; - esp_task_wdt_reset(); // Make sure we don't reset inadvertently. + resetTaskWdt(); // Make sure we don't reset inadvertently. this->backupData = ""; this->backupData.reserve(16384); ShadeConfigFile file; diff --git a/src/TaskWdt.h b/src/TaskWdt.h new file mode 100644 index 0000000..dfe8305 --- /dev/null +++ b/src/TaskWdt.h @@ -0,0 +1,21 @@ +#ifndef TASK_WDT_H +#define TASK_WDT_H +#include + +// Task watchdog calls that are safe from any task. +// +// Only loop() is subscribed to the watchdog. Code it shares with other tasks +// is not: the WiFi event task runs ESPNetwork::setConnected() and async_tcp +// runs the web and socket handlers. A plain esp_task_wdt_reset() there logs +// "esp_task_wdt_reset(707): task not found", and an esp_task_wdt_add() there +// subscribes a task that nothing resets, so it panics once the timeout runs out. + +// Resets the watchdog if the calling task is subscribed to it. +inline void resetTaskWdt() { + if(esp_task_wdt_status(NULL) == ESP_OK) esp_task_wdt_reset(); +} +// Stops watching the calling task around a call that blocks past the timeout. +// True when it was watched, so resumeTaskWdt() knows to add it back. +inline bool pauseTaskWdt() { return esp_task_wdt_delete(NULL) == ESP_OK; } +inline void resumeTaskWdt(bool paused) { if(paused) esp_task_wdt_add(NULL); } +#endif diff --git a/src/entryPoints/WebUiRoutes.cpp b/src/entryPoints/WebUiRoutes.cpp index 9d7559f..2c5710e 100644 --- a/src/entryPoints/WebUiRoutes.cpp +++ b/src/entryPoints/WebUiRoutes.cpp @@ -1,7 +1,7 @@ #include #include #include -#include +#include "TaskWdt.h" #include "esp_log.h" #include "ConfigSettings.h" #include "ConfigFile.h" @@ -275,7 +275,7 @@ void registerWebUiRoutes(AsyncWebServer &server) { else request->send(400, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"The upload didn't complete, so nothing was restored.\"}")); }, [](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) { - esp_task_wdt_reset(); + resetTaskWdt(); if(index == 0) { webServer.uploadSuccess = false; ESP_LOGD(TAG, "Restore: %s", filename.c_str()); @@ -544,7 +544,7 @@ void registerWebUiRoutes(AsyncWebServer &server) { ESP_LOGE(TAG, "Update end failed"); } } - esp_task_wdt_reset(); + resetTaskWdt(); }); // updateShadeConfig - file upload @@ -615,15 +615,14 @@ void registerWebUiRoutes(AsyncWebServer &server) { ESP_LOGE(TAG, "Update end failed"); } } - esp_task_wdt_reset(); + resetTaskWdt(); }); server.on("/scanaps", HTTP_GET, [](AsyncWebServerRequest *request) { - esp_task_wdt_reset(); - esp_task_wdt_delete(NULL); + const bool wdtPaused = pauseTaskWdt(); if(net.softAPOpened) WiFi.disconnect(false); int n = WiFi.scanNetworks(false, true); - esp_task_wdt_add(NULL); + resumeTaskWdt(wdtPaused); ESP_LOGI(TAG, "Scanned %d networks", n); AsyncJsonResp resp; resp.beginResponse(request, g_async_content, sizeof(g_async_content)); diff --git a/src/outputs/SocketEmitter.cpp b/src/outputs/SocketEmitter.cpp index eda960f..85bd85a 100644 --- a/src/outputs/SocketEmitter.cpp +++ b/src/outputs/SocketEmitter.cpp @@ -1,6 +1,6 @@ #include #include -#include +#include "TaskWdt.h" #include "esp_log.h" #include "SocketEmitter.h" #include "ConfigSettings.h" @@ -112,7 +112,7 @@ void SocketEmitter::endEmit(uint8_t num) { uint32_t asyncId = getAsyncId(num); this->json.endEvent(asyncId); } - esp_task_wdt_reset(); + resetTaskWdt(); } void SocketEmitter::endEmitRoom(uint8_t room) { if(room < SOCK_MAX_ROOMS) { @@ -136,7 +136,7 @@ void SocketEmitter::initClients() { uint32_t asyncId = getAsyncId(slot); if(asyncId != 0 && ws.hasClient(asyncId)) { ESP_LOGD(TAG, "Initializing Socket Client %u (asyncId=%lu)", slot, asyncId); - esp_task_wdt_reset(); + resetTaskWdt(); ElapsedTimer total, step; settings.emitSockets(slot); logElapsed(TAG, "Socket init settings", step.lap()); @@ -150,7 +150,7 @@ void SocketEmitter::initClients() { net.emitSockets(slot); logElapsed(TAG, "Socket init network", step.lap()); logElapsed(TAG, "Socket init total", total.us()); - esp_task_wdt_reset(); + resetTaskWdt(); } this->newClients[i] = 255; } diff --git a/src/outputs/SomfyEvents.cpp b/src/outputs/SomfyEvents.cpp index ff87d85..b1ce85d 100644 --- a/src/outputs/SomfyEvents.cpp +++ b/src/outputs/SomfyEvents.cpp @@ -1,4 +1,4 @@ -#include +#include "TaskWdt.h" #include "Somfy.h" #include "SomfyEditor.h" #include "SocketEmitter.h" @@ -128,7 +128,7 @@ void SomfyShadeController::emitState(uint8_t num) { for(uint8_t i = 0; i < SOMFY_MAX_SHADES; i++) { SomfyShade *shade = &this->shades[i]; if(shade->getShadeId() == 255) continue; - esp_task_wdt_reset(); + resetTaskWdt(); shade->emitState(num); } } diff --git a/test/stubs/esp_task_wdt.h b/test/stubs/esp_task_wdt.h new file mode 100644 index 0000000..86c7bc4 --- /dev/null +++ b/test/stubs/esp_task_wdt.h @@ -0,0 +1,34 @@ +#pragma once +// Stands in for ESP-IDF's task watchdog for one calling task. Tests set +// whether it is subscribed and count the resets. +typedef int esp_err_t; +#ifndef ESP_OK +#define ESP_OK 0 +#endif +#ifndef ESP_ERR_NOT_FOUND +#define ESP_ERR_NOT_FOUND 0x105 +#endif + +struct FakeTaskWdt { + bool subscribed = false; + int resets = 0; + int adds = 0; +}; +inline FakeTaskWdt fakeTaskWdt; + +inline esp_err_t esp_task_wdt_status(void *) { return fakeTaskWdt.subscribed ? ESP_OK : ESP_ERR_NOT_FOUND; } +inline esp_err_t esp_task_wdt_reset() { + if(!fakeTaskWdt.subscribed) return ESP_ERR_NOT_FOUND; + fakeTaskWdt.resets++; + return ESP_OK; +} +inline esp_err_t esp_task_wdt_delete(void *) { + if(!fakeTaskWdt.subscribed) return ESP_ERR_NOT_FOUND; + fakeTaskWdt.subscribed = false; + return ESP_OK; +} +inline esp_err_t esp_task_wdt_add(void *) { + fakeTaskWdt.adds++; + fakeTaskWdt.subscribed = true; + return ESP_OK; +} diff --git a/test/test_taskwdt/test_taskwdt.cpp b/test/test_taskwdt/test_taskwdt.cpp new file mode 100644 index 0000000..6fd958f --- /dev/null +++ b/test/test_taskwdt/test_taskwdt.cpp @@ -0,0 +1,49 @@ +#include +#include "TaskWdt.h" + +void setUp() { fakeTaskWdt = FakeTaskWdt(); } +void tearDown() {} + +/********************************************************************* + * resetTaskWdt + ********************************************************************/ +static void test_resetTaskWdt_resets_a_subscribed_task() { + fakeTaskWdt.subscribed = true; + resetTaskWdt(); + TEST_ASSERT_EQUAL_INT(1, fakeTaskWdt.resets); +} +static void test_resetTaskWdt_skips_a_task_that_is_not_subscribed() { + // The WiFi event task and async_tcp. + resetTaskWdt(); + TEST_ASSERT_EQUAL_INT(0, fakeTaskWdt.resets); + TEST_ASSERT_FALSE(fakeTaskWdt.subscribed); +} + +/********************************************************************* + * pauseTaskWdt / resumeTaskWdt + ********************************************************************/ +static void test_pause_resume_puts_a_watched_task_back() { + fakeTaskWdt.subscribed = true; + const bool paused = pauseTaskWdt(); + TEST_ASSERT_TRUE(paused); + TEST_ASSERT_FALSE(fakeTaskWdt.subscribed); + resumeTaskWdt(paused); + TEST_ASSERT_TRUE(fakeTaskWdt.subscribed); +} +static void test_pause_resume_leaves_an_unwatched_task_unwatched() { + // /scanaps on async_tcp: adding it would panic once nothing resets it. + const bool paused = pauseTaskWdt(); + TEST_ASSERT_FALSE(paused); + resumeTaskWdt(paused); + TEST_ASSERT_FALSE(fakeTaskWdt.subscribed); + TEST_ASSERT_EQUAL_INT(0, fakeTaskWdt.adds); +} + +int main(int, char **) { + UNITY_BEGIN(); + RUN_TEST(test_resetTaskWdt_resets_a_subscribed_task); + RUN_TEST(test_resetTaskWdt_skips_a_task_that_is_not_subscribed); + RUN_TEST(test_pause_resume_puts_a_watched_task_back); + RUN_TEST(test_pause_resume_leaves_an_unwatched_task_unwatched); + return UNITY_END(); +}