From 3ffc9930cdaa84e578b8106c54ceafa055d8da19 Mon Sep 17 00:00:00 2001 From: Michele Bigi Date: Fri, 21 Aug 2026 17:11:36 +0200 Subject: [PATCH] Remove intra-boot() BT1035 retry loop; restore validated single-attempt design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git archaeology traced the boot sequence back to fd9d4ae (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, matching fd9d4ae. 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 --- .../drivers/bt1035/src/Bt1035Driver.cpp | 36 ++++---- Software/docs/TODO.md | 24 +++++ .../docs/si4684-rf-investigation-report.md | 90 +++++++++++++++++++ 3 files changed, 131 insertions(+), 19 deletions(-) diff --git a/Software/components/drivers/bt1035/src/Bt1035Driver.cpp b/Software/components/drivers/bt1035/src/Bt1035Driver.cpp index 1b5e7de..60d8ba7 100644 --- a/Software/components/drivers/bt1035/src/Bt1035Driver.cpp +++ b/Software/components/drivers/bt1035/src/Bt1035Driver.cpp @@ -51,10 +51,20 @@ constexpr int kSysCtlSettleMs = 50; * RESET# releases — full BT stack init, not just the internal regulator. * The previous 3500ms wait here was never enough for the module to say * 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 kBootAttempts = 2; -constexpr int kBootRetryDelayMs = 300; 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 // cycles (programming guide §5.1.3), so a stray manual AT+BAUD or // AT+RESTORE sent during earlier interactive testing could have left the @@ -77,8 +87,7 @@ constexpr std::array kBaudProbeCandidates = { void probeBaudRates(int uartPort) noexcept { - ESP_LOGW(kTag, "115200 unresponsive after %d attempts — sweeping baud rates", - kBootAttempts); + ESP_LOGW(kTag, "115200 unresponsive — sweeping baud rates"); for (const int baud : kBaudProbeCandidates) { if (uart_set_baudrate(static_cast(uartPort), baud) != ESP_OK) { @@ -1052,19 +1061,8 @@ std::expected Bt1035Driver::boot() uartInstalled_ = true; } - std::expected init = std::unexpected(Bt1035Error::UnexpectedResponse); - for (int attempt = 1; attempt <= kBootAttempts; ++attempt) { - 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); + if (auto init = resetAndInitOnce(); !init) { + ESP_LOGE(kTag, "AT init failed"); probeBaudRates(uartPort_); return init; } diff --git a/Software/docs/TODO.md b/Software/docs/TODO.md index 5c82b0b..092cb7f 100644 --- a/Software/docs/TODO.md +++ b/Software/docs/TODO.md @@ -134,6 +134,30 @@ Short version: 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 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 load; DAB signal quality still antenna-limited; 24 KB `nvs` partition may be undersized (`saveProfile()` `store_failed` seen intermittently, diff --git a/Software/docs/si4684-rf-investigation-report.md b/Software/docs/si4684-rf-investigation-report.md index 59bc7ef..8937890 100644 --- a/Software/docs/si4684-rf-investigation-report.md +++ b/Software/docs/si4684-rf-investigation-report.md @@ -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 session — worth a dedicated evaluation before committing to it for a 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.