From 25b2a8de532b632892e6ea70cf8e53277f4a0015 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 24 Feb 2022 21:51:34 +0000 Subject: [PATCH 1/4] healthd: Improve readability of BatteryMonitor::init() Declare and assign in a single statement instead of in two separate statements. This patch does not change any functionality. Test: Compile-tested only. Change-Id: I0798cc940f5f2ca329548568ab68e96891521dd8 Signed-off-by: Bart Van Assche --- healthd/BatteryMonitor.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/healthd/BatteryMonitor.cpp b/healthd/BatteryMonitor.cpp index 18076fb23..4bc9c1c8f 100644 --- a/healthd/BatteryMonitor.cpp +++ b/healthd/BatteryMonitor.cpp @@ -608,13 +608,13 @@ void BatteryMonitor::init(struct healthd_config *hc) { while ((entry = readdir(dir.get()))) { const char* name = entry->d_name; - std::vector::iterator itIgnoreName; if (!strcmp(name, ".") || !strcmp(name, "..")) continue; - itIgnoreName = find(hc->ignorePowerSupplyNames.begin(), - hc->ignorePowerSupplyNames.end(), String8(name)); + std::vector::iterator itIgnoreName = + find(hc->ignorePowerSupplyNames.begin(), hc->ignorePowerSupplyNames.end(), + String8(name)); if (itIgnoreName != hc->ignorePowerSupplyNames.end()) continue; From 024e18f06032583c3e9ba36a376a444ef745080c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 24 Feb 2022 21:22:07 +0000 Subject: [PATCH 2/4] healthd: Make initHealthInfo() easier to read Use aggregate initialization instead of memberwise initialization. This patch does not change any functionality. Test: Compile-tested only. Change-Id: I9c7831936e449aec8a48336b1ee76d26638e4f72 Signed-off-by: Bart Van Assche --- healthd/BatteryMonitor.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/healthd/BatteryMonitor.cpp b/healthd/BatteryMonitor.cpp index 4bc9c1c8f..45882a687 100644 --- a/healthd/BatteryMonitor.cpp +++ b/healthd/BatteryMonitor.cpp @@ -121,14 +121,13 @@ static std::optional mapSysfsString(const char* str, SysfsStringEnumMap ma } static void initHealthInfo(HealthInfo* health_info) { - *health_info = HealthInfo{}; - - // Enum values may be zero initialized, so they need to be initialized properly. - health_info->batteryCapacityLevel = BatteryCapacityLevel::UNSUPPORTED; - health_info->batteryChargeTimeToFullNowSeconds = - (int64_t)HealthInfo::BATTERY_CHARGE_TIME_TO_FULL_NOW_SECONDS_UNSUPPORTED; - health_info->batteryStatus = BatteryStatus::UNKNOWN; - health_info->batteryHealth = BatteryHealth::UNKNOWN; + *health_info = { + .batteryCapacityLevel = BatteryCapacityLevel::UNSUPPORTED, + .batteryChargeTimeToFullNowSeconds = + (int64_t)HealthInfo::BATTERY_CHARGE_TIME_TO_FULL_NOW_SECONDS_UNSUPPORTED, + .batteryStatus = BatteryStatus::UNKNOWN, + .batteryHealth = BatteryHealth::UNKNOWN, + }; } BatteryMonitor::BatteryMonitor() From 5a7e5083e60b6ce2037edf8e2b90632fb2ebddab Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 24 Feb 2022 21:40:15 +0000 Subject: [PATCH 3/4] healthd: Fix readFromFile() Not all readFromFile() callers pass an empty string. Make sure that readFromFile() returns 0 (failure) if the *buf argument is not empty and if reading fails. Fixes: 3217c5c7d961 ("batterymonitor: simplify readFromFile and use std::string buffers") Test: Compile-tested only. Change-Id: I6f329491f8aa4a6d378eb7c3cbc17038eb6aa64d Signed-off-by: Bart Van Assche --- healthd/BatteryMonitor.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/healthd/BatteryMonitor.cpp b/healthd/BatteryMonitor.cpp index 45882a687..0566a67aa 100644 --- a/healthd/BatteryMonitor.cpp +++ b/healthd/BatteryMonitor.cpp @@ -228,6 +228,7 @@ BatteryHealth getBatteryHealth(const char* status) { } int BatteryMonitor::readFromFile(const String8& path, std::string* buf) { + buf->clear(); if (android::base::ReadFileToString(path.c_str(), buf)) { *buf = android::base::Trim(*buf); } From 095c944b48d7a3770d591e37b6422ee12bb86148 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 2 Mar 2022 17:36:34 +0000 Subject: [PATCH 4/4] healthd: Convert multiple private methods into static nonmember functions The private methods in class BatteryMonitor do not use the 'this' pointer. Hence convert these into static nonmember functions. Test: Compile-tested only. Change-Id: I4b9b76c9697f1db894ba9fb939ce096ee6b7ff3b Signed-off-by: Bart Van Assche --- healthd/BatteryMonitor.cpp | 70 ++++++++++++------------ healthd/include/healthd/BatteryMonitor.h | 9 --- 2 files changed, 35 insertions(+), 44 deletions(-) diff --git a/healthd/BatteryMonitor.cpp b/healthd/BatteryMonitor.cpp index 0566a67aa..a7571a260 100644 --- a/healthd/BatteryMonitor.cpp +++ b/healthd/BatteryMonitor.cpp @@ -227,7 +227,7 @@ BatteryHealth getBatteryHealth(const char* status) { return *ret; } -int BatteryMonitor::readFromFile(const String8& path, std::string* buf) { +static int readFromFile(const String8& path, std::string* buf) { buf->clear(); if (android::base::ReadFileToString(path.c_str(), buf)) { *buf = android::base::Trim(*buf); @@ -235,39 +235,40 @@ int BatteryMonitor::readFromFile(const String8& path, std::string* buf) { return buf->length(); } -BatteryMonitor::PowerSupplyType BatteryMonitor::readPowerSupplyType(const String8& path) { +static BatteryMonitor::PowerSupplyType readPowerSupplyType(const String8& path) { static SysfsStringEnumMap supplyTypeMap[] = { - {"Unknown", ANDROID_POWER_SUPPLY_TYPE_UNKNOWN}, - {"Battery", ANDROID_POWER_SUPPLY_TYPE_BATTERY}, - {"UPS", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"Mains", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB", ANDROID_POWER_SUPPLY_TYPE_USB}, - {"USB_DCP", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB_HVDCP", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB_CDP", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB_ACA", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB_C", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB_PD", ANDROID_POWER_SUPPLY_TYPE_AC}, - {"USB_PD_DRP", ANDROID_POWER_SUPPLY_TYPE_USB}, - {"Wireless", ANDROID_POWER_SUPPLY_TYPE_WIRELESS}, - {"Dock", ANDROID_POWER_SUPPLY_TYPE_DOCK}, + {"Unknown", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_UNKNOWN}, + {"Battery", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_BATTERY}, + {"UPS", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"Mains", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_USB}, + {"USB_DCP", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB_HVDCP", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB_CDP", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB_ACA", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB_C", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB_PD", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_AC}, + {"USB_PD_DRP", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_USB}, + {"Wireless", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_WIRELESS}, + {"Dock", BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_DOCK}, {NULL, 0}, }; std::string buf; - if (readFromFile(path, &buf) <= 0) - return ANDROID_POWER_SUPPLY_TYPE_UNKNOWN; + if (readFromFile(path, &buf) <= 0) { + return BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_UNKNOWN; + } auto ret = mapSysfsString(buf.c_str(), supplyTypeMap); if (!ret) { KLOG_WARNING(LOG_TAG, "Unknown power supply type '%s'\n", buf.c_str()); - *ret = ANDROID_POWER_SUPPLY_TYPE_UNKNOWN; + *ret = BatteryMonitor::ANDROID_POWER_SUPPLY_TYPE_UNKNOWN; } return static_cast(*ret); } -bool BatteryMonitor::getBooleanField(const String8& path) { +static bool getBooleanField(const String8& path) { std::string buf; bool value = false; @@ -278,7 +279,7 @@ bool BatteryMonitor::getBooleanField(const String8& path) { return value; } -int BatteryMonitor::getIntField(const String8& path) { +static int getIntField(const String8& path) { std::string buf; int value = 0; @@ -288,7 +289,7 @@ int BatteryMonitor::getIntField(const String8& path) { return value; } -bool BatteryMonitor::isScopedPowerSupply(const char* name) { +static bool isScopedPowerSupply(const char* name) { constexpr char kScopeDevice[] = "Device"; String8 path; @@ -411,19 +412,7 @@ void BatteryMonitor::updateValues(void) { } } -void BatteryMonitor::logValues(void) { - logValues(*mHealthInfo, *mHealthdConfig); -} - -void BatteryMonitor::logValues(const HealthInfo_2_1& health_info, - const struct healthd_config& healthd_config) { - HealthInfo aidl_health_info; - (void)android::h2a::translate(health_info, &aidl_health_info); - logValues(aidl_health_info, healthd_config); -} - -void BatteryMonitor::logValues(const HealthInfo& props, - const struct healthd_config& healthd_config) { +static void doLogValues(const HealthInfo& props, const struct healthd_config& healthd_config) { char dmesgline[256]; size_t len; if (props.batteryPresent) { @@ -460,6 +449,17 @@ void BatteryMonitor::logValues(const HealthInfo& props, KLOG_WARNING(LOG_TAG, "%s\n", dmesgline); } +void BatteryMonitor::logValues(const HealthInfo_2_1& health_info, + const struct healthd_config& healthd_config) { + HealthInfo aidl_health_info; + (void)android::h2a::translate(health_info, &aidl_health_info); + doLogValues(aidl_health_info, healthd_config); +} + +void BatteryMonitor::logValues(void) { + doLogValues(*mHealthInfo, *mHealthdConfig); +} + bool BatteryMonitor::isChargerOnline() { const HealthInfo& props = *mHealthInfo; return props.chargerAcOnline | props.chargerUsbOnline | props.chargerWirelessOnline | diff --git a/healthd/include/healthd/BatteryMonitor.h b/healthd/include/healthd/BatteryMonitor.h index 2a7e35329..8cbf5ead6 100644 --- a/healthd/include/healthd/BatteryMonitor.h +++ b/healthd/include/healthd/BatteryMonitor.h @@ -82,15 +82,6 @@ class BatteryMonitor { int mBatteryFixedCapacity; int mBatteryFixedTemperature; std::unique_ptr mHealthInfo; - - int readFromFile(const String8& path, std::string* buf); - PowerSupplyType readPowerSupplyType(const String8& path); - bool getBooleanField(const String8& path); - int getIntField(const String8& path); - bool isScopedPowerSupply(const char* name); - - static void logValues(const aidl::android::hardware::health::HealthInfo& health_info, - const struct healthd_config& healthd_config); }; }; // namespace android