[Bluetooth] Update BluetoothDevice battery level for HID devices

Create a class that listens to PowerManagerClient for changes in the
battery level of Bluetooth HID devices, and update the corresponding
device::BluetoothDevice battery percentage field.

Design doc at go/cros-bt-battery.

Bug: 785758
Change-Id: I3519d6d19c7a24abafc353d4afd3d059034e808b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721087
Commit-Queue: Alberto Herrera <ahrfgb@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683295}

ash/BUILD.gn

@@ -516,6 +516,10 @@ component("ash") {
     "power/gatt_battery_percentage_fetcher.h",
     "power/gatt_battery_poller.cc",
     "power/gatt_battery_poller.h",
+    "power/hid_battery_listener.cc",
+    "power/hid_battery_listener.h",
+    "power/hid_battery_util.cc",
+    "power/hid_battery_util.h",
     "power/peripheral_battery_tracker.cc",
     "power/peripheral_battery_tracker.h",
     "root_window_controller.cc",
@@ -1736,6 +1740,8 @@ test("ash_unittests") {
     "power/gatt_battery_controller_unittest.cc",
     "power/gatt_battery_percentage_fetcher_unittest.cc",
     "power/gatt_battery_poller_unittest.cc",
+    "power/hid_battery_listener_unittest.cc",
+    "power/hid_battery_util_unittest.cc",
     "root_window_controller_unittest.cc",
     "rotator/screen_rotation_animation_unittest.cc",
     "rotator/screen_rotation_animator_unittest.cc",

ash/power/hid_battery_listener.cc

+// Copyright 2019 The Chromium 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 "ash/power/hid_battery_listener.h"
+
+#include "ash/power/hid_battery_util.h"
+#include "device/bluetooth/bluetooth_adapter.h"
+#include "device/bluetooth/bluetooth_device.h"
+
+namespace ash {
+
+HidBatteryListener::HidBatteryListener(
+    scoped_refptr<device::BluetoothAdapter> adapter)
+    : adapter_(adapter) {
+  chromeos::PowerManagerClient::Get()->AddObserver(this);
+}
+
+HidBatteryListener::~HidBatteryListener() {
+  chromeos::PowerManagerClient::Get()->RemoveObserver(this);
+}
+
+void HidBatteryListener::PeripheralBatteryStatusReceived(
+    const std::string& path,
+    const std::string& name,
+    int level) {
+  const std::string bluetooth_address =
+      ExtractBluetoothAddressFromHIDBatteryPath(path);
+  if (bluetooth_address.empty())
+    return;
+
+  device::BluetoothDevice* device = adapter_->GetDevice(bluetooth_address);
+  if (!device)
+    return;
+
+  if (level < 0 || level > 100)
+    device->SetBatteryPercentage(base::nullopt);
+  else
+    device->SetBatteryPercentage(level);
+}
+
+}  // namespace ash

ash/power/hid_battery_listener.h

+// Copyright 2019 The Chromium 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 ASH_POWER_HID_BATTERY_LISTENER_H_
+#define ASH_POWER_HID_BATTERY_LISTENER_H_
+
+#include "ash/ash_export.h"
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "chromeos/dbus/power/power_manager_client.h"
+
+namespace device {
+class BluetoothAdapter;
+}  // namespace device
+
+namespace ash {
+
+// Listens to changes in battery level for HID devices, updating the
+// corresponding device::BluetoothDevice.
+class ASH_EXPORT HidBatteryListener
+    : public chromeos::PowerManagerClient::Observer {
+ public:
+  explicit HidBatteryListener(scoped_refptr<device::BluetoothAdapter> adapter);
+  ~HidBatteryListener() override;
+
+ private:
+  friend class HidBatteryListenerTest;
+
+  // chromeos::PowerManagerClient::Observer:
+  void PeripheralBatteryStatusReceived(const std::string& path,
+                                       const std::string& name,
+                                       int level) override;
+
+  scoped_refptr<device::BluetoothAdapter> adapter_;
+
+  DISALLOW_COPY_AND_ASSIGN(HidBatteryListener);
+};
+
+}  // namespace ash
+
+#endif  // ASH_POWER_HID_BATTERY_LISTENER_H_

ash/power/hid_battery_listener_unittest.cc

+// Copyright 2019 The Chromium 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 "ash/power/hid_battery_listener.h"
+
+#include <memory>
+
+#include "ash/test/ash_test_base.h"
+#include "device/bluetooth/test/mock_bluetooth_adapter.h"
+#include "device/bluetooth/test/mock_bluetooth_device.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+using testing::_;
+using testing::NiceMock;
+using testing::Return;
+
+namespace ash {
+namespace {
+
+const char kPath[] = "/sys/class/power_supply/hid-AA:BB:CC:DD:EE:FF-battery";
+const char kAddress[] = "ff:ee:dd:cc:bb:aa";
+const char kDeviceName[] = "test device";
+
+}  // namespace
+
+class HidBatteryListenerTest : public AshTestBase {
+ public:
+  HidBatteryListenerTest() = default;
+
+  ~HidBatteryListenerTest() override = default;
+
+  void SetUp() override {
+    AshTestBase::SetUp();
+
+    mock_adapter_ =
+        base::MakeRefCounted<NiceMock<device::MockBluetoothAdapter>>();
+
+    mock_device_ = std::make_unique<NiceMock<device::MockBluetoothDevice>>(
+        mock_adapter_.get(), 0 /* bluetooth_class */, kDeviceName, kAddress,
+        true /* paired */, true /* connected */);
+    ASSERT_FALSE(mock_device_->battery_percentage());
+    ON_CALL(*mock_adapter_, GetDevice(kAddress))
+        .WillByDefault(Return(mock_device_.get()));
+
+    listener_ = std::make_unique<HidBatteryListener>(mock_adapter_);
+  }
+
+  void TearDown() override {
+    listener_.reset();
+    AshTestBase::TearDown();
+  }
+
+  void NotifyPeripheralBatteryStatusReceived(const std::string& path,
+                                             const std::string& name,
+                                             int level) {
+    listener_->PeripheralBatteryStatusReceived(path, name, level);
+  }
+
+ protected:
+  scoped_refptr<NiceMock<device::MockBluetoothAdapter>> mock_adapter_;
+  std::unique_ptr<device::MockBluetoothDevice> mock_device_;
+  std::unique_ptr<HidBatteryListener> listener_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(HidBatteryListenerTest);
+};
+
+TEST_F(HidBatteryListenerTest, SetsBatteryToDevice) {
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, 100);
+  EXPECT_EQ(100, mock_device_->battery_percentage());
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, 0);
+  EXPECT_EQ(0, mock_device_->battery_percentage());
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, 55);
+  EXPECT_EQ(55, mock_device_->battery_percentage());
+}
+
+TEST_F(HidBatteryListenerTest, InvalidBatteryLevel) {
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, 101);
+  EXPECT_EQ(base::nullopt, mock_device_->battery_percentage());
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, 200);
+  EXPECT_EQ(base::nullopt, mock_device_->battery_percentage());
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, -1);
+  EXPECT_EQ(base::nullopt, mock_device_->battery_percentage());
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, -100);
+  EXPECT_EQ(base::nullopt, mock_device_->battery_percentage());
+}
+
+TEST_F(HidBatteryListenerTest, InvalidHIDAddress) {
+  EXPECT_CALL(*mock_adapter_, GetDevice(_)).Times(0);
+
+  NotifyPeripheralBatteryStatusReceived("invalid-path", kDeviceName, 100);
+  NotifyPeripheralBatteryStatusReceived("/sys/class/power_supply/hid-battery",
+                                        kDeviceName, 100);
+
+  // 3 characters at the end of the address, "f55".
+  NotifyPeripheralBatteryStatusReceived(
+      "/sys/class/power_supply/hid-A0:b1:C2:d3:E4:f55-battery", kDeviceName,
+      100);
+
+  // 3 characters at the start of the address, "A00".
+  NotifyPeripheralBatteryStatusReceived(
+      "/sys/class/power_supply/hid-A00:b1:C2:d3:E4:f5-battery", kDeviceName,
+      100);
+}
+
+TEST_F(HidBatteryListenerTest, DeviceNotPresentInAdapter) {
+  ON_CALL(*mock_adapter_, GetDevice(kAddress)).WillByDefault(Return(nullptr));
+
+  // Should not crash.
+  NotifyPeripheralBatteryStatusReceived(kPath, kDeviceName, 100);
+}
+
+}  // namespace ash

ash/power/hid_battery_util.cc

+// Copyright 2019 The Chromium 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 "ash/power/hid_battery_util.h"
+
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "third_party/re2/src/re2/re2.h"
+
+namespace ash {
+
+namespace {
+
+// HID device's battery sysfs entry path looks like
+// /sys/class/power_supply/hid-{AA:BB:CC:DD:EE:FF|AAAA:BBBB:CCCC.DDDD}-battery.
+constexpr char kHIDBatteryPrefix[] = "/sys/class/power_supply/hid-";
+constexpr char kHIDBatterySuffix[] = "-battery";
+constexpr size_t kPrefixLen = base::size(kHIDBatteryPrefix) - 1;
+constexpr size_t kPrefixAndSuffixLen =
+    (base::size(kHIDBatteryPrefix) - 1) + (base::size(kHIDBatterySuffix) - 1);
+
+// Regex to check for valid bluetooth addresses.
+constexpr char kBluetoothAddressRegex[] =
+    "^([0-9A-Fa-f]{2}:){5}([0-9A-Fa-f]{2})$";
+
+}  // namespace
+
+bool IsHIDBattery(const std::string& path) {
+  if (!base::StartsWith(path, kHIDBatteryPrefix,
+                        base::CompareCase::INSENSITIVE_ASCII) ||
+      !base::EndsWith(path, kHIDBatterySuffix,
+                      base::CompareCase::INSENSITIVE_ASCII)) {
+    return false;
+  }
+
+  return path.size() > kPrefixAndSuffixLen;
+}
+
+std::string ExtractHIDBatteryIdentifier(const std::string& path) {
+  if (path.size() <= kPrefixAndSuffixLen)
+    return std::string();
+
+  size_t id_len = path.size() - kPrefixAndSuffixLen;
+  return path.substr(kPrefixLen, id_len);
+}
+
+std::string ExtractBluetoothAddressFromHIDBatteryPath(const std::string& path) {
+  if (!IsHIDBattery(path))
+    return std::string();
+
+  std::string identifier = ExtractHIDBatteryIdentifier(path);
+  if (!RE2::FullMatch(identifier, kBluetoothAddressRegex))
+    return std::string();
+
+  std::string reverse_address = base::ToLowerASCII(identifier);
+  std::vector<base::StringPiece> result = base::SplitStringPiece(
+      reverse_address, ":", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+  std::reverse(result.begin(), result.end());
+  return base::JoinString(result, ":");
+}
+
+}  // namespace ash

ash/power/hid_battery_util.h

+// Copyright 2019 The Chromium 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 ASH_POWER_HID_BATTERY_UTIL_H_
+#define ASH_POWER_HID_BATTERY_UTIL_H_
+
+#include <string>
+
+#include "ash/ash_export.h"
+
+namespace ash {
+
+// Checks whether the device at |path| is a HID battery. |path| is a sysfs
+// device path like "/sys/class/power-supply/hid-FF:EE:DD:CC:BB:AA-battery"
+// Returns false if |path| is lacking the HID battery prefix or suffix, or if it
+// contains them but has nothing in between.
+ASH_EXPORT bool IsHIDBattery(const std::string& path);
+
+// Extract the identifier in |path| found between the path prefix and suffix.
+// |path| is a sysfs device path like
+// "/sys/class/power-supply/hid-FF:EE:DD:CC:BB:AA-battery"
+ASH_EXPORT std::string ExtractHIDBatteryIdentifier(const std::string& path);
+
+// Extracts a Bluetooth address (e.g. "AA:BB:CC:DD:EE:FF") from |path|, a sysfs
+// device path like "/sys/class/power-supply/hid-FF:EE:DD:CC:BB:AA-battery".
+// The address supplied in |path| is reversed, so this method will reverse the
+// extracted address. Returns an empty string if |path| does not contain a
+// Bluetooth address.
+ASH_EXPORT std::string ExtractBluetoothAddressFromHIDBatteryPath(
+    const std::string& path);
+
+}  // namespace ash
+
+#endif  // ASH_POWER_HID_BATTERY_UTIL_H_

ash/power/hid_battery_util_unittest.cc

+// Copyright 2019 The Chromium 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 "ash/power/hid_battery_util.h"
+
+#include <string>
+
+#include "base/macros.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ash {
+
+using HidBatteryUtilTest = testing::Test;
+
+TEST_F(HidBatteryUtilTest, IsHIDBattery) {
+  EXPECT_FALSE(IsHIDBattery(std::string()));
+  EXPECT_FALSE(IsHIDBattery("invalid-path"));
+  EXPECT_FALSE(IsHIDBattery("/sys/class/power_supply/hid-"));
+  EXPECT_FALSE(IsHIDBattery("-battery"));
+  EXPECT_FALSE(IsHIDBattery("/sys/class/power_supply/hid--battery"));
+
+  EXPECT_TRUE(
+      IsHIDBattery("/sys/class/power_supply/hid-A0:b1:C2:d3:E4:f5-battery"));
+}
+
+TEST_F(HidBatteryUtilTest, ExtractHIDIdentifier) {
+  EXPECT_EQ(std::string(), ExtractHIDBatteryIdentifier("invalid-path"));
+  EXPECT_EQ("A0:b1:C2:d3:E4:f5",
+            ExtractHIDBatteryIdentifier(
+                "/sys/class/power_supply/hid-A0:b1:C2:d3:E4:f5-battery"));
+}
+
+TEST_F(HidBatteryUtilTest, ExtractBluetoothAddressFromHIDBatteryPath) {
+  EXPECT_EQ(std::string(),
+            ExtractBluetoothAddressFromHIDBatteryPath("invalid-path"));
+
+  // 3 characters at the end of the address, "f55".
+  EXPECT_EQ(std::string(),
+            ExtractBluetoothAddressFromHIDBatteryPath(
+                "/sys/class/power_supply/hid-A0:b1:C2:d3:E4:f55-battery"));
+
+  // 3 characters at the start of the address, "A00".
+  EXPECT_EQ(std::string(),
+            ExtractBluetoothAddressFromHIDBatteryPath(
+                "/sys/class/power_supply/hid-A00:b1:C2:d3:E4:f5-battery"));
+
+  EXPECT_EQ("f5:e4:d3:c2:b1:a0",
+            ExtractBluetoothAddressFromHIDBatteryPath(
+                "/sys/class/power_supply/hid-A0:b1:C2:d3:E4:f5-battery"));
+}
+
+}  // namespace ash

ash/power/peripheral_battery_tracker.cc

@@ -5,6 +5,7 @@
 #include "ash/power/peripheral_battery_tracker.h"
 
 #include "ash/power/gatt_battery_controller.h"
+#include "ash/power/hid_battery_listener.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "device/bluetooth/bluetooth_adapter.h"
@@ -24,6 +25,7 @@ void PeripheralBatteryTracker::InitializeOnBluetoothReady(
     scoped_refptr<device::BluetoothAdapter> adapter) {
   adapter_ = adapter;
   DCHECK(adapter_.get());
+  hid_battery_listener_ = std::make_unique<HidBatteryListener>(adapter_);
   gatt_battery_controller_ = std::make_unique<GattBatteryController>(adapter_);
 }
 

ash/power/peripheral_battery_tracker.h

@@ -16,11 +16,11 @@ class BluetoothAdapter;
 
 namespace ash {
 
+class HidBatteryListener;
 class GattBatteryController;
 
 // Creates instances of classes to collect the battery status of peripheral
-// devices. Currently only tracks Bluetooth devices that support GATT.
-// TODO(https://crbug.com/785758): Add support for other protocols, like HID.
+// devices. Currently only tracks Bluetooth devices that support GATT or HID.
 class ASH_EXPORT PeripheralBatteryTracker {
  public:
   PeripheralBatteryTracker();
@@ -32,6 +32,7 @@ class ASH_EXPORT PeripheralBatteryTracker {
 
   scoped_refptr<device::BluetoothAdapter> adapter_;
 
+  std::unique_ptr<HidBatteryListener> hid_battery_listener_;
   std::unique_ptr<GattBatteryController> gatt_battery_controller_;
 
   base::WeakPtrFactory<PeripheralBatteryTracker> weak_ptr_factory_{this};

ash/system/power/peripheral_battery_notifier.cc

@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "ash/power/hid_battery_util.h"
 #include "ash/public/cpp/notification_utils.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
@@ -15,13 +16,10 @@
 #include "base/macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
-#include "base/strings/string_split.h"
-#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/default_tick_clock.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/bluetooth_device.h"
-#include "third_party/re2/src/re2/re2.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/events/devices/device_data_manager.h"
@@ -49,69 +47,13 @@ const char kNotifierStylusBattery[] = "ash.stylus-battery";
 const char kNotificationOriginUrl[] = "chrome://peripheral-battery";
 const char kNotifierNonStylusBattery[] = "power.peripheral-battery";
 
-// HID device's battery sysfs entry path looks like
-// /sys/class/power_supply/hid-{AA:BB:CC:DD:EE:FF|AAAA:BBBB:CCCC.DDDD}-battery.
-// Here the bluetooth address is showed in reverse order and its true
-// address "FF:EE:DD:CC:BB:AA".
-const char kHIDBatteryPathPrefix[] = "/sys/class/power_supply/hid-";
-const char kHIDBatteryPathSuffix[] = "-battery";
-
 // Prefix added to the address of a Bluetooth device to generate an unique ID
 // when posting a notification to the Message Center.
 const char kBluetoothDeviceIdPrefix[] = "battery_notification_bluetooth-";
 
-// Regex to check for valid bluetooth addresses.
-constexpr char kBluetoothAddressRegex[] =
-    "^([0-9A-Fa-f]{2}:){5}([0-9A-Fa-f]{2})$";
-
-// Checks whether the device at |path| is a HID battery. Returns false if |path|
-// is lacking the HID battery prefix or suffix, or if it contains them but has
-// nothing in between.
-bool IsHIDBattery(const std::string& path) {
-  if (!base::StartsWith(path, kHIDBatteryPathPrefix,
-                        base::CompareCase::INSENSITIVE_ASCII) ||
-      !base::EndsWith(path, kHIDBatteryPathSuffix,
-                      base::CompareCase::INSENSITIVE_ASCII)) {
-    return false;
-  }
-
-  return static_cast<int>(path.size()) -
-             static_cast<int>(strlen(kHIDBatteryPathPrefix) +
-                              strlen(kHIDBatteryPathSuffix)) >
-         0;
-}
-
-// Extract the identifier in |path| found between the path prefix and suffix.
-std::string ExtractIdentifier(const std::string& path) {
-  int header_size = strlen(kHIDBatteryPathPrefix);
-  int end_size = strlen(kHIDBatteryPathSuffix);
-  int key_len = path.size() - header_size - end_size;
-  if (key_len <= 0)
-    return std::string();
-
-  return path.substr(header_size, key_len);
-}
-
-// Extracts a Bluetooth address (e.g. "AA:BB:CC:DD:EE:FF") from |path|, a sysfs
-// device path like "/sys/class/power-supply/hid-AA:BB:CC:DD:EE:FF-battery".
-// The address supplied in |path| is reversed, so this method will reverse the
-// extracted address. Returns an empty string if |path| does not contain a
-// Bluetooth address.
-std::string ExtractBluetoothAddressFromPath(const std::string& path) {
-  std::string identifier = ExtractIdentifier(path);
-  if (!RE2::FullMatch(identifier, kBluetoothAddressRegex))
-    return std::string();
-
-  std::string reverse_address = base::ToLowerASCII(identifier);
-  std::vector<base::StringPiece> result = base::SplitStringPiece(
-      reverse_address, ":", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-  std::reverse(result.begin(), result.end());
-  return base::JoinString(result, ":");
-}
-
 // Checks if the device is an external stylus.
 bool IsStylusDevice(const std::string& path, const std::string& model_name) {
-  std::string identifier = ExtractIdentifier(path);
+  std::string identifier = ExtractHIDBatteryIdentifier(path);
   for (const ui::TouchscreenDevice& device :
        ui::DeviceDataManager::GetInstance()->GetTouchscreenDevices()) {
     if (device.has_stylus &&
@@ -168,7 +110,8 @@ std::string GetMapKeyForBluetoothAddress(const std::string& bluetooth_address) {
 // Returns the corresponding map key for a HID device.
 std::string GetBatteryMapKey(const std::string& path) {
   // Check if the HID path corresponds to a Bluetooth device.
-  const std::string bluetooth_address = ExtractBluetoothAddressFromPath(path);
+  const std::string bluetooth_address =
+      ExtractBluetoothAddressFromHIDBatteryPath(path);
   return bluetooth_address.empty()
              ? path
              : GetMapKeyForBluetoothAddress(bluetooth_address);
@@ -248,7 +191,7 @@ void PeripheralBatteryNotifier::PeripheralBatteryStatusReceived(
 
   BatteryInfo battery{base::ASCIIToUTF16(name), level, base::TimeTicks(),
                       IsStylusDevice(path, name),
-                      ExtractBluetoothAddressFromPath(path)};
+                      ExtractBluetoothAddressFromHIDBatteryPath(path)};
   UpdateBattery(GetBatteryMapKey(path), battery);
 }