Add a watchdog for nRF52 based repeaters

[winnieXY]

Hey,

I've created a watchdog which can be enabled via cli and defaults to off for nRF52 based repeaters. For all other archs are stubs available which are (currently) doing nothing - so there needs to be some code implemented later for esp32 and co.

This patch would mitigate together with the atomic write patch (#2386) issues with failed writes on the internal flash off the p1 - see #2283

In order to test it i provoked a HardFault on the repeater - it came up after 10 seconds again:

This watchdog would e.g. also fix this bug #2320 without adding unnecessary reboots on the devices. For debugging or testing the watchdog can be disabled by setting the timeout to 0 (which is also the default).

@oltaco: could you please review the patches?

[radiomeano]

I noticed a small compile issue in src/helpers/CommonCLI.cpp.
In the non-NRF52 branch of set watchdog, the sprintf() line is missing the final semicolon:
sprintf(reply, "OK - Watchdog currently not implemented on this platform")
Suggested fix:
strcpy(reply, "OK - Watchdog currently not implemented on this platform");
or just add the missing semicolon.
One additional thought: since wdt_timeout_secs is a new preference byte, it may be safer to default it to 0 when loading older /com_prefs files that do not contain it.

[winnieXY]

I noticed a small compile issue in src/helpers/CommonCLI.cpp. In the non-NRF52 branch of set watchdog, the sprintf() line is missing the final semicolon: sprintf(reply, "OK - Watchdog currently not implemented on this platform") Suggested fix: strcpy(reply, "OK - Watchdog currently not implemented on this platform");

Oh - damn. I'm testing it on three different repeaters - but obviously all nRF52 ones - so this line is on all of them dead code and optimized away by gcc - so no compile issues at all :S

[winnieXY]

One additional thought: since wdt_timeout_secs is a new preference byte, it may be safer to default it to 0 when loading older /com_prefs files that do not contain it.

I'll check this. Initially I thought that this should be handled by:

 memset(&_prefs, 0, sizeof(_prefs)); (see line 861)

as a older file is smaller it should defaults to 0 - but I'll double-check that.

[radiomeano]

I noticed a small compile issue in src/helpers/CommonCLI.cpp. In the non-NRF52 branch of set watchdog, the sprintf() line is missing the final semicolon: sprintf(reply, "OK - Watchdog currently not implemented on this platform") Suggested fix: strcpy(reply, "OK - Watchdog currently not implemented on this platform");

Oh - damn. I'm testing it on three different repeaters - but obviously all nRF52 ones - so this line is on all of them dead code and optimized away by gcc - so no compile issues at all :S

No problem Patrick, that makes sense.
Since all your test devices are nRF52, the non-NRF52 branch is never compiled there, so the missing semicolon would only show up when building for another platform.
Thanks for confirming, the watchdog feature itself still looks very useful for remote nRF52 repeaters.

[radiomeano]

One additional thought: since wdt_timeout_secs is a new preference byte, it may be safer to default it to 0 when loading older /com_prefs files that do not contain it.

I'll check this. Initially I thought that this should be handled by:

 memset(&_prefs, 0, sizeof(_prefs)); (see line 861)

as a older file is smaller it should defaults to 0 - but I'll double-check that.

Thanks Patrick for checking this, and thanks for your work on this feature.
If the prefs struct is already zero-initialized before loading /com_prefs, then an older/smaller file should indeed leave wdt_timeout_secs at 0, so watchdog disabled.
I only mentioned it as a migration-safety thought. Maybe just worth double-checking that the memset clears the actual prefs struct, not only the _prefs pointer.
I’m personally very interested in this feature because I have a SenseCAP Solar repeater that I plan to deploy at a high/mountain site, where a manual power cycle would be difficult.