Fix Si4684 DAB response-parsing bugs found in firmware review
readDabServiceData(): 6 of 7 response fields (dataSrc, serviceId, componentId, byteCount, segmentIndex, segmentCount) read one byte too early, using the old raw[4]=RESP4 convention instead of the correct raw[5]=RESP4 (established elsewhere in this driver by getPartInfo() and readDabDigRadStatus()'s own ficQuality/cnrDb fields). dataSrc landing on the wrong byte meant the DAB dynamic label (PAD/now-playing text) check (dataSrc == 2) could essentially never match — it has likely never worked. Header buffer grown 24->25 bytes to fit the correctly-positioned last field. fetchDabServiceList(): didn't match AN649 Table 14's "DAB/DMB Digital Service List" layout at all — serviceCount read from the wrong byte, every per-service field misaligned, componentId assumed 4 bytes wide (actually 2 per the spec), and only the first of a service's possibly-several components was ever skipped past (desyncing every later entry). Rewrote against the actual Table 14 field layout. Confirmed live yesterday this was producing garbled service_id/component_id/label output (component_id values decoding as literal ASCII spaces). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0178rASQ6ZETPMUamvpoR2KR
This commit is contained in:
@@ -962,7 +962,12 @@ Si4684Driver::readDabServiceData(bool statusOnly, bool ack)
|
|||||||
return std::unexpected(Si4684Error::CommandFailed);
|
return std::unexpected(Si4684Error::CommandFailed);
|
||||||
}
|
}
|
||||||
|
|
||||||
std::array<std::uint8_t, 24> header = {};
|
// raw[5]=RESP4 (see readFmRds()). AN649 Command 0x84 response:
|
||||||
|
// RESP4=flags, RESP5=BUFF_COUNT, RESP6=SRV_STATE, RESP7=DATA_SRC/DSCTy,
|
||||||
|
// RESP8-11=SERVICE_ID, RESP12-15=COMP_ID, RESP16-17=UATYPE,
|
||||||
|
// RESP18-19=BYTE_COUNT, RESP20-21=SEG_NUM, RESP22-23=NUM_SEGS — 25
|
||||||
|
// header bytes total (lead-in + STATUS0-3 + RESP4-23).
|
||||||
|
std::array<std::uint8_t, 25> header = {};
|
||||||
if (auto rd = readRaw(header); !rd) {
|
if (auto rd = readRaw(header); !rd) {
|
||||||
return std::unexpected(rd.error());
|
return std::unexpected(rd.error());
|
||||||
}
|
}
|
||||||
@@ -973,11 +978,11 @@ Si4684Driver::readDabServiceData(bool statusOnly, bool ack)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const std::uint16_t byteCount = readLe16(header.data() + 18);
|
const std::uint16_t byteCount = readLe16(header.data() + 19);
|
||||||
if (byteCount == 0U) {
|
if (byteCount == 0U) {
|
||||||
return std::optional<Si4684DabServiceData>{};
|
return std::optional<Si4684DabServiceData>{};
|
||||||
}
|
}
|
||||||
if (byteCount + 24U > kSpiBufferSize) {
|
if (byteCount + 25U > kSpiBufferSize) {
|
||||||
return std::unexpected(Si4684Error::ReplyTooShort);
|
return std::unexpected(Si4684Error::ReplyTooShort);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -989,12 +994,12 @@ Si4684Driver::readDabServiceData(bool statusOnly, bool ack)
|
|||||||
}
|
}
|
||||||
|
|
||||||
Si4684DabServiceData data = {};
|
Si4684DabServiceData data = {};
|
||||||
data.dataSrc = static_cast<std::uint8_t>((header[7] >> 6U) & 0x03U);
|
data.dataSrc = static_cast<std::uint8_t>((header[8] >> 6U) & 0x03U);
|
||||||
data.serviceId = readLe32(header.data() + 8);
|
data.serviceId = readLe32(header.data() + 9);
|
||||||
data.componentId = readLe32(header.data() + 12);
|
data.componentId = readLe32(header.data() + 13);
|
||||||
data.byteCount = byteCount;
|
data.byteCount = byteCount;
|
||||||
data.segmentIndex = readLe16(header.data() + 20);
|
data.segmentIndex = readLe16(header.data() + 21);
|
||||||
data.segmentCount = readLe16(header.data() + 22);
|
data.segmentCount = readLe16(header.data() + 23);
|
||||||
data.payload = std::move(body);
|
data.payload = std::move(body);
|
||||||
return data;
|
return data;
|
||||||
}
|
}
|
||||||
@@ -1115,42 +1120,50 @@ Si4684Driver::fetchDabServiceList()
|
|||||||
return std::unexpected(rd.error());
|
return std::unexpected(rd.error());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// raw[5]=RESP4=SIZE[7:0], raw[6]=RESP5=SIZE[15:8] (see readFmRds()).
|
||||||
const std::uint16_t payloadSize = readLe16(header.data() + 5);
|
const std::uint16_t payloadSize = readLe16(header.data() + 5);
|
||||||
if (payloadSize == 0U || payloadSize + 6U > kSpiBufferSize) {
|
if (payloadSize == 0U || payloadSize + 7U > kSpiBufferSize) {
|
||||||
return std::unexpected(Si4684Error::ReplyTooShort);
|
return std::unexpected(Si4684Error::ReplyTooShort);
|
||||||
}
|
}
|
||||||
|
|
||||||
std::vector<std::uint8_t> body(payloadSize + 6U, 0U);
|
// DATA_0 (first byte of the AN649 Table 14 "DAB/DMB Digital Service
|
||||||
|
// List" structure) is RESP6 = body[7]: lead-in(1) + STATUS0-3(4) +
|
||||||
|
// SIZE(2) = 7 header bytes before it.
|
||||||
|
std::vector<std::uint8_t> body(payloadSize + 7U, 0U);
|
||||||
if (auto rd = readRaw(body); !rd) {
|
if (auto rd = readRaw(body); !rd) {
|
||||||
return std::unexpected(rd.error());
|
return std::unexpected(rd.error());
|
||||||
}
|
}
|
||||||
|
|
||||||
const std::uint8_t serviceCount = body[9];
|
// Table 14: List Size(2) + Version(2) + NumServices(1) + AlignPad(3) =
|
||||||
|
// 8 bytes, then Service 1 begins.
|
||||||
|
const std::uint8_t serviceCount = body[11];
|
||||||
std::vector<Si4684DabService> services;
|
std::vector<Si4684DabService> services;
|
||||||
services.reserve(serviceCount);
|
services.reserve(serviceCount);
|
||||||
|
|
||||||
std::size_t offset = 13U;
|
std::size_t offset = 15U;
|
||||||
for (std::uint8_t i = 0; i < serviceCount; ++i) {
|
for (std::uint8_t i = 0; i < serviceCount; ++i) {
|
||||||
|
// Fixed per-service part: ServiceID(4) + ServiceInfo1-3(3) +
|
||||||
|
// AlignPad(1) + Label(16) = 24 bytes.
|
||||||
if (offset + 24U > body.size()) {
|
if (offset + 24U > body.size()) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
Si4684DabService entry = {};
|
Si4684DabService entry = {};
|
||||||
entry.serviceId = readLe32(body.data() + offset);
|
entry.serviceId = readLe32(body.data() + offset);
|
||||||
offset += 4U;
|
entry.serviceType = body[offset + 4U];
|
||||||
entry.serviceType = body[offset] & 0x3FU;
|
const std::uint8_t componentCount = body[offset + 5U] & 0x0FU;
|
||||||
const std::uint8_t componentCount = body[offset + 1] & 0x0FU;
|
std::memcpy(entry.label.data(), body.data() + offset + 8U, 16U);
|
||||||
offset += 4U;
|
|
||||||
std::memcpy(entry.label.data(), body.data() + offset, 16U);
|
|
||||||
entry.label[16] = '\0';
|
entry.label[16] = '\0';
|
||||||
offset += 16U;
|
offset += 24U;
|
||||||
|
|
||||||
if (componentCount > 0U && offset + 4U <= body.size()) {
|
// Component ID is 2 bytes (AN649 Table 14); only the first
|
||||||
entry.componentId = readLe32(body.data() + offset);
|
// component's ID is exposed on this DTO. Every component (M =
|
||||||
offset += 4U;
|
// componentCount) must still be skipped to keep the next service
|
||||||
if (offset < body.size()) {
|
// entry aligned, each one ComponentID(2) + ComponentInfo(1) +
|
||||||
++offset;
|
// ValidFlags(1) = 4 bytes.
|
||||||
}
|
if (componentCount > 0U && offset + 2U <= body.size()) {
|
||||||
|
entry.componentId = readLe16(body.data() + offset);
|
||||||
}
|
}
|
||||||
|
offset += static_cast<std::size_t>(componentCount) * 4U;
|
||||||
services.push_back(entry);
|
services.push_back(entry);
|
||||||
}
|
}
|
||||||
return services;
|
return services;
|
||||||
|
|||||||
@@ -615,3 +615,20 @@ yet identified; unrelated to the Si4684 investigation (separate chip), but
|
|||||||
treat BT1035 boot failure as non-fatal rather than halting the whole device,
|
treat BT1035 boot failure as non-fatal rather than halting the whole device,
|
||||||
so the rest of the system (Si4684 tuning, web UI, Wi-Fi) remains usable
|
so the rest of the system (Si4684 tuning, web UI, Wi-Fi) remains usable
|
||||||
while this is investigated separately.
|
while this is investigated separately.
|
||||||
|
|
||||||
|
## TODO (next session)
|
||||||
|
|
||||||
|
- **Antenna/front-end calibration, now meaningful.** Before this session's
|
||||||
|
fixes, any ANTCAP sweep or front-end network experiment was untrustworthy
|
||||||
|
— a bad result could have been the software bug, not the antenna. Now
|
||||||
|
that the receiver chain is verified correct end to end (real FM lock,
|
||||||
|
real DAB lock, real audio), redo the ANTCAP sweep and compare the actual
|
||||||
|
front-end network (§ "Front-end network component mismatch" above)
|
||||||
|
against AN851 properly, with results that can actually be trusted.
|
||||||
|
- Fix `fetchDabServiceList()` entry parsing (garbled service_id/component_id/
|
||||||
|
label) against AN649 §7 "Digital Services User's Guide" (~page 418).
|
||||||
|
- Confirm actual DAB audio playback end to end (blocked on the item above).
|
||||||
|
- Try a proper FM antenna to see if the residual noise under the music
|
||||||
|
clears up (suspected antenna quality, not yet confirmed).
|
||||||
|
- BT1035 boot-failure root cause still open (see section above) — non-fatal
|
||||||
|
now, so it's no longer blocking, but still unexplained.
|
||||||
|
|||||||
Reference in New Issue
Block a user