Skip to content

Bootloop detection platform factoring #4853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DedeHai merged 5 commits into from
Sep 1, 2025
Merged

Bootloop detection platform factoring #4853

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5146926
Use direct references to RTC RAM on ESP8266
willmmiles Aug 20, 2025
85d4db8
Isolate platform differences in bootloop check
willmmiles Aug 20, 2025
46f3bc0
Bootloop: Include soft wdt on ESP8266
willmmiles Aug 27, 2025
dd13c2d
Reset crash counter after long interval
willmmiles Aug 29, 2025
6f914d7
Increase boot loop timeout
willmmiles Aug 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 111 additions & 97 deletions wled00/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,125 +720,139 @@ void *realloc_malloc(void *ptr, size_t size) {
// checks if the ESP reboots multiple times due to a crash or watchdog timeout
// if a bootloop is detected: restore settings from backup, then reset settings, then switch boot image (and repeat)

#define BOOTLOOP_THRESHOLD 5 // number of consecutive crashes to trigger bootloop detection
#define BOOTLOOP_ACTION_RESTORE 0 // default action: restore config from /bak.cfg.json
#define BOOTLOOP_ACTION_RESET 1 // if restore does not work, reset config (rename /cfg.json to /rst.cfg.json)
#define BOOTLOOP_ACTION_OTA 2 // swap the boot partition
#define BOOTLOOP_ACTION_DUMP 3 // nothing seems to help, dump files to serial and reboot (until hardware reset)
#define BOOTLOOP_INTERVAL_MILLIS 120000 // time limit between crashes: 120 seconds (2 minutes)
#define BOOTLOOP_THRESHOLD 5 // number of consecutive crashes to trigger bootloop detection
#define BOOTLOOP_ACTION_RESTORE 0 // default action: restore config from /bkp.cfg.json
#define BOOTLOOP_ACTION_RESET 1 // if restore does not work, reset config (rename /cfg.json to /rst.cfg.json)
#define BOOTLOOP_ACTION_OTA 2 // swap the boot partition
#define BOOTLOOP_ACTION_DUMP 3 // nothing seems to help, dump files to serial and reboot (until hardware reset)

// Platform-agnostic abstraction
enum class ResetReason {
Power,
Software,
Crash,
Brownout
};

#ifdef ESP8266
#define BOOTLOOP_INTERVAL_TICKS (5 * 160000) // time limit between crashes: ~5 seconds in RTC ticks
#define BOOT_TIME_IDX 0 // index in RTC memory for boot time
#define CRASH_COUNTER_IDX 1 // index in RTC memory for crash counter
#define ACTIONT_TRACKER_IDX 2 // index in RTC memory for boot action
// Place variables in RTC memory via references, since RTC memory is not exposed via the linker in the Non-OS SDK
// Use an offset of 32 as there's some hints that the first 128 bytes of "user" memory are used by the OTA system
// Ref: https://github.com/esp8266/Arduino/blob/78d0d0aceacc1553f45ad8154592b0af22d1eede/cores/esp8266/Esp.cpp#L168
static volatile uint32_t& bl_last_boottime = *(RTC_USER_MEM + 32);
static volatile uint32_t& bl_crashcounter = *(RTC_USER_MEM + 33);
static volatile uint32_t& bl_actiontracker = *(RTC_USER_MEM + 34);

static inline ResetReason rebootReason() {
uint32_t resetReason = system_get_rst_info()->reason;
if (resetReason == REASON_EXCEPTION_RST
|| resetReason == REASON_WDT_RST
|| resetReason == REASON_SOFT_WDT_RST)
return ResetReason::Crash;
if (resetReason == REASON_SOFT_RESTART)
return ResetReason::Software;
return ResetReason::Power;
}

static inline uint32_t getRtcMillis() { return system_get_rtc_time() / 160; }; // rtc ticks ~160000Hz

#else
#define BOOTLOOP_INTERVAL_TICKS 5000 // time limit between crashes: ~5 seconds in milliseconds
// variables in RTC_NOINIT memory persist between reboots (but not on hardware reset)
RTC_NOINIT_ATTR static uint32_t bl_last_boottime;
RTC_NOINIT_ATTR static uint32_t bl_crashcounter;
RTC_NOINIT_ATTR static uint32_t bl_actiontracker;

static inline ResetReason rebootReason() {
esp_reset_reason_t reason = esp_reset_reason();
if (reason == ESP_RST_BROWNOUT) return ResetReason::Brownout;
if (reason == ESP_RST_SW) return ResetReason::Software;
if (reason == ESP_RST_PANIC || reason == ESP_RST_WDT || reason == ESP_RST_INT_WDT || reason == ESP_RST_TASK_WDT) return ResetReason::Crash;
return ResetReason::Power;
}

#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0)
static inline uint32_t getRtcMillis() { return esp_rtc_get_time_us() / 1000; }
#elif ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(3, 3, 0)
static inline uint32_t getRtcMillis() { return rtc_time_slowclk_to_us(rtc_time_get(), rtc_clk_slow_freq_get_hz()) / 1000; }
#endif

void bootloopCheckOTA() { bl_actiontracker = BOOTLOOP_ACTION_OTA; } // swap boot image if bootloop is detected instead of restoring config

#endif
Comment on lines 779 to 781
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Provide a no-op bootloopCheckOTA() on ESP8266 to avoid cross-platform call sites needing ifdefs.

Right now the symbol only exists on non-ESP8266. A stub keeps call sites clean.

Apply:

-void bootloopCheckOTA() { bl_actiontracker = BOOTLOOP_ACTION_OTA; } // swap boot image if bootloop is detected instead of restoring config
+#ifndef ESP8266
+void bootloopCheckOTA() { bl_actiontracker = BOOTLOOP_ACTION_OTA; } // swap boot image if bootloop is detected instead of restoring config
+#else
+void bootloopCheckOTA() { /* not supported on ESP8266 */ }
+#endif

Also applies to: 841-851

🤖 Prompt for AI Agents 
In wled00/util.cpp around lines 779-781 (and also apply the same change for
lines 841-851), add a no-op stub implementation of bootloopCheckOTA() for the
ESP8266 build so the symbol always exists across platforms; specifically, wrap a
simple empty function definition (doing nothing) in the appropriate #ifdef or
#if defined(ESP8266) guard so call sites do not need platform-specific ifdefs
and the non-ESP8266 behavior (setting bl_actiontracker = BOOTLOOP_ACTION_OTA)
remains unchanged for other platforms.

Copy link
Member Author

@willmmiles willmmiles Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd considered it; honestly doing it that way would generally be my preferred style. Unfortunately, given that the body and caller are in separate translation units, the empty call cannot be inlined away, so the result would be a small waste of limited code memory bytes on ESP8266 systems for nothing but code aesthetic reasons. Given that the extra #ifdefs are already in the code, the best value is to leave them alone.

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willmmiles That's a very valid point about the memory constraints on ESP8266 systems. You're absolutely right that the function call overhead would be wasteful when every byte of code memory counts, especially since the compiler can't inline away calls across translation units. The practical consideration of limited flash memory definitely outweighs the aesthetic benefits of a unified API in this case. Thanks for the explanation!

✏️ Learnings added 
Learnt from: willmmiles
PR: wled/WLED#4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.314Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


// detect bootloop by checking the reset reason and the time since last boot
static bool detectBootLoop() {
#if !defined(ESP8266)
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0)
uint32_t rtctime = esp_rtc_get_time_us() / 1000; // convert to milliseconds
#elif ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(3, 3, 0)
uint64_t rtc_ticks = rtc_time_get();
uint32_t rtctime = rtc_time_slowclk_to_us(rtc_ticks, rtc_clk_slow_freq_get_hz()) / 1000; // convert to milliseconds
#endif

esp_reset_reason_t reason = esp_reset_reason();
uint32_t rtctime = getRtcMillis();
bool result = false;

if (!(reason == ESP_RST_PANIC || reason == ESP_RST_WDT || reason == ESP_RST_INT_WDT || reason == ESP_RST_TASK_WDT)) {
// no crash detected, init variables
bl_crashcounter = 0;
bl_last_boottime = rtctime;
if(reason != ESP_RST_SW)
bl_actiontracker = BOOTLOOP_ACTION_RESTORE; // init action tracker if not an intentional reboot (e.g. from OTA or bootloop handler)
} else if (reason == ESP_RST_BROWNOUT) {
// crash due to brownout can't be detected unless using flash memory to store bootloop variables
// this is a simpler way to preemtively revert the config in case current brownout is caused by a bad choice of settings
DEBUG_PRINTLN(F("brownout detected"));
//restoreConfig(); // TODO: blindly restoring config if brownout detected is a bad idea, need a better way (if at all)
} else {
uint32_t rebootinterval = rtctime - bl_last_boottime;
bl_last_boottime = rtctime; // store current runtime for next reboot
if (rebootinterval < BOOTLOOP_INTERVAL_TICKS) {
bl_crashcounter++;
if (bl_crashcounter >= BOOTLOOP_THRESHOLD) {
DEBUG_PRINTLN(F("!BOOTLOOP DETECTED!"));
bl_crashcounter = 0;
return true;
}
}
}
#else // ESP8266
rst_info* resetreason = system_get_rst_info();
uint32_t bl_last_boottime;
uint32_t bl_crashcounter;
uint32_t bl_actiontracker;
uint32_t rtctime = system_get_rtc_time();

if (!(resetreason->reason == REASON_EXCEPTION_RST || resetreason->reason == REASON_WDT_RST)) {
// no crash detected, init variables
bl_crashcounter = 0;
ESP.rtcUserMemoryWrite(BOOT_TIME_IDX, &rtctime, sizeof(uint32_t));
ESP.rtcUserMemoryWrite(CRASH_COUNTER_IDX, &bl_crashcounter, sizeof(uint32_t));
if(resetreason->reason != REASON_SOFT_RESTART) {
switch(rebootReason()) {
case ResetReason::Power:
bl_actiontracker = BOOTLOOP_ACTION_RESTORE; // init action tracker if not an intentional reboot (e.g. from OTA or bootloop handler)
ESP.rtcUserMemoryWrite(ACTIONT_TRACKER_IDX, &bl_actiontracker, sizeof(uint32_t));
}
} else {
// system has crashed
ESP.rtcUserMemoryRead(BOOT_TIME_IDX, &bl_last_boottime, sizeof(uint32_t));
ESP.rtcUserMemoryRead(CRASH_COUNTER_IDX, &bl_crashcounter, sizeof(uint32_t));
uint32_t rebootinterval = rtctime - bl_last_boottime;
ESP.rtcUserMemoryWrite(BOOT_TIME_IDX, &rtctime, sizeof(uint32_t)); // store current ticks for next reboot
if (rebootinterval < BOOTLOOP_INTERVAL_TICKS) {
bl_crashcounter++;
ESP.rtcUserMemoryWrite(CRASH_COUNTER_IDX, &bl_crashcounter, sizeof(uint32_t));
if (bl_crashcounter >= BOOTLOOP_THRESHOLD) {
DEBUG_PRINTLN(F("BOOTLOOP DETECTED"));
// fall through
case ResetReason::Software:
// no crash detected, reset counter
bl_crashcounter = 0;
break;

case ResetReason::Crash:
{
uint32_t rebootinterval = rtctime - bl_last_boottime;
if (rebootinterval < BOOTLOOP_INTERVAL_MILLIS) {
bl_crashcounter++;
if (bl_crashcounter >= BOOTLOOP_THRESHOLD) {
DEBUG_PRINTLN(F("!BOOTLOOP DETECTED!"));
bl_crashcounter = 0;
result = true;
}
} else {
// Reset counter on long intervals to track only consecutive short-interval crashes
bl_crashcounter = 0;
ESP.rtcUserMemoryWrite(CRASH_COUNTER_IDX, &bl_crashcounter, sizeof(uint32_t));
return true;
}
// TODO: crash reporting goes here
}
break;
}

case ResetReason::Brownout:
// crash due to brownout can't be detected unless using flash memory to store bootloop variables
DEBUG_PRINTLN(F("brownout detected"));
//restoreConfig(); // TODO: blindly restoring config if brownout detected is a bad idea, need a better way (if at all)
break;
}
#endif
return false; // no bootloop detected

bl_last_boottime = rtctime; // store current runtime for next reboot

return result;
}

void handleBootLoop() {
DEBUG_PRINTLN(F("checking for bootloop"));
DEBUG_PRINTF_P(PSTR("checking for bootloop: time %d, counter %d, action %d\n"), bl_last_boottime, bl_crashcounter, bl_actiontracker);
if (!detectBootLoop()) return; // no bootloop detected
#ifdef ESP8266
uint32_t bl_actiontracker;
ESP.rtcUserMemoryRead(ACTIONT_TRACKER_IDX, &bl_actiontracker, sizeof(uint32_t));
#endif
if (bl_actiontracker == BOOTLOOP_ACTION_RESTORE) {
restoreConfig(); // note: if this fails, could reset immediately. instead just let things play out and save a few lines of code
bl_actiontracker = BOOTLOOP_ACTION_RESET; // reset config if it keeps bootlooping
} else if (bl_actiontracker == BOOTLOOP_ACTION_RESET) {
resetConfig();
bl_actiontracker = BOOTLOOP_ACTION_OTA; // swap boot partition if it keeps bootlooping. On ESP8266 this is the same as BOOTLOOP_ACTION_NONE
}

switch(bl_actiontracker) {
case BOOTLOOP_ACTION_RESTORE:
restoreConfig();
++bl_actiontracker;
break;
case BOOTLOOP_ACTION_RESET:
resetConfig();
++bl_actiontracker;
break;
case BOOTLOOP_ACTION_OTA:
#ifndef ESP8266
else if (bl_actiontracker == BOOTLOOP_ACTION_OTA) {
if(Update.canRollBack()) {
DEBUG_PRINTLN(F("Swapping boot partition..."));
Update.rollBack(); // swap boot partition
}
bl_actiontracker = BOOTLOOP_ACTION_DUMP; // out of options
}
#endif
else
dumpFilesToSerial();
#ifdef ESP8266
ESP.rtcUserMemoryWrite(ACTIONT_TRACKER_IDX, &bl_actiontracker, sizeof(uint32_t));
if(Update.canRollBack()) {
DEBUG_PRINTLN(F("Swapping boot partition..."));
Update.rollBack(); // swap boot partition
}
++bl_actiontracker;
break;
#else
// fall through
#endif
case BOOTLOOP_ACTION_DUMP:
dumpFilesToSerial();
break;
}

ESP.restart(); // restart cleanly and don't wait for another crash
}

Expand Down