fix: guard Serial input calls when USB not connected at boot

Fixes device hanging when booted without USB connected. The root cause
was calling Serial.available() and Serial.read() in checkForFlashCommand()
when Serial.begin() was never called (USB not connected at boot).

Changes:
- Add if (!Serial) return guard to checkForFlashCommand()
- Restore upstream while (!Serial) wait loop with 3s timeout
- Remove Serial.setTxTimeoutMs(0) (not in upstream, may cause issues)
- Remove unnecessary if (Serial) guards from EpubReaderActivity.cpp
  (Serial.printf is safe without guards, only input calls need them)

Key insight: Serial.printf() is safe without guards (returns 0 when not
initialized), but Serial.available()/Serial.read() cause undefined
behavior on ESP32-C3 USB CDC when called without Serial.begin().

See: claude_notes/usb-serial-blocking-fix-2026-01-28.md
This commit is contained in:
cottongin 2026-01-28 17:45:00 -05:00
parent 4965e63ad4
commit ffe2aebd7e
No known key found for this signature in database
GPG Key ID: 0ECC91FE4655C262
3 changed files with 158 additions and 38 deletions

View File

@ -0,0 +1,132 @@
# 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.

View File

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

View File

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