Improve current program info validation.

Bug: 74022658
Test: atest VtsHalBroadcastradioV2_0TargetTest
Change-Id: Ifee19249a9033363df863480e8a5da24334f5e59

4 files changed, 64 insertions, 42 deletions

diff --git a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
index 571b80c79..62b00379a 100644
--- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
+++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
@@ -137,29 +137,30 @@ TunerCallbackMock::TunerCallbackMock() {
 }
 
 Return<void> TunerCallbackMock::onCurrentProgramInfoChanged(const ProgramInfo& info) {
-    auto logically = utils::getType(info.logicallyTunedTo);
-    if (logically != IdentifierType::INVALID) {
-        EXPECT_TRUE(logically == IdentifierType::AMFM_FREQUENCY ||
-                    logically == IdentifierType::RDS_PI ||
-                    logically == IdentifierType::HD_STATION_ID_EXT ||
-                    logically == IdentifierType::DAB_SID_EXT ||
-                    logically == IdentifierType::DRMO_SERVICE_ID ||
-                    logically == IdentifierType::SXM_SERVICE_ID ||
-                    (logically >= IdentifierType::VENDOR_START &&
-                     logically <= IdentifierType::VENDOR_END) ||
-                    logically > IdentifierType::SXM_CHANNEL);
+    for (auto&& id : info.selector) {
+        EXPECT_NE(IdentifierType::INVALID, utils::getType(id));
     }
 
+    auto logically = utils::getType(info.logicallyTunedTo);
+    /* This field is required for currently tuned program and should be INVALID
+     * for entries from the program list.
+     */
+    EXPECT_TRUE(
+        logically == IdentifierType::AMFM_FREQUENCY || logically == IdentifierType::RDS_PI ||
+        logically == IdentifierType::HD_STATION_ID_EXT ||
+        logically == IdentifierType::DAB_SID_EXT || logically == IdentifierType::DRMO_SERVICE_ID ||
+        logically == IdentifierType::SXM_SERVICE_ID ||
+        (logically >= IdentifierType::VENDOR_START && logically <= IdentifierType::VENDOR_END) ||
+        logically > IdentifierType::SXM_CHANNEL);
+
     auto physically = utils::getType(info.physicallyTunedTo);
-    if (physically != IdentifierType::INVALID) {
-        EXPECT_TRUE(physically == IdentifierType::AMFM_FREQUENCY ||
-                    physically == IdentifierType::DAB_ENSEMBLE ||
-                    physically == IdentifierType::DRMO_FREQUENCY ||
-                    physically == IdentifierType::SXM_CHANNEL ||
-                    (physically >= IdentifierType::VENDOR_START &&
-                     physically <= IdentifierType::VENDOR_END) ||
-                    physically > IdentifierType::SXM_CHANNEL);
-    }
+    // ditto (see "logically" above)
+    EXPECT_TRUE(
+        physically == IdentifierType::AMFM_FREQUENCY ||
+        physically == IdentifierType::DAB_ENSEMBLE ||
+        physically == IdentifierType::DRMO_FREQUENCY || physically == IdentifierType::SXM_CHANNEL ||
+        (physically >= IdentifierType::VENDOR_START && physically <= IdentifierType::VENDOR_END) ||
+        physically > IdentifierType::SXM_CHANNEL);
 
     if (logically == IdentifierType::AMFM_FREQUENCY) {
         auto ps = utils::getMetadataString(info, MetadataKey::RDS_PS);
diff --git a/broadcastradio/common/tests/IdentifierIterator_test.cpp b/broadcastradio/common/tests/IdentifierIterator_test.cpp
index 5bf222bfd..75e0d49db 100644
--- a/broadcastradio/common/tests/IdentifierIterator_test.cpp
+++ b/broadcastradio/common/tests/IdentifierIterator_test.cpp
@@ -33,8 +33,8 @@ TEST(IdentifierIteratorTest, singleSecondary) {
     };
     // clang-format on
 
-    auto it = utils::begin(sel);
-    auto end = utils::end(sel);
+    auto it = V2_0::begin(sel);
+    auto end = V2_0::end(sel);
 
     ASSERT_NE(end, it);
     EXPECT_EQ(sel.primaryId, *it);
@@ -46,8 +46,8 @@ TEST(IdentifierIteratorTest, singleSecondary) {
 TEST(IdentifierIteratorTest, empty) {
     V2_0::ProgramSelector sel{};
 
-    auto it = utils::begin(sel);
-    auto end = utils::end(sel);
+    auto it = V2_0::begin(sel);
+    auto end = V2_0::end(sel);
 
     ASSERT_NE(end, it++);  // primary id is always present
     ASSERT_EQ(end, it);
@@ -57,8 +57,8 @@ TEST(IdentifierIteratorTest, twoSelectors) {
     V2_0::ProgramSelector sel1{};
     V2_0::ProgramSelector sel2{};
 
-    auto it1 = utils::begin(sel1);
-    auto it2 = utils::begin(sel2);
+    auto it1 = V2_0::begin(sel1);
+    auto it2 = V2_0::begin(sel2);
 
     EXPECT_NE(it1, it2);
 }
@@ -66,8 +66,8 @@ TEST(IdentifierIteratorTest, twoSelectors) {
 TEST(IdentifierIteratorTest, increments) {
     V2_0::ProgramSelector sel{{}, {{}, {}}};
 
-    auto it = utils::begin(sel);
-    auto end = utils::end(sel);
+    auto it = V2_0::begin(sel);
+    auto end = V2_0::end(sel);
     auto pre = it;
     auto post = it;
 
@@ -102,8 +102,8 @@ TEST(IdentifierIteratorTest, findType) {
     auto isRdsPi = std::bind(typeEquals, _1, IdentifierType::RDS_PI);
     auto isFreq = std::bind(typeEquals, _1, IdentifierType::AMFM_FREQUENCY);
 
-    auto end = utils::end(sel);
-    auto it = std::find_if(utils::begin(sel), end, isRdsPi);
+    auto end = V2_0::end(sel);
+    auto it = std::find_if(V2_0::begin(sel), end, isRdsPi);
     ASSERT_NE(end, it);
     EXPECT_EQ(rds_pi1, it->value);
 
@@ -111,7 +111,7 @@ TEST(IdentifierIteratorTest, findType) {
     ASSERT_NE(end, it);
     EXPECT_EQ(rds_pi2, it->value);
 
-    it = std::find_if(utils::begin(sel), end, isFreq);
+    it = std::find_if(V2_0::begin(sel), end, isFreq);
     ASSERT_NE(end, it);
     EXPECT_EQ(freq1, it->value);
 
@@ -120,4 +120,17 @@ TEST(IdentifierIteratorTest, findType) {
     EXPECT_EQ(freq2, it->value);
 }
 
+TEST(IdentifierIteratorTest, rangeLoop) {
+    V2_0::ProgramSelector sel{{}, {{}, {}, {}}};
+
+    unsigned count = 0;
+    for (auto&& id : sel) {
+        ASSERT_EQ(0u, id.type);
+        count++;
+    }
+
+    const auto expectedCount = 1 + sel.secondaryIds.size();
+    ASSERT_EQ(expectedCount, count);
+}
+
 }  // anonymous namespace
diff --git a/broadcastradio/common/utils2x/Utils.cpp b/broadcastradio/common/utils2x/Utils.cpp
index 6fe95549d..3e20b357f 100644
--- a/broadcastradio/common/utils2x/Utils.cpp
+++ b/broadcastradio/common/utils2x/Utils.cpp
@@ -81,14 +81,6 @@ bool IdentifierIterator::operator==(const IdentifierIterator& rhs) const {
     return mPos == rhs.mPos;
 }
 
-IdentifierIterator begin(const V2_0::ProgramSelector& sel) {
-    return IdentifierIterator(sel);
-}
-
-IdentifierIterator end(const V2_0::ProgramSelector& sel) {
-    return IdentifierIterator(sel) + 1 /* primary id */ + sel.secondaryIds.size();
-}
-
 FrequencyBand getBand(uint64_t freq) {
     // keep in sync with
     // frameworks/base/services/core/java/com/android/server/broadcastradio/hal2/Utils.java
@@ -411,6 +403,18 @@ V2_0::ProgramIdentifier make_hdradio_station_name(const std::string& name) {
 }
 
 }  // namespace utils
+
+namespace V2_0 {
+
+utils::IdentifierIterator begin(const ProgramSelector& sel) {
+    return utils::IdentifierIterator(sel);
+}
+
+utils::IdentifierIterator end(const ProgramSelector& sel) {
+    return utils::IdentifierIterator(sel) + 1 /* primary id */ + sel.secondaryIds.size();
+}
+
+}  // namespace V2_0
 }  // namespace broadcastradio
 }  // namespace hardware
 }  // namespace android
diff --git a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
index 5e5194163..c4aecb218 100644
--- a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
+++ b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h
@@ -69,9 +69,6 @@ class IdentifierIterator
     size_t mPos = 0;
 };
 
-IdentifierIterator begin(const V2_0::ProgramSelector& sel);
-IdentifierIterator end(const V2_0::ProgramSelector& sel);
-
 /**
  * Guesses band from the frequency value.
  *
@@ -153,6 +150,13 @@ std::optional<std::string> getMetadataString(const V2_0::ProgramInfo& info,
 V2_0::ProgramIdentifier make_hdradio_station_name(const std::string& name);
 
 }  // namespace utils
+
+namespace V2_0 {
+
+utils::IdentifierIterator begin(const ProgramSelector& sel);
+utils::IdentifierIterator end(const ProgramSelector& sel);
+
+}  // namespace V2_0
 }  // namespace broadcastradio
 }  // namespace hardware
 }  // namespace android