Prepare a best-effort workaround for HD Radio station id abuse.
In theory, 32bit HD Radio station ID + subchannel index (parts of
HD_STATION_ID_EXT) is a globally unique identifier. It allows broadcast
radio framework to determine which programs are the same and allow the
application to match entries from favourite list and the program list
provided by the tuner.
However, some broadcasters don't perform equipment setup correctly and
don't set station ID. As a result, there are some stations with
conflicting IDs.
As a workaround to treat these stations separately in a given location,
FM frequency was added as a part of HD_STATION_ID_EXT. This still doesn't
solve the global uniqueness problem: user might save KCQW 105.5 (sid=0) in
California, travel to Nevada and find KNAB 105.5 (sid=0). It turns out
there is no reliable identifier that might identify the station globally.
As a workaround, shortened station name is added for double-checking.
This is a best-effort fix, so it's not required for such misbehaving
stations to get correctly identified in every corner case.
Bug: 69958777
Test: VTS
Change-Id: Id11243096f1cde7fdda5cb70a7248d1831985cdd
diff --git a/broadcastradio/2.0/types.hal b/broadcastradio/2.0/types.hal
index 38a5709..a4474c8 100644
--- a/broadcastradio/2.0/types.hal
+++ b/broadcastradio/2.0/types.hal
@@ -466,7 +466,12 @@
* Consists of (from the LSB):
* - 32bit: Station ID number;
* - 4bit: HD Radio subchannel;
- * - 18bit: AMFM_FREQUENCY. // TODO(b/69958777): is it necessary?
+ * - 18bit: AMFM_FREQUENCY.
+ *
+ * While station ID number should be unique globally, it sometimes get
+ * abused by broadcasters (i.e. not being set at all). To ensure local
+ * uniqueness, AMFM_FREQUENCY was added here. Global uniqueness is
+ * a best-effort - see HD_STATION_NAME.
*
* HD Radio subchannel is a value in range 0-7.
* This index is 0-based (where 0 is MPS and 1..7 are SPS),
@@ -478,6 +483,22 @@
HD_STATION_ID_EXT,
/**
+ * 64bit additional identifier for HD Radio.
+ *
+ * Due to Station ID abuse, some HD_STATION_ID_EXT identifiers may be not
+ * globally unique. To provide a best-effort solution, a short version of
+ * station name may be carried as additional identifier and may be used
+ * by the tuner hardware to double-check tuning.
+ *
+ * The name is limited to the first 8 A-Z0-9 characters (lowercase letters
+ * must be converted to uppercase). Encoded in little-endian ASCII:
+ * the first character of the name is the LSB.
+ *
+ * For example: "Abc" is encoded as 0x434241.
+ */
+ HD_STATION_NAME,
+
+ /**
* 28bit compound primary identifier for Digital Audio Broadcasting.
*
* Consists of (from the LSB):
@@ -492,7 +513,7 @@
* The remaining bits should be set to zeros when writing on the chip side
* and ignored when read.
*/
- DAB_SID_EXT = HD_STATION_ID_EXT + 2,
+ DAB_SID_EXT,
/** 16bit */
DAB_ENSEMBLE,
diff --git a/broadcastradio/2.0/vts/functional/Android.bp b/broadcastradio/2.0/vts/functional/Android.bp
index 6017b15..6940bca 100644
--- a/broadcastradio/2.0/vts/functional/Android.bp
+++ b/broadcastradio/2.0/vts/functional/Android.bp
@@ -17,6 +17,9 @@
cc_test {
name: "VtsHalBroadcastradioV2_0TargetTest",
defaults: ["VtsHalTargetTestDefaults"],
+ cppflags: [
+ "-std=c++1z",
+ ],
srcs: ["VtsHalBroadcastradioV2_0TargetTest.cpp"],
static_libs: [
"android.hardware.broadcastradio@2.0",
diff --git a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
index 87ac934..151e089 100644
--- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
+++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
@@ -30,6 +30,7 @@
#include <gmock/gmock.h>
#include <chrono>
+#include <optional>
#include <regex>
namespace android {
@@ -96,6 +97,7 @@
bool openSession();
bool getAmFmRegionConfig(bool full, AmFmRegionConfig* config);
+ std::optional<utils::ProgramInfoSet> getProgramList();
sp<IBroadcastRadio> mModule;
Properties mProperties;
@@ -178,6 +180,25 @@
return halResult == Result::OK;
}
+std::optional<utils::ProgramInfoSet> BroadcastRadioHalTest::getProgramList() {
+ EXPECT_TIMEOUT_CALL(*mCallback, onProgramListReady).Times(AnyNumber());
+
+ auto startResult = mSession->startProgramListUpdates({});
+ if (startResult == Result::NOT_SUPPORTED) {
+ printSkipped("Program list not supported");
+ return nullopt;
+ }
+ EXPECT_EQ(Result::OK, startResult);
+ if (startResult != Result::OK) return nullopt;
+
+ EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, timeout::programListScan);
+
+ auto stopResult = mSession->stopProgramListUpdates();
+ EXPECT_TRUE(stopResult.isOk());
+
+ return mCallback->mProgramList;
+}
+
/**
* Test session opening.
*
@@ -645,19 +666,35 @@
TEST_F(BroadcastRadioHalTest, GetProgramList) {
ASSERT_TRUE(openSession());
- EXPECT_TIMEOUT_CALL(*mCallback, onProgramListReady).Times(AnyNumber());
+ getProgramList();
+}
- auto startResult = mSession->startProgramListUpdates({});
- if (startResult == Result::NOT_SUPPORTED) {
- printSkipped("Program list not supported");
- return;
+/**
+ * Test HD_STATION_NAME correctness.
+ *
+ * Verifies that if a program on the list contains HD_STATION_NAME identifier:
+ * - the program provides station name in its metadata;
+ * - the identifier matches the name;
+ * - there is only one identifier of that type.
+ */
+TEST_F(BroadcastRadioHalTest, HdRadioStationNameId) {
+ ASSERT_TRUE(openSession());
+
+ auto list = getProgramList();
+ if (!list) return;
+
+ for (auto&& program : *list) {
+ auto nameIds = utils::getAllIds(program.selector, IdentifierType::HD_STATION_NAME);
+ EXPECT_LE(nameIds.size(), 1u);
+ if (nameIds.size() == 0) continue;
+
+ auto name = utils::getMetadataString(program, MetadataKey::PROGRAM_NAME);
+ if (!name) name = utils::getMetadataString(program, MetadataKey::RDS_PS);
+ ASSERT_TRUE(name.has_value());
+
+ auto expectedId = utils::make_hdradio_station_name(*name);
+ EXPECT_EQ(expectedId.value, nameIds[0]);
}
- ASSERT_EQ(Result::OK, startResult);
-
- EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, timeout::programListScan);
-
- auto stopResult = mSession->stopProgramListUpdates();
- EXPECT_TRUE(stopResult.isOk());
}
// TODO(b/70939328): test ProgramInfo's currentlyTunedId and
diff --git a/broadcastradio/common/tests/Android.bp b/broadcastradio/common/tests/Android.bp
index 512c02e..f6a3b6f 100644
--- a/broadcastradio/common/tests/Android.bp
+++ b/broadcastradio/common/tests/Android.bp
@@ -22,6 +22,9 @@
"-Wextra",
"-Werror",
],
+ cppflags: [
+ "-std=c++1z",
+ ],
srcs: [
"CommonXX_test.cpp",
],
@@ -43,8 +46,12 @@
"-Wextra",
"-Werror",
],
+ cppflags: [
+ "-std=c++1z",
+ ],
srcs: [
"IdentifierIterator_test.cpp",
+ "ProgramIdentifier_test.cpp",
],
static_libs: [
"android.hardware.broadcastradio@common-utils-2x-lib",
diff --git a/broadcastradio/common/tests/ProgramIdentifier_test.cpp b/broadcastradio/common/tests/ProgramIdentifier_test.cpp
new file mode 100644
index 0000000..51ad014
--- /dev/null
+++ b/broadcastradio/common/tests/ProgramIdentifier_test.cpp
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <broadcastradio-utils-2x/Utils.h>
+#include <gtest/gtest.h>
+
+#include <optional>
+
+namespace {
+
+namespace utils = android::hardware::broadcastradio::utils;
+
+TEST(ProgramIdentifierTest, hdRadioStationName) {
+ auto verify = [](std::string name, uint64_t nameId) {
+ auto id = utils::make_hdradio_station_name(name);
+ EXPECT_EQ(nameId, id.value) << "Failed to convert '" << name << "'";
+ };
+
+ verify("", 0);
+ verify("Abc", 0x434241);
+ verify("Some Station 1", 0x54415453454d4f53);
+ verify("Station1", 0x314e4f4954415453);
+ verify("!@#$%^&*()_+", 0);
+ verify("-=[]{};':\"0", 0x30);
+}
+
+} // anonymous namespace
diff --git a/broadcastradio/common/utils2x/Android.bp b/broadcastradio/common/utils2x/Android.bp
index c6b94af..aab94f2 100644
--- a/broadcastradio/common/utils2x/Android.bp
+++ b/broadcastradio/common/utils2x/Android.bp
@@ -23,6 +23,9 @@
"-Wextra",
"-Werror",
],
+ cppflags: [
+ "-std=c++1z",
+ ],
srcs: [
"Utils.cpp",
],
diff --git a/broadcastradio/common/utils2x/Utils.cpp b/broadcastradio/common/utils2x/Utils.cpp
index d825a7a..6fe9554 100644
--- a/broadcastradio/common/utils2x/Utils.cpp
+++ b/broadcastradio/common/utils2x/Utils.cpp
@@ -245,6 +245,15 @@
expect(freq < 10000000u, "f < 10GHz");
break;
}
+ case IdentifierType::HD_STATION_NAME: {
+ while (val > 0) {
+ auto ch = static_cast<char>(val & 0xFF);
+ val >>= 8;
+ expect((ch >= '0' && ch <= '9') || (ch >= 'A' && ch <= 'Z'),
+ "HD_STATION_NAME does not match [A-Z0-9]+");
+ }
+ break;
+ }
case IdentifierType::DAB_SID_EXT: {
auto sid = val & 0xFFFF; // 16bit
val >>= 16;
@@ -367,6 +376,40 @@
}
}
+std::optional<std::string> getMetadataString(const V2_0::ProgramInfo& info,
+ const V2_0::MetadataKey key) {
+ auto isKey = [key](const V2_0::Metadata& item) {
+ return static_cast<V2_0::MetadataKey>(item.key) == key;
+ };
+
+ auto it = std::find_if(info.metadata.begin(), info.metadata.end(), isKey);
+ if (it == info.metadata.end()) return std::nullopt;
+
+ return it->stringValue;
+}
+
+V2_0::ProgramIdentifier make_hdradio_station_name(const std::string& name) {
+ constexpr size_t maxlen = 8;
+
+ std::string shortName;
+ shortName.reserve(maxlen);
+
+ auto&& loc = std::locale::classic();
+ for (char ch : name) {
+ if (!std::isalnum(ch, loc)) continue;
+ shortName.push_back(std::toupper(ch, loc));
+ if (shortName.length() >= maxlen) break;
+ }
+
+ uint64_t val = 0;
+ for (auto rit = shortName.rbegin(); rit != shortName.rend(); ++rit) {
+ val <<= 8;
+ val |= static_cast<uint8_t>(*rit);
+ }
+
+ return make_identifier(IdentifierType::HD_STATION_NAME, val);
+}
+
} // namespace utils
} // namespace broadcastradio
} // namespace hardware
diff --git a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
index e3134f7..5e51941 100644
--- a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
+++ b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
@@ -18,6 +18,7 @@
#include <android/hardware/broadcastradio/2.0/types.h>
#include <chrono>
+#include <optional>
#include <queue>
#include <thread>
#include <unordered_set>
@@ -146,6 +147,11 @@
void updateProgramList(ProgramInfoSet& list, const V2_0::ProgramListChunk& chunk);
+std::optional<std::string> getMetadataString(const V2_0::ProgramInfo& info,
+ const V2_0::MetadataKey key);
+
+V2_0::ProgramIdentifier make_hdradio_station_name(const std::string& name);
+
} // namespace utils
} // namespace broadcastradio
} // namespace hardware