From f232c69809831acbd9b00107f22d5bd38b022eb1 Mon Sep 17 00:00:00 2001
From: Abhishek Pandit-Subedi <abhishekpandit@google.com>
Date: Tue, 28 Feb 2023 15:49:29 -0800
Subject: [PATCH] floss: Refactor le reconnection during suspend
During system suspend, we need to use a separate set of LE scan window
and intervals that are specifically tuned for this purpose. In order to
do this only when suspended, we add |SetSystemSuspendState| to indicate
when we're entering and existing suspend.
We set the suspend state via |AllowWakeByHid| and remove the suspend
state via |RestoreFilterAcceptList|. Currently, we only restore LE hid
devices on resume to the accept list but we should also restore
non-direct gatt connections in the future.
Bug: 271159571
Tag: #floss
Test: Manual test, ChromeOS Autotest and unit tests
Change-Id: I3e67f06f7667dec69379468cc51b8405cf10de10
---
system/bta/dm/bta_dm_act.cc | 5 +++--
system/bta/dm/bta_dm_api.cc | 6 ++++--
system/bta/dm/bta_dm_int.h | 3 ++-
system/bta/include/bta_api.h | 3 ++-
system/btif/include/btif_dm.h | 3 ++-
system/btif/src/bluetooth.cc | 9 +++++++-
system/btif/src/btif_dm.cc | 5 +++--
system/gd/hci/acl_manager.cc | 4 ++++
system/gd/hci/acl_manager.h | 1 +
system/gd/hci/acl_manager/le_impl.h | 14 ++++++++++++
system/gd/rust/linux/stack/src/suspend.rs | 7 +++---
system/main/shim/acl.cc | 8 +++++++
system/main/shim/acl.h | 10 ++++-----
system/main/shim/btm_api.cc | 25 +++++++++++++++++++---
system/main/shim/btm_api.h | 3 ++-
system/test/mock/mock_main_shim_btm_api.cc | 3 ++-
16 files changed, 84 insertions(+), 25 deletions(-)
diff --git a/system/bta/dm/bta_dm_act.cc b/system/bta/dm/bta_dm_act.cc
index 8d869e592db..de91135d16c 100644
--- a/system/bta/dm/bta_dm_act.cc
+++ b/system/bta/dm/bta_dm_act.cc
@@ -4408,9 +4408,10 @@ void bta_dm_allow_wake_by_hid(
* Parameters
*
*******************************************************************************/
-void bta_dm_restore_filter_accept_list() {
+void bta_dm_restore_filter_accept_list(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices) {
// Autoplumbed
- bluetooth::shim::BTM_RestoreFilterAcceptList();
+ bluetooth::shim::BTM_RestoreFilterAcceptList(le_devices);
}
/*******************************************************************************
diff --git a/system/bta/dm/bta_dm_api.cc b/system/bta/dm/bta_dm_api.cc
index 002b39b1b7a..ef3e42305b7 100644
--- a/system/bta/dm/bta_dm_api.cc
+++ b/system/bta/dm/bta_dm_api.cc
@@ -742,9 +742,11 @@ void BTA_DmAllowWakeByHid(
std::move(le_hid_devices)));
}
-void BTA_DmRestoreFilterAcceptList() {
+void BTA_DmRestoreFilterAcceptList(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices) {
APPL_TRACE_API("BTA_DmRestoreFilterAcceptList");
- do_in_main_thread(FROM_HERE, base::Bind(bta_dm_restore_filter_accept_list));
+ do_in_main_thread(FROM_HERE, base::Bind(bta_dm_restore_filter_accept_list,
+ std::move(le_devices)));
}
void BTA_DmSetDefaultEventMaskExcept(uint64_t mask, uint64_t le_mask) {
diff --git a/system/bta/dm/bta_dm_int.h b/system/bta/dm/bta_dm_int.h
index 83370cf44b4..277e9511a0f 100644
--- a/system/bta/dm/bta_dm_int.h
+++ b/system/bta/dm/bta_dm_int.h
@@ -585,7 +585,8 @@ extern void bta_dm_set_event_filter_connection_setup_all_devices();
extern void bta_dm_allow_wake_by_hid(
std::vector<RawAddress> classic_hid_devices,
std::vector<std::pair<RawAddress, uint8_t>> le_hid_devices);
-extern void bta_dm_restore_filter_accept_list();
+extern void bta_dm_restore_filter_accept_list(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices);
extern void bta_dm_set_default_event_mask_except(uint64_t mask,
uint64_t le_mask);
extern void bta_dm_set_event_filter_inquiry_result_all_devices();
diff --git a/system/bta/include/bta_api.h b/system/bta/include/bta_api.h
index f93a9285972..cdfa05a7e3b 100644
--- a/system/bta/include/bta_api.h
+++ b/system/bta/include/bta_api.h
@@ -1324,7 +1324,8 @@ extern void BTA_DmAllowWakeByHid(
* Parameters
*
*******************************************************************************/
-extern void BTA_DmRestoreFilterAcceptList();
+extern void BTA_DmRestoreFilterAcceptList(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices);
/*******************************************************************************
*
diff --git a/system/btif/include/btif_dm.h b/system/btif/include/btif_dm.h
index 59d8da8810c..e995d68d0ba 100644
--- a/system/btif/include/btif_dm.h
+++ b/system/btif/include/btif_dm.h
@@ -104,7 +104,8 @@ void btif_dm_set_event_filter_connection_setup_all_devices();
void btif_dm_allow_wake_by_hid(
std::vector<RawAddress> classic_addrs,
std::vector<std::pair<RawAddress, uint8_t>> le_addrs);
-void btif_dm_restore_filter_accept_list();
+void btif_dm_restore_filter_accept_list(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices);
void btif_dm_set_default_event_mask_except(uint64_t mask, uint64_t le_mask);
void btif_dm_set_event_filter_inquiry_result_all_devices();
void btif_dm_metadata_changed(const RawAddress& remote_bd_addr, int key,
diff --git a/system/btif/src/bluetooth.cc b/system/btif/src/bluetooth.cc
index 6f239977e63..e787e09e246 100644
--- a/system/btif/src/bluetooth.cc
+++ b/system/btif/src/bluetooth.cc
@@ -745,8 +745,15 @@ static int set_default_event_mask_except(uint64_t mask, uint64_t le_mask) {
static int restore_filter_accept_list() {
if (!interface_ready()) return BT_STATUS_NOT_READY;
+ // TODO(b/260922031) - When restoring the filter accept list after a system
+ // suspend, we need to re-arm the LE connections that had `is_direct=False`.
+ // This should be the list of bonded devices and potentially any GATT
+ // connections that have `is_direct=False`. Currently, we only restore LE hid
+ // devices.
+ auto le_hid_addrs = btif_storage_get_le_hid_devices();
do_in_main_thread(FROM_HERE,
- base::BindOnce(btif_dm_restore_filter_accept_list));
+ base::BindOnce(btif_dm_restore_filter_accept_list,
+ std::move(le_hid_addrs)));
return BT_STATUS_SUCCESS;
}
diff --git a/system/btif/src/btif_dm.cc b/system/btif/src/btif_dm.cc
index f0ba61912a7..d23878a7e2d 100644
--- a/system/btif/src/btif_dm.cc
+++ b/system/btif/src/btif_dm.cc
@@ -3887,9 +3887,10 @@ void btif_dm_allow_wake_by_hid(
BTA_DmAllowWakeByHid(std::move(classic_addrs), std::move(le_addrs));
}
-void btif_dm_restore_filter_accept_list() {
+void btif_dm_restore_filter_accept_list(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices) {
// Autoplumbed
- BTA_DmRestoreFilterAcceptList();
+ BTA_DmRestoreFilterAcceptList(std::move(le_devices));
}
void btif_dm_set_default_event_mask_except(uint64_t mask, uint64_t le_mask) {
diff --git a/system/gd/hci/acl_manager.cc b/system/gd/hci/acl_manager.cc
index d321091d695..1f978e4adeb 100644
--- a/system/gd/hci/acl_manager.cc
+++ b/system/gd/hci/acl_manager.cc
@@ -393,6 +393,10 @@ void AclManager::OnLeSuspendInitiatedDisconnect(uint16_t handle, ErrorCode reaso
CallOn(pimpl_->le_impl_, &le_impl::on_le_disconnect, handle, reason);
}
+void AclManager::SetSystemSuspendState(bool suspended) {
+ CallOn(pimpl_->le_impl_, &le_impl::set_system_suspend_state, suspended);
+}
+
LeAddressManager* AclManager::GetLeAddressManager() {
return pimpl_->le_impl_->le_address_manager_;
}
diff --git a/system/gd/hci/acl_manager.h b/system/gd/hci/acl_manager.h
index e82b7bd3142..5e4efcf800e 100644
--- a/system/gd/hci/acl_manager.h
+++ b/system/gd/hci/acl_manager.h
@@ -146,6 +146,7 @@ public:
// Virtual ACL disconnect emitted during suspend.
virtual void OnClassicSuspendInitiatedDisconnect(uint16_t handle, ErrorCode reason);
virtual void OnLeSuspendInitiatedDisconnect(uint16_t handle, ErrorCode reason);
+ virtual void SetSystemSuspendState(bool suspended);
static const ModuleFactory Factory;
diff --git a/system/gd/hci/acl_manager/le_impl.h b/system/gd/hci/acl_manager/le_impl.h
index b0917804f10..776a2ec49cd 100644
--- a/system/gd/hci/acl_manager/le_impl.h
+++ b/system/gd/hci/acl_manager/le_impl.h
@@ -59,6 +59,8 @@ constexpr uint16_t kScanWindow2mFast = 0x0018; /* 15 ms = 24 *0.625 */
constexpr uint16_t kScanWindowCodedFast = 0x0018; /* 15 ms = 24 *0.625 */
constexpr uint16_t kScanIntervalSlow = 0x0800; /* 1.28 s = 2048 *0.625 */
constexpr uint16_t kScanWindowSlow = 0x0030; /* 30 ms = 48 *0.625 */
+constexpr uint16_t kScanIntervalSystemSuspend = 0x0400; /* 640 ms = 1024 * 0.625 */
+constexpr uint16_t kScanWindowSystemSuspend = 0x0012; /* 11.25ms = 18 * 0.625 */
constexpr uint32_t kCreateConnectionTimeoutMs = 30 * 1000;
constexpr uint8_t PHY_LE_NO_PACKET = 0x00;
constexpr uint8_t PHY_LE_1M = 0x01;
@@ -887,6 +889,13 @@ struct le_impl : public bluetooth::hci::LeAddressManagerCallback {
le_scan_window_2m = os::GetSystemPropertyUint32(kPropertyConnScanWindow2mFast, kScanWindow2mFast);
le_scan_window_coded = os::GetSystemPropertyUint32(kPropertyConnScanWindowCodedFast, kScanWindowCodedFast);
}
+ // Use specific parameters when in system suspend.
+ if (system_suspend_) {
+ le_scan_interval = kScanIntervalSystemSuspend;
+ le_scan_window = kScanWindowSystemSuspend;
+ le_scan_window_2m = le_scan_window;
+ le_scan_window_coded = le_scan_window;
+ }
InitiatorFilterPolicy initiator_filter_policy = InitiatorFilterPolicy::USE_FILTER_ACCEPT_LIST;
OwnAddressType own_address_type =
static_cast<OwnAddressType>(le_address_manager_->GetInitiatorAddress().GetAddressType());
@@ -1260,6 +1269,10 @@ struct le_impl : public bluetooth::hci::LeAddressManagerCallback {
}
}
+ void set_system_suspend_state(bool suspended) {
+ system_suspend_ = suspended;
+ }
+
static constexpr uint16_t kMinimumCeLength = 0x0002;
static constexpr uint16_t kMaximumCeLength = 0x0C00;
HciLayer* hci_layer_ = nullptr;
@@ -1281,6 +1294,7 @@ struct le_impl : public bluetooth::hci::LeAddressManagerCallback {
bool ready_to_unregister = false;
bool pause_connection = false;
bool disarmed_while_arming_ = false;
+ bool system_suspend_ = false;
ConnectabilityState connectability_state_{ConnectabilityState::DISARMED};
std::map<AddressWithType, os::Alarm> create_connection_timeout_alarms_{};
};
diff --git a/system/gd/rust/linux/stack/src/suspend.rs b/system/gd/rust/linux/stack/src/suspend.rs
index 3ea1ed1e94e..5c82fde916a 100644
--- a/system/gd/rust/linux/stack/src/suspend.rs
+++ b/system/gd/rust/linux/stack/src/suspend.rs
@@ -242,11 +242,10 @@ impl ISuspend for Suspend {
fn resume(&mut self) -> bool {
self.intf.lock().unwrap().set_default_event_mask_except(0u64, 0u64);
- // TODO(b/260922031) - This needs to be generalized and handled by LE
- // manager to allow other devices to reconnect.
- // Needs to be before `clear_event_filter`.
- self.intf.lock().unwrap().allow_wake_by_hid();
+ // Restore event filter and accept list to normal.
self.intf.lock().unwrap().clear_event_filter();
+ self.intf.lock().unwrap().clear_filter_accept_list();
+ self.intf.lock().unwrap().restore_filter_accept_list();
if !self.audio_reconnect_list.is_empty() {
let reconnect_list = self.audio_reconnect_list.clone();
diff --git a/system/main/shim/acl.cc b/system/main/shim/acl.cc
index f622b8d82ad..482e6b5672f 100644
--- a/system/main/shim/acl.cc
+++ b/system/main/shim/acl.cc
@@ -1135,6 +1135,10 @@ struct shim::legacy::Acl::impl {
GetAclManager()->AddDeviceToFilterAcceptList(address_with_type);
}
+ void SetSystemSuspendState(bool suspended) {
+ GetAclManager()->SetSystemSuspendState(suspended);
+ }
+
void DumpConnectionHistory() const {
std::vector<std::string> history =
connection_history_.ReadElementsAsString();
@@ -1865,3 +1869,7 @@ void shim::legacy::Acl::AddDeviceToFilterAcceptList(
handler_->CallOn(pimpl_.get(), &Acl::impl::AddDeviceToFilterAcceptList,
address_with_type);
}
+
+void shim::legacy::Acl::SetSystemSuspendState(bool suspended) {
+ handler_->CallOn(pimpl_.get(), &Acl::impl::SetSystemSuspendState, suspended);
+}
diff --git a/system/main/shim/acl.h b/system/main/shim/acl.h
index cff1e29de7e..cb6ff3bf3c4 100644
--- a/system/main/shim/acl.h
+++ b/system/main/shim/acl.h
@@ -118,17 +118,15 @@ class Acl : public hci::acl_manager::ConnectionCallbacks,
void Dump(int fd) const;
void DumpConnectionHistory(int fd) const;
- void DisconnectAllForSuspend();
void Shutdown();
void FinalShutdown();
- void LeRand(LeRandCallback cb);
-
- void ClearFilterAcceptList();
void AddDeviceToFilterAcceptList(
const hci::AddressWithType& address_with_type);
-
- void ClearEventFilter();
+ void ClearFilterAcceptList();
+ void DisconnectAllForSuspend();
+ void LeRand(LeRandCallback cb);
+ void SetSystemSuspendState(bool suspended);
protected:
void on_incoming_acl_credits(uint16_t handle, uint16_t credits);
diff --git a/system/main/shim/btm_api.cc b/system/main/shim/btm_api.cc
index 607ffe3fa88..484b841d925 100644
--- a/system/main/shim/btm_api.cc
+++ b/system/main/shim/btm_api.cc
@@ -1357,6 +1357,9 @@ tBTM_STATUS bluetooth::shim::BTM_SetEventFilterConnectionSetupAllDevices() {
tBTM_STATUS bluetooth::shim::BTM_AllowWakeByHid(
std::vector<RawAddress> classic_hid_devices,
std::vector<std::pair<RawAddress, uint8_t>> le_hid_devices) {
+ // First set ACL to suspended state.
+ Stack::GetInstance()->GetAcl()->SetSystemSuspendState(/*suspended=*/true);
+
// Allow classic HID wake.
controller_get_interface()->set_event_filter_allow_device_connection(
std::move(classic_hid_devices));
@@ -1372,12 +1375,28 @@ tBTM_STATUS bluetooth::shim::BTM_AllowWakeByHid(
accept_future.wait();
}
+
return BTM_SUCCESS;
}
-tBTM_STATUS bluetooth::shim::BTM_RestoreFilterAcceptList() {
- LOG_ERROR("%s: TODO(230604670): Figure out what address for A2DP Connected Resume", __func__);
- // TODO(230604670): Figure out what address for A2DP Connected Resume
+tBTM_STATUS bluetooth::shim::BTM_RestoreFilterAcceptList(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices) {
+ // First, mark ACL as no longer suspended.
+ Stack::GetInstance()->GetAcl()->SetSystemSuspendState(/*suspended=*/false);
+
+ // Next, Allow BLE connection from all devices that need to be restored.
+ // This will also re-arm the LE connection.
+ for (auto address_pair : le_devices) {
+ std::promise<bool> accept_promise;
+ auto accept_future = accept_promise.get_future();
+
+ Stack::GetInstance()->GetAcl()->AcceptLeConnectionFrom(
+ ToAddressWithType(address_pair.first, address_pair.second),
+ /*is_direct=*/false, std::move(accept_promise));
+
+ accept_future.wait();
+ }
+
return BTM_SUCCESS;
}
diff --git a/system/main/shim/btm_api.h b/system/main/shim/btm_api.h
index 632ed091d58..f56b4429c59 100644
--- a/system/main/shim/btm_api.h
+++ b/system/main/shim/btm_api.h
@@ -1862,7 +1862,8 @@ tBTM_STATUS BTM_AllowWakeByHid(
* Parameters
*
*******************************************************************************/
-tBTM_STATUS BTM_RestoreFilterAcceptList(void);
+tBTM_STATUS BTM_RestoreFilterAcceptList(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices);
/*******************************************************************************
*
diff --git a/system/test/mock/mock_main_shim_btm_api.cc b/system/test/mock/mock_main_shim_btm_api.cc
index 9ab395a0262..c7f2e3faf79 100644
--- a/system/test/mock/mock_main_shim_btm_api.cc
+++ b/system/test/mock/mock_main_shim_btm_api.cc
@@ -450,7 +450,8 @@ tBTM_STATUS bluetooth::shim::BTM_AllowWakeByHid(
return BTM_SUCCESS;
}
-tBTM_STATUS bluetooth::shim::BTM_RestoreFilterAcceptList() {
+tBTM_STATUS bluetooth::shim::BTM_RestoreFilterAcceptList(
+ std::vector<std::pair<RawAddress, uint8_t>> le_devices) {
inc_func_call_count(__func__);
return BTM_SUCCESS;
}
--
GitLab