From 67a8fd2175b3ff198f4d27d82c43f6296211858b Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Tue, 30 Nov 2021 16:39:00 -0800 Subject: GetBatteryInfo() also reads AIDL health HAL. Test: call GetBatteryInfo manually with and without AIDL health HAL Bug: 170338625 Bug: 177269435 Change-Id: I123739e5bc372d5198fd711f592ceac04d46ab28 --- Android.bp | 1 + minadbd/Android.bp | 2 ++ recovery_utils/Android.bp | 12 +++++++ recovery_utils/battery_utils.cpp | 73 +++++++++++++++++++++++++--------------- tests/Android.bp | 7 ++++ 5 files changed, 67 insertions(+), 28 deletions(-) diff --git a/Android.bp b/Android.bp index 52de77038..bd9570500 100644 --- a/Android.bp +++ b/Android.bp @@ -157,6 +157,7 @@ cc_binary { ], shared_libs: [ + "android.hardware.health-V1-ndk", // from librecovery_utils "librecovery_ui", ], diff --git a/minadbd/Android.bp b/minadbd/Android.bp index 2bcfece40..9d3f7334a 100644 --- a/minadbd/Android.bp +++ b/minadbd/Android.bp @@ -97,6 +97,7 @@ cc_binary { ], shared_libs: [ + "android.hardware.health-V1-ndk", // from librecovery_utils "libbase", "libcrypto", ], @@ -128,6 +129,7 @@ cc_test { ], static_libs: [ + "android.hardware.health-V1-ndk", // from librecovery_utils "libminadbd_services", "libfusesideload", "librecovery_utils", diff --git a/recovery_utils/Android.bp b/recovery_utils/Android.bp index e0e9ec058..9bd66c5d9 100644 --- a/recovery_utils/Android.bp +++ b/recovery_utils/Android.bp @@ -31,6 +31,7 @@ cc_defaults { shared_libs: [ "android.hardware.health@2.0", "libbase", + "libbinder_ndk", "libext4_utils", "libfs_mgr", "libhidlbase", @@ -42,8 +43,10 @@ cc_defaults { "libotautil", // External dependencies. + "android.hardware.health-translate-ndk", "libfstab", "libhealthhalutils", + "libhealthshim", ], } @@ -70,6 +73,15 @@ cc_library_static { "libvold_headers", ], + shared_libs: [ + // The following cannot be placed in librecovery_utils_defaults, + // because at the time of writing, android.hardware.health-V1-ndk.so + // is not installed to the system image yet. (It is installed + // to the recovery ramdisk.) Hence, minadbd_test must link to it + // statically. + "android.hardware.health-V1-ndk", + ], + export_include_dirs: [ "include", ], diff --git a/recovery_utils/battery_utils.cpp b/recovery_utils/battery_utils.cpp index 323f52537..6b126bdf3 100644 --- a/recovery_utils/battery_utils.cpp +++ b/recovery_utils/battery_utils.cpp @@ -20,59 +20,74 @@ #include #include +#include +#include #include BatteryInfo GetBatteryInfo() { - using android::hardware::health::V1_0::BatteryStatus; using android::hardware::health::V2_0::get_health_service; - using android::hardware::health::V2_0::IHealth; - using android::hardware::health::V2_0::Result; - using android::hardware::health::V2_0::toString; + using HidlHealth = android::hardware::health::V2_0::IHealth; + using aidl::android::hardware::health::BatteryStatus; + using aidl::android::hardware::health::HealthShim; + using aidl::android::hardware::health::IHealth; + using aidl::android::hardware::health::toString; + using std::string_literals::operator""s; - android::sp health = get_health_service(); + auto service_name = IHealth::descriptor + "/default"s; + std::shared_ptr health; + if (AServiceManager_isDeclared(service_name.c_str())) { + ndk::SpAIBinder binder(AServiceManager_waitForService(service_name.c_str())); + health = IHealth::fromBinder(binder); + } + if (health == nullptr) { + LOG(INFO) << "Unable to get AIDL health service, trying HIDL..."; + android::sp hidl_health = get_health_service(); + if (hidl_health != nullptr) { + health = ndk::SharedRefBase::make(hidl_health); + } + } + if (health == nullptr) { + LOG(WARNING) << "No health implementation is found; assuming defaults"; + } int wait_second = 0; while (true) { auto charge_status = BatteryStatus::UNKNOWN; - - if (health == nullptr) { - LOG(WARNING) << "No health implementation is found; assuming defaults"; - } else { - health - ->getChargeStatus([&charge_status](auto res, auto out_status) { - if (res == Result::SUCCESS) { - charge_status = out_status; - } - }) - .isOk(); // should not have transport error + if (health != nullptr) { + auto res = health->getChargeStatus(&charge_status); + if (!res.isOk()) { + LOG(WARNING) << "Unable to call getChargeStatus: " << res.getDescription(); + charge_status = BatteryStatus::UNKNOWN; + } } - // Treat unknown status as on charger. See hardware/interfaces/health/1.0/types.hal for the - // meaning of the return values. + // Treat unknown status as on charger. See hardware/interfaces/health/aidl/BatteryStatus.aidl + // for the meaning of the return values. bool charging = (charge_status != BatteryStatus::DISCHARGING && charge_status != BatteryStatus::NOT_CHARGING); - Result res = Result::UNKNOWN; int32_t capacity = INT32_MIN; if (health != nullptr) { - health - ->getCapacity([&res, &capacity](auto out_res, auto out_capacity) { - res = out_res; - capacity = out_capacity; - }) - .isOk(); // should not have transport error + auto res = health->getCapacity(&capacity); + if (!res.isOk()) { + LOG(WARNING) << "Unable to call getCapacity: " << res.getDescription(); + capacity = INT32_MIN; + } } LOG(INFO) << "charge_status " << toString(charge_status) << ", charging " << charging - << ", status " << toString(res) << ", capacity " << capacity; + << ", capacity " << capacity; constexpr int BATTERY_READ_TIMEOUT_IN_SEC = 10; // At startup, the battery drivers in devices like N5X/N6P take some time to load // the battery profile. Before the load finishes, it reports value 50 as a fake // capacity. BATTERY_READ_TIMEOUT_IN_SEC is set that the battery drivers are expected // to finish loading the battery profile earlier than 10 seconds after kernel startup. - if (res == Result::SUCCESS && capacity == 50) { + if (capacity == 50) { if (wait_second < BATTERY_READ_TIMEOUT_IN_SEC) { + LOG(INFO) << "Battery capacity == 50, waiting " + << (BATTERY_READ_TIMEOUT_IN_SEC - wait_second) + << " seconds to ensure this is not a fake value..."; sleep(1); wait_second++; continue; @@ -80,10 +95,12 @@ BatteryInfo GetBatteryInfo() { } // If we can't read battery percentage, it may be a device without battery. In this // situation, use 100 as a fake battery percentage. - if (res != Result::SUCCESS) { + if (capacity == INT32_MIN) { + LOG(WARNING) << "Using fake battery capacity 100."; capacity = 100; } + LOG(INFO) << "GetBatteryInfo() reporting charging " << charging << ", capacity " << capacity; return BatteryInfo{ charging, capacity }; } } diff --git a/tests/Android.bp b/tests/Android.bp index 5ef4d58c0..7f00adcd4 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -138,7 +138,14 @@ cc_test { "unit/*.cpp", ], + shared_libs: [ + "libbinder_ndk", + ], + static_libs: libapplypatch_static_libs + librecovery_static_libs + [ + "android.hardware.health-translate-ndk", + "android.hardware.health-V1-ndk", + "libhealthshim", "librecovery_ui", "libfusesideload", "libminui", -- cgit v1.2.3