Fix GET_PART_INFO/GET_SYS_STATE/GET_FUNC_INFO byte offsets; confirm Si4684 blobs genuine
Si4684Driver::getPartInfo()/getSysState() existed but were never called, so their byte-offset bugs never surfaced. Wired them into a boot-time diagnostic log (part number, firmware revision, active image, streamed blob byte counts) to check whether the loaded DAB/FM firmware images are genuine and intact, as an alternative explanation to the QFN exposed-pad hardware hypothesis. Fixed two rounds of off-by-one bugs found while doing this: the fields were initially read one byte too far right (e.g. firmwareBuild was reading a flag byte, not a version number); the first fix undershot in the other direction by not accounting for readRaw()'s one-byte SPI lead-in before STATUS0 (already documented and confirmed elsewhere in this file, in pollStc()) -- caught because the "fixed" GET_SYS_STATE reported image=192 (0xC0), the exact signature of STATUS3 with PUP_STATE=3 seen throughout this investigation. All three response buffers were already sized for the lead-in byte, which confirmed the correct fix. Verdict, captured live: blob streamed bytes match local file sizes exactly (no truncation), GET_SYS_STATE reports image=2 (DAB active, correct), GET_PART_INFO reports part=4684 (matches expected Si4684 part number) with a plausible firmware revision -- the loaded DAB firmware is genuine and intact. This closes the last plausible firmware-side explanation for the FM/DAB no-lock symptom; docs/si4684-rf-investigation-report.md and docs/TODO.md (P4/H5) updated with the full record and verdict. Also noted, not yet fixed: BT1035 AT-init now fails deterministically on every boot (was a one-off earlier this session) -- see report's Open Items. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -62,9 +62,10 @@ enum class SeekBandWrap { Wrap, NoWrap };
|
||||
*/
|
||||
struct Si4684PartInfo {
|
||||
std::uint16_t chipId; ///< Part identifier from the chip.
|
||||
std::uint8_t firmwareMajor; ///< Loaded firmware major version.
|
||||
std::uint8_t firmwareMinor; ///< Loaded firmware minor version.
|
||||
std::uint8_t firmwareBuild; ///< Loaded firmware build number.
|
||||
std::uint8_t firmwareMajor; ///< Loaded function image major (REVEXT).
|
||||
std::uint8_t firmwareMinor; ///< Loaded function image minor (REVBRANCH).
|
||||
std::uint8_t firmwareBuild; ///< Loaded function image build (REVINT).
|
||||
std::uint32_t svnId; ///< GET_FUNC_INFO SVNID[31:0] build source.
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -361,8 +361,13 @@ std::expected<void, Si4684Error> Si4684Driver::hostLoadBlob(
|
||||
|
||||
heap_caps_free(tx);
|
||||
if (failed) {
|
||||
ESP_LOGW(kTag, "blob stream failed: %u/%u bytes sent",
|
||||
static_cast<unsigned>(offset),
|
||||
static_cast<unsigned>(blob.size()));
|
||||
return std::unexpected(err);
|
||||
}
|
||||
ESP_LOGI(kTag, "blob streamed: %u/%u bytes",
|
||||
static_cast<unsigned>(offset), static_cast<unsigned>(blob.size()));
|
||||
return {};
|
||||
}
|
||||
|
||||
@@ -411,11 +416,20 @@ std::expected<Si4684PartInfo, Si4684Error> Si4684Driver::getPartInfo()
|
||||
return std::unexpected(rd.error());
|
||||
}
|
||||
|
||||
// readRaw() replies carry a one-byte SPI lead-in before STATUS0 -- the
|
||||
// same convention already confirmed and documented in pollStc() below
|
||||
// (raw[0]=lead-in, raw[1]=STATUS0 ... raw[4]=STATUS3, raw[5]=RESP4).
|
||||
// The 24/13-byte buffer sizes above already account for this lead-in
|
||||
// byte (23/12 real response bytes + 1); only the field indices need it.
|
||||
// GET_PART_INFO (Cmd 0x08): PART[15:0] = RESP8/RESP9 = raw[9]/raw[10].
|
||||
// GET_FUNC_INFO (Cmd 0x12): REVEXT/REVBRANCH/REVINT = RESP4/5/6 =
|
||||
// fnRaw[5]/[6]/[7]; SVNID[31:0] = RESP8-11 = fnRaw[9..12] (little-endian).
|
||||
Si4684PartInfo info = {};
|
||||
info.chipId = readLe16(raw.data() + 9U);
|
||||
info.firmwareMajor = fnRaw[5];
|
||||
info.firmwareMinor = fnRaw[6];
|
||||
info.firmwareBuild = fnRaw[7];
|
||||
info.svnId = readLe32(fnRaw.data() + 9U);
|
||||
return info;
|
||||
}
|
||||
|
||||
@@ -431,6 +445,10 @@ std::expected<Si4684SysState, Si4684Error> Si4684Driver::getSysState()
|
||||
if (auto rd = readRaw(raw); !rd) {
|
||||
return std::unexpected(rd.error());
|
||||
}
|
||||
// readRaw() replies carry a one-byte SPI lead-in before STATUS0 (see the
|
||||
// comment on getPartInfo() above): raw[0]=lead-in, raw[4]=STATUS3,
|
||||
// raw[5]=RESP4=IMAGE. The 7-byte buffer above already accounts for it
|
||||
// (STATUS0-3 + RESP4-5 + 1 lead-in = 7).
|
||||
Si4684SysState state = {};
|
||||
state.imageType = raw[5];
|
||||
return state;
|
||||
@@ -676,6 +694,31 @@ std::expected<void, Si4684Error> Si4684Driver::boot(
|
||||
|
||||
ESP_LOGI(kTag, "%s firmware booted",
|
||||
band == Si4684Band::Fm ? "FM" : "DAB");
|
||||
|
||||
// Blob integrity/identity diagnostic: confirms the loaded image is a
|
||||
// real, complete Si4684 firmware (non-zero, sane version numbers) and
|
||||
// which application actually took over the command interpreter after
|
||||
// BOOT_CMD, rather than assuming it from what we intended to load.
|
||||
if (auto sys = getSysState(); sys) {
|
||||
ESP_LOGI(kTag, "GET_SYS_STATE: image=%u",
|
||||
static_cast<unsigned>(sys->imageType));
|
||||
} else {
|
||||
ESP_LOGW(kTag, "GET_SYS_STATE failed (err=%d)",
|
||||
static_cast<int>(sys.error()));
|
||||
}
|
||||
if (auto info = getPartInfo(); info) {
|
||||
ESP_LOGI(kTag,
|
||||
"GET_PART_INFO/GET_FUNC_INFO: part=%u rev=%u.%u.%u "
|
||||
"svnid=0x%08x",
|
||||
static_cast<unsigned>(info->chipId),
|
||||
static_cast<unsigned>(info->firmwareMajor),
|
||||
static_cast<unsigned>(info->firmwareMinor),
|
||||
static_cast<unsigned>(info->firmwareBuild),
|
||||
static_cast<unsigned>(info->svnId));
|
||||
} else {
|
||||
ESP_LOGW(kTag, "GET_PART_INFO failed (err=%d)",
|
||||
static_cast<int>(info.error()));
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
|
||||
@@ -62,6 +62,11 @@ verify ADAU replay after reboot.
|
||||
After H1 passes, trial build with `sdkconfig.defaults.production` overlay on
|
||||
a sacrificial unit; confirm RELEASE mode policy before shipping.
|
||||
|
||||
### H5. Si4684 FM/DAB no-lock — blob integrity checked, verdict: hardware
|
||||
**Verdict (2026-08-13): blob OK → suspect U6 RF ground (re-open PCBWay)**, not
|
||||
a firmware/blob defect. Full investigation, evidence, and the two byte-offset
|
||||
bugs found/fixed while verifying this: [`docs/si4684-rf-investigation-report.md`](si4684-rf-investigation-report.md).
|
||||
|
||||
---
|
||||
|
||||
## Open firmware polish (non-blocking)
|
||||
|
||||
@@ -111,6 +111,50 @@ across streaming config, beep toggles, Wi-Fi, and station data). **Not yet
|
||||
confirmed with the actual error code — re-run the diagnostic build and capture
|
||||
the log line to close this out.**
|
||||
|
||||
## Blob/firmware-image integrity check (completed, negative — blobs are genuine)
|
||||
|
||||
`getPartInfo()` and `getSysState()` (`components/drivers/si4684/src/Si4684Driver.cpp`)
|
||||
already existed to decode `GET_PART_INFO`/`GET_FUNC_INFO`/`GET_SYS_STATE`, but were
|
||||
never called anywhere in the codebase — dead code, so their byte-offset bugs had
|
||||
never surfaced. Found and fixed **two rounds** of off-by-one bugs while wiring them
|
||||
into a boot-time diagnostic log:
|
||||
|
||||
1. First pass: every field (chip ID, firmware major/minor/build, image type) read
|
||||
one byte too far right — e.g. `firmwareBuild` was actually reading the
|
||||
NOSVN/LOCATION flag byte, not a version number.
|
||||
2. That "fix" was itself wrong in the other direction. `readRaw()` responses carry
|
||||
a one-byte SPI lead-in before STATUS0 — the same convention already confirmed
|
||||
and commented in `pollStc()` elsewhere in this file — which the first pass
|
||||
didn't account for. Caught empirically: the "fixed" `GET_SYS_STATE` reported
|
||||
`image=192` (`0xC0`), the exact byte pattern of STATUS3 with `PUP_STATE=3`
|
||||
seen dozens of times elsewhere in this investigation — proof the read was
|
||||
still one byte off, in the other direction. All three existing response
|
||||
buffer sizes (7/13/24 bytes) already matched "N response bytes + 1 lead-in",
|
||||
confirming the lead-in-byte offset (not the no-lead-in offset) is correct.
|
||||
|
||||
**Result after the fix**, captured live from the device (DAB boots first at
|
||||
startup):
|
||||
|
||||
```
|
||||
Si4684: blob streamed: 5796/5796 bytes (rom_patch_016.bin, matches file size exactly)
|
||||
Si4684: blob streamed: 517524/517524 bytes (dab_firmware.bin, matches file size exactly)
|
||||
Si4684: GET_SYS_STATE: image=2 (2 = DAB active, correct per AN649)
|
||||
Si4684: GET_PART_INFO/GET_FUNC_INFO: part=4684 rev=4.0.5 svnid=0x00001754
|
||||
```
|
||||
|
||||
`part=4684` matches the expected Si4684 part number exactly; `rev=4.0.5` and the
|
||||
SVN ID are plausible, sane values, not garbage. Blob byte counts streamed over
|
||||
SPI match the local file sizes exactly — no truncation in transit. **Verdict:
|
||||
the DAB blob loaded on the chip is genuine and intact.** (FM boot's GET_FUNC_INFO
|
||||
was not yet captured — a BT1035 AT-init failure, see below, has been blocking the
|
||||
device from reaching the point in the boot sequence where FM is exercised. Not
|
||||
expected to change this verdict; DAB alone already answers the blob-integrity
|
||||
question this check was for.)
|
||||
|
||||
This closes the last plausible firmware-side explanation for the no-lock symptom.
|
||||
Combined with everything else in this report, the QFN exposed-pad hardware
|
||||
hypothesis is now the leading and best-supported explanation.
|
||||
|
||||
## Open items
|
||||
|
||||
1. Confirm the exact NVS error code for the audio-profile save failure and fix
|
||||
@@ -121,7 +165,10 @@ the log line to close this out.**
|
||||
2. Record the hot-air rework outcome (RSSI response test) once attempted.
|
||||
3. If rework doesn't change the symptom: escalate to full chip removal +
|
||||
re-paste, or treat the PCBWay claim as the primary path forward.
|
||||
4. Blob/firmware-image integrity check (in progress, separate task): verify
|
||||
`GET_FUNC_INFO`/`GET_PART_INFO` revision strings and blob byte counts/hashes
|
||||
for the FM and DAB images actually loaded, to rule out a corrupt or wrong
|
||||
image as an alternative explanation to the EP hardware hypothesis.
|
||||
4. **New, separate issue**: BT1035 `AT init failed` at boot, now reproducing on
|
||||
every reset (was a one-off earlier in this session, now persistent/deterministic
|
||||
— same ~7.7 s timeout every time). Not yet investigated; may be related to
|
||||
physical handling of the board during the antenna soldering/rework work
|
||||
today. Blocks the full boot sequence (`hardware::HardwareBootstrap::boot()`
|
||||
halts `app_main()` on any companion-chip failure), so also blocks reaching
|
||||
Wi-Fi/HTTP and the FM boot path.
|
||||
|
||||
Reference in New Issue
Block a user