Skip to content

GitLab

T
tangled
Commit 296680d1 authored by Sonny Sasaka's avatar Sonny Sasaka Committed by Chromium LUCI CQ
Browse files
Options

[Bluetooth] Remove HidBatteryListener and PeripheralBatteryTracker 

Battery reporting via HID is now consolidated into BlueZ (CL:2593287),
so there is no need to listen to powerd directly. This only affects
Bluetooth HID devies and does not affect non-Bluetooth HID battery
notification.

Bug: b:166543531, b:172361193
Change-Id: I222677b96463120acdd2f7665d9a3c87df32346e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594158
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842579}
parent f975321f
Hide whitespace changes
Inline Side-by-side
Showing with 2 additions and 331 deletions
ash/BUILD.gn
View file @ 296680d1
......@@ -715,12 +715,8 @@ component("ash") {
"multi_user/user_switch_animator.h",
"policy/policy_recommendation_restorer.cc",
"policy/policy_recommendation_restorer.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",
"quick_answers/quick_answers_controller_impl.cc",
"quick_answers/quick_answers_controller_impl.h",
"quick_answers/quick_answers_ui_controller.cc",
......@@ -2126,7 +2122,6 @@ test("ash_unittests") {
"metrics/user_metrics_recorder_unittest.cc",
"multi_device_setup/multi_device_notification_presenter_unittest.cc",
"policy/policy_recommendation_restorer_unittest.cc",
"power/hid_battery_listener_unittest.cc",
"power/hid_battery_util_unittest.cc",
"quick_answers/quick_answers_controller_unittest.cc",
"quick_answers/quick_answers_ui_controller_unittest.cc",
......
ash/power/hid_battery_listener.cc deleted 100644 → 0
View file @ f975321f
// 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);
adapter_->AddObserver(this);
// We may be late for DeviceAdded notifications. So for the already added
// devices, simulate DeviceAdded events.
for (auto* const device : adapter_->GetDevices())
DeviceAdded(adapter_.get(), device);
}
HidBatteryListener::~HidBatteryListener() {
chromeos::PowerManagerClient::Get()->RemoveObserver(this);
adapter_->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);
}
void HidBatteryListener::DeviceAdded(device::BluetoothAdapter* adapter,
device::BluetoothDevice* device) {
chromeos::PowerManagerClient::Get()->RefreshBluetoothBattery(
device->GetAddress());
}
} // namespace ash
ash/power/hid_battery_listener.h deleted 100644 → 0
View file @ f975321f
// 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"
#include "device/bluetooth/bluetooth_adapter.h"
namespace ash {
// Listens to changes in battery level for HID devices, updating the
// corresponding device::BluetoothDevice.
// TODO(b/166543531): Remove after migrated to BlueZ Battery Provider API.
class ASH_EXPORT HidBatteryListener
: public chromeos::PowerManagerClient::Observer,
public device::BluetoothAdapter::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;
// device::BluetoothAdapter::Observer:
void DeviceAdded(device::BluetoothAdapter* adapter,
device::BluetoothDevice* device) 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 deleted 100644 → 0
View file @ f975321f
// 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 "chromeos/dbus/power/fake_power_manager_client.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[] = "aa:bb:cc:dd:ee:ff";
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()));
// Check that HidBatteryListener subscribes to be adapter observer.
EXPECT_CALL(*mock_adapter_, AddObserver(_))
.WillOnce(testing::SaveArg<0>(&adapter_observer_));
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_;
device::BluetoothAdapter::Observer* adapter_observer_;
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);
}
TEST_F(HidBatteryListenerTest, RefreshBluetoothBattery) {
// Start with no battery info.
EXPECT_FALSE(mock_device_->battery_percentage());
// Simulate DeviceAdded observer event.
chromeos::FakePowerManagerClient::Get()->set_peripheral_battery_refresh_level(
kAddress, 60);
adapter_observer_->DeviceAdded(mock_adapter_.get(), mock_device_.get());
EXPECT_EQ(60, mock_device_->battery_percentage());
// Simulate DeviceAdded observer event with a different battery value.
chromeos::FakePowerManagerClient::Get()->set_peripheral_battery_refresh_level(
kAddress, 50);
adapter_observer_->DeviceAdded(mock_adapter_.get(), mock_device_.get());
EXPECT_EQ(50, mock_device_->battery_percentage());
}
} // namespace ash
ash/power/peripheral_battery_tracker.cc deleted 100644 → 0
View file @ f975321f
// 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/peripheral_battery_tracker.h"
#include "ash/power/hid_battery_listener.h"
#include "base/bind.h"
#include "base/check.h"
#include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_adapter_factory.h"
namespace ash {
PeripheralBatteryTracker::PeripheralBatteryTracker() {
device::BluetoothAdapterFactory::Get()->GetAdapter(
base::BindOnce(&PeripheralBatteryTracker::InitializeOnBluetoothReady,
weak_ptr_factory_.GetWeakPtr()));
}
PeripheralBatteryTracker::~PeripheralBatteryTracker() = default;
void PeripheralBatteryTracker::InitializeOnBluetoothReady(
scoped_refptr<device::BluetoothAdapter> adapter) {
adapter_ = adapter;
DCHECK(adapter_.get());
hid_battery_listener_ = std::make_unique<HidBatteryListener>(adapter_);
// GATT and HFP Battery reporting is handled by
// device::BluetoothBatteryClient.
}
} // namespace ash
ash/power/peripheral_battery_tracker.h deleted 100644 → 0
View file @ f975321f
// 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_PERIPHERAL_BATTERY_TRACKER_H_
#define ASH_POWER_PERIPHERAL_BATTERY_TRACKER_H_
#include "ash/ash_export.h"
#include "base/macros.h"
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
namespace device {
class BluetoothAdapter;
} // namespace device
namespace ash {
class HfpBatteryListener;
class HidBatteryListener;
// Creates instances of classes to collect the battery status of peripheral
// devices. Currently only tracks Bluetooth devices that support GATT, HFP or
// HID.
class ASH_EXPORT PeripheralBatteryTracker {
public:
PeripheralBatteryTracker();
~PeripheralBatteryTracker();
private:
void InitializeOnBluetoothReady(
scoped_refptr<device::BluetoothAdapter> adapter);
scoped_refptr<device::BluetoothAdapter> adapter_;
std::unique_ptr<HidBatteryListener> hid_battery_listener_;
base::WeakPtrFactory<PeripheralBatteryTracker> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(PeripheralBatteryTracker);
};
} // namespace ash
#endif // ASH_POWER_PERIPHERAL_BATTERY_TRACKER_H_
ash/shell.cc
View file @ 296680d1
......@@ -74,7 +74,6 @@
#include "ash/media/media_notification_controller_impl.h"
#include "ash/multi_device_setup/multi_device_notification_presenter.h"
#include "ash/policy/policy_recommendation_restorer.h"
#include "ash/power/peripheral_battery_tracker.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/ash_prefs.h"
......@@ -944,10 +943,6 @@ void Shell::Init(
peripheral_battery_notifier_ = std::make_unique<PeripheralBatteryNotifier>(
peripheral_battery_listener_.get());
if (base::FeatureList::IsEnabled(
chromeos::features::kShowBluetoothDeviceBattery)) {
peripheral_battery_tracker_ = std::make_unique<PeripheralBatteryTracker>();
}
power_event_observer_.reset(new PowerEventObserver());
if (features::IsCaptureModeEnabled()) {
......
ash/shell.h
View file @ 296680d1
......@@ -152,7 +152,6 @@ class ParentAccessController;
class PartialMagnificationController;
class PeripheralBatteryListener;
class PeripheralBatteryNotifier;
class PeripheralBatteryTracker;
class PersistentWindowController;
class PolicyRecommendationRestorer;
class PowerButtonController;
......@@ -793,7 +792,6 @@ class ASH_EXPORT Shell : public SessionObserver,
std::unique_ptr<PeripheralBatteryListener> peripheral_battery_listener_;
std::unique_ptr<PeripheralBatteryNotifier> peripheral_battery_notifier_;
std::unique_ptr<PeripheralBatteryTracker> peripheral_battery_tracker_;
std::unique_ptr<PowerEventObserver> power_event_observer_;
std::unique_ptr<PowerPrefs> power_prefs_;
std::unique_ptr<ui::UserActivityPowerManagerNotifier> user_activity_notifier_;
......
device/bluetooth/bluetooth_device.h
View file @ 296680d1
......@@ -605,12 +605,8 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDevice {
#if defined(OS_CHROMEOS) || defined(OS_LINUX)
// Set the remaining battery of the device to show in the UI. This value must
// be between 0 and 100, inclusive.
// TODO(http://b/160905785): Battery percentage is populated by
// ash::HfpBatteryListener, ash::HidBatteryListener, and
// device::BluetoothAdapterBlueZ. When Battery information is entirely
// consolidated in BlueZ's Battery API, only device::BluetoothAdapterBlueZ
// should have control over this field with the value originating from a
// single source, the BlueZ Battery API..
// Only device::BluetoothAdapterBlueZ has control over this field with the
// value originating from a single source, the BlueZ Battery API.
void SetBatteryPercentage(base::Optional<uint8_t> battery_percentage);
// Returns the remaining battery for the device.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment