Merge "Improve DAB support in broadcast radio AIDL HAL"
diff --git a/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl b/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl
index 0484d02..646c502 100644
--- a/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl
+++ b/broadcastradio/aidl/android/hardware/broadcastradio/IdentifierType.aidl
@@ -110,10 +110,11 @@
     HD_STATION_NAME,
 
     /**
-     * 28bit compound primary identifier for Digital Audio Broadcasting.
+     * 44bit compound primary identifier for Digital Audio Broadcasting and
+     * Digital Multimeida Broadcasting.
      *
      * Consists of (from the LSB):
-     * - 16bit: SId;
+     * - 32bit: SId;
      * - 8bit: ECC code;
      * - 4bit: SCIdS.
      *
diff --git a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl
index 3e2c9cc..d650239 100644
--- a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl
+++ b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramInfo.aidl
@@ -120,7 +120,7 @@
      *
      * Only physical identifiers are valid:
      *  - AMFM_FREQUENCY_KHZ;
-     *  - DAB_ENSEMBLE;
+     *  - DAB_FREQUENCY_KHZ;
      *  - DRMO_FREQUENCY_KHZ;
      *  - SXM_CHANNEL;
      *  - VENDOR_*;
diff --git a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl
index 8bd3fd4..93b0e12 100644
--- a/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl
+++ b/broadcastradio/aidl/android/hardware/broadcastradio/ProgramSelector.aidl
@@ -51,7 +51,8 @@
      *  - analogue AM/FM: AMFM_FREQUENCY_KHZ;
      *  - FM RDS: RDS_PI;
      *  - HD Radio: HD_STATION_ID_EXT;
-     *  - DAB: DAB_SID_EXT;
+     *  - DAB/DMB: DAB_SID_EXT (when used, DAB_ENSEMBLE and DAB_FREQUENCY_KHZ
+     *    must present in secondaryIds);
      *  - Digital Radio Mondiale: DRMO_SERVICE_ID;
      *  - SiriusXM: SXM_SERVICE_ID;
      *  - vendor-specific: VENDOR_START..VENDOR_END.
diff --git a/broadcastradio/aidl/default/BroadcastRadio.cpp b/broadcastradio/aidl/default/BroadcastRadio.cpp
index 42e550e..36520fb 100644
--- a/broadcastradio/aidl/default/BroadcastRadio.cpp
+++ b/broadcastradio/aidl/default/BroadcastRadio.cpp
@@ -589,22 +589,28 @@
     }
     ProgramSelector sel = {};
     if (isDab) {
-        if (numArgs != 4) {
+        if (numArgs != 5) {
             dprintf(fd,
-                    "Invalid number of arguments: please provide --tune dab <SID> <ENSEMBLE>\n");
+                    "Invalid number of arguments: please provide "
+                    "--tune dab <SID> <ENSEMBLE> <FREQUENCY>\n");
             return STATUS_BAD_VALUE;
         }
         int sid;
         if (!utils::parseArgInt(string(args[2]), &sid)) {
-            dprintf(fd, "Non-integer sid provided with tune: %s\n", string(args[2]).c_str());
+            dprintf(fd, "Non-integer sid provided with tune: %s\n", args[2]);
             return STATUS_BAD_VALUE;
         }
         int ensemble;
         if (!utils::parseArgInt(string(args[3]), &ensemble)) {
-            dprintf(fd, "Non-integer ensemble provided with tune: %s\n", string(args[3]).c_str());
+            dprintf(fd, "Non-integer ensemble provided with tune: %s\n", args[3]);
             return STATUS_BAD_VALUE;
         }
-        sel = utils::makeSelectorDab(sid, ensemble);
+        int freq;
+        if (!utils::parseArgInt(string(args[4]), &freq)) {
+            dprintf(fd, "Non-integer frequency provided with tune: %s\n", args[4]);
+            return STATUS_BAD_VALUE;
+        }
+        sel = utils::makeSelectorDab(sid, ensemble, freq);
     } else {
         if (numArgs != 3) {
             dprintf(fd, "Invalid number of arguments: please provide --tune amfm <FREQUENCY>\n");
@@ -612,7 +618,7 @@
         }
         int freq;
         if (!utils::parseArgInt(string(args[2]), &freq)) {
-            dprintf(fd, "Non-integer frequency provided with tune: %s\n", string(args[2]).c_str());
+            dprintf(fd, "Non-integer frequency provided with tune: %s\n", args[2]);
             return STATUS_BAD_VALUE;
         }
         sel = utils::makeSelectorAmfm(freq);
diff --git a/broadcastradio/aidl/default/VirtualProgram.cpp b/broadcastradio/aidl/default/VirtualProgram.cpp
index 0df0a82..4fe6567 100644
--- a/broadcastradio/aidl/default/VirtualProgram.cpp
+++ b/broadcastradio/aidl/default/VirtualProgram.cpp
@@ -53,7 +53,7 @@
             break;
         case IdentifierType::DAB_SID_EXT:
             info.logicallyTunedTo = selectId(IdentifierType::DAB_SID_EXT);
-            info.physicallyTunedTo = selectId(IdentifierType::DAB_ENSEMBLE);
+            info.physicallyTunedTo = selectId(IdentifierType::DAB_FREQUENCY_KHZ);
             break;
         case IdentifierType::DRMO_SERVICE_ID:
             info.logicallyTunedTo = selectId(IdentifierType::DRMO_SERVICE_ID);
diff --git a/broadcastradio/aidl/default/VirtualRadio.cpp b/broadcastradio/aidl/default/VirtualRadio.cpp
index 851543b..126bcff 100644
--- a/broadcastradio/aidl/default/VirtualRadio.cpp
+++ b/broadcastradio/aidl/default/VirtualRadio.cpp
@@ -53,15 +53,18 @@
     static VirtualRadio amFmRadioMock(
         "AM/FM radio mock",
         {
-            {makeSelectorAmfm(94900), "Wild 94.9", "Drake ft. Rihanna", "Too Good"},
-            {makeSelectorAmfm(96500), "KOIT", "Celine Dion", "All By Myself"},
-            {makeSelectorAmfm(97300), "Alice@97.3", "Drops of Jupiter", "Train"},
-            {makeSelectorAmfm(99700), "99.7 Now!", "The Chainsmokers", "Closer"},
-            {makeSelectorAmfm(101300), "101-3 KISS-FM", "Justin Timberlake", "Rock Your Body"},
-            {makeSelectorAmfm(103700), "iHeart80s @ 103.7", "Michael Jackson", "Billie Jean"},
-            {makeSelectorAmfm(106100), "106 KMEL", "Drake", "Marvins Room"},
-            {makeSelectorAmfm(700), "700 AM", "Artist700", "Title700"},
-            {makeSelectorAmfm(1700), "1700 AM", "Artist1700", "Title1700"},
+            {makeSelectorAmfm(/* frequency= */ 94900), "Wild 94.9", "Drake ft. Rihanna",
+                "Too Good"},
+            {makeSelectorAmfm(/* frequency= */ 96500), "KOIT", "Celine Dion", "All By Myself"},
+            {makeSelectorAmfm(/* frequency= */ 97300), "Alice@97.3", "Drops of Jupiter", "Train"},
+            {makeSelectorAmfm(/* frequency= */ 99700), "99.7 Now!", "The Chainsmokers", "Closer"},
+            {makeSelectorAmfm(/* frequency= */ 101300), "101-3 KISS-FM", "Justin Timberlake",
+                "Rock Your Body"},
+            {makeSelectorAmfm(/* frequency= */ 103700), "iHeart80s @ 103.7", "Michael Jackson",
+                "Billie Jean"},
+            {makeSelectorAmfm(/* frequency= */ 106100), "106 KMEL", "Drake", "Marvins Room"},
+            {makeSelectorAmfm(/* frequency= */ 700), "700 AM", "Artist700", "Title700"},
+            {makeSelectorAmfm(/* frequency= */ 1700), "1700 AM", "Artist1700", "Title1700"},
         });
     // clang-format on
     return amFmRadioMock;
@@ -73,9 +76,14 @@
     static VirtualRadio dabRadioMock(
         "DAB radio mock",
         {
-            {makeSelectorDab(0xA00001u, 0x0001u), "BBC Radio 1", "Khalid", "Talk"},
-            {makeSelectorDab(0xB00001u, 0x1001u), "Classic FM", "Jean Sibelius", "Andante Festivo"},
-            {makeSelectorDab(0xB00002u, 0x1001u), "Absolute Radio", "Coldplay", "Clocks"},
+            {makeSelectorDab(/* sidExt= */ 0xA000000001u, /* ensemble= */ 0x0001u,
+                /* freq= */ 225648), "BBC Radio 1", "Khalid", "Talk"},
+            {makeSelectorDab(/* sidExt= */ 0xB000000001u, /* ensemble= */ 0x1001u,
+                /* freq= */ 222064), "Classic FM", "Jean Sibelius", "Andante Festivo"},
+            {makeSelectorDab(/* sidExt= */ 0xB000000002u, /* ensemble= */ 0x1002u,
+                /* freq= */ 227360), "Absolute Radio", "Coldplay", "Clocks"},
+            {makeSelectorDab(/* sidExt= */ 0xB000000002u, /* ensemble= */ 0x1002u,
+                /* freq= */ 222064), "Absolute Radio", "Coldplay", "Clocks"},
         });
     // clang-format on
     return dabRadioMock;
diff --git a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp
index 5a56846..356673f 100644
--- a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp
+++ b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp
@@ -46,6 +46,7 @@
 
 using ::aidl::android::hardware::broadcastradio::utils::makeIdentifier;
 using ::aidl::android::hardware::broadcastradio::utils::makeSelectorAmfm;
+using ::aidl::android::hardware::broadcastradio::utils::makeSelectorDab;
 using ::aidl::android::hardware::broadcastradio::utils::resultToInt;
 using ::ndk::ScopedAStatus;
 using ::ndk::SharedRefBase;
@@ -110,8 +111,7 @@
 class TunerCallbackMock : public BnTunerCallback {
   public:
     TunerCallbackMock();
-
-    MOCK_METHOD2(onTuneFailed, ScopedAStatus(Result, const ProgramSelector&));
+    ScopedAStatus onTuneFailed(Result result, const ProgramSelector& selector) override;
     MOCK_TIMEOUT_METHOD1(onCurrentProgramInfoChangedMock, ScopedAStatus(const ProgramInfo&));
     ScopedAStatus onCurrentProgramInfoChanged(const ProgramInfo& info) override;
     ScopedAStatus onProgramListUpdated(const ProgramListChunk& chunk) override;
@@ -154,6 +154,12 @@
     EXPECT_CALL(*this, onAntennaStateChange(false)).Times(0);
 }
 
+ScopedAStatus TunerCallbackMock::onTuneFailed(Result result, const ProgramSelector& selector) {
+    LOG(DEBUG) << "Tune failed for selector" << selector.toString();
+    EXPECT_TRUE(result == Result::CANCELED);
+    return ndk::ScopedAStatus::ok();
+}
+
 ScopedAStatus TunerCallbackMock::onCurrentProgramInfoChanged(const ProgramInfo& info) {
     for (const auto& id : info.selector) {
         EXPECT_NE(id.type, IdentifierType::INVALID);
@@ -175,7 +181,7 @@
     IdentifierType physically = info.physicallyTunedTo.type;
     // ditto (see "logically" above)
     EXPECT_TRUE(physically == IdentifierType::AMFM_FREQUENCY_KHZ ||
-                physically == IdentifierType::DAB_ENSEMBLE ||
+                physically == IdentifierType::DAB_FREQUENCY_KHZ ||
                 physically == IdentifierType::DRMO_FREQUENCY_KHZ ||
                 physically == IdentifierType::SXM_CHANNEL ||
                 (physically >= IdentifierType::VENDOR_START &&
@@ -593,10 +599,43 @@
     ASSERT_TRUE(halResult.isOk());
     ASSERT_NE(config.size(), 0U);
 
-    // TODO(245787803): use a DAB frequency that can actually be tuned to.
+    auto programList = getProgramList();
+
+    if (!programList) {
+        printSkipped("Empty DAB station list, tune cannot be performed");
+        return;
+    }
+
     ProgramSelector sel = {};
-    int64_t freq = config[config.size() / 2].frequencyKhz;
-    sel.primaryId = makeIdentifier(IdentifierType::DAB_FREQUENCY_KHZ, freq);
+    uint64_t freq = 0;
+    bool dabStationPresent = false;
+    for (auto&& programInfo : *programList) {
+        if (!utils::hasId(programInfo.selector, IdentifierType::DAB_FREQUENCY_KHZ)) {
+            continue;
+        }
+        for (auto&& config_entry : config) {
+            if (config_entry.frequencyKhz ==
+                utils::getId(programInfo.selector, IdentifierType::DAB_FREQUENCY_KHZ, 0)) {
+                freq = config_entry.frequencyKhz;
+                break;
+            }
+        }
+        // Do not trigger a tune request if the programList entry does not contain
+        // a valid DAB frequency.
+        if (freq == 0) {
+            continue;
+        }
+        int64_t dabSidExt = utils::getId(programInfo.selector, IdentifierType::DAB_SID_EXT, 0);
+        int64_t dabEns = utils::getId(programInfo.selector, IdentifierType::DAB_ENSEMBLE, 0);
+        sel = makeSelectorDab(dabSidExt, (int32_t)dabEns, freq);
+        dabStationPresent = true;
+        break;
+    }
+
+    if (!dabStationPresent) {
+        printSkipped("No DAB stations in the list, tune cannot be performed");
+        return;
+    }
 
     // try tuning
     ProgramInfo infoCb = {};
@@ -623,7 +662,6 @@
     vector<int> freqs = bcutils::getAllIds(infoCb.selector, IdentifierType::DAB_FREQUENCY_KHZ);
     EXPECT_NE(freqs.end(), find(freqs.begin(), freqs.end(), freq))
             << "DAB freq " << freq << " kHz is not sent back by callback.";
-    ;
 }
 
 /**
diff --git a/broadcastradio/common/utilsaidl/Utils.cpp b/broadcastradio/common/utilsaidl/Utils.cpp
index 52c7b40..ad82366 100644
--- a/broadcastradio/common/utilsaidl/Utils.cpp
+++ b/broadcastradio/common/utilsaidl/Utils.cpp
@@ -136,7 +136,9 @@
             return getHdSubchannel(b) == 0 &&
                    haveEqualIds(a, b, IdentifierType::AMFM_FREQUENCY_KHZ);
         case IdentifierType::DAB_SID_EXT:
-            return haveEqualIds(a, b, IdentifierType::DAB_SID_EXT);
+            return haveEqualIds(a, b, IdentifierType::DAB_SID_EXT) &&
+                   haveEqualIds(a, b, IdentifierType::DAB_ENSEMBLE) &&
+                   haveEqualIds(a, b, IdentifierType::DAB_FREQUENCY_KHZ);
         case IdentifierType::DRMO_SERVICE_ID:
             return haveEqualIds(a, b, IdentifierType::DRMO_SERVICE_ID);
         case IdentifierType::SXM_SERVICE_ID:
@@ -241,8 +243,8 @@
             break;
         }
         case IdentifierType::DAB_SID_EXT: {
-            int64_t sid = val & 0xFFFF;  // 16bit
-            val >>= 16;
+            int64_t sid = val & 0xFFFFFFFF;  // 32bit
+            val >>= 32;
             int64_t ecc = val & 0xFF;  // 8bit
             expect(sid != 0u, "DAB SId != 0");
             expect(ecc >= 0xA0u && ecc <= 0xF6u, "Invalid ECC, see ETSI TS 101 756 V2.1.1");
@@ -277,13 +279,35 @@
 }
 
 bool isValid(const ProgramSelector& sel) {
-    // iterate through primaryId and secondaryIds
-    for (auto it = begin(sel); it != end(sel); it++) {
+    if (sel.primaryId.type != IdentifierType::AMFM_FREQUENCY_KHZ &&
+        sel.primaryId.type != IdentifierType::RDS_PI &&
+        sel.primaryId.type != IdentifierType::HD_STATION_ID_EXT &&
+        sel.primaryId.type != IdentifierType::DAB_SID_EXT &&
+        sel.primaryId.type != IdentifierType::DRMO_SERVICE_ID &&
+        sel.primaryId.type != IdentifierType::SXM_SERVICE_ID &&
+        (sel.primaryId.type < IdentifierType::VENDOR_START ||
+         sel.primaryId.type > IdentifierType::VENDOR_END)) {
+        return false;
+    }
+    if (!isValid(sel.primaryId)) {
+        return false;
+    }
+
+    bool isDab = sel.primaryId.type == IdentifierType::DAB_SID_EXT;
+    bool hasDabEnsemble = false;
+    bool hasDabFrequency = false;
+    for (auto it = sel.secondaryIds.begin(); it != sel.secondaryIds.end(); it++) {
         if (!isValid(*it)) {
             return false;
         }
+        if (isDab && it->type == IdentifierType::DAB_ENSEMBLE) {
+            hasDabEnsemble = true;
+        }
+        if (isDab && it->type == IdentifierType::DAB_FREQUENCY_KHZ) {
+            hasDabFrequency = true;
+        }
     }
-    return true;
+    return !isDab || (hasDabEnsemble && hasDabFrequency);
 }
 
 ProgramIdentifier makeIdentifier(IdentifierType type, int64_t value) {
@@ -296,16 +320,12 @@
     return sel;
 }
 
-ProgramSelector makeSelectorDab(int32_t sidExt, int32_t ensemble) {
+ProgramSelector makeSelectorDab(int64_t sidExt, int32_t ensemble, int64_t freq) {
     ProgramSelector sel = {};
-    // TODO(243686545): Have a helper function to create the sidExt instead of
-    // passing the whole identifier here. Something like makeDabSidExt.
     sel.primaryId = makeIdentifier(IdentifierType::DAB_SID_EXT, sidExt);
     vector<ProgramIdentifier> secondaryIds = {
             makeIdentifier(IdentifierType::DAB_ENSEMBLE, ensemble),
-            // TODO(243686545): Include frequency here when the helper method to
-            // translate between ensemble and frequency is implemented.
-    };
+            makeIdentifier(IdentifierType::DAB_FREQUENCY_KHZ, freq)};
     sel.secondaryIds = std::move(secondaryIds);
     return sel;
 }
@@ -330,10 +350,6 @@
         }
     }
 
-    if (!filter.includeCategories && sel.primaryId.type == IdentifierType::DAB_ENSEMBLE) {
-        return false;
-    }
-
     return true;
 }
 
diff --git a/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h b/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h
index 8ea6319..beebd03 100644
--- a/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h
+++ b/broadcastradio/common/utilsaidl/include/broadcastradio-utils-aidl/Utils.h
@@ -138,7 +138,7 @@
 
 ProgramIdentifier makeIdentifier(IdentifierType type, int64_t value);
 ProgramSelector makeSelectorAmfm(int32_t frequency);
-ProgramSelector makeSelectorDab(int32_t sidExt, int32_t ensemble);
+ProgramSelector makeSelectorDab(int64_t sidExt, int32_t ensemble, int64_t freq);
 
 bool satisfies(const ProgramFilter& filter, const ProgramSelector& sel);