Fix remaining instrumentation tests.
Bug: 69958777
Test: VTS, Instrumentation
Change-Id: I98956ea4b1d2953b9159fb7d53ce61e57f80a90f
diff --git a/broadcastradio/2.0/default/BroadcastRadio.cpp b/broadcastradio/2.0/default/BroadcastRadio.cpp
index aa5afad..0148fec 100644
--- a/broadcastradio/2.0/default/BroadcastRadio.cpp
+++ b/broadcastradio/2.0/default/BroadcastRadio.cpp
@@ -112,6 +112,12 @@
Return<void> BroadcastRadio::openSession(const sp<ITunerCallback>& callback,
openSession_cb _hidl_cb) {
ALOGV("%s", __func__);
+
+ /* For the needs of default implementation it's fine to instantiate new session object
+ * out of the lock scope. If your implementation needs it, use reentrant lock.
+ */
+ sp<TunerSession> newSession = new TunerSession(*this, callback);
+
lock_guard<mutex> lk(mMut);
auto oldSession = mSession.promote();
@@ -121,7 +127,6 @@
mSession = nullptr;
}
- sp<TunerSession> newSession = new TunerSession(*this, callback);
mSession = newSession;
_hidl_cb(Result::OK, newSession);
diff --git a/broadcastradio/2.0/default/TunerSession.cpp b/broadcastradio/2.0/default/TunerSession.cpp
index 3166d86..ba8285f 100644
--- a/broadcastradio/2.0/default/TunerSession.cpp
+++ b/broadcastradio/2.0/default/TunerSession.cpp
@@ -50,7 +50,12 @@
} // namespace delay
TunerSession::TunerSession(BroadcastRadio& module, const sp<ITunerCallback>& callback)
- : mCallback(callback), mModule(module) {}
+ : mCallback(callback), mModule(module) {
+ auto&& ranges = module.getAmFmConfig().ranges;
+ if (ranges.size() > 0) {
+ tuneInternalLocked(utils::make_selector_amfm(ranges[0].lowerBound));
+ }
+}
// makes ProgramInfo that points to no program
static ProgramInfo makeDummyProgramInfo(const ProgramSelector& selector) {
@@ -63,6 +68,8 @@
}
void TunerSession::tuneInternalLocked(const ProgramSelector& sel) {
+ ALOGV("%s(%s)", __func__, toString(sel).c_str());
+
VirtualProgram virtualProgram;
ProgramInfo programInfo;
if (virtualRadio().getProgram(sel, virtualProgram)) {
@@ -100,6 +107,8 @@
return Result::INVALID_ARGUMENTS;
}
+ cancelLocked();
+
mIsTuneCompleted = false;
auto task = [this, sel]() {
lock_guard<mutex> lk(mMut);
@@ -115,6 +124,8 @@
lock_guard<mutex> lk(mMut);
if (mIsClosed) return Result::INVALID_STATE;
+ cancelLocked();
+
auto list = virtualRadio().getProgramList();
if (list.empty()) {
@@ -166,13 +177,13 @@
lock_guard<mutex> lk(mMut);
if (mIsClosed) return Result::INVALID_STATE;
+ cancelLocked();
+
if (!utils::hasId(mCurrentProgram, IdentifierType::AMFM_FREQUENCY)) {
ALOGE("Can't step in anything else than AM/FM");
return Result::NOT_SUPPORTED;
}
- mIsTuneCompleted = false;
-
auto stepTo = utils::getId(mCurrentProgram, IdentifierType::AMFM_FREQUENCY);
auto range = getAmFmRangeLocked();
if (!range) {
@@ -188,6 +199,7 @@
if (stepTo > range->upperBound) stepTo = range->lowerBound;
if (stepTo < range->lowerBound) stepTo = range->upperBound;
+ mIsTuneCompleted = false;
auto task = [this, stepTo]() {
ALOGI("Performing step to %s", std::to_string(stepTo).c_str());
@@ -200,12 +212,22 @@
return Result::OK;
}
+void TunerSession::cancelLocked() {
+ ALOGV("%s", __func__);
+
+ mThread.cancelAll();
+ if (utils::getType(mCurrentProgram.primaryId) != IdentifierType::INVALID) {
+ mIsTuneCompleted = true;
+ }
+}
+
Return<void> TunerSession::cancel() {
ALOGV("%s", __func__);
lock_guard<mutex> lk(mMut);
if (mIsClosed) return {};
- mThread.cancelAll();
+ cancelLocked();
+
return {};
}
@@ -281,7 +303,10 @@
}
std::optional<AmFmBandRange> TunerSession::getAmFmRangeLocked() const {
- if (!mIsTuneCompleted) return {};
+ if (!mIsTuneCompleted) {
+ ALOGW("tune operation in process");
+ return {};
+ }
if (!utils::hasId(mCurrentProgram, IdentifierType::AMFM_FREQUENCY)) return {};
auto freq = utils::getId(mCurrentProgram, IdentifierType::AMFM_FREQUENCY);
diff --git a/broadcastradio/2.0/default/TunerSession.h b/broadcastradio/2.0/default/TunerSession.h
index 5d27b1e..4130341 100644
--- a/broadcastradio/2.0/default/TunerSession.h
+++ b/broadcastradio/2.0/default/TunerSession.h
@@ -63,6 +63,7 @@
bool mIsTuneCompleted = false;
ProgramSelector mCurrentProgram = {};
+ void cancelLocked();
void tuneInternalLocked(const ProgramSelector& sel);
const VirtualRadio& virtualRadio() const;
const BroadcastRadio& module() const;
diff --git a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
index 8d9d622..feb62c6 100644
--- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
+++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
@@ -15,6 +15,8 @@
*/
#define LOG_TAG "BcRadio.vts"
+#define LOG_NDEBUG 0
+#define EGMOCK_VERBOSE 1
#include <VtsHalHidlTargetTestBase.h>
#include <android-base/logging.h>
@@ -113,7 +115,15 @@
std::cout << "[ SKIPPED ] " << msg << std::endl;
}
+MATCHER_P(InfoHasId, id,
+ std::string(negation ? "does not contain" : "contains") + " " + toString(id)) {
+ auto ids = utils::getAllIds(arg.selector, utils::getType(id));
+ return ids.end() != find(ids.begin(), ids.end(), id.value);
+}
+
TunerCallbackMock::TunerCallbackMock() {
+ EXPECT_TIMEOUT_CALL(*this, onCurrentProgramInfoChanged, _).Times(AnyNumber());
+
// we expect the antenna is connected through the whole test
EXPECT_CALL(*this, onAntennaStateChange(false)).Times(0);
}
@@ -362,9 +372,19 @@
uint64_t freq = 100100; // 100.1 FM
auto sel = make_selector_amfm(freq);
+ /* TODO(b/69958777): there is a race condition between tune() and onCurrentProgramInfoChanged
+ * callback setting infoCb, because egmock cannot distinguish calls with different matchers
+ * (there is one here and one in callback constructor).
+ *
+ * This sleep workaround will fix default implementation, but the real HW tests will still be
+ * flaky. We probably need to implement egmock alternative based on actions.
+ */
+ std::this_thread::sleep_for(100ms);
+
// try tuning
ProgramInfo infoCb = {};
- EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChanged, _)
+ EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChanged,
+ InfoHasId(utils::make_identifier(IdentifierType::AMFM_FREQUENCY, freq)))
.Times(AnyNumber())
.WillOnce(DoAll(SaveArg<0>(&infoCb), testing::Return(ByMove(Void()))));
auto result = mSession->tune(sel);
@@ -379,6 +399,8 @@
EXPECT_EQ(Result::OK, result);
EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChanged, timeout::tune);
+ ALOGD("current program info: %s", toString(infoCb).c_str());
+
// it should tune exactly to what was requested
auto freqs = utils::getAllIds(infoCb.selector, IdentifierType::AMFM_FREQUENCY);
EXPECT_NE(freqs.end(), find(freqs.begin(), freqs.end(), freq));
@@ -442,6 +464,9 @@
TEST_F(BroadcastRadioHalTest, Scan) {
ASSERT_TRUE(openSession());
+ // TODO(b/69958777): see FmTune workaround
+ std::this_thread::sleep_for(100ms);
+
EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChanged, _);
auto result = mSession->scan(true /* up */, true /* skip subchannel */);
EXPECT_EQ(Result::OK, result);
@@ -464,9 +489,15 @@
TEST_F(BroadcastRadioHalTest, Step) {
ASSERT_TRUE(openSession());
+ // TODO(b/69958777): see FmTune workaround
+ std::this_thread::sleep_for(100ms);
+
EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChanged, _).Times(AnyNumber());
auto result = mSession->step(true /* up */);
- if (result == Result::NOT_SUPPORTED) return;
+ if (result == Result::NOT_SUPPORTED) {
+ printSkipped("step not supported");
+ return;
+ }
EXPECT_EQ(Result::OK, result);
EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChanged, timeout::tune);