[esp32][rp2] Print the previous boot crash report before the logger reads it (#19351)

esphome/components/esp32/crash_handler.cpp

@@ -173,7 +173,10 @@ static const char *const TAG = "esp32.crash";
 // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static uint32_t s_current_build_time = static_cast<uint32_t>(ESPHOME_BUILD_TIME);
 
-void crash_handler_read_and_clear() {
+// Validate the NOINIT record. Runs on every has_data() call; re-running is
+// harmless and the magic is left alone so the record survives an OTA
+// rollback reboot, crash_handler_clear() drops it once an API client has it.
+static void read_crash_data() {
   if (s_raw_crash_data.magic == CRASH_MAGIC && s_raw_crash_data.version == CRASH_DATA_VERSION) {
     s_crash_data_valid = true;
     // Clamp counts to prevent out-of-bounds reads from corrupt .noinit data
@@ -194,11 +197,12 @@ void crash_handler_read_and_clear() {
       s_raw_crash_data.other_reg_frame_count = s_raw_crash_data.other_backtrace_count;
 #endif
   }
-  // Don't clear magic here — crash data must survive OTA rollback reboots.
-  // Magic is cleared by crash_handler_clear() after an API client receives the data.
 }
 
-bool crash_handler_has_data() { return s_crash_data_valid; }
+bool crash_handler_has_data() {
+  read_crash_data();
+  return s_crash_data_valid;
+}
 
 void crash_handler_clear() {
   // Only clear the magic so data doesn't survive the next reboot.
@@ -426,7 +430,7 @@ static void log_foreign_addresses() {
 // crashes again during boot, and allowing the CLI's process_stacktrace to match
 // and decode each address individually.
 void crash_handler_log() {
-  if (!s_crash_data_valid)
+  if (!crash_handler_has_data())
     return;
 
   ESP_LOGE(TAG, "*** CRASH DETECTED ON PREVIOUS BOOT ***");

esphome/components/esp32/crash_handler.h

@@ -4,11 +4,6 @@
 
 namespace esphome::esp32 {
 
-/// Read and validate crash data from NOINIT memory.
-/// Does not clear the magic marker — call crash_handler_clear() after
-/// the data has been delivered to an API client so it survives OTA rollback reboots.
-void crash_handler_read_and_clear();
-
 /// Log crash data if a crash was detected on previous boot.
 void crash_handler_log();
 
@@ -16,7 +11,7 @@ void crash_handler_log();
 /// Call after the data has been delivered to an API client.
 void crash_handler_clear();
 
-/// Returns true if crash data was found this boot.
+/// Returns true if crash data was found this boot, reading it first if needed.
 bool crash_handler_has_data();
 
 }  // namespace esphome::esp32

esphome/components/esp32/hal.cpp

@@ -1,9 +1,6 @@
 #ifdef USE_ESP32
 
-// defines.h must come before crash_handler.h so USE_ESP32_CRASH_HANDLER is set
-// before crash_handler.h's #ifdef-guarded namespace block is parsed.
 #include "esphome/core/defines.h"
-#include "crash_handler.h"
 #include "esphome/core/hal.h"
 
 #include <esp_clk_tree.h>
@@ -45,11 +42,6 @@ void arch_restart() {
 }
 
 void arch_init() {
-#ifdef USE_ESP32_CRASH_HANDLER
-  // Read crash data from previous boot before anything else
-  esp32::crash_handler_read_and_clear();
-#endif
-
   // Enable the task watchdog only on the loop task (from which we're currently running)
   esp_task_wdt_add(nullptr);
 

esphome/components/rp2/crash_handler.cpp

@@ -55,8 +55,7 @@ namespace esphome::rp2 {
 
 static const char *const TAG = "rp2.crash";
 
-// Placed in .noinit so BSS zero-init cannot race with crash_handler_read_and_clear().
-// The valid field is explicitly cleared in crash_handler_read_and_clear() instead.
+// Filled from the watchdog scratch registers on the first read.
 static struct CrashData {
   bool valid;
   uint32_t pc;
@@ -64,11 +63,24 @@ static struct CrashData {
   uint32_t sp;
   uint32_t backtrace[MAX_BACKTRACE];
   uint8_t backtrace_count;
-} s_crash_data __attribute__((section(".noinit")));  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+} s_crash_data;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
 
-bool crash_handler_has_data() { return s_crash_data.valid; }
+// Logger::pre_setup() logs the record before App.pre_setup() reaches
+// arch_init(), so the first caller reads it and later calls are no-ops.
+// The read clears the scratch registers, so it must not run twice, and
+// arch_init() keeps its call so the read precedes watchdog_enable(), which
+// overwrites scratch[4].
+static bool s_crash_data_read = false;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+
+bool crash_handler_has_data() {
+  crash_handler_read_and_clear();
+  return s_crash_data.valid;
+}
 
 void crash_handler_read_and_clear() {
+  if (s_crash_data_read)
+    return;
+  s_crash_data_read = true;
   s_crash_data.valid = false;
   uint32_t magic = watchdog_hw->scratch[0];
   if ((magic & 0xFFFF0000) == CRASH_MAGIC_SENTINEL && (magic & 0xFFFF) == CRASH_DATA_VERSION) {
@@ -97,7 +109,7 @@ void crash_handler_read_and_clear() {
 // the device crashes again during boot, and allowing the CLI's process_stacktrace
 // to match and decode each address individually.
 void crash_handler_log() {
-  if (!s_crash_data.valid)
+  if (!crash_handler_has_data())
     return;
 
   ESP_LOGE(TAG, "*** CRASH DETECTED ON PREVIOUS BOOT ***");

esphome/components/rp2/crash_handler.h

@@ -9,12 +9,13 @@
 namespace esphome::rp2 {
 
 /// Read crash data from watchdog scratch registers and clear them.
+/// Only the first call reads; later calls are no-ops.
 void crash_handler_read_and_clear();
 
 /// Log crash data if a crash was detected on previous boot.
 void crash_handler_log();
 
-/// Returns true if crash data was found this boot.
+/// Returns true if crash data was found this boot, reading it first if needed.
 bool crash_handler_has_data();
 
 }  // namespace esphome::rp2

esphome/core/application.h

@@ -67,7 +67,7 @@ static constexpr uint32_t TEARDOWN_TIMEOUT_REBOOT_MS = 1000;  // 1 second for qu
 class Application {
  public:
 #ifdef ESPHOME_NAME_ADD_MAC_SUFFIX
-  // Called before Logger::pre_setup() — must not log (global_logger is not yet set).
+  // Runs after Logger::pre_setup() (emitted at EARLY_INIT priority), so the app name is not set yet there.
   /// Pre-setup with MAC suffix: overwrites placeholder in mutable static buffers with actual MAC.
   void pre_setup(char *name, size_t name_len, char *friendly_name, size_t friendly_name_len) {
     arch_init();
@@ -87,7 +87,7 @@ class Application {
     this->friendly_name_ = StringRef(friendly_name, friendly_name_len);
   }
 #else
-  // Called before Logger::pre_setup() — must not log (global_logger is not yet set).
+  // Runs after Logger::pre_setup() (emitted at EARLY_INIT priority), so the app name is not set yet there.
   /// Pre-setup without MAC suffix: StringRef points directly at const string literals in flash.
   void pre_setup(const char *name, size_t name_len, const char *friendly_name, size_t friendly_name_len) {
     arch_init();