diff --git a/Software/components/core/include/core/RdsMetadataAccumulator.hpp b/Software/components/core/include/core/RdsMetadataAccumulator.hpp index 3126817..b78e955 100644 --- a/Software/components/core/include/core/RdsMetadataAccumulator.hpp +++ b/Software/components/core/include/core/RdsMetadataAccumulator.hpp @@ -47,10 +47,12 @@ public: * @brief applyGroup — ingest one RDS group (blocks A–D). * * @dname applyGroup - * @param blockA RDS block A (PI code). - * @param blockB RDS block B (group type and address). - * @param blockC RDS block C (text payload). - * @param blockD RDS block D (text payload for RT). + * @param blockA RDS block A (PI code, currently unused). + * @param blockB RDS block B (group type and segment address). + * @param blockC RDS block C (RT text for group 2 only; group 0's + * AF list/repeated PI is not text and is ignored). + * @param blockD RDS block D (PS text for group 0; RT text for + * group 2, per ETSI EN 62106 §3.1.5/§3.1.5.3). * @pubstate updates PS/RT buffers for group types 0A and 2A. * * @author Michele Bigi diff --git a/Software/components/core/src/RdsMetadataAccumulator.cpp b/Software/components/core/src/RdsMetadataAccumulator.cpp index 8ef84b2..9df17eb 100644 --- a/Software/components/core/src/RdsMetadataAccumulator.cpp +++ b/Software/components/core/src/RdsMetadataAccumulator.cpp @@ -35,12 +35,15 @@ void RdsMetadataAccumulator::applyGroup(std::uint16_t blockA, static_cast((blockB >> 12U) & 0x0FU); if (groupType == 0U) { - const std::size_t index = - static_cast((blockB >> 1U) & 0x03U); + // Group type 0 (both 0A and 0B): the two Program Service name + // characters are always in Block D. Block C differs by version -- + // 0A carries alternate-frequency codes, 0B repeats the PI code -- + // but never PS text either way (ETSI EN 62106 §3.1.5). + const std::size_t index = static_cast(blockB & 0x03U); if (index < kPsSegments) { psBuffer_[index * 2U] = - static_cast((blockC >> 8U) & 0xFFU); - psBuffer_[index * 2U + 1U] = static_cast(blockC & 0xFFU); + static_cast((blockD >> 8U) & 0xFFU); + psBuffer_[index * 2U + 1U] = static_cast(blockD & 0xFFU); psSegmentValid_[index] = true; } return; diff --git a/Software/components/core/test/CMakeLists.txt b/Software/components/core/test/CMakeLists.txt index b9dbbc6..785f35f 100644 --- a/Software/components/core/test/CMakeLists.txt +++ b/Software/components/core/test/CMakeLists.txt @@ -129,6 +129,10 @@ add_executable(bt1035_at_test bt1035_at_test.cpp) target_link_libraries(bt1035_at_test PRIVATE digiradio_core) add_test(NAME bt1035_at_test COMMAND bt1035_at_test) +add_executable(rds_metadata_accumulator_test rds_metadata_accumulator_test.cpp) +target_link_libraries(rds_metadata_accumulator_test PRIVATE digiradio_core) +add_test(NAME rds_metadata_accumulator_test COMMAND rds_metadata_accumulator_test) + add_executable(broadcast_metadata_test broadcast_metadata_test.cpp) target_link_libraries(broadcast_metadata_test PRIVATE digiradio_core) add_test(NAME broadcast_metadata_test COMMAND broadcast_metadata_test) diff --git a/Software/components/core/test/broadcast_metadata_test.cpp b/Software/components/core/test/broadcast_metadata_test.cpp index fca2b6e..6be4c9d 100644 --- a/Software/components/core/test/broadcast_metadata_test.cpp +++ b/Software/components/core/test/broadcast_metadata_test.cpp @@ -34,11 +34,13 @@ namespace { [[nodiscard]] int runRdsProgramNameTest() { + // Group type 0, segment address in blockB bits[1:0] (no shift); PS + // characters are in blockD, never blockC (ETSI EN 62106 §3.1.5). core::RdsMetadataAccumulator acc; - acc.applyGroup(0U, 0x0000U, 0x5445U, 0U); - acc.applyGroup(0U, 0x0002U, 0x5354U, 0U); - acc.applyGroup(0U, 0x0004U, 0x2020U, 0U); - acc.applyGroup(0U, 0x0006U, 0x2020U, 0U); + acc.applyGroup(0U, 0x0000U, 0U, 0x5445U); + acc.applyGroup(0U, 0x0001U, 0U, 0x5354U); + acc.applyGroup(0U, 0x0002U, 0U, 0x2020U); + acc.applyGroup(0U, 0x0003U, 0U, 0x2020U); const auto name = acc.programName(); if (!name || name->value() != "TEST") { diff --git a/Software/components/core/test/rds_metadata_accumulator_test.cpp b/Software/components/core/test/rds_metadata_accumulator_test.cpp new file mode 100644 index 0000000..6542c00 --- /dev/null +++ b/Software/components/core/test/rds_metadata_accumulator_test.cpp @@ -0,0 +1,126 @@ +/** + * @file rds_metadata_accumulator_test.cpp + * @brief Host tests for RdsMetadataAccumulator against ETSI EN 62106 + * group 0B/2A block layouts. + * + * DigiRadio firmware — https://github.com/manvalan/DigiRadio + * + * Copyright 2026 Michele Bigi + * SPDX-License-Identifier: Apache-2.0 + * + * @author Michele Bigi + * @date 2026-08-25 + */ + +#include "core/RdsMetadataAccumulator.hpp" + +#include +#include + +namespace { + +/* + * Group type 0B (block B bit 11 set), segment address in bits[1:0]: the two + * Program Service characters for that segment live in Block D (high byte + * first), never Block C -- Block C for group 0 carries alternate-frequency + * codes (0A) or a repeated PI code (0B), not text. This is the exact bug + * fixed 2026-08-25 (characters were being read from Block C, which made + * every FM station name silently fail to accumulate). + */ +[[nodiscard]] int runProgramServiceNameTest() +{ + core::RdsMetadataAccumulator acc; + + // "RADIO101" across the 4 PS segments, group type 0B. + struct + { + std::uint16_t segment; + char c0; + char c1; + } segments[] = { + {0U, 'R', 'A'}, + {1U, 'D', 'I'}, + {2U, 'O', '1'}, + {3U, '0', '1'}, + }; + + for (const auto &seg : segments) { + const std::uint16_t blockB = + static_cast(0x0800U | seg.segment); + const std::uint16_t blockD = static_cast( + (static_cast(seg.c0) << 8) | + static_cast(seg.c1)); + // Block C deliberately holds garbage (a repeated-PI-like value that + // is NOT the expected text) to prove the decoder ignores it for + // group 0, rather than happening to read the right bytes by luck. + acc.applyGroup(0x1234U, blockB, 0xBEEFU, blockD); + } + + const auto name = acc.programName(); + if (!name) { + std::cerr << "expected a program name after 4 PS segments\n"; + return EXIT_FAILURE; + } + if (name->value() != "RADIO101") { + std::cerr << "program name mismatch: got '" << name->value() + << "'\n"; + return EXIT_FAILURE; + } + return EXIT_SUCCESS; +} + +[[nodiscard]] int runIncompleteSegmentsTest() +{ + core::RdsMetadataAccumulator acc; + acc.applyGroup(0x1234U, 0x0800U, 0xBEEFU, 0x5241U); // segment 0 only + if (acc.programName()) { + std::cerr << "program name should be absent with 3 segments " + "missing\n"; + return EXIT_FAILURE; + } + return EXIT_SUCCESS; +} + +/* + * Group type 2A: RadioText characters split across Block C (2 chars) and + * Block D (2 chars) for the same segment -- unlike group 0, Block C really + * does carry text here, so this is a regression guard against ever + * "fixing" group 2 the same way group 0 needed fixing. + */ +[[nodiscard]] int runRadiotextTest() +{ + core::RdsMetadataAccumulator acc; + const std::uint16_t blockB = 0x2000U; // group type 2, version A, seg 0 + const std::uint16_t blockC = + (static_cast('T') << 8) | static_cast('e'); + const std::uint16_t blockD = + (static_cast('s') << 8) | static_cast('t'); + acc.applyGroup(0x1234U, blockB, blockC, blockD); + + const auto rt = acc.radiotext(); + if (!rt) { + std::cerr << "expected radiotext after one 2A segment\n"; + return EXIT_FAILURE; + } + if (rt->value() != "Test") { + std::cerr << "radiotext mismatch: got '" << rt->value() << "'\n"; + return EXIT_FAILURE; + } + return EXIT_SUCCESS; +} + +} // namespace + +int main() +{ + if (runProgramServiceNameTest() != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (runIncompleteSegmentsTest() != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (runRadiotextTest() != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + return EXIT_SUCCESS; +} diff --git a/Software/components/drivers/si4684/src/Si4684Driver.cpp b/Software/components/drivers/si4684/src/Si4684Driver.cpp index 5934402..4ff7390 100644 --- a/Software/components/drivers/si4684/src/Si4684Driver.cpp +++ b/Software/components/drivers/si4684/src/Si4684Driver.cpp @@ -66,6 +66,11 @@ constexpr std::uint16_t kSi4684I2sOutEnable = 0x8002U; /** Si4684 volume: 0=mute, 63=max (AN649 AUDIO_ANALOG_VOLUME). */ constexpr std::uint8_t kSi4684VolumeMax = 63U; constexpr std::uint16_t kPropFmRdsConfig = 0x3C02U; +/** AN649 §0x3C01 FM_RDS_INTERRUPT_FIFO_COUNT: DEPTH[7:0], groups needed + * before RDSFIFOINT (FM_RDS_STATUS RESP4 bit0, our readFmRds() "received" + * bit) ever sets. Default 0 disables it permanently regardless of + * FM_RDS_CONFIG -- must be nonzero for any RDS group to ever be seen. */ +constexpr std::uint16_t kPropFmRdsInterruptFifoCount = 0x3C01U; /** AN649 FM_AUDIO_DE_EMPHASIS (0x3900): 0=75us/US (chip default), 1=50us/ * Europe, 2=disabled. FM seek band/spacing above is already the European * 87.5-107.9 MHz/100 kHz plan, so the chip must not stay on its 75us/US @@ -557,9 +562,22 @@ std::expected Si4684Driver::configureAfterBoot( return set; } } - if (auto rds = setProperty(kPropFmRdsConfig, 0x0001U); !rds) { + // AN649 §0x3C02 FM_RDS_CONFIG: BLETHB[7:6]/BLETHCD[5:4] block-error + // thresholds, RDSEN[0]. 0x0001 (thresholds at 0, "no block errors") + // rejected almost every real-world group -- any bit error at all + // (routine with multipath/noise, even on a strong signal) dropped + // the group from the FIFO, so accumulated station names/RadioText + // never completed. 0x00A1 keeps both thresholds at the datasheet's + // most tolerant recommended setting (2 = "3-5 bit errors detected + // and corrected"), still discarding uncorrectable (3) groups. + if (auto rds = setProperty(kPropFmRdsConfig, 0x00A1U); !rds) { return rds; } + if (auto rdsFifo = + setProperty(kPropFmRdsInterruptFifoCount, 0x0001U); + !rdsFifo) { + return rdsFifo; + } if (auto deEmph = setProperty(kPropFmAudioDeEmphasis, kFmAudioDeEmphasisEurope); !deEmph) {