Add BatteryHealthObserver to SystemDataProvider

Allows clients to implement the BatteryHealthObserver interface and
register themselves as an observer in order to receive periodic updates
about the battery health.

Bug: 1128204
Change-Id: Ib4ffc60ec5fd30bda37c255bd17b669eaa4b9f74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438474
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812822}

chromeos/components/diagnostics_ui/backend/system_data_provider.cc

@@ -29,7 +29,9 @@ using PhysicalCpuInfos = std::vector<healthd::PhysicalCpuInfoPtr>;
 using PowerSupplyProperties = power_manager::PowerSupplyProperties;
 using ProbeCategories = healthd::ProbeCategoryEnum;
 
+constexpr int kBatteryHealthRefreshIntervalInSeconds = 60;
 constexpr int kChargeStatusRefreshIntervalInSeconds = 15;
+constexpr int kMilliampsInAnAmp = 1000;
 
 void PopulateBoardName(const healthd::SystemInfo& system_info,
                        mojom::SystemInfo& out_system_info) {
@@ -86,9 +88,8 @@ void PopulateDeviceCapabilities(const healthd::TelemetryInfo& telemetry_info,
 void PopulateBatteryInfo(const healthd::BatteryInfo& battery_info,
                          mojom::BatteryInfo& out_battery_info) {
   out_battery_info.manufacturer = battery_info.vendor;
-  // Multiply by 1000 to convert amps to milliamps.
   out_battery_info.charge_full_design_milliamp_hours =
-      battery_info.charge_full_design * 1000;
+      battery_info.charge_full_design * kMilliampsInAnAmp;
 }
 
 void PopulatePowerInfo(const PowerSupplyProperties& power_supply_properties,
@@ -109,15 +110,29 @@ void PopulateBatteryChargeStatus(
     mojom::BatteryChargeStatus& out_charge_status) {
   PopulatePowerInfo(power_supply_properties, out_charge_status);
 
-  // Multiply by 1000 to convert amps to milliamps.
-  out_charge_status.current_now_milliamps = battery_info.current_now * 1000;
-  out_charge_status.charge_now_milliamp_hours = battery_info.charge_now * 1000;
+  out_charge_status.current_now_milliamps =
+      battery_info.current_now * kMilliampsInAnAmp;
+  out_charge_status.charge_now_milliamp_hours =
+      battery_info.charge_now * kMilliampsInAnAmp;
+}
+
+void PopulateBatteryHealth(const healthd::BatteryInfo& battery_info,
+                           mojom::BatteryHealth& out_battery_health) {
+  out_battery_health.charge_full_now_milliamp_hours =
+      battery_info.charge_full * kMilliampsInAnAmp;
+  out_battery_health.charge_full_design_milliamp_hours =
+      battery_info.charge_full_design * kMilliampsInAnAmp;
+  out_battery_health.cycle_count = battery_info.cycle_count;
+  out_battery_health.battery_wear_percentage =
+      out_battery_health.charge_full_now_milliamp_hours /
+      out_battery_health.charge_full_design_milliamp_hours;
 }
 
 }  // namespace
 
 SystemDataProvider::SystemDataProvider() {
   battery_charge_status_timer_ = std::make_unique<base::RepeatingTimer>();
+  battery_health_timer_ = std::make_unique<base::RepeatingTimer>();
   PowerManagerClient::Get()->AddObserver(this);
 }
 
@@ -158,6 +173,20 @@ void SystemDataProvider::ObserveBatteryChargeStatus(
   UpdateBatteryChargeStatus();
 }
 
+void SystemDataProvider::ObserveBatteryHealth(
+    mojo::PendingRemote<mojom::BatteryHealthObserver> observer) {
+  battery_health_observers_.Add(std::move(observer));
+
+  if (!battery_health_timer_->IsRunning()) {
+    battery_health_timer_->Start(
+        FROM_HERE,
+        base::TimeDelta::FromSeconds(kBatteryHealthRefreshIntervalInSeconds),
+        base::BindRepeating(&SystemDataProvider::UpdateBatteryHealth,
+                            base::Unretained(this)));
+  }
+  UpdateBatteryHealth();
+}
+
 void SystemDataProvider::PowerChanged(
     const power_manager::PowerSupplyProperties& proto) {
   if (battery_charge_status_observers_.empty()) {
@@ -177,6 +206,11 @@ void SystemDataProvider::SetBatteryChargeStatusTimerForTesting(
   battery_charge_status_timer_ = std::move(timer);
 }
 
+void SystemDataProvider::SetBatteryHealthTimerForTesting(
+    std::unique_ptr<base::RepeatingTimer> timer) {
+  battery_health_timer_ = std::move(timer);
+}
+
 void SystemDataProvider::OnSystemInfoProbeResponse(
     GetSystemInfoCallback callback,
     healthd::TelemetryInfoPtr info_ptr) {
@@ -258,6 +292,15 @@ void SystemDataProvider::UpdateBatteryChargeStatus() {
                      base::Unretained(this), properties));
 }
 
+void SystemDataProvider::UpdateBatteryHealth() {
+  BindCrosHealthdProbeServiceIfNeccessary();
+
+  probe_service_->ProbeTelemetryInfo(
+      {ProbeCategories::kBattery},
+      base::BindOnce(&SystemDataProvider::OnBatteryHealthUpdated,
+                     base::Unretained(this)));
+}
+
 void SystemDataProvider::OnBatteryChargeStatusUpdated(
     const base::Optional<PowerSupplyProperties>& power_supply_properties,
     healthd::TelemetryInfoPtr info_ptr) {
@@ -294,6 +337,28 @@ void SystemDataProvider::OnBatteryChargeStatusUpdated(
   NotifyBatteryChargeStatusObservers(battery_charge_status);
 }
 
+void SystemDataProvider::OnBatteryHealthUpdated(
+    healthd::TelemetryInfoPtr info_ptr) {
+  mojom::BatteryHealthPtr battery_health = mojom::BatteryHealth::New();
+
+  if (info_ptr.is_null()) {
+    LOG(ERROR) << "Null response from croshealthd::ProbeTelemetryInfo.";
+    NotifyBatteryHealthObservers(battery_health);
+    battery_health_timer_.reset();
+    return;
+  }
+
+  if (!DoesDeviceHaveBattery(*info_ptr)) {
+    NotifyBatteryHealthObservers(battery_health);
+    battery_health_timer_.reset();
+    return;
+  }
+
+  PopulateBatteryHealth(*diagnostics::GetBatteryInfo(*info_ptr),
+                        *battery_health.get());
+  NotifyBatteryHealthObservers(battery_health);
+}
+
 void SystemDataProvider::NotifyBatteryChargeStatusObservers(
     const mojom::BatteryChargeStatusPtr& battery_charge_status) {
   for (auto& observer : battery_charge_status_observers_) {
@@ -301,6 +366,13 @@ void SystemDataProvider::NotifyBatteryChargeStatusObservers(
   }
 }
 
+void SystemDataProvider::NotifyBatteryHealthObservers(
+    const mojom::BatteryHealthPtr& battery_health) {
+  for (auto& observer : battery_health_observers_) {
+    observer->OnBatteryHealthUpdated(battery_health.Clone());
+  }
+}
+
 void SystemDataProvider::BindCrosHealthdProbeServiceIfNeccessary() {
   if (!probe_service_ || !probe_service_.is_connected()) {
     cros_healthd::ServiceConnection::GetInstance()->GetProbeService(

chromeos/components/diagnostics_ui/backend/system_data_provider.h

@@ -37,6 +37,8 @@ class SystemDataProvider : public mojom::SystemDataProvider,
   void ObserveBatteryChargeStatus(
       mojo::PendingRemote<mojom::BatteryChargeStatusObserver> observer)
       override;
+  void ObserveBatteryHealth(
+      mojo::PendingRemote<mojom::BatteryHealthObserver> observer) override;
 
   // PowerManagerClient::Observer:
   void PowerChanged(const power_manager::PowerSupplyProperties& proto) override;
@@ -44,6 +46,9 @@ class SystemDataProvider : public mojom::SystemDataProvider,
   void SetBatteryChargeStatusTimerForTesting(
       std::unique_ptr<base::RepeatingTimer> timer);
 
+  void SetBatteryHealthTimerForTesting(
+      std::unique_ptr<base::RepeatingTimer> timer);
+
  private:
   void BindCrosHealthdProbeServiceIfNeccessary();
 
@@ -59,19 +64,28 @@ class SystemDataProvider : public mojom::SystemDataProvider,
 
   void UpdateBatteryChargeStatus();
 
+  void UpdateBatteryHealth();
+
   void NotifyBatteryChargeStatusObservers(
       const mojom::BatteryChargeStatusPtr& battery_charge_status);
 
+  void NotifyBatteryHealthObservers(
+      const mojom::BatteryHealthPtr& battery_health);
+
   void OnBatteryChargeStatusUpdated(
       const base::Optional<power_manager::PowerSupplyProperties>&
           power_supply_properties,
       cros_healthd::mojom::TelemetryInfoPtr info_ptr);
 
+  void OnBatteryHealthUpdated(cros_healthd::mojom::TelemetryInfoPtr info_ptr);
+
   mojo::Remote<cros_healthd::mojom::CrosHealthdProbeService> probe_service_;
   mojo::RemoteSet<mojom::BatteryChargeStatusObserver>
       battery_charge_status_observers_;
+  mojo::RemoteSet<mojom::BatteryHealthObserver> battery_health_observers_;
 
   std::unique_ptr<base::RepeatingTimer> battery_charge_status_timer_;
+  std::unique_ptr<base::RepeatingTimer> battery_health_timer_;
 };
 
 }  // namespace diagnostics

chromeos/components/diagnostics_ui/backend/system_data_provider_unittest.cc

@@ -156,6 +156,27 @@ CreateCrosHealthdBatteryChargeStatusResponse(double charge_now,
       /*temperature=*/0);
 }
 
+cros_healthd::mojom::BatteryInfoPtr CreateCrosHealthdBatteryHealthResponse(
+    double charge_full_now,
+    double charge_full_design,
+    int32_t cycle_count) {
+  return CreateCrosHealthdBatteryInfoResponse(
+      /*cycle_count=*/cycle_count,
+      /*voltage_now=*/0,
+      /*vendor=*/"",
+      /*serial_number=*/"",
+      /*charge_full_design=*/charge_full_design,
+      /*charge_full=*/charge_full_now,
+      /*voltage_min_design=*/0,
+      /*model_name=*/"",
+      /*charge_now=*/0,
+      /*current_now=*/0,
+      /*technology=*/"",
+      /*status=*/"",
+      /*manufacture_date=*/base::nullopt,
+      /*temperature=*/0);
+}
+
 void SetCrosHealthdBatteryInfoResponse(const std::string& vendor,
                                        double charge_full_design) {
   cros_healthd::mojom::BatteryInfoPtr battery_info =
@@ -174,6 +195,17 @@ void SetCrosHealthdBatteryChargeStatusResponse(double charge_now,
                                 /*memory_info=*/nullptr);
 }
 
+void SetCrosHealthdBatteryHealthResponse(double charge_full_now,
+                                         double charge_full_design,
+                                         int32_t cycle_count) {
+  cros_healthd::mojom::BatteryInfoPtr battery_info =
+      CreateCrosHealthdBatteryHealthResponse(charge_full_now,
+                                             charge_full_design, cycle_count);
+  SetProbeTelemetryInfoResponse(std::move(battery_info), /*cpu_info=*/nullptr,
+                                /*memory_info=*/nullptr,
+                                /*memory_info=*/nullptr);
+}
+
 bool AreValidPowerTimes(int64_t time_to_full, int64_t time_to_empty) {
   // Exactly one of |time_to_full| or |time_to_empty| must be zero. The other
   // can be a positive integer to represent the time to charge/discharge or -1
@@ -209,9 +241,9 @@ power_manager::PowerSupplyProperties ConstructPowerSupplyProperties(
 
 // Sets the PowerSupplyProperties on FakePowerManagerClient. Calling this
 // method immediately notifies PowerManagerClient observers. One of
-// |time_to_full| or |time_to_empty| must be either -1 or a positive number. The
-// other must be 0. If |battery_state| is NOT_PRESENT, both |time_to_full| and
-// |time_to_empty| will be left unset.
+// |time_to_full| or |time_to_empty| must be either -1 or a positive number.
+// The other must be 0. If |battery_state| is NOT_PRESENT, both |time_to_full|
+// and |time_to_empty| will be left unset.
 void SetPowerManagerProperties(
     power_manager::PowerSupplyProperties::ExternalPower power_source,
     power_manager::PowerSupplyProperties::BatteryState battery_state,
@@ -263,6 +295,26 @@ void VerifyChargeStatusResult(
   EXPECT_EQ(expected_power_time, update->power_time);
 }
 
+void VerifyHealthResult(const mojom::BatteryHealthPtr& update,
+                        double charge_full_now,
+                        double charge_full_design,
+                        int32_t expected_cycle_count) {
+  const int32_t expected_charge_full_now_milliamp_hours =
+      charge_full_now * 1000;
+  const int32_t expected_charge_full_design_milliamp_hours =
+      charge_full_design * 1000;
+  const int8_t expected_battery_wear_percentage =
+      expected_charge_full_now_milliamp_hours /
+      expected_charge_full_design_milliamp_hours;
+
+  EXPECT_EQ(expected_charge_full_now_milliamp_hours,
+            update->charge_full_now_milliamp_hours);
+  EXPECT_EQ(expected_charge_full_design_milliamp_hours,
+            update->charge_full_design_milliamp_hours);
+  EXPECT_EQ(expected_cycle_count, update->cycle_count);
+  EXPECT_EQ(expected_battery_wear_percentage, update->battery_wear_percentage);
+}
+
 }  // namespace
 
 struct FakeBatteryChargeStatusObserver
@@ -280,6 +332,19 @@ struct FakeBatteryChargeStatusObserver
   mojo::Receiver<mojom::BatteryChargeStatusObserver> receiver{this};
 };
 
+struct FakeBatteryHealthObserver : public mojom::BatteryHealthObserver {
+  // mojom::BatteryHealthObserver
+  void OnBatteryHealthUpdated(mojom::BatteryHealthPtr status_ptr) override {
+    updates.emplace_back(std::move(status_ptr));
+  }
+
+  // Tracks calls to OnBatteryHealthUpdated. Each call adds an element to
+  // the vector.
+  std::vector<mojom::BatteryHealthPtr> updates;
+
+  mojo::Receiver<mojom::BatteryHealthObserver> receiver{this};
+};
+
 class SystemDataProviderTest : public testing::Test {
  public:
   SystemDataProviderTest() {
@@ -446,5 +511,53 @@ TEST_F(SystemDataProviderTest, BatteryChargeStatusObserver) {
                            new_time_to_full_secs, time_to_empty_secs);
 }
 
+TEST_F(SystemDataProviderTest, BatteryHealthObserver) {
+  // Setup Timer
+  auto timer = std::make_unique<base::MockRepeatingTimer>();
+  auto* timer_ptr = timer.get();
+  system_data_provider_->SetBatteryHealthTimerForTesting(std::move(timer));
+
+  // Setup initial data
+  const double charge_full_now = 20;
+  const double charge_full_design = 26;
+  const int32_t cycle_count = 500;
+
+  SetCrosHealthdBatteryHealthResponse(charge_full_now, charge_full_design,
+                                      cycle_count);
+
+  // Registering as an observer should trigger one update.
+  FakeBatteryHealthObserver health_observer;
+  system_data_provider_->ObserveBatteryHealth(
+      health_observer.receiver.BindNewPipeAndPassRemote());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(1u, health_observer.updates.size());
+  VerifyHealthResult(health_observer.updates[0], charge_full_now,
+                     charge_full_design, cycle_count);
+
+  // Firing the timer should trigger another.
+  timer_ptr->Fire();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(2u, health_observer.updates.size());
+  VerifyHealthResult(health_observer.updates[1], charge_full_now,
+                     charge_full_design, cycle_count);
+
+  // Updating the information in Croshealthd does not trigger an update until
+  // the timer fires
+  const int32_t new_cycle_count = cycle_count + 1;
+  SetCrosHealthdBatteryHealthResponse(charge_full_now, charge_full_design,
+                                      new_cycle_count);
+
+  EXPECT_EQ(2u, health_observer.updates.size());
+
+  timer_ptr->Fire();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(3u, health_observer.updates.size());
+  VerifyHealthResult(health_observer.updates[2], charge_full_now,
+                     charge_full_design, new_cycle_count);
+}
+
 }  // namespace diagnostics
 }  // namespace chromeos

chromeos/components/diagnostics_ui/mojom/system_data_provider.mojom

@@ -55,6 +55,14 @@ struct BatteryChargeStatus {
   ExternalPowerSource power_adapter_status;
 };
 
+// Contains information about the health of the battery.
+struct BatteryHealth {
+  int32 charge_full_now_milliamp_hours;
+  int32 charge_full_design_milliamp_hours;
+  int32 cycle_count;
+  int8 battery_wear_percentage;
+};
+
 // Implemented by clients that wish to be updated periodically about changes to
 // the battery charge status.
 interface BatteryChargeStatusObserver {
@@ -65,6 +73,15 @@ interface BatteryChargeStatusObserver {
   OnBatteryChargeStatusUpdated(BatteryChargeStatus battery_charge_status);
 };
 
+// Implemented by clients that wish to be updated periodically about the health
+// of the device battery.
+interface BatteryHealthObserver {
+  // OnBatteryHealthUpdated calls can be triggered by either of 2 conditions:
+  // 1) A BatteryHealthObserver is registered with SystemDataProvider
+  // 2) A periodic update is sent by SystemDataProvider
+  OnBatteryHealthUpdated(BatteryHealth battery_health);
+};
+
 // Provides telemetric information about the system. This API is exposed to the
 // Diagnostics SWA.
 interface SystemDataProvider {
@@ -80,4 +97,7 @@ interface SystemDataProvider {
   // Registers an observer of BatteryChargeStatus information.
   ObserveBatteryChargeStatus(
       pending_remote<BatteryChargeStatusObserver> observer);
+
+  // Registers an observer of BatteryHealth inforamation.
+  ObserveBatteryHealth(pending_remote<BatteryHealthObserver> observer);
 };