chromium/src
0
Fork 0
Code Activity

[Floss] Change GetBondedDevices to update on generic property cb

Browse Source 
Currently UI calls GetBondedDevices when it gets the hci enabled cb,
but occasionally the stack actually hasn't gotten the bonded devices
yet, so it returns an empty list and never calls GetBondedDevices again.
This CL uses the generic adapter properties changed cb to call
GetBondedDevices again when that property updates.

Bug: b/233184735
Change-Id: I1566b160b4346685d6477d475ad7637ef1760c9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3698983
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@google.com>
Commit-Queue: Katherine Lai <laikatherine@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013562}
This commit is contained in:
Katherine Lai
2022-06-13 18:08:26 +00:00
committed by Chromium LUCI CQ
parent a98d038d3f
commit 62624b714f
9 changed files with 148 additions and 38 deletions

22
device/bluetooth/floss/bluetooth_adapter_floss.cc

@ -131,9 +131,7 @@ void BluetoothAdapterFloss::RemoveAdapter() {
}
void BluetoothAdapterFloss::PopulateInitialDevices() {
FlossDBusManager::Get()->GetAdapterClient()->GetBondedDevices(
base::BindOnce(&BluetoothAdapterFloss::OnGetBondedDevices,
weak_ptr_factory_.GetWeakPtr()));
FlossDBusManager::Get()->GetAdapterClient()->GetBondedDevices();
}
void BluetoothAdapterFloss::ClearAllDevices() {
@ -389,24 +387,6 @@ void BluetoothAdapterFloss::OnStopDiscovery(
std::move(callback).Run(false, UMABluetoothDiscoverySessionOutcome::SUCCESS);
}
void BluetoothAdapterFloss::OnGetBondedDevices(
const absl::optional<std::vector<FlossDeviceId>>& ret,
const absl::optional<Error>& error) {
if (error.has_value()) {
LOG(ERROR) << "Error on GetBondedDevices: " << error->name;
return;
}
if (!ret.has_value()) {
LOG(ERROR) << "Error on GetBondedDevices: No return value";
return;
}
for (const auto& device_id : *ret) {
AdapterFoundDevice(device_id);
}
}
void BluetoothAdapterFloss::OnGetConnectionState(
const FlossDeviceId& device_id,
const absl::optional<uint32_t>& ret,

2
device/bluetooth/floss/bluetooth_adapter_floss.h

@ -157,8 +157,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterFloss final
void OnStopDiscovery(DiscoverySessionResultCallback callback,
const absl::optional<Void>& ret,
const absl::optional<Error>& error);
void OnGetBondedDevices(const absl::optional<std::vector<FlossDeviceId>>& ret,
const absl::optional<Error>& error);
void OnGetConnectionState(const FlossDeviceId& device_id,
const absl::optional<uint32_t>& ret,
const absl::optional<Error>& error);

11
device/bluetooth/floss/fake_floss_adapter_client.cc

@ -222,16 +222,17 @@ void FakeFlossAdapterClient::SetPin(ResponseCallback<Void> callback,
/*err=*/absl::nullopt));
}
void FakeFlossAdapterClient::GetBondedDevices(
ResponseCallback<std::vector<FlossDeviceId>> callback) {
void FakeFlossAdapterClient::GetBondedDevices() {
std::vector<FlossDeviceId> known_devices;
known_devices.push_back(
FlossDeviceId({.address = kBondedAddress1, .name = ""}));
known_devices.push_back(
FlossDeviceId({.address = kBondedAddress2, .name = ""}));
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), known_devices,
/*err=*/absl::nullopt));
for (const auto& device_id : known_devices) {
for (auto& observer : observers_) {
observer.AdapterFoundDevice(device_id);
}
}
}
} // namespace floss

3
device/bluetooth/floss/fake_floss_adapter_client.h

@ -61,8 +61,7 @@ class DEVICE_BLUETOOTH_EXPORT FakeFlossAdapterClient
const FlossDeviceId& device,
bool accept,
const std::vector<uint8_t>& pin) override;
void GetBondedDevices(
ResponseCallback<std::vector<FlossDeviceId>> callback) override;
void GetBondedDevices() override;
// Helper for posting a delayed task.
void PostDelayedTask(base::OnceClosure callback);

59
device/bluetooth/floss/floss_adapter_client.cc

@ -145,10 +145,11 @@ void FlossAdapterClient::SetPasskey(ResponseCallback<Void> callback,
accept, passkey);
}
void FlossAdapterClient::GetBondedDevices(
ResponseCallback<std::vector<FlossDeviceId>> callback) {
CallAdapterMethod0<std::vector<FlossDeviceId>>(std::move(callback),
adapter::kGetBondedDevices);
void FlossAdapterClient::GetBondedDevices() {
CallAdapterMethod0<std::vector<FlossDeviceId>>(
base::BindOnce(&FlossAdapterClient::OnGetBondedDevices,
weak_ptr_factory_.GetWeakPtr()),
adapter::kGetBondedDevices);
}
void FlossAdapterClient::Init(dbus::Bus* bus,
@ -192,6 +193,12 @@ void FlossAdapterClient::Init(dbus::Bus* bus,
}
// Register callbacks for the adapter.
callbacks->ExportMethod(
adapter::kCallbackInterface, adapter::kOnAdapterPropertyChanged,
base::BindRepeating(&FlossAdapterClient::OnAdapterPropertyChanged,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&HandleExported, adapter::kOnAdapterPropertyChanged));
callbacks->ExportMethod(
adapter::kCallbackInterface, adapter::kOnAddressChanged,
base::BindRepeating(&FlossAdapterClient::OnAddressChanged,
@ -277,6 +284,30 @@ void FlossAdapterClient::Init(dbus::Bus* bus,
adapter::kRegisterCallback));
}
void FlossAdapterClient::OnAdapterPropertyChanged(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
dbus::MessageReader msg(method_call);
uint32_t prop;
if (!msg.PopUint32(&prop)) {
std::move(response_sender)
.Run(dbus::ErrorResponse::FromMethodCall(
method_call, kErrorInvalidParameters, std::string()));
return;
}
BtPropertyType prop_type = static_cast<BtPropertyType>(prop);
switch (prop_type) {
case BtPropertyType::kAdapterBondedDevices:
GetBondedDevices();
break;
default:; // Do nothing for other property types
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
void FlossAdapterClient::HandleGetAddress(dbus::Response* response,
dbus::ErrorResponse* error_response) {
if (!response) {
@ -477,6 +508,26 @@ void FlossAdapterClient::OnSspRequest(
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
void FlossAdapterClient::OnGetBondedDevices(
const absl::optional<std::vector<FlossDeviceId>>& ret,
const absl::optional<Error>& error) {
if (error.has_value()) {
LOG(ERROR) << "Error on GetBondedDevices: " << error->name;
return;
}
if (!ret.has_value()) {
LOG(ERROR) << "Error on GetBondedDevices: No return value";
return;
}
for (const auto& device_id : *ret) {
for (auto& observer : observers_) {
observer.AdapterFoundDevice(device_id);
}
}
}
void FlossAdapterClient::OnBondStateChanged(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {

34
device/bluetooth/floss/floss_adapter_client.h

@ -59,6 +59,28 @@ class DEVICE_BLUETOOTH_EXPORT FlossAdapterClient : public FlossDBusClient {
kBonded = 2,
};
enum class BtPropertyType {
kBdName = 0x1,
kBdAddr,
kUuids,
kClassOfDevice,
kTypeOfDevice,
kServiceRecord,
kAdapterScanMode,
kAdapterBondedDevices,
kAdapterDiscoverableTimeout,
kRemoteFriendlyName,
kRemoteRssi,
kRemoteVersionInfo,
kLocalLeFeatures,
kLocalIoCaps,
kLocalIoCapsBle,
kDynamicAudioBuffer,
kUnknown = 0xFE,
kRemoteDeviceTimestamp = 0xFF,
};
// Adopted from bt_status_t in system/include/hardware/bluetooth.h
enum class BtifStatus {
kSuccess = 0,
@ -239,8 +261,7 @@ class DEVICE_BLUETOOTH_EXPORT FlossAdapterClient : public FlossDBusClient {
const std::vector<uint8_t>& passkey);
// Returns bonded devices.
virtual void GetBondedDevices(
ResponseCallback<std::vector<FlossDeviceId>> callback);
virtual void GetBondedDevices();
// Get the object path for this adapter.
const dbus::ObjectPath* GetObjectPath() const { return &adapter_path_; }
@ -257,6 +278,11 @@ class DEVICE_BLUETOOTH_EXPORT FlossAdapterClient : public FlossDBusClient {
void HandleGetAddress(dbus::Response* response,
dbus::ErrorResponse* error_response);
// Handle callback |OnAdapterPropertyChanged| on exported object path.
void OnAdapterPropertyChanged(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender);
// Handle callback |OnAddressChanged| on exported object path.
void OnAddressChanged(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender);
@ -308,6 +334,10 @@ class DEVICE_BLUETOOTH_EXPORT FlossAdapterClient : public FlossDBusClient {
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender);
// Handle GetBondedDevices
void OnGetBondedDevices(const absl::optional<std::vector<FlossDeviceId>>& ret,
const absl::optional<Error>& error);
// List of observers interested in event notifications from this client.
base::ObserverList<Observer> observers_;

53
device/bluetooth/floss/floss_adapter_client_unittest.cc

@ -146,7 +146,7 @@ class FlossAdapterClientTest : public testing::Test {
// Make sure we export all callbacks. This will need to be updated once new
// callbacks are added.
EXPECT_CALL(*exported_callbacks_.get(), ExportMethod).Times(10);
EXPECT_CALL(*exported_callbacks_.get(), ExportMethod).Times(11);
// Handle method calls on the object proxy
ON_CALL(
@ -396,6 +396,20 @@ class FlossAdapterClientTest : public testing::Test {
client_->OnSspRequest(&method_call, std::move(response));
}
void SendAdapterPropertyChangedCallback(
bool error,
FlossAdapterClient::BtPropertyType type,
dbus::ExportedObject::ResponseSender response) {
dbus::MethodCall method_call(adapter::kCallbackInterface,
adapter::kOnAdapterPropertyChanged);
method_call.SetSerial(serial_++);
dbus::MessageWriter writer(&method_call);
writer.AppendUint32(static_cast<uint32_t>(type));
client_->OnAdapterPropertyChanged(&method_call, std::move(response));
}
int serial_ = 1;
int adapter_index_ = 5;
dbus::ObjectPath adapter_path_;
@ -999,7 +1013,7 @@ TEST_F(FlossAdapterClientTest, GenericMethodGetRemoteUuids) {
TEST_F(FlossAdapterClientTest, GenericMethodGetRemoteType) {
client_->Init(bus_.get(), kAdapterInterface, adapter_path_.value());
// Method of 1 parameter with UUID response.
// Method of 1 parameter with BluetoothDeviceType response.
EXPECT_CALL(
*adapter_object_proxy_.get(),
DoCallMethodWithErrorResponse(HasMemberOf(adapter::kGetRemoteType), _, _))
@ -1035,4 +1049,39 @@ TEST_F(FlossAdapterClientTest, GenericMethodGetRemoteType) {
run_loop.Run();
}
TEST_F(FlossAdapterClientTest, OnAdapterPropertyChanged) {
TestAdapterObserver test_observer(client_.get());
client_->Init(bus_.get(), kAdapterInterface, adapter_path_.value());
EXPECT_EQ(test_observer.found_device_count_, 0);
// Method of no parameters with vector of FlossDeviceId response.
EXPECT_CALL(*adapter_object_proxy_.get(),
DoCallMethodWithErrorResponse(
HasMemberOf(adapter::kGetBondedDevices), _, _))
.WillOnce([this](::dbus::MethodCall* method_call, int timeout_ms,
::dbus::ObjectProxy::ResponseOrErrorCallback* cb) {
dbus::MessageReader msg(method_call);
// D-Bus method call should have no parameters.
EXPECT_FALSE(msg.HasMoreData());
// Create a response with valid array of FlossDeviceIds
FlossDeviceId device_id = {.address = "66:55:44:33:22:11",
.name = "First"};
auto response = ::dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response.get());
dbus::MessageWriter array(nullptr);
writer.OpenArray("a{sv}", &array);
this->EncodeFlossDeviceId(&array, device_id,
/*include_required_keys=*/true,
/*include_extra_keys=*/false);
writer.CloseContainer(&array);
std::move(*cb).Run(response.get(), /*err=*/nullptr);
});
SendAdapterPropertyChangedCallback(
/*error=*/false,
FlossAdapterClient::BtPropertyType::kAdapterBondedDevices,
base::BindOnce(&FlossAdapterClientTest::ExpectNormalResponse,
weak_ptr_factory_.GetWeakPtr()));
EXPECT_EQ(test_observer.found_device_count_, 1);
}
} // namespace floss

1
device/bluetooth/floss/floss_dbus_client.cc

@ -123,6 +123,7 @@ const char kCallbackInterface[] = "org.chromium.bluetooth.BluetoothCallback";
const char kConnectionCallbackInterface[] =
"org.chromium.bluetooth.BluetoothConnectionCallback";
const char kOnAdapterPropertyChanged[] = "OnAdapterPropertyChanged";
const char kOnAddressChanged[] = "OnAddressChanged";
const char kOnNameChanged[] = "OnNameChanged";
const char kOnDiscoverableChanged[] = "OnDiscoverableChanged";

1
device/bluetooth/floss/floss_dbus_client.h

@ -58,6 +58,7 @@ extern DEVICE_BLUETOOTH_EXPORT const char kSetPin[];
extern DEVICE_BLUETOOTH_EXPORT const char kSetPasskey[];
extern DEVICE_BLUETOOTH_EXPORT const char kGetBondedDevices[];
extern DEVICE_BLUETOOTH_EXPORT const char kOnAdapterPropertyChanged[];
extern DEVICE_BLUETOOTH_EXPORT const char kOnAddressChanged[];
extern DEVICE_BLUETOOTH_EXPORT const char kOnNameChanged[];
extern DEVICE_BLUETOOTH_EXPORT const char kOnDiscoverableChanged[];