From 84ec4e14f66f47e9339aa44ec69a3ca7c95106dd Mon Sep 17 00:00:00 2001 From: Tomasz Wasilczyk Date: Tue, 13 Nov 2018 11:26:23 -0800 Subject: [PATCH] Migrate broadcast radio default HAL to the new logging API. Bug: 112540729 Test: flash and boot Change-Id: I18d8b508971cd9a9b7b2c9c221674d862ff351d6 --- broadcastradio/2.0/default/Android.bp | 1 - broadcastradio/2.0/default/BroadcastRadio.cpp | 22 +++----- broadcastradio/2.0/default/TunerSession.cpp | 52 ++++++++----------- broadcastradio/2.0/default/VirtualProgram.cpp | 5 +- broadcastradio/2.0/default/VirtualRadio.cpp | 4 -- broadcastradio/2.0/default/service.cpp | 6 +-- .../VtsHalBroadcastradioV2_0TargetTest.cpp | 10 ++-- broadcastradio/common/utils/Android.bp | 1 - broadcastradio/common/utils/WorkerThread.cpp | 11 ---- broadcastradio/common/utils2x/Utils.cpp | 8 ++- .../broadcastradio-vts-utils/mock-timeout.h | 2 +- 11 files changed, 42 insertions(+), 80 deletions(-) diff --git a/broadcastradio/2.0/default/Android.bp b/broadcastradio/2.0/default/Android.bp index 900454e78b..840c4b864e 100644 --- a/broadcastradio/2.0/default/Android.bp +++ b/broadcastradio/2.0/default/Android.bp @@ -43,7 +43,6 @@ cc_binary { "libbase", "libhidlbase", "libhidltransport", - "liblog", "libutils", ], } diff --git a/broadcastradio/2.0/default/BroadcastRadio.cpp b/broadcastradio/2.0/default/BroadcastRadio.cpp index 0148fecc48..28a0dd504a 100644 --- a/broadcastradio/2.0/default/BroadcastRadio.cpp +++ b/broadcastradio/2.0/default/BroadcastRadio.cpp @@ -13,15 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define LOG_TAG "BcRadioDef.module" -#define LOG_NDEBUG 0 - #include "BroadcastRadio.h" -#include - #include "resources.h" +#include + namespace android { namespace hardware { namespace broadcastradio { @@ -66,7 +63,6 @@ BroadcastRadio::BroadcastRadio(const VirtualRadio& virtualRadio) mAmFmConfig(gDefaultAmFmConfig) {} Return BroadcastRadio::getProperties(getProperties_cb _hidl_cb) { - ALOGV("%s", __func__); _hidl_cb(mProperties); return {}; } @@ -77,8 +73,6 @@ AmFmRegionConfig BroadcastRadio::getAmFmConfig() const { } Return BroadcastRadio::getAmFmRegionConfig(bool full, getAmFmRegionConfig_cb _hidl_cb) { - ALOGV("%s(%d)", __func__, full); - if (full) { AmFmRegionConfig config = {}; config.ranges = hidl_vec({ @@ -96,8 +90,6 @@ Return BroadcastRadio::getAmFmRegionConfig(bool full, getAmFmRegionConfig_ } Return BroadcastRadio::getDabRegionConfig(getDabRegionConfig_cb _hidl_cb) { - ALOGV("%s", __func__); - hidl_vec config = { {"5A", 174928}, {"7D", 194064}, {"8A", 195936}, {"8B", 197648}, {"9A", 202928}, {"9B", 204640}, {"9C", 206352}, {"10B", 211648}, {"10C", 213360}, {"10D", 215072}, @@ -111,7 +103,7 @@ Return BroadcastRadio::getDabRegionConfig(getDabRegionConfig_cb _hidl_cb) Return BroadcastRadio::openSession(const sp& callback, openSession_cb _hidl_cb) { - ALOGV("%s", __func__); + LOG(DEBUG) << "opening new session..."; /* 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. @@ -122,7 +114,7 @@ Return BroadcastRadio::openSession(const sp& callback, auto oldSession = mSession.promote(); if (oldSession != nullptr) { - ALOGI("Closing previously opened tuner"); + LOG(INFO) << "closing previously opened tuner"; oldSession->close(); mSession = nullptr; } @@ -134,14 +126,14 @@ Return BroadcastRadio::openSession(const sp& callback, } Return BroadcastRadio::getImage(uint32_t id, getImage_cb _hidl_cb) { - ALOGV("%s(%x)", __func__, id); + LOG(DEBUG) << "fetching image " << std::hex << id; if (id == resources::demoPngId) { _hidl_cb(std::vector(resources::demoPng, std::end(resources::demoPng))); return {}; } - ALOGI("Image %x doesn't exists", id); + LOG(INFO) << "image " << std::hex << id << " doesn't exists"; _hidl_cb({}); return {}; } @@ -149,7 +141,7 @@ Return BroadcastRadio::getImage(uint32_t id, getImage_cb _hidl_cb) { Return BroadcastRadio::registerAnnouncementListener( const hidl_vec& enabled, const sp& /* listener */, registerAnnouncementListener_cb _hidl_cb) { - ALOGV("%s(%s)", __func__, toString(enabled).c_str()); + LOG(DEBUG) << "registering announcement listener for " << toString(enabled); _hidl_cb(Result::NOT_SUPPORTED, nullptr); return {}; diff --git a/broadcastradio/2.0/default/TunerSession.cpp b/broadcastradio/2.0/default/TunerSession.cpp index da9756208f..2ba4d02092 100644 --- a/broadcastradio/2.0/default/TunerSession.cpp +++ b/broadcastradio/2.0/default/TunerSession.cpp @@ -14,15 +14,12 @@ * limitations under the License. */ -#define LOG_TAG "BcRadioDef.tuner" -#define LOG_NDEBUG 0 - #include "TunerSession.h" #include "BroadcastRadio.h" +#include #include -#include namespace android { namespace hardware { @@ -68,7 +65,7 @@ static ProgramInfo makeDummyProgramInfo(const ProgramSelector& selector) { } void TunerSession::tuneInternalLocked(const ProgramSelector& sel) { - ALOGV("%s(%s)", __func__, toString(sel).c_str()); + LOG(VERBOSE) << "tune (internal) to " << toString(sel); VirtualProgram virtualProgram; ProgramInfo programInfo; @@ -93,17 +90,18 @@ const VirtualRadio& TunerSession::virtualRadio() const { } Return TunerSession::tune(const ProgramSelector& sel) { - ALOGV("%s(%s)", __func__, toString(sel).c_str()); + LOG(DEBUG) << "tune to " << toString(sel); + lock_guard lk(mMut); if (mIsClosed) return Result::INVALID_STATE; if (!utils::isSupported(module().mProperties, sel)) { - ALOGW("Selector not supported"); + LOG(WARNING) << "selector not supported: " << toString(sel); return Result::NOT_SUPPORTED; } if (!utils::isValid(sel)) { - ALOGE("ProgramSelector is not valid"); + LOG(ERROR) << "selector is not valid: " << toString(sel); return Result::INVALID_ARGUMENTS; } @@ -119,8 +117,9 @@ Return TunerSession::tune(const ProgramSelector& sel) { return Result::OK; } -Return TunerSession::scan(bool directionUp, bool /* skipSubChannel */) { - ALOGV("%s", __func__); +Return TunerSession::scan(bool directionUp, bool skipSubChannel) { + LOG(DEBUG) << "seek up=" << directionUp << " skipSubChannel=" << skipSubChannel; + lock_guard lk(mMut); if (mIsClosed) return Result::INVALID_STATE; @@ -130,8 +129,8 @@ Return TunerSession::scan(bool directionUp, bool /* skipSubChannel */) { if (list.empty()) { mIsTuneCompleted = false; - auto task = [this, directionUp]() { - ALOGI("Performing failed seek up=%d", directionUp); + auto task = [this]() { + LOG(DEBUG) << "program list is empty, seek couldn't stop"; mCallback->onTuneFailed(Result::TIMEOUT, {}); }; @@ -162,7 +161,7 @@ Return TunerSession::scan(bool directionUp, bool /* skipSubChannel */) { mIsTuneCompleted = false; auto task = [this, tuneTo, directionUp]() { - ALOGI("Performing seek up=%d", directionUp); + LOG(VERBOSE) << "executing seek up=" << directionUp; lock_guard lk(mMut); tuneInternalLocked(tuneTo); @@ -173,21 +172,21 @@ Return TunerSession::scan(bool directionUp, bool /* skipSubChannel */) { } Return TunerSession::step(bool directionUp) { - ALOGV("%s", __func__); + LOG(DEBUG) << "step up=" << directionUp; lock_guard 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"); + LOG(WARNING) << "can't step in anything else than AM/FM"; return Result::NOT_SUPPORTED; } auto stepTo = utils::getId(mCurrentProgram, IdentifierType::AMFM_FREQUENCY); auto range = getAmFmRangeLocked(); if (!range) { - ALOGE("Can't find current band"); + LOG(ERROR) << "can't find current band"; return Result::INTERNAL_ERROR; } @@ -201,7 +200,7 @@ Return TunerSession::step(bool directionUp) { mIsTuneCompleted = false; auto task = [this, stepTo]() { - ALOGI("Performing step to %s", std::to_string(stepTo).c_str()); + LOG(VERBOSE) << "executing step to " << stepTo; lock_guard lk(mMut); @@ -213,7 +212,7 @@ Return TunerSession::step(bool directionUp) { } void TunerSession::cancelLocked() { - ALOGV("%s", __func__); + LOG(VERBOSE) << "cancelling current operations..."; mThread.cancelAll(); if (utils::getType(mCurrentProgram.primaryId) != IdentifierType::INVALID) { @@ -222,7 +221,6 @@ void TunerSession::cancelLocked() { } Return TunerSession::cancel() { - ALOGV("%s", __func__); lock_guard lk(mMut); if (mIsClosed) return {}; @@ -232,7 +230,7 @@ Return TunerSession::cancel() { } Return TunerSession::startProgramListUpdates(const ProgramFilter& filter) { - ALOGV("%s(%s)", __func__, toString(filter).c_str()); + LOG(DEBUG) << "requested program list updates, filter=" << toString(filter); lock_guard lk(mMut); if (mIsClosed) return Result::INVALID_STATE; @@ -259,41 +257,37 @@ Return TunerSession::startProgramListUpdates(const ProgramFilter& filter } Return TunerSession::stopProgramListUpdates() { - ALOGV("%s", __func__); + LOG(DEBUG) << "requested program list updates to stop"; return {}; } Return TunerSession::isConfigFlagSet(ConfigFlag flag, isConfigFlagSet_cb _hidl_cb) { - ALOGV("%s(%s)", __func__, toString(flag).c_str()); + LOG(VERBOSE) << __func__ << " " << toString(flag); _hidl_cb(Result::NOT_SUPPORTED, false); return {}; } Return TunerSession::setConfigFlag(ConfigFlag flag, bool value) { - ALOGV("%s(%s, %d)", __func__, toString(flag).c_str(), value); + LOG(VERBOSE) << __func__ << " " << toString(flag) << " " << value; return Result::NOT_SUPPORTED; } Return TunerSession::setParameters(const hidl_vec& /* parameters */, setParameters_cb _hidl_cb) { - ALOGV("%s", __func__); - _hidl_cb({}); return {}; } Return TunerSession::getParameters(const hidl_vec& /* keys */, getParameters_cb _hidl_cb) { - ALOGV("%s", __func__); - _hidl_cb({}); return {}; } Return TunerSession::close() { - ALOGV("%s", __func__); + LOG(DEBUG) << "closing session..."; lock_guard lk(mMut); if (mIsClosed) return {}; @@ -304,7 +298,7 @@ Return TunerSession::close() { std::optional TunerSession::getAmFmRangeLocked() const { if (!mIsTuneCompleted) { - ALOGW("tune operation in process"); + LOG(WARNING) << "tune operation is in process"; return {}; } if (!utils::hasId(mCurrentProgram, IdentifierType::AMFM_FREQUENCY)) return {}; diff --git a/broadcastradio/2.0/default/VirtualProgram.cpp b/broadcastradio/2.0/default/VirtualProgram.cpp index acde704f9b..a971927610 100644 --- a/broadcastradio/2.0/default/VirtualProgram.cpp +++ b/broadcastradio/2.0/default/VirtualProgram.cpp @@ -13,15 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define LOG_TAG "BcRadioDef.VirtualProgram" - #include "VirtualProgram.h" #include "resources.h" #include #include -#include namespace android { namespace hardware { @@ -72,7 +69,7 @@ VirtualProgram::operator ProgramInfo() const { info.physicallyTunedTo = selectId(IdentifierType::SXM_CHANNEL); break; default: - LOG(FATAL) << "Unsupported program type: " << toString(pType); + LOG(FATAL) << "unsupported program type: " << toString(pType); } info.infoFlags |= ProgramInfoFlags::TUNED; diff --git a/broadcastradio/2.0/default/VirtualRadio.cpp b/broadcastradio/2.0/default/VirtualRadio.cpp index f601d41637..0b65979b76 100644 --- a/broadcastradio/2.0/default/VirtualRadio.cpp +++ b/broadcastradio/2.0/default/VirtualRadio.cpp @@ -13,13 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define LOG_TAG "BcRadioDef.VirtualRadio" -//#define LOG_NDEBUG 0 - #include "VirtualRadio.h" #include -#include namespace android { namespace hardware { diff --git a/broadcastradio/2.0/default/service.cpp b/broadcastradio/2.0/default/service.cpp index 7e677a1323..af96dad47f 100644 --- a/broadcastradio/2.0/default/service.cpp +++ b/broadcastradio/2.0/default/service.cpp @@ -13,8 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define LOG_TAG "BcRadioDef.service" - #include #include @@ -26,7 +24,9 @@ using android::hardware::joinRpcThreadpool; using android::hardware::broadcastradio::V2_0::implementation::BroadcastRadio; using android::hardware::broadcastradio::V2_0::implementation::gAmFmRadio; -int main(int /* argc */, char** /* argv */) { +int main() { + android::base::SetDefaultTag("BcRadioDef"); + android::base::SetMinimumLogSeverity(android::base::VERBOSE); configureRpcThreadpool(4, true); BroadcastRadio broadcastRadio(gAmFmRadio); diff --git a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp index 3d7039dc9b..c523013f42 100644 --- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp +++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp @@ -14,8 +14,6 @@ * limitations under the License. */ -#define LOG_TAG "BcRadio.vts" -#define LOG_NDEBUG 0 #define EGMOCK_VERBOSE 1 #include @@ -446,7 +444,7 @@ TEST_F(BroadcastRadioHalTest, FmTune) { EXPECT_EQ(Result::OK, result); EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChanged_, timeout::tune); - ALOGD("current program info: %s", toString(infoCb).c_str()); + LOG(DEBUG) << "current program info: " << toString(infoCb); // it should tune exactly to what was requested auto freqs = utils::getAllIds(infoCb.selector, IdentifierType::AMFM_FREQUENCY); @@ -823,11 +821,11 @@ int main(int argc, char** argv) { using android::hardware::broadcastradio::V2_0::vts::gEnv; using android::hardware::broadcastradio::V2_0::IBroadcastRadio; using android::hardware::broadcastradio::vts::BroadcastRadioHidlEnvironment; + android::base::SetDefaultTag("BcRadio.vts"); + android::base::SetMinimumLogSeverity(android::base::VERBOSE); gEnv = new BroadcastRadioHidlEnvironment; ::testing::AddGlobalTestEnvironment(gEnv); ::testing::InitGoogleTest(&argc, argv); gEnv->init(&argc, argv); - int status = RUN_ALL_TESTS(); - ALOGI("Test result = %d", status); - return status; + return RUN_ALL_TESTS(); } diff --git a/broadcastradio/common/utils/Android.bp b/broadcastradio/common/utils/Android.bp index 33ba7da22f..32e06d7bdf 100644 --- a/broadcastradio/common/utils/Android.bp +++ b/broadcastradio/common/utils/Android.bp @@ -29,7 +29,6 @@ cc_library_static { export_include_dirs: ["include"], shared_libs: [ "libbase", - "liblog", "libutils", ], } diff --git a/broadcastradio/common/utils/WorkerThread.cpp b/broadcastradio/common/utils/WorkerThread.cpp index bfcbb390e8..31f4d3f83e 100644 --- a/broadcastradio/common/utils/WorkerThread.cpp +++ b/broadcastradio/common/utils/WorkerThread.cpp @@ -14,13 +14,8 @@ * limitations under the License. */ -#define LOG_TAG "WorkerThread" -//#define LOG_NDEBUG 0 - #include -#include - namespace android { using std::chrono::milliseconds; @@ -39,7 +34,6 @@ bool operator<(const WorkerThread::Task& lhs, const WorkerThread::Task& rhs) { WorkerThread::WorkerThread() : mIsTerminating(false), mThread(&WorkerThread::threadLoop, this) {} WorkerThread::~WorkerThread() { - ALOGV("%s", __func__); { lock_guard lk(mMut); mIsTerminating = true; @@ -49,8 +43,6 @@ WorkerThread::~WorkerThread() { } void WorkerThread::schedule(function task, milliseconds delay) { - ALOGV("%s", __func__); - auto when = steady_clock::now() + delay; lock_guard lk(mMut); @@ -59,14 +51,11 @@ void WorkerThread::schedule(function task, milliseconds delay) { } void WorkerThread::cancelAll() { - ALOGV("%s", __func__); - lock_guard lk(mMut); priority_queue().swap(mTasks); // empty queue } void WorkerThread::threadLoop() { - ALOGV("%s", __func__); while (!mIsTerminating) { unique_lock lk(mMut); if (mTasks.empty()) { diff --git a/broadcastradio/common/utils2x/Utils.cpp b/broadcastradio/common/utils2x/Utils.cpp index f292c0855f..789265328f 100644 --- a/broadcastradio/common/utils2x/Utils.cpp +++ b/broadcastradio/common/utils2x/Utils.cpp @@ -14,12 +14,10 @@ * limitations under the License. */ #define LOG_TAG "BcRadioDef.utils" -//#define LOG_NDEBUG 0 #include #include -#include namespace android { namespace hardware { @@ -130,7 +128,7 @@ bool tunesTo(const ProgramSelector& a, const ProgramSelector& b) { case IdentifierType::SXM_SERVICE_ID: return haveEqualIds(a, b, IdentifierType::SXM_SERVICE_ID); default: // includes all vendor types - ALOGW("Unsupported program type: %s", toString(type).c_str()); + LOG(WARNING) << "unsupported program type: " << toString(type); return false; } } @@ -166,7 +164,7 @@ uint64_t getId(const ProgramSelector& sel, const IdentifierType type) { return val; } - ALOGW("Identifier %s not found", toString(type).c_str()); + LOG(WARNING) << "identifier not found: " << toString(type); return 0; } @@ -205,7 +203,7 @@ bool isValid(const ProgramIdentifier& id) { auto expect = [&valid](bool condition, std::string message) { if (!condition) { valid = false; - ALOGE("Identifier not valid, expected %s", message.c_str()); + LOG(ERROR) << "identifier not valid, expected " << message; } }; diff --git a/broadcastradio/common/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h b/broadcastradio/common/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h index 1f716f1bce..f6cd6aed4a 100644 --- a/broadcastradio/common/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h +++ b/broadcastradio/common/vts/utils/include/broadcastradio-vts-utils/mock-timeout.h @@ -29,7 +29,7 @@ * INTERNAL IMPLEMENTATION - don't use in user code. */ #if EGMOCK_VERBOSE -#define EGMOCK_LOG_(...) ALOGV("egmock: " __VA_ARGS__) +#define EGMOCK_LOG_(...) LOG(VERBOSE) << "egmock: " << __VA_ARGS__ #else #define EGMOCK_LOG_(...) #endif