diagnostics: Add prober for battery metrics
Implement the prober for battery metrics. Use powerd as the retrieval
source. In the future, hook this up to Chromium's DeviceStatusCollector.
Run 'cros_healthd --probe_battery_metrics' to exercise this behavior.
BUG=chromium:978128
TEST=unit tests and manual testing on DUT (running cros_healthd
--probe_battery_metrics)
Change-Id: I6dab2574a9b325927583e5a6be572003e4382d51
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1682268
Tested-by: Kartik Hegde <khegde@chromium.org>
Tested-by: Justin TerAvest <teravest@chromium.org>
Reviewed-by: Justin TerAvest <teravest@chromium.org>
Reviewed-by: Will Bradley <wbbradley@chromium.org>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
diff --git a/diagnostics/BUILD.gn b/diagnostics/BUILD.gn
index 5e2cad1..8af5b67 100644
--- a/diagnostics/BUILD.gn
+++ b/diagnostics/BUILD.gn
@@ -72,15 +72,18 @@
"libchrome-${libbase_ver}",
"libmojo-${libbase_ver}",
"libudev",
+ "system_api",
]
}
static_library("libcros_healthd") {
deps = [
":diagnostics_mojo_bindings",
+ "//diagnostics/grpc:diagnostics_grpc_protos",
]
all_dependent_configs = [ ":libcros_healthd_pkg_deps" ]
sources = [
+ "common/battery_utils.cc",
"common/disk_utils.cc",
"cros_healthd/cros_healthd_mojo_service.cc",
"cros_healthd/cros_healthd_routine_service_impl.cc",
@@ -181,6 +184,7 @@
":libcros_healthd",
]
sources = [
+ "common/battery_utils.cc",
"common/disk_utils.cc",
"cros_healthd/main.cc",
]
@@ -289,6 +293,7 @@
"../common-mk/testrunner:testrunner",
]
sources = [
+ "common/battery_utils_test.cc",
"cros_healthd/cros_healthd_mojo_service_test.cc",
]
}
diff --git a/diagnostics/common/battery_utils.cc b/diagnostics/common/battery_utils.cc
new file mode 100644
index 0000000..c6d8822
--- /dev/null
+++ b/diagnostics/common/battery_utils.cc
@@ -0,0 +1,105 @@
+// Copyright 2019 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "diagnostics/common/battery_utils.h"
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include <chromeos/dbus/service_constants.h>
+#include <dbus/bus.h>
+#include <dbus/message.h>
+#include <dbus/object_proxy.h>
+
+#include "power_manager/proto_bindings/power_supply_properties.pb.h"
+
+namespace diagnostics {
+
+using BatteryInfoPtr = ::chromeos::cros_healthd::mojom::BatteryInfoPtr;
+using BatteryInfo = ::chromeos::cros_healthd::mojom::BatteryInfo;
+
+// Extract the battery metrics from the PowerSupplyProperties protobuf.
+// Return true if the metrics could be successfully extracted from |response|
+// and put into |output_info|.
+bool ExtractBatteryMetrics(dbus::Response* response,
+ BatteryInfoPtr* output_info) {
+ DCHECK(response);
+ DCHECK(output_info);
+
+ BatteryInfo info;
+ power_manager::PowerSupplyProperties power_supply_proto;
+ dbus::MessageReader reader_response(response);
+ if (!reader_response.PopArrayOfBytesAsProto(&power_supply_proto)) {
+ LOG(ERROR) << "Could not successfully read power supply protobuf";
+ return false;
+ }
+
+ if (power_supply_proto.has_battery_cycle_count()) {
+ info.cycle_count = power_supply_proto.battery_cycle_count();
+ }
+ if (power_supply_proto.has_battery_vendor()) {
+ info.vendor = power_supply_proto.battery_vendor();
+ }
+ if (power_supply_proto.has_battery_voltage()) {
+ info.voltage_now = power_supply_proto.battery_voltage();
+ }
+ if (power_supply_proto.has_battery_charge_full()) {
+ info.charge_full = power_supply_proto.battery_charge_full();
+ }
+ if (power_supply_proto.has_battery_charge_full_design()) {
+ info.charge_full_design = power_supply_proto.battery_charge_full_design();
+ }
+ if (power_supply_proto.has_battery_serial_number()) {
+ info.serial_number = power_supply_proto.battery_serial_number();
+ }
+ if (power_supply_proto.has_battery_voltage_min_design()) {
+ info.voltage_min_design = power_supply_proto.battery_voltage_min_design();
+ }
+
+ *output_info = info.Clone();
+
+ return true;
+}
+
+namespace {
+
+// Make a D-bus call to get the PowerSupplyProperties proto, which contains the
+// battery metrics.
+bool FetchBatteryMetrics(BatteryInfoPtr* output_info) {
+ dbus::Bus::Options options;
+ scoped_refptr<dbus::Bus> bus(new dbus::Bus(options));
+ if (!bus->Connect()) {
+ LOG(ERROR) << "Failed to connect to the system bus";
+ return false;
+ }
+ dbus::ObjectProxy* bus_proxy = bus->GetObjectProxy(
+ power_manager::kPowerManagerServiceName,
+ dbus::ObjectPath(power_manager::kPowerManagerServicePath));
+ constexpr base::TimeDelta kPowerManagerDBusTimeout =
+ base::TimeDelta::FromSeconds(3);
+ dbus::MethodCall method_call(power_manager::kPowerManagerInterface,
+ power_manager::kGetPowerSupplyPropertiesMethod);
+ auto response = bus_proxy->CallMethodAndBlock(
+ &method_call, kPowerManagerDBusTimeout.InMilliseconds());
+ return ExtractBatteryMetrics(response.get(), output_info);
+}
+
+} // namespace
+
+std::vector<BatteryInfoPtr> FetchBatteryInfo() {
+ // Since Chromebooks currently only support a single battery (main battery),
+ // the vector should have a size of one. In the future, if Chromebooks
+ // contain more batteries, they can easily be supported by the vector.
+ std::vector<BatteryInfoPtr> batteries{};
+
+ BatteryInfoPtr info;
+ if (FetchBatteryMetrics(&info)) {
+ batteries.push_back(std::move(info));
+ }
+
+ return batteries;
+}
+
+} // namespace diagnostics
diff --git a/diagnostics/common/battery_utils.h b/diagnostics/common/battery_utils.h
new file mode 100644
index 0000000..185e19b
--- /dev/null
+++ b/diagnostics/common/battery_utils.h
@@ -0,0 +1,19 @@
+// Copyright 2019 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DIAGNOSTICS_COMMON_BATTERY_UTILS_H_
+#define DIAGNOSTICS_COMMON_BATTERY_UTILS_H_
+
+#include <vector>
+
+#include "mojo/cros_healthd_probe.mojom.h"
+
+namespace diagnostics {
+
+// Retrieves the main battery metrics from powerd over D-bus.
+std::vector<chromeos::cros_healthd::mojom::BatteryInfoPtr> FetchBatteryInfo();
+
+} // namespace diagnostics
+
+#endif // DIAGNOSTICS_COMMON_BATTERY_UTILS_H_
diff --git a/diagnostics/common/battery_utils_test.cc b/diagnostics/common/battery_utils_test.cc
new file mode 100644
index 0000000..70eb168
--- /dev/null
+++ b/diagnostics/common/battery_utils_test.cc
@@ -0,0 +1,83 @@
+// Copyright 2019 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "diagnostics/common/battery_utils.h"
+
+#include <map>
+#include <memory>
+#include <utility>
+
+#include <base/message_loop/message_loop.h>
+#include <base/time/time.h>
+#include <dbus/message.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "power_manager/proto_bindings/power_supply_properties.pb.h"
+
+namespace diagnostics {
+
+using ::chromeos::cros_healthd::mojom::BatteryInfoPtr;
+using ::testing::_;
+
+bool ExtractBatteryMetrics(dbus::Response* response,
+ BatteryInfoPtr* output_info);
+
+namespace {
+
+const char kBatteryVendor[] = "TEST_MFR";
+const double kBatteryVoltage = 127.45;
+const int kBatteryCycleCount = 2;
+const char kBatterySerialNumber[] = "1000";
+const double kBatteryVoltageMinDesign = 114.00;
+const double kBatteryChargeFull = 4.3;
+const double kBatteryChargeFullDesign = 3.92;
+
+// Test the expected path of extracting battery metrics from a D-bus response.
+TEST(BatteryUtils, TestExtractingBatteryMetrics) {
+ // Create PowerSupplyProperties response protobuf.
+ std::unique_ptr<dbus::Response> response = dbus::Response::CreateEmpty();
+ dbus::MessageWriter writer(response.get());
+ power_manager::PowerSupplyProperties power_supply_proto;
+ power_supply_proto.set_battery_vendor(kBatteryVendor);
+ power_supply_proto.set_battery_voltage(kBatteryVoltage);
+ power_supply_proto.set_battery_cycle_count(kBatteryCycleCount);
+ power_supply_proto.set_battery_charge_full(kBatteryChargeFull);
+ power_supply_proto.set_battery_charge_full_design(kBatteryChargeFullDesign);
+ power_supply_proto.set_battery_serial_number(kBatterySerialNumber);
+ power_supply_proto.set_battery_voltage_min_design(kBatteryVoltageMinDesign);
+ writer.AppendProtoAsArrayOfBytes(power_supply_proto);
+
+ BatteryInfoPtr info;
+
+ ExtractBatteryMetrics(response.get(), &info);
+ ASSERT_EQ(kBatteryCycleCount, info->cycle_count);
+ ASSERT_EQ(kBatteryVendor, info->vendor);
+ ASSERT_EQ(kBatteryVoltage, info->voltage_now);
+ ASSERT_EQ(kBatteryChargeFull, info->charge_full);
+ ASSERT_EQ(kBatteryChargeFullDesign, info->charge_full_design);
+ ASSERT_EQ(kBatterySerialNumber, info->serial_number);
+ ASSERT_EQ(kBatteryVoltageMinDesign, info->voltage_min_design);
+}
+
+// Test that ExtractBatteryMetrics exits safely (returning false) when it
+// receves a bad D-bus response.
+TEST(BatteryUtils, TestBadDbusResponse) {
+ std::unique_ptr<dbus::Response> response = dbus::Response::CreateEmpty();
+ dbus::MessageWriter writer(response.get());
+ writer.AppendVariantOfBool(false);
+
+ dbus::MessageReader reader_response(response.get());
+ power_manager::PowerSupplyProperties power_supply_properties_proto;
+ ASSERT_FALSE(
+ reader_response.PopArrayOfBytesAsProto(&power_supply_properties_proto));
+
+ BatteryInfoPtr info;
+
+ ASSERT_FALSE(ExtractBatteryMetrics(response.get(), &info));
+}
+
+} // namespace
+
+} // namespace diagnostics
diff --git a/diagnostics/cros_healthd/main.cc b/diagnostics/cros_healthd/main.cc
index 186fcff..9ad170d 100644
--- a/diagnostics/cros_healthd/main.cc
+++ b/diagnostics/cros_healthd/main.cc
@@ -2,18 +2,22 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <cstdio>
#include <cstdlib>
#include <iostream>
+#include <string>
#include <brillo/flag_helper.h>
#include <brillo/syslog_logging.h>
+#include "diagnostics/common/battery_utils.h"
#include "diagnostics/common/disk_utils.h"
int main(int argc, char** argv) {
DEFINE_bool(probe_block_devices, false,
"Exercise the ProbeNonRemovableBlockDeviceInfo routine");
- DEFINE_bool(probe_batteries, false, "Exercise the ProbeBatteryInfo routine");
+ DEFINE_bool(probe_battery_metrics, false,
+ "Exercise the ProbeBatteryInfo routine");
brillo::FlagHelper::Init(
argc, argv, "cros_healthd - Device telemetry and diagnostics daemon.");
@@ -26,16 +30,27 @@
diagnostics::disk_utils::FetchNonRemovableBlockDevicesInfo(root_dir);
VLOG(1) << "Found " << devices.size() << " non-removable block device(s)."
<< std::endl;
- std::cout << "path,size,type,manfid,name,serial" << std::endl;
- for (auto& device : devices) {
- std::cout << device->path << "," << std::dec << device->size << ","
- << device->type << ",0x" << std::hex
- << static_cast<int>(device->manufacturer_id) << ","
- << device->name << ",0x" << std::hex << device->serial
- << std::endl;
+ printf("path,size,type,manfid,name,serial\n");
+ for (const auto& device : devices) {
+ printf("%s,%ld,%s,0x%x,%s,0x%x\n", device->path.c_str(), device->size,
+ device->type.c_str(), static_cast<int>(device->manufacturer_id),
+ device->name.c_str(), device->serial);
}
- } else if (FLAGS_probe_batteries) {
- NOTIMPLEMENTED();
+ } else if (FLAGS_probe_battery_metrics) {
+ auto batteries = diagnostics::FetchBatteryInfo();
+ if (batteries.size() != 1) {
+ LOG(ERROR) << "Did not properly fetch information for main battery.";
+ return EXIT_FAILURE;
+ }
+ VLOG(1) << "Found information for main battery.";
+ printf(
+ "charge_full,charge_full_design,cycle_count,serial_number,"
+ "vendor(manufacturer),voltage_now,voltage_min_design\n");
+ const auto& battery = batteries[0];
+ printf("%f,%f,%ld,%s,%s,%f,%f\n", battery->charge_full,
+ battery->charge_full_design, battery->cycle_count,
+ battery->serial_number.c_str(), battery->vendor.c_str(),
+ battery->voltage_now, battery->voltage_min_design);
} else {
// TODO(pmoy): implement daemon
}
diff --git a/diagnostics/mojo/cros_healthd_probe.mojom b/diagnostics/mojo/cros_healthd_probe.mojom
index a39e53f..408138d 100644
--- a/diagnostics/mojo/cros_healthd_probe.mojom
+++ b/diagnostics/mojo/cros_healthd_probe.mojom
@@ -11,42 +11,20 @@
module chromeos.cros_healthd.mojom;
struct BatteryInfo {
- // (1) Most fields are following the naming of exposed ACPI interface.
- // (2) Most units are in µ because of this unexplained kernel patch:
- // https://chromium.git.corp.google.com/chromiumos/third_party/kernel/+/d7380965752505951668e85de59c128d1d6fd21f%5E%21/#F1
-
- // Index number of this battery, starts from 1.
- int32 index@0;
- // Manufacturer for length not exceeding EC_COMM_TEXT_MAX.
- string manufacturer@1;
- // Model name for length not exceeding EC_COMM_TEXT_MAX.
- string model_name@2;
- // Serial number for length not exceeding EC_COMM_TEXT_MAX.
- string serial_number@3;
- // Design Capacity (µAh).
- int32 charge_full_design@4;
- // Full Capacity (µAh, might change occasionally).
- int32 charge_full@5;
- // Remaining capacity (µAh)
- int32 charge_now@6;
- // Current Battery voltage (µV)
- int32 voltage_now@7;
- // Designed minimum output voltage (µV)
- int32 voltage_min_design@8;
- // Smart Battery Cycle Count in http://sbs-forum.org/specs/sbdat110.pdf
- int32 cycle_count_smart@9;
- // Smart Battery Status defined in http://sbs-forum.org/specs/sbdat110.pdf
- int32 status_smart@10;
- // Temperature in 0.1°K as Smart Battery Temperature defined in
- // http://sbs-forum.org/specs/sbdat110.pdf
- int32 temperature_smart@11;
- // The path of this battery in system. It is useful if caller needs to
- // correlate with other information
- string path@12;
- // Smart Manufacture Date is defined in
- // http://sbs-forum.org/specs/sbdat110.pdf.
- // The value is calculated by ((year-1980) * 512 + month * 32 + day).
- int32 manufacture_date_smart@13;
+ // TODO(https://crbug.com/979245): Update "smart" cycle count.
+ int64 cycle_count;
+ // Current battery voltage (V)
+ double voltage_now;
+ // Manufacturer of the battery
+ string vendor;
+ // Serial number of the battery
+ string serial_number;
+ // Design capacity (Ah)
+ double charge_full_design;
+ // Full capacity (Ah)
+ double charge_full;
+ // Desired minimum output voltage (V)
+ double voltage_min_design;
};
struct NonRemovableBlockDeviceInfo {
