133 lines
4.4 KiB
Markdown
133 lines
4.4 KiB
Markdown
|
|
# 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.
|