Merge "Prepare a best-effort workaround for HD Radio station id abuse."
diff --git a/broadcastradio/2.0/types.hal b/broadcastradio/2.0/types.hal
index d77e8c1..c5880e2 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 aa75442..1111478 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 {
@@ -100,6 +101,7 @@
bool openSession();
bool getAmFmRegionConfig(bool full, AmFmRegionConfig* config);
+ std::optional<utils::ProgramInfoSet> getProgramList();
sp<IBroadcastRadio> mModule;
Properties mProperties;
@@ -182,6 +184,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.
*
@@ -649,19 +670,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());
}
/**
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