diff --git a/broadcastradio/1.1/vts/functional/Android.bp b/broadcastradio/1.1/vts/functional/Android.bp index 3d4fe05678..c016c168d2 100644 --- a/broadcastradio/1.1/vts/functional/Android.bp +++ b/broadcastradio/1.1/vts/functional/Android.bp @@ -21,6 +21,7 @@ cc_test { static_libs: [ "android.hardware.broadcastradio@1.0", "android.hardware.broadcastradio@1.1", + "android.hardware.broadcastradio@1.2", // common-utils-lib dependency "android.hardware.broadcastradio@common-utils-lib", "android.hardware.broadcastradio@vts-utils-lib", "libgmock", diff --git a/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp b/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp index a46378ee6a..bb490c941d 100644 --- a/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp +++ b/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp @@ -525,6 +525,98 @@ TEST_P(BroadcastRadioHalTest, AnalogForcedSwitch) { ASSERT_FALSE(forced); } +static void verifyIdentifier(const ProgramIdentifier& id) { + EXPECT_NE(id.type, 0u); + auto val = id.value; + + switch (static_cast(id.type)) { + case IdentifierType::AMFM_FREQUENCY: + case IdentifierType::DAB_FREQUENCY: + case IdentifierType::DRMO_FREQUENCY: + EXPECT_GT(val, 100u) << "Expected f > 100kHz"; + EXPECT_LT(val, 10000000u) << "Expected f < 10GHz"; + break; + case IdentifierType::RDS_PI: + EXPECT_GT(val, 0u); + EXPECT_LE(val, 0xFFFFu) << "Expected 16bit id"; + break; + case IdentifierType::HD_STATION_ID_EXT: { + auto stationId = val & 0xFFFFFFFF; // 32bit + val >>= 32; + auto subchannel = val & 0xF; // 4bit + val >>= 4; + auto freq = val & 0x3FFFF; // 18bit + EXPECT_GT(stationId, 0u); + EXPECT_LT(subchannel, 8u) << "Expected ch < 8"; + EXPECT_GT(freq, 100u) << "Expected f > 100kHz"; + EXPECT_LT(freq, 10000000u) << "Expected f < 10GHz"; + break; + } + case IdentifierType::HD_SUBCHANNEL: + EXPECT_LT(val, 8u) << "Expected ch < 8"; + break; + case IdentifierType::DAB_SIDECC: { + auto sid = val & 0xFFFF; // 16bit + val >>= 16; + auto ecc = val & 0xFF; // 8bit + EXPECT_NE(sid, 0u); + EXPECT_GE(ecc, 0xA0u) << "Invalid ECC, see ETSI TS 101 756 V2.1.1"; + EXPECT_LE(ecc, 0xF6u) << "Invalid ECC, see ETSI TS 101 756 V2.1.1"; + break; + } + case IdentifierType::DAB_ENSEMBLE: + EXPECT_GT(val, 0u); + EXPECT_LE(val, 0xFFFFu) << "Expected 16bit id"; + break; + case IdentifierType::DAB_SCID: + EXPECT_GT(val, 0xFu) << "Expected 12bit SCId (not 4bit SCIdS)"; + EXPECT_LE(val, 0xFFFu) << "Expected 12bit id"; + break; + case IdentifierType::DRMO_SERVICE_ID: + EXPECT_GT(val, 0u); + EXPECT_LE(val, 0xFFFFFFu) << "Expected 24bit id"; + break; + case IdentifierType::DRMO_MODULATION: + EXPECT_GE(val, static_cast(Modulation::AM)); + EXPECT_LE(val, static_cast(Modulation::FM)); + break; + case IdentifierType::SXM_SERVICE_ID: + EXPECT_GT(val, 0u); + EXPECT_LE(val, 0xFFFFFFFFu) << "Expected 32bit id"; + break; + case IdentifierType::SXM_CHANNEL: + EXPECT_LT(val, 1000u); + break; + case IdentifierType::VENDOR_PRIMARY_START: + case IdentifierType::VENDOR_PRIMARY_END: + // skip + break; + } +} + +/** + * Test ProgramIdentifier format. + * + * Verifies that: + * - values of ProgramIdentifier match their definitions at IdentifierType. + */ +TEST_P(BroadcastRadioHalTest, VerifyIdentifiersFormat) { + if (skipped) return; + ASSERT_TRUE(openTuner()); + + do { + auto getCb = [&](const hidl_vec& list) { + for (auto&& program : list) { + verifyIdentifier(program.selector.primaryId); + for (auto&& id : program.selector.secondaryIds) { + verifyIdentifier(id); + } + } + }; + getProgramList(getCb); + } while (nextBand()); +} + INSTANTIATE_TEST_CASE_P(BroadcastRadioHalTestCases, BroadcastRadioHalTest, ::testing::Values(Class::AM_FM, Class::SAT, Class::DT)); diff --git a/broadcastradio/1.2/default/Tuner.cpp b/broadcastradio/1.2/default/Tuner.cpp index 70418cfbc6..6209dc1953 100644 --- a/broadcastradio/1.2/default/Tuner.cpp +++ b/broadcastradio/1.2/default/Tuner.cpp @@ -35,13 +35,13 @@ using V1_0::Band; using V1_0::BandConfig; using V1_0::Class; using V1_0::Direction; -using V1_1::IdentifierType; using V1_1::ProgramInfo; using V1_1::ProgramInfoFlags; using V1_1::ProgramListResult; using V1_1::ProgramSelector; using V1_1::ProgramType; using V1_1::VendorKeyValue; +using V1_2::IdentifierType; using utils::HalRevision; using std::chrono::milliseconds; @@ -282,7 +282,7 @@ Return Tuner::tuneByProgramSelector(const ProgramSelector& sel) { return Result::INVALID_ARGUMENTS; } } else if (programType == ProgramType::DAB) { - if (!utils::hasId(sel, IdentifierType::DAB_SIDECC)) return Result::INVALID_ARGUMENTS; + if (!utils::hasId(sel, IdentifierType::DAB_SID_EXT)) return Result::INVALID_ARGUMENTS; } else if (programType == ProgramType::DRMO) { if (!utils::hasId(sel, IdentifierType::DRMO_SERVICE_ID)) return Result::INVALID_ARGUMENTS; } else if (programType == ProgramType::SXM) { diff --git a/broadcastradio/1.2/default/VirtualProgram.cpp b/broadcastradio/1.2/default/VirtualProgram.cpp index 95879e34df..3284bd1273 100644 --- a/broadcastradio/1.2/default/VirtualProgram.cpp +++ b/broadcastradio/1.2/default/VirtualProgram.cpp @@ -30,9 +30,9 @@ using std::vector; using V1_0::MetaData; using V1_0::MetadataKey; using V1_0::MetadataType; -using V1_1::IdentifierType; using V1_1::ProgramInfo; using V1_1::VendorKeyValue; +using V1_2::IdentifierType; using utils::HalRevision; static MetaData createDemoBitmap(MetadataKey key, HalRevision halRev) { diff --git a/broadcastradio/1.2/types.hal b/broadcastradio/1.2/types.hal index 5edb097aaf..7301e13ccf 100644 --- a/broadcastradio/1.2/types.hal +++ b/broadcastradio/1.2/types.hal @@ -16,8 +16,35 @@ package android.hardware.broadcastradio@1.2; +import @1.1::IdentifierType; import @1.1::Result; import @1.1::VendorKeyValue; typedef @1.1::Result Result; typedef @1.1::VendorKeyValue VendorKeyValue; + +enum IdentifierType : @1.1::IdentifierType { + /** + * 28bit compound primary identifier for DAB. + * + * Consists of (from the LSB): + * - 16bit: SId; + * - 8bit: ECC code; + * - 4bit: SCIdS (optional). + * + * SCIdS (Service Component Identifier within the Service) value + * of 0 represents the main service, while 1 and above represents + * secondary services. + * + * The remaining bits should be set to zeros when writing on the chip side + * and ignored when read. + * + * This identifier deprecates DAB_SIDECC and makes new primary identifier + * for DAB. If the hal implementation detects 1.2 client (by casting + * V1_0::ITunerCallback to V1_2::ITunerCallback), it must use DAB_SID_EXT + * as a primary identifier for DAB program type. If the hal client detects + * either 1.1 or 1.2 HAL, it must convert those identifiers to the + * correct version. + */ + DAB_SID_EXT = SXM_CHANNEL + 1, +}; diff --git a/broadcastradio/1.2/vts/functional/VtsHalBroadcastradioV1_2TargetTest.cpp b/broadcastradio/1.2/vts/functional/VtsHalBroadcastradioV1_2TargetTest.cpp index f075945947..f3552a8be3 100644 --- a/broadcastradio/1.2/vts/functional/VtsHalBroadcastradioV1_2TargetTest.cpp +++ b/broadcastradio/1.2/vts/functional/VtsHalBroadcastradioV1_2TargetTest.cpp @@ -295,6 +295,9 @@ TEST_P(BroadcastRadioHalTest, UnknownParameters) { ASSERT_EQ(0u, halResults.size()); } +// TODO(b/69860743): implement VerifyIdentifiersFormat test when +// the new program list fetching mechanism is implemented + INSTANTIATE_TEST_CASE_P(BroadcastRadioHalTestCases, BroadcastRadioHalTest, ::testing::Values(Class::AM_FM, Class::SAT, Class::DT)); diff --git a/broadcastradio/common/utils/Android.bp b/broadcastradio/common/utils/Android.bp index d8bd12556c..d29d05c8ff 100644 --- a/broadcastradio/common/utils/Android.bp +++ b/broadcastradio/common/utils/Android.bp @@ -29,6 +29,6 @@ cc_library_static { ], export_include_dirs: ["include"], shared_libs: [ - "android.hardware.broadcastradio@1.1", + "android.hardware.broadcastradio@1.2", ], } diff --git a/broadcastradio/common/utils/Utils.cpp b/broadcastradio/common/utils/Utils.cpp index bdaf8e8cf2..22a697066a 100644 --- a/broadcastradio/common/utils/Utils.cpp +++ b/broadcastradio/common/utils/Utils.cpp @@ -26,10 +26,10 @@ namespace broadcastradio { namespace utils { using V1_0::Band; -using V1_1::IdentifierType; using V1_1::ProgramIdentifier; using V1_1::ProgramSelector; using V1_1::ProgramType; +using V1_2::IdentifierType; static bool isCompatibleProgramType(const uint32_t ia, const uint32_t ib) { auto a = static_cast(ia); @@ -88,7 +88,7 @@ bool tunesTo(const ProgramSelector& a, const ProgramSelector& b) { return haveEqualIds(a, b, IdentifierType::AMFM_FREQUENCY); case ProgramType::DAB: - return haveEqualIds(a, b, IdentifierType::DAB_SIDECC); + return haveEqualIds(a, b, IdentifierType::DAB_SID_EXT); case ProgramType::DRMO: return haveEqualIds(a, b, IdentifierType::DRMO_SERVICE_ID); case ProgramType::SXM: @@ -126,23 +126,50 @@ bool isFm(const Band band) { return band == Band::FM || band == Band::FM_HD; } -bool hasId(const ProgramSelector& sel, const IdentifierType type) { +static bool maybeGetId(const ProgramSelector& sel, const IdentifierType type, uint64_t* val) { auto itype = static_cast(type); - if (sel.primaryId.type == itype) return true; - // not optimal, but we don't care in default impl - for (auto&& id : sel.secondaryIds) { - if (id.type == itype) return true; + auto itypeAlt = itype; + if (type == IdentifierType::DAB_SIDECC) { + itypeAlt = static_cast(IdentifierType::DAB_SID_EXT); } - return false; + if (type == IdentifierType::DAB_SID_EXT) { + itypeAlt = static_cast(IdentifierType::DAB_SIDECC); + } + + if (sel.primaryId.type == itype || sel.primaryId.type == itypeAlt) { + if (val) *val = sel.primaryId.value; + return true; + } + + // not optimal, but we don't care in default impl + bool gotAlt = false; + for (auto&& id : sel.secondaryIds) { + if (id.type == itype) { + if (val) *val = id.value; + return true; + } + // alternative identifier is a backup, we prefer original value + if (id.type == itypeAlt) { + if (val) *val = id.value; + gotAlt = true; + continue; + } + } + + return gotAlt; +} + +bool hasId(const ProgramSelector& sel, const IdentifierType type) { + return maybeGetId(sel, type, nullptr); } uint64_t getId(const ProgramSelector& sel, const IdentifierType type) { - auto itype = static_cast(type); - if (sel.primaryId.type == itype) return sel.primaryId.value; - // not optimal, but we don't care in default impl - for (auto&& id : sel.secondaryIds) { - if (id.type == itype) return id.value; + uint64_t val; + + if (maybeGetId(sel, type, &val)) { + return val; } + ALOGW("Identifier %s not found", toString(type).c_str()); return 0; } diff --git a/broadcastradio/common/utils/include/broadcastradio-utils/Utils.h b/broadcastradio/common/utils/include/broadcastradio-utils/Utils.h index b07ce79be2..9cdc629d76 100644 --- a/broadcastradio/common/utils/include/broadcastradio-utils/Utils.h +++ b/broadcastradio/common/utils/include/broadcastradio-utils/Utils.h @@ -16,7 +16,7 @@ #ifndef ANDROID_HARDWARE_BROADCASTRADIO_COMMON_UTILS_H #define ANDROID_HARDWARE_BROADCASTRADIO_COMMON_UTILS_H -#include +#include #include #include #include @@ -50,21 +50,21 @@ bool isAmFm(const V1_1::ProgramType type); bool isAm(const V1_0::Band band); bool isFm(const V1_0::Band band); -bool hasId(const V1_1::ProgramSelector& sel, const V1_1::IdentifierType type); +bool hasId(const V1_1::ProgramSelector& sel, const V1_2::IdentifierType type); /** * Returns ID (either primary or secondary) for a given program selector. * * If the selector does not contain given type, returns 0 and emits a warning. */ -uint64_t getId(const V1_1::ProgramSelector& sel, const V1_1::IdentifierType type); +uint64_t getId(const V1_1::ProgramSelector& sel, const V1_2::IdentifierType type); /** * Returns ID (either primary or secondary) for a given program selector. * * If the selector does not contain given type, returns default value. */ -uint64_t getId(const V1_1::ProgramSelector& sel, const V1_1::IdentifierType type, uint64_t defval); +uint64_t getId(const V1_1::ProgramSelector& sel, const V1_2::IdentifierType type, uint64_t defval); V1_1::ProgramSelector make_selector(V1_0::Band band, uint32_t channel, uint32_t subChannel = 0);