Merge "Address Broadcast Radio HAL review notes." into oc-mr1-dev

This commit is contained in:
Tomasz Wasilczyk
2017-08-03 16:52:31 +00:00
committed by Android (Google) Code Review
6 changed files with 57 additions and 41 deletions

View File

@@ -40,6 +40,19 @@ interface IBroadcastRadio extends @1.0::IBroadcastRadio {
* The data should be a valid PNG, JPEG, GIF or BMP file.
* Invalid format must be handled gracefully as if the image was missing.
*
* The image identifier may become invalid after some time from passing it
* with metadata struct (due to resource cleanup at the HAL implementation).
* However, it must remain valid for a currently tuned program at least
* until currentProgramInfoChanged or programListChanged is called and
* metadata changes for the current program.
*
* There is still a race condition possible (if the HAL deletes the old
* image immediately after notifying about the new one) between
* currentProgramInfoChanged callback propagating through the framework and
* the HAL implementation removing previous image. In such case, client
* application may expect the new currentProgramInfoChanged callback with
* updated image identifier.
*
* @param id Identifier of an image;
* value of 0 is reserved and should be treated as invalid image.
* @return image A binary blob with image data

View File

@@ -39,7 +39,7 @@ interface ITuner extends @1.0::ITuner {
* INVALID_ARGUMENTS if invalid arguments are passed.
* NOT_INITIALIZED if another error occurs.
*/
tune_1_1(ProgramSelector program) generates (Result result);
tuneByProgramSelector(ProgramSelector program) generates (Result result);
/**
* Cancels announcement.
@@ -121,19 +121,6 @@ interface ITuner extends @1.0::ITuner {
getProgramList(string filter)
generates (ProgramListResult result, vec<ProgramInfo> programList);
/**
* Checks, if the analog playback is forced, see setAnalogForced.
*
* The isForced value is only valid if result was OK.
*
* @return result OK if the call succeeded and isForced is valid.
* INVALID_STATE if the switch is not supported at current
* configuration.
* NOT_INITIALIZED if any other error occurs.
* @return isForced true if analog is forced, false otherwise.
*/
isAnalogForced() generates (Result result, bool isForced);
/**
* Forces the analog playback for the supporting radio technology.
*
@@ -150,4 +137,17 @@ interface ITuner extends @1.0::ITuner {
* NOT_INITIALIZED if any other error occurs.
*/
setAnalogForced(bool isForced) generates (Result result);
/**
* Checks, if the analog playback is forced, see setAnalogForced.
*
* The isForced value is only valid if result was OK.
*
* @return result OK if the call succeeded and isForced is valid.
* INVALID_STATE if the switch is not supported at current
* configuration.
* NOT_INITIALIZED if any other error occurs.
* @return isForced true if analog is forced, false otherwise.
*/
isAnalogForced() generates (Result result, bool isForced);
};

View File

@@ -29,9 +29,9 @@ interface ITunerCallback extends @1.0::ITunerCallback {
* Method called by the HAL when a tuning operation completes
* following a step(), scan() or tune() command.
*
* This callback supersedes V1_0::tuneComplete. For performance reasons,
* the 1.0 callback may not be called when HAL implementation detects 1.1
* client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).
* This callback supersedes V1_0::tuneComplete.
* The 1.0 callback must not be called when HAL implementation detects
* 1.1 client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).
*
* @param result OK if tune succeeded or TIMEOUT in case of time out.
* @param selector A ProgramSelector structure describing the tuned station.
@@ -41,9 +41,9 @@ interface ITunerCallback extends @1.0::ITunerCallback {
/**
* Method called by the HAL when a frequency switch occurs.
*
* This callback supersedes V1_0::afSwitch. For performance reasons,
* the 1.0 callback may not be called when HAL implementation detects 1.1
* client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).
* This callback supersedes V1_0::afSwitch.
* The 1.0 callback must not be called when HAL implementation detects
* 1.1 client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).
*
* @param selector A ProgramSelector structure describing the tuned station.
*/
@@ -73,6 +73,9 @@ interface ITunerCallback extends @1.0::ITunerCallback {
* call it immediately, ie. it may wait for a short time to accumulate
* multiple list change notifications into a single event.
*
* This callback is only for notifying about insertions and deletions,
* not about metadata changes.
*
* It may be triggered either by an explicitly issued background scan,
* or a scan issued by the device internally.
*
@@ -89,10 +92,10 @@ interface ITunerCallback extends @1.0::ITunerCallback {
*
* This may be called together with tuneComplete_1_1 or afSwitch_1_1.
*
* This callback supersedes V1_0::tuneComplete, V1_0::afSwitch and
* newMetadata. For performance reasons, these callbacks may not be called
* when HAL implementation detects 1.1 client (by casting
* V1_0::ITunerCallback to V1_1::ITunerCallback).
* This callback supersedes V1_0::newMetadata and partly V1_0::tuneComplete
* and V1_0::afSwitch.
* 1.0 callbacks must not be called when HAL implementation detects
* 1.1 client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).
*/
oneway programInfoChanged();
oneway currentProgramInfoChanged();
};

View File

@@ -248,10 +248,10 @@ Return<Result> Tuner::tune(uint32_t channel, uint32_t subChannel) {
lock_guard<mutex> lk(mMut);
band = mAmfmConfig.type;
}
return tune_1_1(utils::make_selector(band, channel, subChannel));
return tuneByProgramSelector(utils::make_selector(band, channel, subChannel));
}
Return<Result> Tuner::tune_1_1(const ProgramSelector& sel) {
Return<Result> Tuner::tuneByProgramSelector(const ProgramSelector& sel) {
ALOGV("%s(%s)", __func__, toString(sel).c_str());
lock_guard<mutex> lk(mMut);
if (mIsClosed) return Result::NOT_INITIALIZED;
@@ -336,6 +336,15 @@ Return<void> Tuner::getProgramList(const hidl_string& filter, getProgramList_cb
return {};
}
Return<Result> Tuner::setAnalogForced(bool isForced) {
ALOGV("%s", __func__);
lock_guard<mutex> lk(mMut);
if (mIsClosed) return Result::NOT_INITIALIZED;
mIsAnalogForced = isForced;
return Result::OK;
}
Return<void> Tuner::isAnalogForced(isAnalogForced_cb _hidl_cb) {
ALOGV("%s", __func__);
lock_guard<mutex> lk(mMut);
@@ -348,15 +357,6 @@ Return<void> Tuner::isAnalogForced(isAnalogForced_cb _hidl_cb) {
return {};
}
Return<Result> Tuner::setAnalogForced(bool isForced) {
ALOGV("%s", __func__);
lock_guard<mutex> lk(mMut);
if (mIsClosed) return Result::NOT_INITIALIZED;
mIsAnalogForced = isForced;
return Result::OK;
}
} // namespace implementation
} // namespace V1_1
} // namespace broadcastradio

View File

@@ -39,7 +39,7 @@ struct Tuner : public ITuner {
virtual Return<Result> scan(V1_0::Direction direction, bool skipSubChannel) override;
virtual Return<Result> step(V1_0::Direction direction, bool skipSubChannel) override;
virtual Return<Result> tune(uint32_t channel, uint32_t subChannel) override;
virtual Return<Result> tune_1_1(const ProgramSelector& program) override;
virtual Return<Result> tuneByProgramSelector(const ProgramSelector& program) override;
virtual Return<Result> cancel() override;
virtual Return<Result> cancelAnnouncement() override;
virtual Return<void> getProgramInformation(getProgramInformation_cb _hidl_cb) override;
@@ -47,8 +47,8 @@ struct Tuner : public ITuner {
virtual Return<ProgramListResult> startBackgroundScan() override;
virtual Return<void> getProgramList(const hidl_string& filter,
getProgramList_cb _hidl_cb) override;
virtual Return<void> isAnalogForced(isAnalogForced_cb _hidl_cb) override;
virtual Return<Result> setAnalogForced(bool isForced) override;
virtual Return<void> isAnalogForced(isAnalogForced_cb _hidl_cb) override;
private:
std::mutex mMut;

View File

@@ -84,7 +84,7 @@ struct TunerCallbackMock : public ITunerCallback {
MOCK_METHOD1(backgroundScanAvailable, Return<void>(bool));
MOCK_TIMEOUT_METHOD1(backgroundScanComplete, Return<void>(ProgramListResult));
MOCK_METHOD0(programListChanged, Return<void>());
MOCK_METHOD0(programInfoChanged, Return<void>());
MOCK_METHOD0(currentProgramInfoChanged, Return<void>());
};
class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase,
@@ -315,7 +315,7 @@ TEST_P(BroadcastRadioHalTest, OpenTunerTwice) {
* - getProgramList either succeeds or returns NOT_STARTED/NOT_READY status;
* - if the program list is NOT_STARTED, startBackgroundScan makes it completed
* within a full scan timeout and the next getProgramList call succeeds;
* - if the program list is not empty, tune_1_1 call succeeds.
* - if the program list is not empty, tuneByProgramSelector call succeeds.
*/
TEST_P(BroadcastRadioHalTest, TuneFromProgramList) {
if (skipped) return;
@@ -340,7 +340,7 @@ TEST_P(BroadcastRadioHalTest, TuneFromProgramList) {
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);
auto tuneResult = mTuner->tuneByProgramSelector(firstProgram.selector);
ASSERT_EQ(Result::OK, tuneResult);
EXPECT_TIMEOUT_CALL_WAIT(*mCallback, tuneComplete_1_1, kTuneTimeout);
EXPECT_EQ(firstProgram.selector.primaryId, selCb.primaryId);