Remove intra-boot() BT1035 retry loop; restore validated single-attempt design
Git archaeology traced the boot sequence back tofd9d4ae(2026-08-15, documented 5/5 clean boots), which removed a redundant AT+RESET and added the boot-banner listen window in a single commit. Comparing that validated design to today's working tree found one real structural deviation: an intra-boot() retry loop (2 attempts, only 300ms between hardware reset pulses) added earlier today, which never existed in the validated baseline. The BT1035 datasheet's own Reset Protection timeout (typically >1.8s) means a second pulse fired only 300ms later may not reach a clean power-off state before repowering. Removed the intra-boot() retry loop entirely (kBootAttempts, kBootRetryDelayMs deleted) — boot() now makes exactly one attempt per call, matchingfd9d4ae. Retries remain exclusively at the bt1035RetryTask level (whole clean boot() calls, confirmed live at ~31.8s apart). Banner wait (25s) and GPIO readback left untouched. Documented the full commit-by-commit analysis and live test result (structurally correct, hit-rate inconclusive on this sample) in the RF investigation report and TODO for future sessions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -51,10 +51,20 @@ constexpr int kSysCtlSettleMs = 50;
|
|||||||
* RESET# releases — full BT stack init, not just the internal regulator.
|
* RESET# releases — full BT stack init, not just the internal regulator.
|
||||||
* The previous 3500ms wait here was never enough for the module to say
|
* The previous 3500ms wait here was never enough for the module to say
|
||||||
* anything, so every prior boot attempt cut power and restarted before
|
* anything, so every prior boot attempt cut power and restarted before
|
||||||
* the module could finish booting even once. See kBootAttempts below. */
|
* the module could finish booting even once.
|
||||||
|
*
|
||||||
|
* boot() makes exactly one resetAndInitOnce() attempt per call (matching
|
||||||
|
* fd9d4ae's validated 5/5-clean-boot design): the BT1035 datasheet's own
|
||||||
|
* "Reset Protection timeout (typically >1.8s)" means a second
|
||||||
|
* SYS_CTRL/RESET pulse fired shortly after a failed attempt would not
|
||||||
|
* reliably reach a clean power-off state before repowering — an internal
|
||||||
|
* retry loop here risks re-interrupting the module mid bring-up, the
|
||||||
|
* same class of bug fixed in fd9d4ae (redundant AT+RESET). Retries now
|
||||||
|
* live one layer up, in hardware::bt1035RetryTask
|
||||||
|
* (main/hardware_bootstrap.cpp), which only re-invokes a full, clean
|
||||||
|
* boot() call — never re-pulses the pins faster than a whole boot cycle
|
||||||
|
* apart. */
|
||||||
constexpr int kBootBannerWaitMs = 25000;
|
constexpr int kBootBannerWaitMs = 25000;
|
||||||
constexpr int kBootAttempts = 2;
|
|
||||||
constexpr int kBootRetryDelayMs = 300;
|
|
||||||
|
|
||||||
void flushUartRx(int uartPort) noexcept
|
void flushUartRx(int uartPort) noexcept
|
||||||
{
|
{
|
||||||
@@ -66,7 +76,7 @@ void flushUartRx(int uartPort) noexcept
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Diagnostic only, run once if all kBootAttempts fail at 115200 (the
|
// Diagnostic only, run if the single boot attempt fails at 115200 (the
|
||||||
// datasheet's own default). AT+BAUD persists across RESET#/SYS_CTRL power
|
// datasheet's own default). AT+BAUD persists across RESET#/SYS_CTRL power
|
||||||
// cycles (programming guide §5.1.3), so a stray manual AT+BAUD or
|
// cycles (programming guide §5.1.3), so a stray manual AT+BAUD or
|
||||||
// AT+RESTORE sent during earlier interactive testing could have left the
|
// AT+RESTORE sent during earlier interactive testing could have left the
|
||||||
@@ -77,8 +87,7 @@ constexpr std::array<int, 8> kBaudProbeCandidates = {
|
|||||||
|
|
||||||
void probeBaudRates(int uartPort) noexcept
|
void probeBaudRates(int uartPort) noexcept
|
||||||
{
|
{
|
||||||
ESP_LOGW(kTag, "115200 unresponsive after %d attempts — sweeping baud rates",
|
ESP_LOGW(kTag, "115200 unresponsive — sweeping baud rates");
|
||||||
kBootAttempts);
|
|
||||||
for (const int baud : kBaudProbeCandidates) {
|
for (const int baud : kBaudProbeCandidates) {
|
||||||
if (uart_set_baudrate(static_cast<uart_port_t>(uartPort), baud)
|
if (uart_set_baudrate(static_cast<uart_port_t>(uartPort), baud)
|
||||||
!= ESP_OK) {
|
!= ESP_OK) {
|
||||||
@@ -1052,19 +1061,8 @@ std::expected<void, Bt1035Error> Bt1035Driver::boot()
|
|||||||
uartInstalled_ = true;
|
uartInstalled_ = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::expected<void, Bt1035Error> init = std::unexpected(Bt1035Error::UnexpectedResponse);
|
if (auto init = resetAndInitOnce(); !init) {
|
||||||
for (int attempt = 1; attempt <= kBootAttempts; ++attempt) {
|
ESP_LOGE(kTag, "AT init failed");
|
||||||
init = resetAndInitOnce();
|
|
||||||
if (init) {
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
ESP_LOGW(kTag, "boot attempt %d/%d failed", attempt, kBootAttempts);
|
|
||||||
if (attempt < kBootAttempts) {
|
|
||||||
vTaskDelay(pdMS_TO_TICKS(kBootRetryDelayMs));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (!init) {
|
|
||||||
ESP_LOGE(kTag, "AT init failed after %d attempts", kBootAttempts);
|
|
||||||
probeBaudRates(uartPort_);
|
probeBaudRates(uartPort_);
|
||||||
return init;
|
return init;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -134,6 +134,30 @@ Short version:
|
|||||||
session narrative, including a UART TX/RX loopback test attempt that was
|
session narrative, including a UART TX/RX loopback test attempt that was
|
||||||
inconclusive (bridging the ESP32's own TX/RX pins from cold boot caused
|
inconclusive (bridging the ESP32's own TX/RX pins from cold boot caused
|
||||||
an unrelated, reproducible, harmless early-boot hang, not yet explained).
|
an unrelated, reproducible, harmless early-boot hang, not yet explained).
|
||||||
|
- **BT1035 — git archaeology + minimal patch, follow-up (2026-08-21).**
|
||||||
|
Traced the full commit history of `Bt1035Driver.cpp` from the last
|
||||||
|
documented-good boot (`6ca40f1`) through the regression (`6f7b6dd`, a
|
||||||
|
redundant `AT+RESET`) and its fix (`fd9d4ae`, 5/5 clean boots — removed
|
||||||
|
the `AT+RESET` and introduced the boot-banner listen at 3500ms in the
|
||||||
|
same commit). Comparing `fd9d4ae` to this session's working tree found
|
||||||
|
one real structural difference beyond the justified 25s banner window:
|
||||||
|
today's earlier commit (`3a58d33`) had added an intra-`boot()` retry
|
||||||
|
loop (2 attempts, only 300ms between hardware reset pulses) that never
|
||||||
|
existed in the validated baseline — shorter than the BT1035 datasheet's
|
||||||
|
own "Reset Protection timeout (typically >1.8s)", so the second pulse
|
||||||
|
may not have reached a clean power-off state. **Fixed**: removed the
|
||||||
|
intra-`boot()` retry loop entirely (`kBootAttempts`/`kBootRetryDelayMs`
|
||||||
|
deleted); `boot()` now makes exactly one attempt per call, matching
|
||||||
|
`fd9d4ae`. Retries remain exclusively at the `bt1035RetryTask` level
|
||||||
|
(whole clean `boot()` calls, never re-pulsing pins faster than one full
|
||||||
|
cycle apart — confirmed live, ~31.8s between attempts). Host tests
|
||||||
|
(20/20) and firmware build green; flashed and observed live. **Result
|
||||||
|
inconclusive on hit rate**: a 20-minute post-flash window captured 31
|
||||||
|
consecutive silent retry attempts, zero successes — worse than earlier
|
||||||
|
the same day. The patch is kept because it's structurally correct (only
|
||||||
|
known deviation from the historically validated design removed), not
|
||||||
|
because this sample proved a better success rate. Root cause of the
|
||||||
|
underlying intermittent silence is still open (see entry above).
|
||||||
- **Still open**: intermittent multi-second HTTP unresponsiveness under
|
- **Still open**: intermittent multi-second HTTP unresponsiveness under
|
||||||
load; DAB signal quality still antenna-limited; 24 KB `nvs` partition
|
load; DAB signal quality still antenna-limited; 24 KB `nvs` partition
|
||||||
may be undersized (`saveProfile()` `store_failed` seen intermittently,
|
may be undersized (`saveProfile()` `store_failed` seen intermittently,
|
||||||
|
|||||||
@@ -908,3 +908,93 @@ would be a significant main-MCU redesign, not a drop-in swap, and its
|
|||||||
ESP-IDF support maturity/availability wasn't independently verified this
|
ESP-IDF support maturity/availability wasn't independently verified this
|
||||||
session — worth a dedicated evaluation before committing to it for a
|
session — worth a dedicated evaluation before committing to it for a
|
||||||
future hardware revision.
|
future hardware revision.
|
||||||
|
|
||||||
|
## 2026-08-21 follow-up: git archaeology on the boot-retry structure;
|
||||||
|
## minimal patch to restore the validated single-attempt design
|
||||||
|
|
||||||
|
Separate follow-up session, requested specifically to re-derive the
|
||||||
|
BT1035 boot regression analysis directly from git history rather than
|
||||||
|
from further live hardware probing, per the project's own house rule
|
||||||
|
(2026-08-14 postmortem): exhaust the code-path diff against a known-good
|
||||||
|
commit before floating new hardware theories.
|
||||||
|
|
||||||
|
**Full commit archaeology** (`git log --follow` on
|
||||||
|
`Bt1035Driver.cpp`):
|
||||||
|
```
|
||||||
|
6ca40f1 "all companion chips ready" — baseline, 0 known bugs
|
||||||
|
6f7b6dd added a redundant AT+RESET right after the hardware reset pulse
|
||||||
|
fd9d4ae (2026-08-15) fixed 6f7b6dd in one commit: removed the redundant
|
||||||
|
AT+RESET AND introduced logRawUartBoot() for the first time,
|
||||||
|
already at its final 3500ms window (the "1500ms too short"
|
||||||
|
text in the report/commit message describes an intermediate
|
||||||
|
value tried live during that debugging session, never itself
|
||||||
|
committed) — 5/5 clean boots documented after this fix.
|
||||||
|
3a58d33 (2026-08-20, this project's own earlier commit today) widened
|
||||||
|
the banner wait 3500ms → 25000ms (real banner measured arriving
|
||||||
|
up to ~18.5-42s post-reset) AND, in the same commit, introduced
|
||||||
|
a NEW intra-boot() retry loop (kBootAttempts=2, only
|
||||||
|
kBootRetryDelayMs=300ms between the two hardware reset pulses)
|
||||||
|
that did not exist in fd9d4ae's validated design.
|
||||||
|
```
|
||||||
|
|
||||||
|
**Finding**: comparing `fd9d4ae` (the last commit with a documented,
|
||||||
|
validated 5/5 clean-boot run) against the working tree confirmed exactly
|
||||||
|
three differences, only one of them structural:
|
||||||
|
1. Banner wait 3500ms → 25000ms — justified by this session's own real
|
||||||
|
measurements, kept.
|
||||||
|
2. `GPIO_MODE_OUTPUT` → `GPIO_MODE_INPUT_OUTPUT` on RESET/SYS_CTRL —
|
||||||
|
purely additive (enables `gpio_get_level()` readback for the
|
||||||
|
pre-power/post-syscl/post-reset diagnostic logs), electrically
|
||||||
|
neutral, kept.
|
||||||
|
3. **A new intra-`boot()` retry loop with only 300ms between the two
|
||||||
|
hardware reset pulses — this did not exist in the validated baseline.**
|
||||||
|
The BT1035 datasheet's own "Reset Protection timeout (typically
|
||||||
|
>1.8s)" (already gathered earlier this session) means a second
|
||||||
|
SYS_CTRL/RESET pulse fired only 300ms after a failed attempt would not
|
||||||
|
reliably reach a clean power-off state — risking re-interrupting the
|
||||||
|
module mid bring-up, the same class of bug 6f7b6dd/fd9d4ae already
|
||||||
|
dealt with once (redundant AT+RESET). This is the only difference
|
||||||
|
flagged as a plausible contributor, not asserted as certain.
|
||||||
|
|
||||||
|
Also confirmed via repo-wide search: `AT+RESET` (`Bt1035AtCommand::Reset`)
|
||||||
|
is referenced only in the unit test, never in production code; no other
|
||||||
|
task/thread touches the BT1035 UART during its boot window
|
||||||
|
(`savedSpeakerReconnectTask` only starts after `HardwareBootstrap::boot()`
|
||||||
|
returns; the new `bt1035RetryTask` calls `boot()` sequentially, never
|
||||||
|
concurrently). `logRawUartBoot()`'s single `uart_read_bytes()` call and
|
||||||
|
the following `uart_flush_input()` were confirmed, both by code reading
|
||||||
|
and by this session's own successful-boot log capture (banner appeared,
|
||||||
|
then `AT`→`OK` immediately after, no stall), to not swallow or discard
|
||||||
|
data that `runInitSequence()` would otherwise need — `runInitSequence()`
|
||||||
|
does its own fresh TX/RX cycle regardless of what the banner-capture step
|
||||||
|
saw.
|
||||||
|
|
||||||
|
**Minimal patch applied** (user-directed, exact scope agreed before
|
||||||
|
touching code): removed the intra-`boot()` retry loop entirely —
|
||||||
|
`boot()` now makes exactly one `resetAndInitOnce()` call per invocation,
|
||||||
|
structurally identical to `fd9d4ae`. Removed `kBootAttempts` and
|
||||||
|
`kBootRetryDelayMs` (dead after the loop's removal); `probeBaudRates()`'s
|
||||||
|
log line adjusted accordingly (no longer references the removed
|
||||||
|
attempt count). `kBootBannerWaitMs=25000` and the `GPIO_MODE_INPUT_OUTPUT`
|
||||||
|
readback were explicitly left untouched. Retries now live exclusively one
|
||||||
|
layer up, in `hardware::bt1035RetryTask` (`main/hardware_bootstrap.cpp`,
|
||||||
|
added earlier this session), which only re-invokes a full, clean `boot()`
|
||||||
|
call — never re-pulses the pins faster than one whole boot cycle apart.
|
||||||
|
Host tests (20/20) and firmware build both green before flashing.
|
||||||
|
|
||||||
|
**Live result after flashing**: structurally the retry cadence is now
|
||||||
|
clean — confirmed via serial log, each `bt1035RetryTask` iteration is
|
||||||
|
spaced ~31.8s apart (25s banner wait + ~2s AT timeout + ~4s baud sweep,
|
||||||
|
no extra gap), matching the intended single-attempt-per-call design
|
||||||
|
exactly, versus the old back-to-back double-pulse. **However, a 20-minute
|
||||||
|
monitoring window immediately after flashing captured 31 consecutive
|
||||||
|
retry attempts, all silent — zero successes**, a worse hit rate in this
|
||||||
|
specific sample than earlier in the day (which had at least one clean
|
||||||
|
success among fewer attempts). This neither confirms nor refutes the
|
||||||
|
Reset-Protection-timing hypothesis on its own — the patch is kept because
|
||||||
|
it's structurally correct (matches the one historically validated design,
|
||||||
|
removes the only unexplained difference from it), not because this
|
||||||
|
sample proves it improved the success rate. The underlying intermittent
|
||||||
|
root cause (most likely the module's internal, sealed 32MHz crystal
|
||||||
|
startup margin — see the 2026-08-21 entry above) remains unresolved and
|
||||||
|
would need an oscilloscope to pin down further.
|
||||||
|
|||||||
Reference in New Issue
Block a user