From c863a957d4c8d111aa33ea7d0ced841b265449a5 Mon Sep 17 00:00:00 2001 From: Tomasz Wasilczyk Date: Wed, 7 Mar 2018 14:28:55 -0800 Subject: [PATCH] Fix boadcastradio HAL 1.1 tuneByProgramSelector implementation. It was not compliant with the HAL definition - it didn't auto-change band if necessary. Bug: 74353024 Test: manual Change-Id: I015faffc42778fa27fca3030306f31b0abe409c7 --- broadcastradio/1.1/default/BroadcastRadio.cpp | 52 +++++++++++-------- broadcastradio/1.1/default/BroadcastRadio.h | 2 + broadcastradio/1.1/default/Tuner.cpp | 49 +++++++++++------ broadcastradio/1.1/default/Tuner.h | 8 ++- 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/broadcastradio/1.1/default/BroadcastRadio.cpp b/broadcastradio/1.1/default/BroadcastRadio.cpp index 2933aa7dd5..e01812e6e6 100644 --- a/broadcastradio/1.1/default/BroadcastRadio.cpp +++ b/broadcastradio/1.1/default/BroadcastRadio.cpp @@ -122,28 +122,7 @@ Return BroadcastRadio::getProperties_1_1(getProperties_1_1_cb _hidl_cb) { {"com.google.dummy", "dummy"}, }); - prop10.bands.resize(mConfig.amFmBands.size()); - for (size_t i = 0; i < mConfig.amFmBands.size(); i++) { - auto& src = mConfig.amFmBands[i]; - auto& dst = prop10.bands[i]; - - dst.type = src.type; - dst.antennaConnected = true; - dst.lowerLimit = src.lowerLimit; - dst.upperLimit = src.upperLimit; - dst.spacings = src.spacings; - - if (utils::isAm(src.type)) { - dst.ext.am.stereo = true; - } else if (utils::isFm(src.type)) { - dst.ext.fm.deemphasis = static_cast(Deemphasis::D50 | Deemphasis::D75); - dst.ext.fm.stereo = true; - dst.ext.fm.rds = static_cast(Rds::WORLD | Rds::US); - dst.ext.fm.ta = true; - dst.ext.fm.af = true; - dst.ext.fm.ea = true; - } - } + prop10.bands = getAmFmBands(); _hidl_cb(prop11); return Void(); @@ -162,7 +141,7 @@ Return BroadcastRadio::openTuner(const BandConfig& config, bool audio __un mTuner = nullptr; } - sp newTuner = new Tuner(mClassId, callback); + sp newTuner = new Tuner(this, mClassId, callback); mTuner = newTuner; if (mClassId == Class::AM_FM) { auto ret = newTuner->setConfiguration(config); @@ -189,6 +168,33 @@ Return BroadcastRadio::getImage(int32_t id, getImage_cb _hidl_cb) { return Void(); } +std::vector BroadcastRadio::getAmFmBands() const { + std::vector out; + for (auto&& src : mConfig.amFmBands) { + V1_0::BandConfig dst; + + dst.type = src.type; + dst.antennaConnected = true; + dst.lowerLimit = src.lowerLimit; + dst.upperLimit = src.upperLimit; + dst.spacings = src.spacings; + + if (utils::isAm(src.type)) { + dst.ext.am.stereo = true; + } else if (utils::isFm(src.type)) { + dst.ext.fm.deemphasis = static_cast(Deemphasis::D50 | Deemphasis::D75); + dst.ext.fm.stereo = true; + dst.ext.fm.rds = static_cast(Rds::WORLD | Rds::US); + dst.ext.fm.ta = true; + dst.ext.fm.af = true; + dst.ext.fm.ea = true; + } + + out.push_back(dst); + } + return out; +} + } // namespace implementation } // namespace V1_1 } // namespace broadcastradio diff --git a/broadcastradio/1.1/default/BroadcastRadio.h b/broadcastradio/1.1/default/BroadcastRadio.h index bdf3b87458..d0a73d9955 100644 --- a/broadcastradio/1.1/default/BroadcastRadio.h +++ b/broadcastradio/1.1/default/BroadcastRadio.h @@ -65,6 +65,8 @@ struct BroadcastRadio : public V1_1::IBroadcastRadio { openTuner_cb _hidl_cb) override; Return getImage(int32_t id, getImage_cb _hidl_cb); + std::vector getAmFmBands() const; + private: std::mutex mMut; V1_0::Class mClassId; diff --git a/broadcastradio/1.1/default/Tuner.cpp b/broadcastradio/1.1/default/Tuner.cpp index 2be070d965..ae0187987a 100644 --- a/broadcastradio/1.1/default/Tuner.cpp +++ b/broadcastradio/1.1/default/Tuner.cpp @@ -58,8 +58,10 @@ const struct { milliseconds tune = 150ms; } gDefaultDelay; -Tuner::Tuner(V1_0::Class classId, const sp& callback) - : mClassId(classId), +Tuner::Tuner(const sp module, V1_0::Class classId, + const sp& callback) + : mModule(module), + mClassId(classId), mCallback(callback), mCallback1_1(V1_1::ITunerCallback::castFrom(callback).withDefault(nullptr)), mVirtualRadio(getRadio(classId)), @@ -71,6 +73,33 @@ void Tuner::forceClose() { mThread.cancelAll(); } +void Tuner::setConfigurationInternalLocked(const BandConfig& config) { + mAmfmConfig = config; + mAmfmConfig.antennaConnected = true; + mCurrentProgram = utils::make_selector(mAmfmConfig.type, mAmfmConfig.lowerLimit); + + if (utils::isFm(mAmfmConfig.type)) { + mVirtualRadio = std::ref(getFmRadio()); + } else { + mVirtualRadio = std::ref(getAmRadio()); + } + + mIsAmfmConfigSet = true; + mCallback->configChange(Result::OK, mAmfmConfig); +} + +bool Tuner::autoConfigureLocked(uint64_t frequency) { + for (auto&& config : mModule->getAmFmBands()) { + // The check here is rather poor, but it's enough for default implementation. + if (config.lowerLimit <= frequency && config.upperLimit >= frequency) { + ALOGI("Auto-switching band to %s", toString(config).c_str()); + setConfigurationInternalLocked(config); + return true; + } + } + return false; +} + Return Tuner::setConfiguration(const BandConfig& config) { ALOGV("%s", __func__); lock_guard lk(mMut); @@ -85,19 +114,7 @@ Return Tuner::setConfiguration(const BandConfig& config) { auto task = [this, config]() { ALOGI("Setting AM/FM config"); lock_guard lk(mMut); - - mAmfmConfig = move(config); - mAmfmConfig.antennaConnected = true; - mCurrentProgram = utils::make_selector(mAmfmConfig.type, mAmfmConfig.lowerLimit); - - if (utils::isFm(mAmfmConfig.type)) { - mVirtualRadio = std::ref(getFmRadio()); - } else { - mVirtualRadio = std::ref(getAmRadio()); - } - - mIsAmfmConfigSet = true; - mCallback->configChange(Result::OK, mAmfmConfig); + setConfigurationInternalLocked(config); }; mThread.schedule(task, gDefaultDelay.config); @@ -276,7 +293,7 @@ Return Tuner::tuneByProgramSelector(const ProgramSelector& sel) { auto freq = utils::getId(sel, IdentifierType::AMFM_FREQUENCY); if (freq < mAmfmConfig.lowerLimit || freq > mAmfmConfig.upperLimit) { - return Result::INVALID_ARGUMENTS; + if (!autoConfigureLocked(freq)) return Result::INVALID_ARGUMENTS; } } else if (programType == ProgramType::DAB) { if (!utils::hasId(sel, IdentifierType::DAB_SIDECC)) return Result::INVALID_ARGUMENTS; diff --git a/broadcastradio/1.1/default/Tuner.h b/broadcastradio/1.1/default/Tuner.h index 764d5b3d28..e2668d878c 100644 --- a/broadcastradio/1.1/default/Tuner.h +++ b/broadcastradio/1.1/default/Tuner.h @@ -28,8 +28,11 @@ namespace broadcastradio { namespace V1_1 { namespace implementation { +struct BroadcastRadio; + struct Tuner : public ITuner { - Tuner(V1_0::Class classId, const sp& callback); + Tuner(const sp module, V1_0::Class classId, + const sp& callback); void forceClose(); @@ -55,6 +58,7 @@ struct Tuner : public ITuner { WorkerThread mThread; bool mIsClosed = false; + const sp mModule; V1_0::Class mClassId; const sp mCallback; const sp mCallback1_1; @@ -68,7 +72,9 @@ struct Tuner : public ITuner { std::atomic mIsAnalogForced; utils::HalRevision getHalRev() const; + void setConfigurationInternalLocked(const V1_0::BandConfig& config); void tuneInternalLocked(const V1_1::ProgramSelector& sel); + bool autoConfigureLocked(uint64_t frequency); }; } // namespace implementation