claude_notes/usb-serial-blocking-fix-2026-01-28.md

@@ -1,132 +0,0 @@
-# USB Serial Blocking Issue - Root Cause and Fix
-
-**Date:** 2026-01-28  
-**Issue:** Device blocking/hanging when USB is not connected at boot
-
----
-
-## Problem Description
-
-The device would hang or behave unpredictably when booted without USB connected. This was traced to improper Serial handling on ESP32-C3 with USB CDC.
-
-## Root Cause Analysis
-
-### Factor A: `checkForFlashCommand()` Called Without Serial Initialization
-
-The most critical issue was in `checkForFlashCommand()`, which is called at the start of every `loop()` iteration:
-
-```cpp
-void loop() {
-  checkForFlashCommand();  // Called EVERY loop iteration
-  // ...
-}
-
-void checkForFlashCommand() {
-  while (Serial.available()) {  // Called even when Serial.begin() was never called!
-    char c = Serial.read();
-    // ...
-  }
-}
-```
-
-When USB is not connected at boot, `Serial.begin()` is never called. Then in `loop()`, `checkForFlashCommand()` calls `Serial.available()` and `Serial.read()` on an uninitialized Serial object. On ESP32-C3 with USB CDC, this causes undefined behavior or blocking.
-
-### Factor B: Removed `while (!Serial)` Wait Loop
-
-The upstream 0.16.0 code included a 3-second wait loop after `Serial.begin()`:
-
-```cpp
-if (isUsbConnected()) {
-  Serial.begin(115200);
-  unsigned long start = millis();
-  while (!Serial && (millis() - start) < 3000) {
-    delay(10);
-  }
-}
-```
-
-This wait loop was removed in an earlier attempt to fix boot delays, but it may be necessary for proper USB CDC initialization.
-
-### Factor C: `Serial.setTxTimeoutMs(0)` Added Too Early
-
-`Serial.setTxTimeoutMs(0)` was added immediately after `Serial.begin()` to make TX non-blocking. However, calling this before the Serial connection is fully established may interfere with USB CDC initialization.
-
----
-
-## The Fix
-
-### 1. Guard `checkForFlashCommand()` with Serial Check
-
-```cpp
-void checkForFlashCommand() {
-  if (!Serial) return;  // Early exit if Serial not initialized
-  while (Serial.available()) {
-    // ... rest unchanged
-  }
-}
-```
-
-### 2. Restore Upstream Serial Initialization Pattern
-
-```cpp
-void setup() {
-  t1 = millis();
-
-  // Only start serial if USB connected
-  pinMode(UART0_RXD, INPUT);
-  if (isUsbConnected()) {
-    Serial.begin(115200);
-    // Wait up to 3 seconds for Serial to be ready to catch early logs
-    unsigned long start = millis();
-    while (!Serial && (millis() - start) < 3000) {
-      delay(10);
-    }
-  }
-  // ... rest of setup
-}
-```
-
-### 3. Remove `Serial.setTxTimeoutMs(0)`
-
-This call was removed entirely as it's not present in upstream and may cause issues.
-
-### 4. Remove Unnecessary `if (Serial)` Guards
-
-The 15 `if (Serial)` guards added to `EpubReaderActivity.cpp` were removed. `Serial.printf()` is safe to call when Serial isn't initialized (it simply returns 0), so guards around output calls are unnecessary.
-
-**Key distinction:**
-- `Serial.printf()` / `Serial.println()` - Safe without guards (no-op when not initialized)
-- `Serial.available()` / `Serial.read()` - **MUST** be guarded (undefined behavior when not initialized)
-
----
-
-## Files Changed
-
-| File | Change |
-|------|--------|
-| `src/main.cpp` | Removed `Serial.setTxTimeoutMs(0)`, restored `while (!Serial)` wait, added guard to `checkForFlashCommand()` |
-| `src/activities/reader/EpubReaderActivity.cpp` | Removed all 15 `if (Serial)` guards |
-
----
-
-## Testing Checklist
-
-After applying fixes, verify:
-
-1. ✅ Boot with USB connected, serial monitor open - should work
-2. ✅ Boot with USB connected, NO serial monitor - should work (3s delay then continue)
-3. ✅ Boot without USB - should work immediately (no blocking)
-4. ✅ Sleep without USB, plug in USB during sleep, wake - should work
-5. ✅ Sleep with USB, unplug during sleep, wake - should work
-
----
-
-## Lessons Learned
-
-1. **Always guard Serial input operations**: `Serial.available()` and `Serial.read()` must be guarded with `if (Serial)` or `if (!Serial) return` when Serial initialization is conditional.
-
-2. **Serial output is safe without guards**: `Serial.printf()` and similar output functions are safe to call even when Serial is not initialized - they simply return 0.
-
-3. **Don't remove initialization waits without understanding why they exist**: The `while (!Serial)` wait loop exists for proper USB CDC initialization and shouldn't be removed without careful testing.
-
-4. **Upstream patterns exist for a reason**: When diverging from upstream behavior, especially around low-level hardware initialization, be extra cautious and test thoroughly.

platformio.ini

@@ -2,7 +2,7 @@
 default_envs = default
 
 [crosspoint]
-version = ef-0.15.99
+version = ef-0.15.9
 
 [base]
 platform = espressif32 @ 6.12.0

src/activities/reader/EpubReaderActivity.cpp

@@ -127,8 +127,9 @@ void EpubReaderActivity::onEnter() {
         nextPageNumber = pageNumber;
         hasContentOffset = true;
 
-        Serial.printf("[%lu] [ERS] Loaded progress v1: spine %d, page %d, offset %u\n", millis(), currentSpineIndex,
-                      nextPageNumber, savedContentOffset);
+        if (Serial)
+          Serial.printf("[%lu] [ERS] Loaded progress v1: spine %d, page %d, offset %u\n", millis(), currentSpineIndex,
+                        nextPageNumber, savedContentOffset);
       } else {
         // Unknown version, try legacy format
         f.seek(0);
@@ -137,8 +138,9 @@ void EpubReaderActivity::onEnter() {
           currentSpineIndex = data[0] + (data[1] << 8);
           nextPageNumber = data[2] + (data[3] << 8);
           hasContentOffset = false;
-          Serial.printf("[%lu] [ERS] Loaded legacy progress (unknown version %d): spine %d, page %d\n", millis(),
-                      version, currentSpineIndex, nextPageNumber);
+          if (Serial)
+            Serial.printf("[%lu] [ERS] Loaded legacy progress (unknown version %d): spine %d, page %d\n", millis(),
+                          version, currentSpineIndex, nextPageNumber);
         }
       }
     } else if (fileSize >= 4) {
@@ -148,8 +150,9 @@ void EpubReaderActivity::onEnter() {
         currentSpineIndex = data[0] + (data[1] << 8);
         nextPageNumber = data[2] + (data[3] << 8);
         hasContentOffset = false;
-        Serial.printf("[%lu] [ERS] Loaded legacy progress: spine %d, page %d\n", millis(), currentSpineIndex,
-                      nextPageNumber);
+        if (Serial)
+          Serial.printf("[%lu] [ERS] Loaded legacy progress: spine %d, page %d\n", millis(), currentSpineIndex,
+                        nextPageNumber);
       }
     }
     f.close();
@@ -161,8 +164,9 @@ void EpubReaderActivity::onEnter() {
     int textSpineIndex = epub->getSpineIndexForTextReference();
     if (textSpineIndex != 0) {
       currentSpineIndex = textSpineIndex;
-      Serial.printf("[%lu] [ERS] Opened for first time, navigating to text reference at index %d\n", millis(),
-                    textSpineIndex);
+      if (Serial)
+        Serial.printf("[%lu] [ERS] Opened for first time, navigating to text reference at index %d\n", millis(),
+                      textSpineIndex);
     }
   }
 
@@ -635,7 +639,8 @@ void EpubReaderActivity::renderScreen() {
 
   if (!section) {
     const auto filepath = epub->getSpineItem(currentSpineIndex).href;
-    Serial.printf("[%lu] [ERS] Loading file: %s, index: %d\n", millis(), filepath.c_str(), currentSpineIndex);
+    if (Serial)
+      Serial.printf("[%lu] [ERS] Loading file: %s, index: %d\n", millis(), filepath.c_str(), currentSpineIndex);
     section = std::unique_ptr<Section>(new Section(epub, currentSpineIndex, renderer));
 
     const uint16_t viewportWidth = renderer.getScreenWidth() - orientedMarginLeft - orientedMarginRight;
@@ -646,7 +651,7 @@ void EpubReaderActivity::renderScreen() {
     if (!section->loadSectionFile(SETTINGS.getReaderFontId(), SETTINGS.getReaderLineCompression(),
                                   SETTINGS.extraParagraphSpacing, SETTINGS.paragraphAlignment, viewportWidth,
                                   viewportHeight, SETTINGS.hyphenationEnabled)) {
-      Serial.printf("[%lu] [ERS] Cache not found, building...\n", millis());
+      if (Serial) Serial.printf("[%lu] [ERS] Cache not found, building...\n", millis());
       sectionWasReIndexed = true;
 
       // Progress bar dimensions
@@ -692,12 +697,12 @@ void EpubReaderActivity::renderScreen() {
       if (!section->createSectionFile(SETTINGS.getReaderFontId(), SETTINGS.getReaderLineCompression(),
                                       SETTINGS.extraParagraphSpacing, SETTINGS.paragraphAlignment, viewportWidth,
                                       viewportHeight, SETTINGS.hyphenationEnabled, progressSetup, progressCallback)) {
-        Serial.printf("[%lu] [ERS] Failed to persist page data to SD\n", millis());
+        if (Serial) Serial.printf("[%lu] [ERS] Failed to persist page data to SD\n", millis());
         section.reset();
         return;
       }
     } else {
-      Serial.printf("[%lu] [ERS] Cache found, skipping build...\n", millis());
+      if (Serial) Serial.printf("[%lu] [ERS] Cache found, skipping build...\n", millis());
     }
 
     // Determine the correct page to display
@@ -709,8 +714,9 @@ void EpubReaderActivity::renderScreen() {
       // Use the offset to find the correct page
       const int restoredPage = section->findPageForContentOffset(savedContentOffset);
       section->currentPage = restoredPage;
-      Serial.printf("[%lu] [ERS] Restored position via offset: %u -> page %d (was page %d)\n", millis(),
-                    savedContentOffset, restoredPage, nextPageNumber);
+      if (Serial)
+        Serial.printf("[%lu] [ERS] Restored position via offset: %u -> page %d (was page %d)\n", millis(),
+                      savedContentOffset, restoredPage, nextPageNumber);
       // Clear the offset flag since we've used it
       hasContentOffset = false;
     } else {
@@ -722,7 +728,7 @@ void EpubReaderActivity::renderScreen() {
   renderer.clearScreen();
 
   if (section->pageCount == 0) {
-    Serial.printf("[%lu] [ERS] No pages to render\n", millis());
+    if (Serial) Serial.printf("[%lu] [ERS] No pages to render\n", millis());
     renderer.drawCenteredText(UI_12_FONT_ID, 300, "Empty chapter", true, EpdFontFamily::BOLD);
     renderStatusBar(orientedMarginRight, orientedMarginBottom, orientedMarginLeft);
     renderer.displayBuffer();
@@ -730,8 +736,9 @@ void EpubReaderActivity::renderScreen() {
   }
 
   if (section->currentPage < 0 || section->currentPage >= section->pageCount) {
-    Serial.printf("[%lu] [ERS] Page out of bounds: %d (max %d)\n", millis(), section->currentPage,
-                  section->pageCount);
+    if (Serial)
+      Serial.printf("[%lu] [ERS] Page out of bounds: %d (max %d)\n", millis(), section->currentPage,
+                    section->pageCount);
     renderer.drawCenteredText(UI_12_FONT_ID, 300, "Out of bounds", true, EpdFontFamily::BOLD);
     renderStatusBar(orientedMarginRight, orientedMarginBottom, orientedMarginLeft);
     renderer.displayBuffer();
@@ -741,7 +748,7 @@ void EpubReaderActivity::renderScreen() {
   {
     auto p = section->loadPageFromSectionFile();
     if (!p) {
-      Serial.printf("[%lu] [ERS] Failed to load page from SD - clearing section cache\n", millis());
+      if (Serial) Serial.printf("[%lu] [ERS] Failed to load page from SD - clearing section cache\n", millis());
       section->clearCache();
       section.reset();
       return renderScreen();
@@ -749,7 +756,7 @@ void EpubReaderActivity::renderScreen() {
 
     // Handle empty pages (e.g., from malformed chapters that couldn't be parsed)
     if (p->elements.empty()) {
-      Serial.printf("[%lu] [ERS] Page has no content (possibly malformed chapter)\n", millis());
+      if (Serial) Serial.printf("[%lu] [ERS] Page has no content (possibly malformed chapter)\n", millis());
       renderer.drawCenteredText(UI_12_FONT_ID, 280, "Chapter content unavailable", true, EpdFontFamily::BOLD);
       renderer.drawCenteredText(UI_10_FONT_ID, 320, "(File may be malformed)");
       renderStatusBar(orientedMarginRight, orientedMarginBottom, orientedMarginLeft);
@@ -759,7 +766,7 @@ void EpubReaderActivity::renderScreen() {
 
     const auto start = millis();
     renderContents(std::move(p), orientedMarginTop, orientedMarginRight, orientedMarginBottom, orientedMarginLeft);
-    Serial.printf("[%lu] [ERS] Rendered page in %dms\n", millis(), millis() - start);
+    if (Serial) Serial.printf("[%lu] [ERS] Rendered page in %dms\n", millis(), millis() - start);
   }
 
   // Save progress with content offset for position restoration after re-indexing
@@ -775,8 +782,9 @@ void EpubReaderActivity::renderScreen() {
     serialization::writePod(f, contentOffset);
 
     f.close();
-    Serial.printf("[%lu] [ERS] Saved progress: spine %d, page %d, offset %u\n", millis(), currentSpineIndex,
-                  section->currentPage, contentOffset);
+    if (Serial)
+      Serial.printf("[%lu] [ERS] Saved progress: spine %d, page %d, offset %u\n", millis(), currentSpineIndex,
+                    section->currentPage, contentOffset);
   }
 }
 

src/main.cpp

@@ -134,7 +134,6 @@ void logMemoryState(const char* tag, const char* context) {
 static String flashCmdBuffer;
 
 void checkForFlashCommand() {
-  if (!Serial) return;  // Early exit if Serial not initialized
   while (Serial.available()) {
     char c = Serial.read();
     if (c == '\n') {
@@ -458,10 +457,14 @@ bool isWakeupByPowerButton() {
 void setup() {
   t1 = millis();
 
-  // Only start serial if USB connected
+  // Always initialize Serial but make it non-blocking
+  // This prevents Serial.printf from blocking when USB is disconnected
+  Serial.begin(115200);
+  Serial.setTxTimeoutMs(0);  // Non-blocking TX - critical for USB disconnect handling
+
+  // Only wait for Serial to be ready if USB is connected
   pinMode(UART0_RXD, INPUT);
   if (isUsbConnected()) {
-    Serial.begin(115200);
     // Wait up to 3 seconds for Serial to be ready to catch early logs
     unsigned long start = millis();
     while (!Serial && (millis() - start) < 3000) {
@@ -557,7 +560,8 @@ void loop() {
 
   const unsigned long sleepTimeoutMs = SETTINGS.getSleepTimeoutMs();
   if (millis() - lastActivityTime >= sleepTimeoutMs) {
-    Serial.printf("[%lu] [SLP] Auto-sleep triggered after %lu ms of inactivity\n", millis(), sleepTimeoutMs);
+    if (Serial)
+      Serial.printf("[%lu] [SLP] Auto-sleep triggered after %lu ms of inactivity\n", millis(), sleepTimeoutMs);
     enterDeepSleep();
     // This should never be hit as `enterDeepSleep` calls esp_deep_sleep_start
     return;