Fix remaining broadcastradio 1.1 VTS TODOs.
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
diff --git a/broadcastradio/1.1/default/Tuner.cpp b/broadcastradio/1.1/default/Tuner.cpp
index 0a45208..133593e 100644
--- a/broadcastradio/1.1/default/Tuner.cpp
+++ b/broadcastradio/1.1/default/Tuner.cpp
@@ -20,7 +20,7 @@
#include "BroadcastRadio.h"
#include "Tuner.h"
-#include <Utils.h>
+#include <broadcastradio-utils/Utils.h>
#include <log/log.h>
namespace android {
diff --git a/broadcastradio/1.1/default/Tuner.h b/broadcastradio/1.1/default/Tuner.h
index 2ab4f40..2222e5a 100644
--- a/broadcastradio/1.1/default/Tuner.h
+++ b/broadcastradio/1.1/default/Tuner.h
@@ -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 {
diff --git a/broadcastradio/1.1/default/VirtualProgram.cpp b/broadcastradio/1.1/default/VirtualProgram.cpp
index 1f3b693..4c6b3b1 100644
--- a/broadcastradio/1.1/default/VirtualProgram.cpp
+++ b/broadcastradio/1.1/default/VirtualProgram.cpp
@@ -15,7 +15,7 @@
*/
#include "VirtualProgram.h"
-#include <Utils.h>
+#include <broadcastradio-utils/Utils.h>
#include "resources.h"
diff --git a/broadcastradio/1.1/default/VirtualRadio.cpp b/broadcastradio/1.1/default/VirtualRadio.cpp
index 89b2b0a..692e7bc 100644
--- a/broadcastradio/1.1/default/VirtualRadio.cpp
+++ b/broadcastradio/1.1/default/VirtualRadio.cpp
@@ -15,7 +15,7 @@
*/
#include "VirtualRadio.h"
-#include <Utils.h>
+#include <broadcastradio-utils/Utils.h>
namespace android {
namespace hardware {
diff --git a/broadcastradio/1.1/tests/WorkerThread_test.cpp b/broadcastradio/1.1/tests/WorkerThread_test.cpp
index a0e0ebb..ed36de3 100644
--- a/broadcastradio/1.1/tests/WorkerThread_test.cpp
+++ b/broadcastradio/1.1/tests/WorkerThread_test.cpp
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-#include <WorkerThread.h>
+#include <broadcastradio-utils/WorkerThread.h>
#include <gtest/gtest.h>
namespace {
diff --git a/broadcastradio/1.1/utils/Android.bp b/broadcastradio/1.1/utils/Android.bp
index 73c6680..e80d133 100644
--- a/broadcastradio/1.1/utils/Android.bp
+++ b/broadcastradio/1.1/utils/Android.bp
@@ -27,7 +27,7 @@
"Utils.cpp",
"WorkerThread.cpp",
],
- export_include_dirs: ["."],
+ export_include_dirs: ["include"],
shared_libs: [
"android.hardware.broadcastradio@1.1",
],
diff --git a/broadcastradio/1.1/utils/Utils.cpp b/broadcastradio/1.1/utils/Utils.cpp
index f8a4479..8bb7691 100644
--- a/broadcastradio/1.1/utils/Utils.cpp
+++ b/broadcastradio/1.1/utils/Utils.cpp
@@ -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 @@
} // 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
diff --git a/broadcastradio/1.1/utils/WorkerThread.cpp b/broadcastradio/1.1/utils/WorkerThread.cpp
index a3ceaa1..bfcbb39 100644
--- a/broadcastradio/1.1/utils/WorkerThread.cpp
+++ b/broadcastradio/1.1/utils/WorkerThread.cpp
@@ -17,7 +17,7 @@
#define LOG_TAG "WorkerThread"
//#define LOG_NDEBUG 0
-#include "WorkerThread.h"
+#include <broadcastradio-utils/WorkerThread.h>
#include <log/log.h>
diff --git a/broadcastradio/1.1/utils/Utils.h b/broadcastradio/1.1/utils/include/broadcastradio-utils/Utils.h
similarity index 95%
rename from broadcastradio/1.1/utils/Utils.h
rename to broadcastradio/1.1/utils/include/broadcastradio-utils/Utils.h
index cd86ffa..a7da9fe 100644
--- a/broadcastradio/1.1/utils/Utils.h
+++ b/broadcastradio/1.1/utils/include/broadcastradio-utils/Utils.h
@@ -66,6 +66,13 @@
} // 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
diff --git a/broadcastradio/1.1/utils/WorkerThread.h b/broadcastradio/1.1/utils/include/broadcastradio-utils/WorkerThread.h
similarity index 100%
rename from broadcastradio/1.1/utils/WorkerThread.h
rename to broadcastradio/1.1/utils/include/broadcastradio-utils/WorkerThread.h
diff --git a/broadcastradio/1.1/vts/functional/Android.bp b/broadcastradio/1.1/vts/functional/Android.bp
index c136019..6e5c84c 100644
--- a/broadcastradio/1.1/vts/functional/Android.bp
+++ b/broadcastradio/1.1/vts/functional/Android.bp
@@ -31,7 +31,8 @@
],
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 @@
"-g",
],
}
-
-cc_library_static {
- name: "broadcastradio-vts-call-barrier",
- srcs: [
- "call-barrier.cpp",
- ],
- export_include_dirs: ["."],
- cflags: [
- "-Wall",
- "-Wextra",
- "-Werror",
- ],
-}
diff --git a/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp b/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp
index d20452b..c6bc344 100644
--- a/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp
+++ b/broadcastradio/1.1/vts/functional/VtsHalBroadcastradioV1_1TargetTest.cpp
@@ -17,8 +17,15 @@
#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>
@@ -27,14 +34,6 @@
#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::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 @@
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 @@
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::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 @@
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 @@
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 @@
EXPECT_EQ(ProgramListResult::OK, getListResult);
}
- if (isListEmpty) {
- printSkipped("Program list is empty.");
- return false;
- }
- return true;
+ return !isListEmpty;
}
/**
@@ -263,13 +299,13 @@
*/
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, 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, 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, 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
index 0000000..0c7e2a4
--- /dev/null
+++ b/broadcastradio/1.1/vts/utils/Android.bp
@@ -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",
+ ],
+}
diff --git a/broadcastradio/1.1/vts/functional/call-barrier.cpp b/broadcastradio/1.1/vts/utils/call-barrier.cpp
similarity index 95%
rename from broadcastradio/1.1/vts/functional/call-barrier.cpp
rename to broadcastradio/1.1/vts/utils/call-barrier.cpp
index fede297..d8c4716 100644
--- a/broadcastradio/1.1/vts/functional/call-barrier.cpp
+++ b/broadcastradio/1.1/vts/utils/call-barrier.cpp
@@ -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 {
diff --git a/broadcastradio/1.1/vts/functional/call-barrier.h b/broadcastradio/1.1/vts/utils/include/broadcastradio-vts-utils/call-barrier.h
similarity index 100%
rename from broadcastradio/1.1/vts/functional/call-barrier.h
rename to broadcastradio/1.1/vts/utils/include/broadcastradio-vts-utils/call-barrier.h
diff --git a/broadcastradio/1.1/vts/functional/mock-timeout.h b/broadcastradio/1.1/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h
similarity index 100%
rename from broadcastradio/1.1/vts/functional/mock-timeout.h
rename to broadcastradio/1.1/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h