Skip to content
This repository was archived by the owner on Feb 4, 2023. It is now read-only.
This repository was archived by the owner on Feb 4, 2023. It is now read-only.

ESPAsync_WiFiManager::startConfigPortal() will cause a watchdog timeout when called from a higher-priority task. #39

Closed
@russelljahn

Description

@russelljahn

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:

  1. Network Task (Priority of 1). This fires ESPAsync_WiFiManager::startConfigPortal() inside of a task.
  2. 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 a yield() statement in the spin loop waiting for the ESPAsync_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.
  • 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() in startConfigPortal(). 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() with delay() 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. 😃

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions