OSDN Git Service

Fix remaining broadcastradio 1.1 VTS TODOs.
authorTomasz Wasilczyk <twasilczyk@google.com>
Tue, 25 Jul 2017 17:01:17 +0000 (10:01 -0700)
committerTomasz Wasilczyk <twasilczyk@google.com>
Wed, 26 Jul 2017 21:41:35 +0000 (14:41 -0700)
This includes:
- cover all AM/FM bands, not just first one
- fix flakiness on late callback dereference
- fix 1.0 tuneComplete check
- move utils includes into separate subdirectories

Bug: b/36864490
Test: VTS
Change-Id: I6e2427ac29abd6278c9783cf83b4df05195ac7ea

17 files changed:
broadcastradio/1.1/default/Tuner.cpp
broadcastradio/1.1/default/Tuner.h
broadcastradio/1.1/default/VirtualProgram.cpp
broadcastradio/1.1/default/VirtualRadio.cpp
broadcastradio/1.1/tests/WorkerThread_test.cpp
broadcastradio/1.1/utils/Android.bp
broadcastradio/1.1/utils/Utils.cpp
broadcastradio/1.1/utils/WorkerThread.cpp
broadcastradio/1.1/utils/include/broadcastradio-utils/Utils.h [moved from broadcastradio/1.1/utils/Utils.h with 96% similarity]
broadcastradio/1.1/utils/include/broadcastradio-utils/WorkerThread.h [moved from broadcastradio/1.1/utils/WorkerThread.h with 100% similarity]
broadcastradio/1.1/vts/functional/Android.bp
broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp
broadcastradio/1.1/vts/utils/Android.bp [new file with mode: 0644]
broadcastradio/1.1/vts/utils/call-barrier.cpp [moved from broadcastradio/1.1/vts/functional/call-barrier.cpp with 95% similarity]
broadcastradio/1.1/vts/utils/include/broadcastradio-vts-utils/call-barrier.h [moved from broadcastradio/1.1/vts/functional/call-barrier.h with 100% similarity]
broadcastradio/1.1/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h [moved from broadcastradio/1.1/vts/functional/mock-timeout.h with 100% similarity]
broadcastradio/Android.bp

index 0a45208..133593e 100644 (file)
@@ -20,7 +20,7 @@
 #include "BroadcastRadio.h"
 #include "Tuner.h"
 
-#include <Utils.h>
+#include <broadcastradio-utils/Utils.h>
 #include <log/log.h>
 
 namespace android {
index 2ab4f40..2222e5a 100644 (file)
@@ -18,9 +18,9 @@
 
 #include "VirtualRadio.h"
 
-#include <WorkerThread.h>
 #include <android/hardware/broadcastradio/1.1/ITuner.h>
 #include <android/hardware/broadcastradio/1.1/ITunerCallback.h>
+#include <broadcastradio-utils/WorkerThread.h>
 
 namespace android {
 namespace hardware {
index 1f3b693..4c6b3b1 100644 (file)
@@ -15,7 +15,7 @@
  */
 #include "VirtualProgram.h"
 
-#include <Utils.h>
+#include <broadcastradio-utils/Utils.h>
 
 #include "resources.h"
 
index 89b2b0a..692e7bc 100644 (file)
@@ -15,7 +15,7 @@
  */
 #include "VirtualRadio.h"
 
-#include <Utils.h>
+#include <broadcastradio-utils/Utils.h>
 
 namespace android {
 namespace hardware {
index a0e0ebb..ed36de3 100644 (file)
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#include <WorkerThread.h>
+#include <broadcastradio-utils/WorkerThread.h>
 #include <gtest/gtest.h>
 
 namespace {
index 73c6680..e80d133 100644 (file)
@@ -27,7 +27,7 @@ cc_library_static {
         "Utils.cpp",
         "WorkerThread.cpp",
     ],
-    export_include_dirs: ["."],
+    export_include_dirs: ["include"],
     shared_libs: [
         "android.hardware.broadcastradio@1.1",
     ],
index f8a4479..8bb7691 100644 (file)
@@ -16,7 +16,7 @@
 #define LOG_TAG "BroadcastRadioDefault.utils"
 //#define LOG_NDEBUG 0
 
-#include "Utils.h"
+#include <broadcastradio-utils/Utils.h>
 
 #include <log/log.h>
 
@@ -208,6 +208,26 @@ bool isDigital(const ProgramSelector& sel) {
 
 }  // namespace utils
 }  // namespace V1_1
+
+namespace V1_0 {
+
+bool operator==(const BandConfig& l, const BandConfig& r) {
+    if (l.type != r.type) return false;
+    if (l.antennaConnected != r.antennaConnected) return false;
+    if (l.lowerLimit != r.lowerLimit) return false;
+    if (l.upperLimit != r.upperLimit) return false;
+    if (l.spacings != r.spacings) return false;
+    if (l.type == Band::AM || l.type == Band::AM_HD) {
+        return l.ext.am == r.ext.am;
+    } else if (l.type == Band::FM || l.type == Band::FM_HD) {
+        return l.ext.fm == r.ext.fm;
+    } else {
+        ALOGW("Unsupported band config type: %s", toString(l.type).c_str());
+        return false;
+    }
+}
+
+}  // namespace V1_0
 }  // namespace broadcastradio
 }  // namespace hardware
 }  // namespace android
index a3ceaa1..bfcbb39 100644 (file)
@@ -17,7 +17,7 @@
 #define LOG_TAG "WorkerThread"
 //#define LOG_NDEBUG 0
 
-#include "WorkerThread.h"
+#include <broadcastradio-utils/WorkerThread.h>
 
 #include <log/log.h>
 
@@ -66,6 +66,13 @@ bool isDigital(const ProgramSelector& sel);
 
 }  // namespace utils
 }  // namespace V1_1
+
+namespace V1_0 {
+
+bool operator==(const BandConfig& l, const BandConfig& r);
+
+}  // namespace V1_0
+
 }  // namespace broadcastradio
 }  // namespace hardware
 }  // namespace android
index c136019..6e5c84c 100644 (file)
@@ -31,7 +31,8 @@ cc_test {
     ],
     static_libs: [
         "VtsHalHidlTargetTestBase",
-        "broadcastradio-vts-call-barrier",
+        "android.hardware.broadcastradio@1.1-utils-lib",
+        "android.hardware.broadcastradio@1.1-vts-utils-lib",
         "libgmock",
     ],
     cflags: [
@@ -40,16 +41,3 @@ cc_test {
         "-g",
     ],
 }
-
-cc_library_static {
-    name: "broadcastradio-vts-call-barrier",
-    srcs: [
-        "call-barrier.cpp",
-    ],
-    export_include_dirs: ["."],
-    cflags: [
-        "-Wall",
-        "-Wextra",
-        "-Werror",
-    ],
-}
index d20452b..c6bc344 100644 (file)
 #define LOG_TAG "broadcastradio.vts"
 
 #include <VtsHalHidlTargetTestBase.h>
+#include <android/hardware/broadcastradio/1.1/IBroadcastRadio.h>
+#include <android/hardware/broadcastradio/1.1/IBroadcastRadioFactory.h>
+#include <android/hardware/broadcastradio/1.1/ITuner.h>
+#include <android/hardware/broadcastradio/1.1/ITunerCallback.h>
+#include <android/hardware/broadcastradio/1.1/types.h>
 #include <android-base/logging.h>
-#include <call-barrier.h>
+#include <broadcastradio-utils/Utils.h>
+#include <broadcastradio-vts-utils/call-barrier.h>
+#include <broadcastradio-vts-utils/mock-timeout.h>
 #include <cutils/native_handle.h>
 #include <cutils/properties.h>
 #include <gmock/gmock.h>
 
 #include <chrono>
 
-#include <android/hardware/broadcastradio/1.1/IBroadcastRadio.h>
-#include <android/hardware/broadcastradio/1.1/IBroadcastRadioFactory.h>
-#include <android/hardware/broadcastradio/1.1/ITuner.h>
-#include <android/hardware/broadcastradio/1.1/ITunerCallback.h>
-#include <android/hardware/broadcastradio/1.1/types.h>
-
-#include "mock-timeout.h"
-
 namespace android {
 namespace hardware {
 namespace broadcastradio {
@@ -57,6 +56,9 @@ using V1_0::MetaData;
 using V1_0::MetadataKey;
 using V1_0::MetadataType;
 
+using std::chrono::steady_clock;
+using std::this_thread::sleep_for;
+
 static constexpr auto kConfigTimeout = 10s;
 static constexpr auto kConnectModuleTimeout = 1s;
 static constexpr auto kTuneTimeout = 30s;
@@ -91,10 +93,8 @@ class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase,
     virtual void SetUp() override;
     virtual void TearDown() override;
 
-    // TODO(b/36864490): check all bands for good test conditions (ie. FM is more likely to have
-    // any stations on the list, so don't pick AM blindly).
-    bool openTuner(unsigned band);
-    const BandConfig& getBand(unsigned idx);
+    bool openTuner();
+    bool nextBand();
     bool getProgramList(std::function<void(const hidl_vec<ProgramInfo>& list)> cb);
 
     Class radioClass;
@@ -105,9 +105,33 @@ class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase,
     sp<TunerCallbackMock> mCallback = new TunerCallbackMock();
 
    private:
+    const BandConfig& getBand(unsigned idx);
+
+    unsigned currentBandIndex = 0;
     hidl_vec<BandConfig> mBands;
 };
 
+/**
+ * Clears strong pointer and waits until the object gets destroyed.
+ *
+ * @param ptr The pointer to get cleared.
+ * @param timeout Time to wait for other references.
+ */
+template <typename T>
+static void clearAndWait(sp<T>& ptr, std::chrono::milliseconds timeout) {
+    wp<T> wptr = ptr;
+    ptr.clear();
+    auto limit = steady_clock::now() + timeout;
+    while (wptr.promote() != nullptr) {
+        constexpr auto step = 10ms;
+        if (steady_clock::now() + step > limit) {
+            FAIL() << "Pointer was not released within timeout";
+            break;
+        }
+        sleep_for(step);
+    }
+}
+
 void BroadcastRadioHalTest::SetUp() {
     radioClass = GetParam();
 
@@ -153,10 +177,10 @@ void BroadcastRadioHalTest::SetUp() {
 void BroadcastRadioHalTest::TearDown() {
     mTuner.clear();
     mRadioModule.clear();
-    // TODO(b/36864490): wait (with timeout) until mCallback has only one reference
+    clearAndWait(mCallback, 1s);
 }
 
-bool BroadcastRadioHalTest::openTuner(unsigned band) {
+bool BroadcastRadioHalTest::openTuner() {
     EXPECT_EQ(nullptr, mTuner.get());
 
     if (radioClass == Class::AM_FM) {
@@ -169,7 +193,8 @@ bool BroadcastRadioHalTest::openTuner(unsigned band) {
         if (result != Result::OK) return;
         mTuner = ITuner::castFrom(tuner);
     };
-    auto hidlResult = mRadioModule->openTuner(getBand(band), true, mCallback, openCb);
+    currentBandIndex = 0;
+    auto hidlResult = mRadioModule->openTuner(getBand(0), true, mCallback, openCb);
 
     EXPECT_TRUE(hidlResult.isOk());
     EXPECT_EQ(Result::OK, halResult);
@@ -210,6 +235,21 @@ const BandConfig& BroadcastRadioHalTest::getBand(unsigned idx) {
     return band;
 }
 
+bool BroadcastRadioHalTest::nextBand() {
+    if (currentBandIndex + 1 >= mBands.size()) return false;
+    currentBandIndex++;
+
+    BandConfig bandCb;
+    EXPECT_TIMEOUT_CALL(*mCallback, configChange, Result::OK, _)
+        .WillOnce(DoAll(SaveArg<1>(&bandCb), testing::Return(ByMove(Void()))));
+    auto hidlResult = mTuner->setConfiguration(getBand(currentBandIndex));
+    EXPECT_EQ(Result::OK, hidlResult);
+    EXPECT_TIMEOUT_CALL_WAIT(*mCallback, configChange, kConfigTimeout);
+    EXPECT_EQ(getBand(currentBandIndex), bandCb);
+
+    return true;
+}
+
 bool BroadcastRadioHalTest::getProgramList(
     std::function<void(const hidl_vec<ProgramInfo>& list)> cb) {
     ProgramListResult getListResult = ProgramListResult::NOT_INITIALIZED;
@@ -244,11 +284,7 @@ bool BroadcastRadioHalTest::getProgramList(
         EXPECT_EQ(ProgramListResult::OK, getListResult);
     }
 
-    if (isListEmpty) {
-        printSkipped("Program list is empty.");
-        return false;
-    }
-    return true;
+    return !isListEmpty;
 }
 
 /**
@@ -263,13 +299,13 @@ bool BroadcastRadioHalTest::getProgramList(
  */
 TEST_P(BroadcastRadioHalTest, OpenTunerTwice) {
     if (skipped) return;
-    ASSERT_TRUE(openTuner(0));
 
-    Result halResult = Result::NOT_INITIALIZED;
-    auto openCb = [&](Result result, const sp<V1_0::ITuner>&) { halResult = result; };
-    auto hidlResult = mRadioModule->openTuner(getBand(0), true, mCallback, openCb);
-    ASSERT_TRUE(hidlResult.isOk());
-    ASSERT_EQ(Result::OK, halResult);
+    ASSERT_TRUE(openTuner());
+
+    auto secondTuner = mTuner;
+    mTuner.clear();
+
+    ASSERT_TRUE(openTuner());
 }
 
 /**
@@ -283,17 +319,25 @@ TEST_P(BroadcastRadioHalTest, OpenTunerTwice) {
  */
 TEST_P(BroadcastRadioHalTest, TuneFromProgramList) {
     if (skipped) return;
-    ASSERT_TRUE(openTuner(0));
+    ASSERT_TRUE(openTuner());
 
     ProgramInfo firstProgram;
-    auto getCb = [&](const hidl_vec<ProgramInfo>& list) {
-        // don't copy the whole list out, it might be heavy
-        firstProgram = list[0];
-    };
-    if (!getProgramList(getCb)) return;
+    bool foundAny = false;
+    do {
+        auto getCb = [&](const hidl_vec<ProgramInfo>& list) {
+            // don't copy the whole list out, it might be heavy
+            firstProgram = list[0];
+        };
+        if (getProgramList(getCb)) foundAny = true;
+    } while (nextBand());
+    if (HasFailure()) return;
+    if (!foundAny) {
+        printSkipped("Program list is empty.");
+        return;
+    }
 
     ProgramSelector selCb;
-    EXPECT_CALL(*mCallback, tuneComplete(_, _));
+    EXPECT_CALL(*mCallback, tuneComplete(_, _)).Times(0);
     EXPECT_TIMEOUT_CALL(*mCallback, tuneComplete_1_1, Result::OK, _)
         .WillOnce(DoAll(SaveArg<1>(&selCb), testing::Return(ByMove(Void()))));
     auto tuneResult = mTuner->tune_1_1(firstProgram.selector);
@@ -304,7 +348,7 @@ TEST_P(BroadcastRadioHalTest, TuneFromProgramList) {
 
 TEST_P(BroadcastRadioHalTest, CancelAnnouncement) {
     if (skipped) return;
-    ASSERT_TRUE(openTuner(0));
+    ASSERT_TRUE(openTuner());
 
     auto hidlResult = mTuner->cancelAnnouncement();
     EXPECT_EQ(Result::OK, hidlResult);
@@ -336,23 +380,24 @@ TEST_P(BroadcastRadioHalTest, GetNoImage) {
  */
 TEST_P(BroadcastRadioHalTest, OobImagesOnly) {
     if (skipped) return;
-    ASSERT_TRUE(openTuner(0));
+    ASSERT_TRUE(openTuner());
 
     std::vector<int> imageIds;
 
-    ProgramInfo firstProgram;
-    auto getCb = [&](const hidl_vec<ProgramInfo>& list) {
-        for (auto&& program : list) {
-            for (auto&& entry : program.base.metadata) {
-                EXPECT_NE(MetadataType::RAW, entry.type);
-                if (entry.key != MetadataKey::ICON && entry.key != MetadataKey::ART) continue;
-                EXPECT_NE(0, entry.intValue);
-                EXPECT_EQ(0u, entry.rawValue.size());
-                if (entry.intValue != 0) imageIds.push_back(entry.intValue);
+    do {
+        auto getCb = [&](const hidl_vec<ProgramInfo>& list) {
+            for (auto&& program : list) {
+                for (auto&& entry : program.base.metadata) {
+                    EXPECT_NE(MetadataType::RAW, entry.type);
+                    if (entry.key != MetadataKey::ICON && entry.key != MetadataKey::ART) continue;
+                    EXPECT_NE(0, entry.intValue);
+                    EXPECT_EQ(0u, entry.rawValue.size());
+                    if (entry.intValue != 0) imageIds.push_back(entry.intValue);
+                }
             }
-        }
-    };
-    if (!getProgramList(getCb)) return;
+        };
+        getProgramList(getCb);
+    } while (nextBand());
 
     if (imageIds.size() == 0) {
         printSkipped("No images found");
diff --git a/broadcastradio/1.1/vts/utils/Android.bp b/broadcastradio/1.1/vts/utils/Android.bp
new file mode 100644 (file)
index 0000000..0c7e2a4
--- /dev/null
@@ -0,0 +1,28 @@
+//
+// 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.
+//
+
+cc_library_static {
+    name: "android.hardware.broadcastradio@1.1-vts-utils-lib",
+    srcs: [
+        "call-barrier.cpp",
+    ],
+    export_include_dirs: ["include"],
+    cflags: [
+        "-Wall",
+        "-Wextra",
+        "-Werror",
+    ],
+}
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "call-barrier.h"
+#include <broadcastradio-vts-utils/call-barrier.h>
 
 namespace android {
 namespace hardware {
index a5ad5e7..8c65bf6 100644 (file)
@@ -8,4 +8,5 @@ subdirs = [
     "1.1/tests",
     "1.1/utils",
     "1.1/vts/functional",
+    "1.1/vts/utils",
 ]