diff --git a/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl b/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl index 0484d02a9e..646c502660 100644 --- a/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl +++ b/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl @@ -110,10 +110,11 @@ enum IdentifierType { HD_STATION_NAME, /** - * 28bit compound primary identifier for Digital Audio Broadcasting. + * 44bit compound primary identifier for Digital Audio Broadcasting and + * Digital Multimeida Broadcasting. * * Consists of (from the LSB): - * - 16bit: SId; + * - 32bit: SId; * - 8bit: ECC code; * - 4bit: SCIdS. * diff --git a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl index 3e2c9cca7d..d6502394ec 100644 --- a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl +++ b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl @@ -120,7 +120,7 @@ parcelable ProgramInfo { * * Only physical identifiers are valid: * - AMFM_FREQUENCY_KHZ; - * - DAB_ENSEMBLE; + * - DAB_FREQUENCY_KHZ; * - DRMO_FREQUENCY_KHZ; * - SXM_CHANNEL; * - VENDOR_*; diff --git a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl index 8bd3fd41e4..93b0e12674 100644 --- a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl +++ b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl @@ -51,7 +51,8 @@ parcelable ProgramSelector { * - analogue AM/FM: AMFM_FREQUENCY_KHZ; * - FM RDS: RDS_PI; * - HD Radio: HD_STATION_ID_EXT; - * - DAB: DAB_SID_EXT; + * - DAB/DMB: DAB_SID_EXT (when used, DAB_ENSEMBLE and DAB_FREQUENCY_KHZ + * must present in secondaryIds); * - Digital Radio Mondiale: DRMO_SERVICE_ID; * - SiriusXM: SXM_SERVICE_ID; * - vendor-specific: VENDOR_START..VENDOR_END. diff --git a/broadcastradio/aidl/default/BroadcastRadio.cpp b/broadcastradio/aidl/default/BroadcastRadio.cpp index 42e550e75a..36520fb387 100644 --- a/broadcastradio/aidl/default/BroadcastRadio.cpp +++ b/broadcastradio/aidl/default/BroadcastRadio.cpp @@ -589,22 +589,28 @@ binder_status_t BroadcastRadio::cmdTune(int fd, const char** args, uint32_t numA } ProgramSelector sel = {}; if (isDab) { - if (numArgs != 4) { + if (numArgs != 5) { dprintf(fd, - "Invalid number of arguments: please provide --tune dab \n"); + "Invalid number of arguments: please provide " + "--tune dab \n"); return STATUS_BAD_VALUE; } int sid; if (!utils::parseArgInt(string(args[2]), &sid)) { - dprintf(fd, "Non-integer sid provided with tune: %s\n", string(args[2]).c_str()); + dprintf(fd, "Non-integer sid provided with tune: %s\n", args[2]); return STATUS_BAD_VALUE; } int ensemble; if (!utils::parseArgInt(string(args[3]), &ensemble)) { - dprintf(fd, "Non-integer ensemble provided with tune: %s\n", string(args[3]).c_str()); + dprintf(fd, "Non-integer ensemble provided with tune: %s\n", args[3]); return STATUS_BAD_VALUE; } - sel = utils::makeSelectorDab(sid, ensemble); + int freq; + if (!utils::parseArgInt(string(args[4]), &freq)) { + dprintf(fd, "Non-integer frequency provided with tune: %s\n", args[4]); + return STATUS_BAD_VALUE; + } + sel = utils::makeSelectorDab(sid, ensemble, freq); } else { if (numArgs != 3) { dprintf(fd, "Invalid number of arguments: please provide --tune amfm \n"); @@ -612,7 +618,7 @@ binder_status_t BroadcastRadio::cmdTune(int fd, const char** args, uint32_t numA } int freq; if (!utils::parseArgInt(string(args[2]), &freq)) { - dprintf(fd, "Non-integer frequency provided with tune: %s\n", string(args[2]).c_str()); + dprintf(fd, "Non-integer frequency provided with tune: %s\n", args[2]); return STATUS_BAD_VALUE; } sel = utils::makeSelectorAmfm(freq); diff --git a/broadcastradio/aidl/default/VirtualProgram.cpp b/broadcastradio/aidl/default/VirtualProgram.cpp index 0df0a82fa4..4fe6567d17 100644 --- a/broadcastradio/aidl/default/VirtualProgram.cpp +++ b/broadcastradio/aidl/default/VirtualProgram.cpp @@ -53,7 +53,7 @@ VirtualProgram::operator ProgramInfo() const { break; case IdentifierType::DAB_SID_EXT: info.logicallyTunedTo = selectId(IdentifierType::DAB_SID_EXT); - info.physicallyTunedTo = selectId(IdentifierType::DAB_ENSEMBLE); + info.physicallyTunedTo = selectId(IdentifierType::DAB_FREQUENCY_KHZ); break; case IdentifierType::DRMO_SERVICE_ID: info.logicallyTunedTo = selectId(IdentifierType::DRMO_SERVICE_ID); diff --git a/broadcastradio/aidl/default/VirtualRadio.cpp b/broadcastradio/aidl/default/VirtualRadio.cpp index 851543b0aa..126bcff621 100644 --- a/broadcastradio/aidl/default/VirtualRadio.cpp +++ b/broadcastradio/aidl/default/VirtualRadio.cpp @@ -53,15 +53,18 @@ const VirtualRadio& VirtualRadio::getAmFmRadio() { static VirtualRadio amFmRadioMock( "AM/FM radio mock", { - {makeSelectorAmfm(94900), "Wild 94.9", "Drake ft. Rihanna", "Too Good"}, - {makeSelectorAmfm(96500), "KOIT", "Celine Dion", "All By Myself"}, - {makeSelectorAmfm(97300), "Alice@97.3", "Drops of Jupiter", "Train"}, - {makeSelectorAmfm(99700), "99.7 Now!", "The Chainsmokers", "Closer"}, - {makeSelectorAmfm(101300), "101-3 KISS-FM", "Justin Timberlake", "Rock Your Body"}, - {makeSelectorAmfm(103700), "iHeart80s @ 103.7", "Michael Jackson", "Billie Jean"}, - {makeSelectorAmfm(106100), "106 KMEL", "Drake", "Marvins Room"}, - {makeSelectorAmfm(700), "700 AM", "Artist700", "Title700"}, - {makeSelectorAmfm(1700), "1700 AM", "Artist1700", "Title1700"}, + {makeSelectorAmfm(/* frequency= */ 94900), "Wild 94.9", "Drake ft. Rihanna", + "Too Good"}, + {makeSelectorAmfm(/* frequency= */ 96500), "KOIT", "Celine Dion", "All By Myself"}, + {makeSelectorAmfm(/* frequency= */ 97300), "Alice@97.3", "Drops of Jupiter", "Train"}, + {makeSelectorAmfm(/* frequency= */ 99700), "99.7 Now!", "The Chainsmokers", "Closer"}, + {makeSelectorAmfm(/* frequency= */ 101300), "101-3 KISS-FM", "Justin Timberlake", + "Rock Your Body"}, + {makeSelectorAmfm(/* frequency= */ 103700), "iHeart80s @ 103.7", "Michael Jackson", + "Billie Jean"}, + {makeSelectorAmfm(/* frequency= */ 106100), "106 KMEL", "Drake", "Marvins Room"}, + {makeSelectorAmfm(/* frequency= */ 700), "700 AM", "Artist700", "Title700"}, + {makeSelectorAmfm(/* frequency= */ 1700), "1700 AM", "Artist1700", "Title1700"}, }); // clang-format on return amFmRadioMock; @@ -73,9 +76,14 @@ const VirtualRadio& VirtualRadio::getDabRadio() { static VirtualRadio dabRadioMock( "DAB radio mock", { - {makeSelectorDab(0xA00001u, 0x0001u), "BBC Radio 1", "Khalid", "Talk"}, - {makeSelectorDab(0xB00001u, 0x1001u), "Classic FM", "Jean Sibelius", "Andante Festivo"}, - {makeSelectorDab(0xB00002u, 0x1001u), "Absolute Radio", "Coldplay", "Clocks"}, + {makeSelectorDab(/* sidExt= */ 0xA000000001u, /* ensemble= */ 0x0001u, + /* freq= */ 225648), "BBC Radio 1", "Khalid", "Talk"}, + {makeSelectorDab(/* sidExt= */ 0xB000000001u, /* ensemble= */ 0x1001u, + /* freq= */ 222064), "Classic FM", "Jean Sibelius", "Andante Festivo"}, + {makeSelectorDab(/* sidExt= */ 0xB000000002u, /* ensemble= */ 0x1002u, + /* freq= */ 227360), "Absolute Radio", "Coldplay", "Clocks"}, + {makeSelectorDab(/* sidExt= */ 0xB000000002u, /* ensemble= */ 0x1002u, + /* freq= */ 222064), "Absolute Radio", "Coldplay", "Clocks"}, }); // clang-format on return dabRadioMock; diff --git a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp index 5a568464ec..356673f715 100644 --- a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp +++ b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp @@ -46,6 +46,7 @@ namespace { using ::aidl::android::hardware::broadcastradio::utils::makeIdentifier; using ::aidl::android::hardware::broadcastradio::utils::makeSelectorAmfm; +using ::aidl::android::hardware::broadcastradio::utils::makeSelectorDab; using ::aidl::android::hardware::broadcastradio::utils::resultToInt; using ::ndk::ScopedAStatus; using ::ndk::SharedRefBase; @@ -110,8 +111,7 @@ bool supportsFM(const AmFmRegionConfig& config) { class TunerCallbackMock : public BnTunerCallback { public: TunerCallbackMock(); - - MOCK_METHOD2(onTuneFailed, ScopedAStatus(Result, const ProgramSelector&)); + ScopedAStatus onTuneFailed(Result result, const ProgramSelector& selector) override; MOCK_TIMEOUT_METHOD1(onCurrentProgramInfoChangedMock, ScopedAStatus(const ProgramInfo&)); ScopedAStatus onCurrentProgramInfoChanged(const ProgramInfo& info) override; ScopedAStatus onProgramListUpdated(const ProgramListChunk& chunk) override; @@ -154,6 +154,12 @@ TunerCallbackMock::TunerCallbackMock() { EXPECT_CALL(*this, onAntennaStateChange(false)).Times(0); } +ScopedAStatus TunerCallbackMock::onTuneFailed(Result result, const ProgramSelector& selector) { + LOG(DEBUG) << "Tune failed for selector" << selector.toString(); + EXPECT_TRUE(result == Result::CANCELED); + return ndk::ScopedAStatus::ok(); +} + ScopedAStatus TunerCallbackMock::onCurrentProgramInfoChanged(const ProgramInfo& info) { for (const auto& id : info.selector) { EXPECT_NE(id.type, IdentifierType::INVALID); @@ -175,7 +181,7 @@ ScopedAStatus TunerCallbackMock::onCurrentProgramInfoChanged(const ProgramInfo& IdentifierType physically = info.physicallyTunedTo.type; // ditto (see "logically" above) EXPECT_TRUE(physically == IdentifierType::AMFM_FREQUENCY_KHZ || - physically == IdentifierType::DAB_ENSEMBLE || + physically == IdentifierType::DAB_FREQUENCY_KHZ || physically == IdentifierType::DRMO_FREQUENCY_KHZ || physically == IdentifierType::SXM_CHANNEL || (physically >= IdentifierType::VENDOR_START && @@ -593,10 +599,43 @@ TEST_P(BroadcastRadioHalTest, DabTune) { ASSERT_TRUE(halResult.isOk()); ASSERT_NE(config.size(), 0U); - // TODO(245787803): use a DAB frequency that can actually be tuned to. + auto programList = getProgramList(); + + if (!programList) { + printSkipped("Empty DAB station list, tune cannot be performed"); + return; + } + ProgramSelector sel = {}; - int64_t freq = config[config.size() / 2].frequencyKhz; - sel.primaryId = makeIdentifier(IdentifierType::DAB_FREQUENCY_KHZ, freq); + uint64_t freq = 0; + bool dabStationPresent = false; + for (auto&& programInfo : *programList) { + if (!utils::hasId(programInfo.selector, IdentifierType::DAB_FREQUENCY_KHZ)) { + continue; + } + for (auto&& config_entry : config) { + if (config_entry.frequencyKhz == + utils::getId(programInfo.selector, IdentifierType::DAB_FREQUENCY_KHZ, 0)) { + freq = config_entry.frequencyKhz; + break; + } + } + // Do not trigger a tune request if the programList entry does not contain + // a valid DAB frequency. + if (freq == 0) { + continue; + } + int64_t dabSidExt = utils::getId(programInfo.selector, IdentifierType::DAB_SID_EXT, 0); + int64_t dabEns = utils::getId(programInfo.selector, IdentifierType::DAB_ENSEMBLE, 0); + sel = makeSelectorDab(dabSidExt, (int32_t)dabEns, freq); + dabStationPresent = true; + break; + } + + if (!dabStationPresent) { + printSkipped("No DAB stations in the list, tune cannot be performed"); + return; + } // try tuning ProgramInfo infoCb = {}; @@ -623,7 +662,6 @@ TEST_P(BroadcastRadioHalTest, DabTune) { vector freqs = bcutils::getAllIds(infoCb.selector, IdentifierType::DAB_FREQUENCY_KHZ); EXPECT_NE(freqs.end(), find(freqs.begin(), freqs.end(), freq)) << "DAB freq " << freq << " kHz is not sent back by callback."; - ; } /** diff --git a/broadcastradio/common/utilsaidl/Utils.cpp b/broadcastradio/common/utilsaidl/Utils.cpp index 52c7b40c7a..ad823661db 100644 --- a/broadcastradio/common/utilsaidl/Utils.cpp +++ b/broadcastradio/common/utilsaidl/Utils.cpp @@ -136,7 +136,9 @@ bool tunesTo(const ProgramSelector& a, const ProgramSelector& b) { return getHdSubchannel(b) == 0 && haveEqualIds(a, b, IdentifierType::AMFM_FREQUENCY_KHZ); case IdentifierType::DAB_SID_EXT: - return haveEqualIds(a, b, IdentifierType::DAB_SID_EXT); + return haveEqualIds(a, b, IdentifierType::DAB_SID_EXT) && + haveEqualIds(a, b, IdentifierType::DAB_ENSEMBLE) && + haveEqualIds(a, b, IdentifierType::DAB_FREQUENCY_KHZ); case IdentifierType::DRMO_SERVICE_ID: return haveEqualIds(a, b, IdentifierType::DRMO_SERVICE_ID); case IdentifierType::SXM_SERVICE_ID: @@ -241,8 +243,8 @@ bool isValid(const ProgramIdentifier& id) { break; } case IdentifierType::DAB_SID_EXT: { - int64_t sid = val & 0xFFFF; // 16bit - val >>= 16; + int64_t sid = val & 0xFFFFFFFF; // 32bit + val >>= 32; int64_t ecc = val & 0xFF; // 8bit expect(sid != 0u, "DAB SId != 0"); expect(ecc >= 0xA0u && ecc <= 0xF6u, "Invalid ECC, see ETSI TS 101 756 V2.1.1"); @@ -277,13 +279,35 @@ bool isValid(const ProgramIdentifier& id) { } bool isValid(const ProgramSelector& sel) { - // iterate through primaryId and secondaryIds - for (auto it = begin(sel); it != end(sel); it++) { + if (sel.primaryId.type != IdentifierType::AMFM_FREQUENCY_KHZ && + sel.primaryId.type != IdentifierType::RDS_PI && + sel.primaryId.type != IdentifierType::HD_STATION_ID_EXT && + sel.primaryId.type != IdentifierType::DAB_SID_EXT && + sel.primaryId.type != IdentifierType::DRMO_SERVICE_ID && + sel.primaryId.type != IdentifierType::SXM_SERVICE_ID && + (sel.primaryId.type < IdentifierType::VENDOR_START || + sel.primaryId.type > IdentifierType::VENDOR_END)) { + return false; + } + if (!isValid(sel.primaryId)) { + return false; + } + + bool isDab = sel.primaryId.type == IdentifierType::DAB_SID_EXT; + bool hasDabEnsemble = false; + bool hasDabFrequency = false; + for (auto it = sel.secondaryIds.begin(); it != sel.secondaryIds.end(); it++) { if (!isValid(*it)) { return false; } + if (isDab && it->type == IdentifierType::DAB_ENSEMBLE) { + hasDabEnsemble = true; + } + if (isDab && it->type == IdentifierType::DAB_FREQUENCY_KHZ) { + hasDabFrequency = true; + } } - return true; + return !isDab || (hasDabEnsemble && hasDabFrequency); } ProgramIdentifier makeIdentifier(IdentifierType type, int64_t value) { @@ -296,16 +320,12 @@ ProgramSelector makeSelectorAmfm(int32_t frequency) { return sel; } -ProgramSelector makeSelectorDab(int32_t sidExt, int32_t ensemble) { +ProgramSelector makeSelectorDab(int64_t sidExt, int32_t ensemble, int64_t freq) { ProgramSelector sel = {}; - // TODO(243686545): Have a helper function to create the sidExt instead of - // passing the whole identifier here. Something like makeDabSidExt. sel.primaryId = makeIdentifier(IdentifierType::DAB_SID_EXT, sidExt); vector secondaryIds = { makeIdentifier(IdentifierType::DAB_ENSEMBLE, ensemble), - // TODO(243686545): Include frequency here when the helper method to - // translate between ensemble and frequency is implemented. - }; + makeIdentifier(IdentifierType::DAB_FREQUENCY_KHZ, freq)}; sel.secondaryIds = std::move(secondaryIds); return sel; } @@ -330,10 +350,6 @@ bool satisfies(const ProgramFilter& filter, const ProgramSelector& sel) { } } - if (!filter.includeCategories && sel.primaryId.type == IdentifierType::DAB_ENSEMBLE) { - return false; - } - return true; } diff --git a/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h b/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h index 8ea6319ce7..beebd037c2 100644 --- a/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h +++ b/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h @@ -138,7 +138,7 @@ bool isValid(const ProgramSelector& sel); ProgramIdentifier makeIdentifier(IdentifierType type, int64_t value); ProgramSelector makeSelectorAmfm(int32_t frequency); -ProgramSelector makeSelectorDab(int32_t sidExt, int32_t ensemble); +ProgramSelector makeSelectorDab(int64_t sidExt, int32_t ensemble, int64_t freq); bool satisfies(const ProgramFilter& filter, const ProgramSelector& sel);