From 832c9cd24fcff55da0460b8587a1c5e6145e8ab5 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 27 Sep 2019 23:32:00 -0700 Subject: [PATCH] Refactor battery info querying functions into librecovery_utils. Bug: 134560109 Test: Run recovery_unit_test. Change-Id: Ibbcdcfd507fa23657ee7ff677208b0003ec382ba --- Android.bp | 5 +- recovery.cpp | 85 +++--------------- recovery_utils/Android.bp | 43 +++++---- recovery_utils/battery_utils.cpp | 89 +++++++++++++++++++ .../include/recovery_utils/battery_utils.h | 33 +++++++ tests/unit/battery_utils_test.cpp | 27 ++++++ 6 files changed, 191 insertions(+), 91 deletions(-) create mode 100644 recovery_utils/battery_utils.cpp create mode 100644 recovery_utils/include/recovery_utils/battery_utils.h create mode 100644 tests/unit/battery_utils_test.cpp diff --git a/Android.bp b/Android.bp index 0759e08d..45aafb04 100644 --- a/Android.bp +++ b/Android.bp @@ -58,7 +58,6 @@ cc_defaults { ], shared_libs: [ - "android.hardware.health@2.0", "libbase", "libbootloader_message", "libcrypto", @@ -74,9 +73,6 @@ cc_defaults { "libminui", "librecovery_utils", "libotautil", - - // external dependencies - "libhealthhalutils", ], } @@ -105,6 +101,7 @@ cc_binary { defaults: [ "libinstall_defaults", "librecovery_defaults", + "librecovery_utils_defaults", ], srcs: [ diff --git a/recovery.cpp b/recovery.cpp index 682ddbc4..f59a940f 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -42,7 +42,6 @@ #include #include /* for property_list */ #include -#include #include #include "bootloader_message/bootloader_message.h" @@ -59,6 +58,7 @@ #include "otautil/sysutil.h" #include "recovery_ui/screen_ui.h" #include "recovery_ui/ui.h" +#include "recovery_utils/battery_utils.h" #include "recovery_utils/logging.h" #include "recovery_utils/roots.h" @@ -453,74 +453,17 @@ static void print_property(const char* key, const char* name, void* /* cookie */ printf("%s=%s\n", key, name); } -static bool is_battery_ok(int* required_battery_level) { - 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; +static bool IsBatteryOk(int* required_battery_level) { + // GmsCore enters recovery mode to install package when having enough battery percentage. + // Normally, the threshold is 40% without charger and 20% with charger. So we check the battery + // level against a slightly lower limit. + constexpr int BATTERY_OK_PERCENTAGE = 20; + constexpr int BATTERY_WITH_CHARGER_OK_PERCENTAGE = 15; - android::sp health = get_health_service(); - - static constexpr int BATTERY_READ_TIMEOUT_IN_SEC = 10; - 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 - } - - // Treat unknown status as charged. - bool charged = (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 - } - - LOG(INFO) << "charge_status " << toString(charge_status) << ", charged " << charged - << ", status " << toString(res) << ", capacity " << capacity; - // 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 (wait_second < BATTERY_READ_TIMEOUT_IN_SEC) { - sleep(1); - wait_second++; - continue; - } - } - // 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) { - capacity = 100; - } - - // GmsCore enters recovery mode to install package when having enough battery percentage. - // Normally, the threshold is 40% without charger and 20% with charger. So we should check - // battery with a slightly lower limitation. - static constexpr int BATTERY_OK_PERCENTAGE = 20; - static constexpr int BATTERY_WITH_CHARGER_OK_PERCENTAGE = 15; - *required_battery_level = charged ? BATTERY_WITH_CHARGER_OK_PERCENTAGE : BATTERY_OK_PERCENTAGE; - return capacity >= *required_battery_level; - } + auto battery_info = GetBatteryInfo(); + *required_battery_level = + battery_info.charging ? BATTERY_WITH_CHARGER_OK_PERCENTAGE : BATTERY_OK_PERCENTAGE; + return battery_info.capacity >= *required_battery_level; } // Set the retry count to |retry_count| in BCB. @@ -713,12 +656,10 @@ Device::BuiltinAction start_recovery(Device* device, const std::vectorPrint("battery capacity is not enough for installing package: %d%% needed\n", required_battery_level); - // Log the error code to last_install when installation skips due to - // low battery. + // Log the error code to last_install when installation skips due to low battery. log_failure_code(kLowBattery, update_package); status = INSTALL_SKIPPED; } else if (retry_count == 0 && bootreason_in_blacklist()) { diff --git a/recovery_utils/Android.bp b/recovery_utils/Android.bp index 271d0799..463f27fd 100644 --- a/recovery_utils/Android.bp +++ b/recovery_utils/Android.bp @@ -12,6 +12,32 @@ // See the License for the specific language governing permissions and // limitations under the License. +cc_defaults { + name: "librecovery_utils_defaults", + + defaults: [ + "recovery_defaults", + ], + + shared_libs: [ + "android.hardware.health@2.0", + "libbase", + "libext4_utils", + "libfs_mgr", + "libhidlbase", + "libselinux", + "libutils", + ], + + static_libs: [ + "libotautil", + + // External dependencies. + "libfstab", + "libhealthhalutils", + ], +} + // A utility lib that's local to recovery (in contrast, libotautil is exposed to device-specific // recovery_ui lib as well as device-specific updater). cc_library_static { @@ -20,23 +46,17 @@ cc_library_static { recovery_available: true, defaults: [ - "recovery_defaults", + "librecovery_utils_defaults", ], srcs: [ + "battery_utils.cpp", "logging.cpp", "parse_install_logs.cpp", "roots.cpp", "thermalutil.cpp", ], - shared_libs: [ - "libbase", - "libext4_utils", - "libfs_mgr", - "libselinux", - ], - export_include_dirs: [ "include", ], @@ -45,13 +65,6 @@ cc_library_static { "system/vold", ], - static_libs: [ - "libotautil", - - // external dependency - "libfstab", - ], - export_static_lib_headers: [ "libfstab", ], diff --git a/recovery_utils/battery_utils.cpp b/recovery_utils/battery_utils.cpp new file mode 100644 index 00000000..323f5253 --- /dev/null +++ b/recovery_utils/battery_utils.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "recovery_utils/battery_utils.h" + +#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; + + android::sp health = get_health_service(); + + 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 + } + + // Treat unknown status as on charger. See hardware/interfaces/health/1.0/types.hal 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 + } + + LOG(INFO) << "charge_status " << toString(charge_status) << ", charging " << charging + << ", status " << toString(res) << ", 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 (wait_second < BATTERY_READ_TIMEOUT_IN_SEC) { + sleep(1); + wait_second++; + continue; + } + } + // 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) { + capacity = 100; + } + + return BatteryInfo{ charging, capacity }; + } +} diff --git a/recovery_utils/include/recovery_utils/battery_utils.h b/recovery_utils/include/recovery_utils/battery_utils.h new file mode 100644 index 00000000..a95f71dc --- /dev/null +++ b/recovery_utils/include/recovery_utils/battery_utils.h @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +struct BatteryInfo { + // Whether the device is on charger. Note that the value will be `true` if the battery status is + // unknown (BATTERY_STATUS_UNKNOWN). + bool charging; + + // The remaining battery capacity percentage (i.e. between 0 and 100). See getCapacity in + // hardware/interfaces/health/2.0/IHealth.hal. Returns 100 in case it fails to read a value from + // the health HAL. + int32_t capacity; +}; + +// Returns the battery status for OTA installation purpose. +BatteryInfo GetBatteryInfo(); diff --git a/tests/unit/battery_utils_test.cpp b/tests/unit/battery_utils_test.cpp new file mode 100644 index 00000000..55639fdb --- /dev/null +++ b/tests/unit/battery_utils_test.cpp @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agree to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include "recovery_utils/battery_utils.h" + +TEST(BatteryInfoTest, GetBatteryInfo) { + auto info = GetBatteryInfo(); + // 0 <= capacity <= 100 + ASSERT_LE(0, info.capacity); + ASSERT_LE(info.capacity, 100); +}