Implement machine.WDT()

[MrSurly]

esp32/machine_wdt.c: Implement machine.WDT()
esp32/modmachine.c: Implement machine.WDT()
esp32/modmachine.h: Implement machine.WDT()
esp32/sdkconfig.h: Implement machine.WDT()

Derivative of esp8266/machine_wdt.c, for ESP32, same API.

[dpgeorge]

Did you test it, does it actually reset the MCU if you don't feed it? If you don't create a WDT does everything work the same before (ie the WDT is disabled)?

[dpgeorge]

Please rename to machine_wdt_type.

[MrSurly]

Will do.

[MrSurly]

Did you test it, does it actually reset the MCU if you don't feed it? If you don't create a WDT does everything work the same before (ie the WDT is disabled)?

Of course. I eat my own dog food =) That's why I added CONFIG_TASK_WDT_PANIC to sdkconfig.h, otherwise, it just prints a message from the ISR.

It works the same as the ESP8266: Creating a WDT object doesn't really do anything. As soon as you call feed(), it adds the µPy task to a list of tasks (it maintains it's own linked list of tasks) that need to feed it. After that you must feed it. This matches what's documented (which states you can set the timeout) and what's implemented for the ESP8266 (where you can't).

I should note that the task WDT was already enabled in the ESP32 branch; the ISR just returns if the task list is empty. I simply copied the µPy API from the ESP8266, and changed the function names to match the IDF.

If desirable, this API can be updated to match the docs WRT timeout -- I didn't do that because I wasn't sure if it should match the ESP8266, or the docs (which don't cover ESP8266 WDT)

[MrSurly]

@dpgeorge If I get this fixed up and merging cleanly, will you consider merging it?

[dpgeorge]

@MrSurly sorry I let this one slip...

As per the current docs I think it makes sense to start the WDT as soon as it's created, which can be accomplished here by simply calling the feed function in the constructor.

It's also not possible to stop a WDT on some MCUs (for safety reasons), and that's also the behaviour described in the docs. So maybe remove the deinit() method.

Does ESP32 support a custom WDT timeout, configured at runtime? Well, for starters it's ok to not support this parameter and just use the default.

[MrSurly]

@dpgeorge

machine_wdt.c is directly copied from the ESP8266 port with very minimal editing to make it work on the ESP32.

I'm OK making your suggested changes for both ports, but it will be a change in the current behavior on ESP8266.

Does ESP32 support a custom WDT timeout, configured at runtime? Well, for starters it's ok to not support this parameter and just use the default.

Does ESP32 support a custom WDT timeout, configured at runtime? Well, for starters it's ok to not support this parameter and just use the default.

It looks like the ESP32 does (see code below, from here), but the IDF interface hard-codes it based on the CONFIG_TASK_WDT_TIMEOUT_S from sdkconfig.h. It's presently set to 5 seconds.

Implementing a variable time would be trivial, but again, this PR is just a quick-and-dirty port of the ESP8266 functionality.

I'm totally fine with making it variable (per the docs), and fixing this up, if there's interest. I have ESP8266 boards on-hand to test any changes made there.

void esp_task_wdt_init() {
    TIMERG0.wdt_wprotect=TIMG_WDT_WKEY_VALUE;
    TIMERG0.wdt_config0.sys_reset_length=7;                 //3.2uS
    TIMERG0.wdt_config0.cpu_reset_length=7;                 //3.2uS
    TIMERG0.wdt_config0.level_int_en=1;
    TIMERG0.wdt_config0.stg0=TIMG_WDT_STG_SEL_INT;          //1st stage timeout: interrupt
    TIMERG0.wdt_config0.stg1=TIMG_WDT_STG_SEL_RESET_SYSTEM; //2nd stage timeout: reset system
    TIMERG0.wdt_config1.clk_prescale=80*500;                //Prescaler: wdt counts in ticks of 0.5mS
    TIMERG0.wdt_config2=CONFIG_TASK_WDT_TIMEOUT_S*2000;     //Set timeout before interrupt
    TIMERG0.wdt_config3=CONFIG_TASK_WDT_TIMEOUT_S*4000;     //Set timeout before reset
    TIMERG0.wdt_config0.en=1;
    TIMERG0.wdt_feed=1;
    TIMERG0.wdt_wprotect=0;
#if CONFIG_TASK_WDT_CHECK_IDLE_TASK
    esp_register_freertos_idle_hook(idle_hook);
#endif
    esp_intr_alloc(ETS_TG0_WDT_LEVEL_INTR_SOURCE, 0, task_wdt_isr, NULL, NULL);
}

[dpgeorge]

I'm OK making your suggested changes for both ports, but it will be a change in the current behavior on ESP8266.

AFAIK the esp8266's WDT doesn't work (at least the uPy implementation) so there's no behaviour to change (in the sense of the WDT not starting until the first feed is called).

It looks like the ESP32 does (see code below, from here), but the IDF interface hard-codes it based on the CONFIG_TASK_WDT_TIMEOUT_S from sdkconfig.h. It's presently set to 5 seconds.

Right, then just leave that for now. There might be side-effects of changing the timeout (eg wifi timing out).

So I suggest just calling feed() in the constructor to start the WDT, and removing deinit(). Doesn't need to be more involved than that, and at least then it's usable.

[MrSurly]

Okay, adding to the queue.

[dpgeorge]

@MrSurly I see you pushed some commits. Please let me know when it's ready to review.

[MrSurly]

@dpgeorge

ESP32: Is ready. I've been using it heavily in my current project development (along with ethernet, and deep sleep!)
ESP8266: Is untested. I didn't even compile it. The changes are the same as for ESP32, but if it didn't work before, it probably doesn't now.

I'm only working on it since it's in-line with my current development deadline.

NB: I really think WDT timeout should be configurable, but that's for a future PR.

[nickzoic]

I've still got a pile of '66 stuff lying around so I'm happy to test in the next couple of days.

[dpgeorge]

Ok, thanks for updating, this was merged in d7b373c

Re esp8266 changes: I didn't merge these, this repo is just for esp32 changes. If it can be shown that the esp8266 WDT can be made to function with the extra call to system_soft_wdt_feed() then that can be changed upstream.

[MrSurly]

@nickzoic Did WDT work on the 8266?