ESPAsync_WiFiManager::startConfigPortal() will cause a watchdog timeout when called from a higher-priority task. #39
Description
Hi, first I want to say thank you for this library. It's fantastically well documented. I've also compared the old version of this library to this newer async version, and the improved snappiness of the experience is night & day.
I'm working on fairly complex firmware for a sensor-based project, and can consistently repro a watchdog timeout crash that only happens when startConfigPortal()
is called from a task with a priority above the default of 0.
Debugging my firmware, I've simplified the code down to these 2 tasks:
- Network Task (Priority of 1). This fires
ESPAsync_WiFiManager::startConfigPortal()
inside of a task. - Idle Task (Priority of 0). This is the default implicit FreeRTOS task for that feeds the Task watchdog: https://gitdemo.readthedocs.io/en/latest/api/system/wdts.html
I noted that if I decrease the Network Task's priority to 0, the watchdog crash will not happen. This is intuitive if I examine the source code:
- On line 887 of
ESPAsync_WiFiManager.cpp
, there's ayield()
statement in the spin loop waiting for theESPAsync_WiFiManager
config portal to timeout or be closed. - Unlike
delay(milliseconds)
/vTaskDelay(ticks)
, which suspends the currently running Task and triggers the Task scheduler to schedule the next available Task,yield()
doesn't necessarily suspend the current Task.yield()
actually moves the current Task to the bottom of the current priority group, and triggers the Task scheduler to run again. If the currently running Task is the highest priority one,yield()
will reschedule this same Task again. This is the expected FreeRTOS behaviour:
https://www.freertos.org/FreeRTOS_Support_Forum_Archive/February_2007/freertos_taskYIELD_from_highest_priority_task._1663277.html
- This would naturally cause a watchdog crash - in my case, the Network Task with a priority of 1 never yields time to the Idle Task do to the
yield()
instartConfigPortal()
. This starves the watchdog and eventually triggering a crash. - Decreasing the Network Task's priority to 0 allows the Idle Task to get yielded time and feed the watchdog.
- And lastly, replacing that
yield()
withdelay()
fixes the crash(!)
I'd suggest using delay()
in line 887 instead, as that will allow yielding processing time to lower priority tasks. This allows using the library without modifications for more complex projects like mine, which require different task priorities. If you'd like to keep the same default behaviour for existing users, this would be easy to #ifdef with a flag like your other opt-in features.
I'm more than happy to create a PR with the fix if you'd review it. 😃