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

8 files changed, 172 insertions, 12 deletions

diff --git a/broadcastradio/2.0/types.hal b/broadcastradio/2.0/types.hal
index 38a5709a..a4474c8c 100644
--- a/broadcastradio/2.0/types.hal
+++ b/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),
@@ -478,6 +483,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.
      *
      * Consists of (from the LSB):
@@ -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,
diff --git a/broadcastradio/2.0/vts/functional/Android.bp b/broadcastradio/2.0/vts/functional/Android.bp
index 6017b152..6940bca4 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 87ac9341..151e089a 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 @@ class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase {
 
     bool openSession();
     bool getAmFmRegionConfig(bool full, AmFmRegionConfig* config);
+    std::optional<utils::ProgramInfoSet> getProgramList();
 
     sp<IBroadcastRadio> mModule;
     Properties mProperties;
@@ -178,6 +180,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.
  *
@@ -645,19 +666,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;
-    }
-    ASSERT_EQ(Result::OK, startResult);
+/**
+ * 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());
 
-    EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, timeout::programListScan);
+    auto list = getProgramList();
+    if (!list) return;
 
-    auto stopResult = mSession->stopProgramListUpdates();
-    EXPECT_TRUE(stopResult.isOk());
+    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]);
+    }
 }
 
 // TODO(b/70939328): test ProgramInfo's currentlyTunedId and
diff --git a/broadcastradio/common/tests/Android.bp b/broadcastradio/common/tests/Android.bp
index 512c02e4..f6a3b6f4 100644
--- a/broadcastradio/common/tests/Android.bp
+++ b/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",
diff --git a/broadcastradio/common/tests/ProgramIdentifier_test.cpp b/broadcastradio/common/tests/ProgramIdentifier_test.cpp
new file mode 100644
index 00000000..51ad0145
--- /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 c6b94afb..aab94f2a 100644
--- a/broadcastradio/common/utils2x/Android.bp
+++ b/broadcastradio/common/utils2x/Android.bp
@@ -23,6 +23,9 @@ cc_library_static {
         "-Wextra",
         "-Werror",
     ],
+    cppflags: [
+        "-std=c++1z",
+    ],
     srcs: [
         "Utils.cpp",
     ],
diff --git a/broadcastradio/common/utils2x/Utils.cpp b/broadcastradio/common/utils2x/Utils.cpp
index d825a7a0..6fe95549 100644
--- a/broadcastradio/common/utils2x/Utils.cpp
+++ b/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
diff --git a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
index e3134f7b..5e519416 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 @@ 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