Merge "Prepare a best-effort workaround for HD Radio station id abuse."

broadcastradio/2.0/types.hal

@@ -466,7 +466,12 @@ enum IdentifierType : uint32_t {
      * 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),
@@ -477,6 +482,22 @@ enum IdentifierType : uint32_t {
      */
     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.
      *
@@ -492,7 +513,7 @@ enum IdentifierType : uint32_t {
      * 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,

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",

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 @@ class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase {
 
     bool openSession();
     bool getAmFmRegionConfig(bool full, AmFmRegionConfig* config);
+    std::optional<utils::ProgramInfoSet> getProgramList();
 
     sp<IBroadcastRadio> mModule;
     Properties mProperties;
@@ -182,6 +184,25 @@ bool BroadcastRadioHalTest::getAmFmRegionConfig(bool full, AmFmRegionConfig* con
     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, SetConfigFlags) {
 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());
 }
 
 /**

broadcastradio/common/tests/Android.bp

@@ -22,6 +22,9 @@ cc_test {
         "-Wextra",
         "-Werror",
     ],
+    cppflags: [
+        "-std=c++1z",
+    ],
     srcs: [
         "CommonXX_test.cpp",
     ],
@@ -43,8 +46,12 @@ cc_test {
         "-Wextra",
         "-Werror",
     ],
+    cppflags: [
+        "-std=c++1z",
+    ],
     srcs: [
         "IdentifierIterator_test.cpp",
+        "ProgramIdentifier_test.cpp",
     ],
     static_libs: [
         "android.hardware.broadcastradio@common-utils-2x-lib",

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

broadcastradio/common/utils2x/Android.bp

@@ -23,6 +23,9 @@ cc_library_static {
         "-Wextra",
         "-Werror",
     ],
+    cppflags: [
+        "-std=c++1z",
+    ],
     srcs: [
         "Utils.cpp",
     ],

broadcastradio/common/utils2x/Utils.cpp

@@ -245,6 +245,15 @@ bool isValid(const ProgramIdentifier& id) {
             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 @@ void updateProgramList(ProgramInfoSet& list, const ProgramListChunk& chunk) {
     }
 }
 
+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

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 @@ typedef std::unordered_set<V2_0::ProgramInfo, ProgramInfoHasher, ProgramInfoKeyE
 
 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