* [Bluez PATCH v5 1/4] adapter: Refactor kernel feature globals
2020-09-15 17:41 [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Abhishek Pandit-Subedi
@ 2020-09-15 17:41 ` Abhishek Pandit-Subedi
2020-09-15 17:41 ` [Bluez PATCH v5 2/4] adapter: Handle controller resume and notify drivers Abhishek Pandit-Subedi
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-09-15 17:41 UTC (permalink / raw)
To: luiz.dentz, marcel
Cc: chromeos-bluetooth-upstreaming, linux-bluetooth, Abhishek Pandit-Subedi
Move all the kernel specific feature globals into a single
kernel_features bitfield and replace all uses with the bitfield instead.
---
Changes in v5:
- Remove use of !! in has_kernel_features
Changes in v4: None
Changes in v3: None
Changes in v2: None
src/adapter.c | 59 ++++++++++++++++++++++++++-------------------------
src/adapter.h | 9 ++++++++
2 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 1435e2bd7..88b5202d9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -116,13 +116,7 @@ static const struct mgmt_blocked_key_info blocked_keys[] = {
static DBusConnection *dbus_conn = NULL;
-static bool kernel_conn_control = false;
-
-static bool kernel_blocked_keys_supported = false;
-
-static bool kernel_set_system_config = false;
-
-static bool kernel_exp_features = false;
+static uint32_t kernel_features = 0;
static GList *adapter_list = NULL;
static unsigned int adapter_remaining = 0;
@@ -678,7 +672,7 @@ static bool set_discoverable(struct btd_adapter *adapter, uint8_t mode,
DBG("sending set mode command for index %u", adapter->dev_id);
- if (kernel_conn_control) {
+ if (has_kernel_features(KERNEL_CONN_CONTROL)) {
if (mode)
set_mode(adapter, MGMT_OP_SET_CONNECTABLE, mode);
else
@@ -1334,7 +1328,7 @@ static void trigger_passive_scanning(struct btd_adapter *adapter)
* no need to start any discovery. The kernel will keep scanning
* as long as devices are in its auto-connection list.
*/
- if (kernel_conn_control)
+ if (has_kernel_features(KERNEL_CONN_CONTROL))
return;
/*
@@ -1385,7 +1379,7 @@ static void stop_passive_scanning_complete(uint8_t status, uint16_t length,
* no need to stop any discovery. The kernel will handle the
* auto-connection by itself.
*/
- if (kernel_conn_control)
+ if (has_kernel_features(KERNEL_CONN_CONTROL))
return;
/*
@@ -2816,7 +2810,7 @@ static void property_set_mode_complete(uint8_t status, uint16_t length,
static void clear_discoverable(struct btd_adapter *adapter)
{
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return;
if (!(adapter->current_settings & MGMT_SETTING_DISCOVERABLE))
@@ -2876,7 +2870,7 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
break;
case MGMT_SETTING_DISCOVERABLE:
- if (kernel_conn_control) {
+ if (has_kernel_features(KERNEL_CONN_CONTROL)) {
if (mode) {
set_mode(adapter, MGMT_OP_SET_CONNECTABLE,
mode);
@@ -4193,7 +4187,8 @@ static void load_default_system_params(struct btd_adapter *adapter)
size_t len = 0;
unsigned int err;
- if (!main_opts.default_params.num_entries || !kernel_set_system_config)
+ if (!main_opts.default_params.num_entries ||
+ !has_kernel_features(KERNEL_SET_SYSTEM_CONFIG))
return;
params = malloc0(sizeof(*params) *
@@ -4878,7 +4873,7 @@ int adapter_connect_list_add(struct btd_adapter *adapter,
* adapter_auto_connect_add() function is used to maintain what to
* connect.
*/
- if (kernel_conn_control)
+ if (has_kernel_features(KERNEL_CONN_CONTROL))
return 0;
if (g_slist_find(adapter->connect_list, device)) {
@@ -4918,7 +4913,7 @@ void adapter_connect_list_remove(struct btd_adapter *adapter,
if (device == adapter->connect_le)
adapter->connect_le = NULL;
- if (kernel_conn_control)
+ if (has_kernel_features(KERNEL_CONN_CONTROL))
return;
if (!g_slist_find(adapter->connect_list, device)) {
@@ -4980,7 +4975,7 @@ void adapter_whitelist_add(struct btd_adapter *adapter, struct btd_device *dev)
{
struct mgmt_cp_add_device cp;
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return;
memset(&cp, 0, sizeof(cp));
@@ -5019,7 +5014,7 @@ void adapter_whitelist_remove(struct btd_adapter *adapter, struct btd_device *de
{
struct mgmt_cp_remove_device cp;
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return;
memset(&cp, 0, sizeof(cp));
@@ -5075,7 +5070,7 @@ void adapter_auto_connect_add(struct btd_adapter *adapter,
uint8_t bdaddr_type;
unsigned int id;
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return;
if (g_slist_find(adapter->connect_list, device)) {
@@ -5147,7 +5142,7 @@ void adapter_set_device_wakeable(struct btd_adapter *adapter,
const bdaddr_t *bdaddr;
uint8_t bdaddr_type;
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return;
bdaddr = device_get_address(device);
@@ -5224,7 +5219,7 @@ void adapter_auto_connect_remove(struct btd_adapter *adapter,
uint8_t bdaddr_type;
unsigned int id;
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return;
if (!g_slist_find(adapter->connect_list, device)) {
@@ -6764,7 +6759,7 @@ connect_le:
* If kernel background scan is used then the kernel is
* responsible for connecting.
*/
- if (kernel_conn_control)
+ if (has_kernel_features(KERNEL_CONN_CONTROL))
return;
/*
@@ -8964,7 +8959,7 @@ static int clear_devices(struct btd_adapter *adapter)
{
struct mgmt_cp_remove_device cp;
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
return 0;
memset(&cp, 0, sizeof(cp));
@@ -9282,7 +9277,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
(missing_settings & MGMT_SETTING_FAST_CONNECTABLE))
set_mode(adapter, MGMT_OP_SET_FAST_CONNECTABLE, 0x01);
- if (kernel_exp_features)
+ if (has_kernel_features(KERNEL_EXP_FEATURES))
read_exp_features(adapter);
err = adapter_register(adapter);
@@ -9403,7 +9398,8 @@ static void read_info_complete(uint8_t status, uint16_t length,
set_name(adapter, btd_adapter_get_name(adapter));
- if (kernel_blocked_keys_supported && !set_blocked_keys(adapter)) {
+ if (has_kernel_features(KERNEL_BLOCKED_KEYS_SUPPORTED) &&
+ !set_blocked_keys(adapter)) {
btd_error(adapter->dev_id,
"Failed to set blocked keys for index %u",
adapter->dev_id);
@@ -9414,7 +9410,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
!(adapter->current_settings & MGMT_SETTING_BONDABLE))
set_mode(adapter, MGMT_OP_SET_BONDABLE, 0x01);
- if (!kernel_conn_control)
+ if (!has_kernel_features(KERNEL_CONN_CONTROL))
set_mode(adapter, MGMT_OP_SET_CONNECTABLE, 0x01);
else if (adapter->current_settings & MGMT_SETTING_CONNECTABLE)
set_mode(adapter, MGMT_OP_SET_CONNECTABLE, 0x00);
@@ -9590,19 +9586,19 @@ static void read_commands_complete(uint8_t status, uint16_t length,
switch (op) {
case MGMT_OP_ADD_DEVICE:
DBG("enabling kernel-side connection control");
- kernel_conn_control = true;
+ kernel_features |= KERNEL_CONN_CONTROL;
break;
case MGMT_OP_SET_BLOCKED_KEYS:
DBG("kernel supports the set_blocked_keys op");
- kernel_blocked_keys_supported = true;
+ kernel_features |= KERNEL_BLOCKED_KEYS_SUPPORTED;
break;
case MGMT_OP_SET_DEF_SYSTEM_CONFIG:
DBG("kernel supports set system confic");
- kernel_set_system_config = true;
+ kernel_features |= KERNEL_SET_SYSTEM_CONFIG;
break;
case MGMT_OP_READ_EXP_FEATURES_INFO:
DBG("kernel supports exp features");
- kernel_exp_features = true;
+ kernel_features |= KERNEL_EXP_FEATURES;
break;
default:
break;
@@ -9768,3 +9764,8 @@ bool btd_le_connect_before_pairing(void)
return false;
}
+
+bool has_kernel_features(uint32_t features)
+{
+ return (kernel_features & features);
+}
diff --git a/src/adapter.h b/src/adapter.h
index f8ac20261..b0ed4915f 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -233,3 +233,12 @@ void btd_adapter_for_each_device(struct btd_adapter *adapter,
void *data);
bool btd_le_connect_before_pairing(void);
+
+enum kernel_features {
+ KERNEL_CONN_CONTROL = 1 << 0,
+ KERNEL_BLOCKED_KEYS_SUPPORTED = 1 << 1,
+ KERNEL_SET_SYSTEM_CONFIG = 1 << 2,
+ KERNEL_EXP_FEATURES = 1 << 3,
+};
+
+bool has_kernel_features(uint32_t feature);
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Bluez PATCH v5 2/4] adapter: Handle controller resume and notify drivers
2020-09-15 17:41 [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Abhishek Pandit-Subedi
2020-09-15 17:41 ` [Bluez PATCH v5 1/4] adapter: Refactor kernel feature globals Abhishek Pandit-Subedi
@ 2020-09-15 17:41 ` Abhishek Pandit-Subedi
2020-09-15 17:41 ` [Bluez PATCH v5 3/4] policy: Enable reconnect for a2dp-sink in defaults Abhishek Pandit-Subedi
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-09-15 17:41 UTC (permalink / raw)
To: luiz.dentz, marcel
Cc: chromeos-bluetooth-upstreaming, linux-bluetooth, Abhishek Pandit-Subedi
Register for controller resume notification and notify the adapter
drivers when it occurs. Also adds the resume event kernel feature to
make sure the kernel supports this event.
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
src/adapter.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/adapter.h | 2 ++
2 files changed, 45 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index 88b5202d9..b431097f2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -8771,6 +8771,33 @@ static void connected_callback(uint16_t index, uint16_t length,
eir_data_free(&eir_data);
}
+static void controller_resume_notify(struct btd_adapter *adapter)
+{
+ GSList *l;
+
+ for (l = adapter->drivers; l; l = g_slist_next(l)) {
+ struct btd_adapter_driver *driver = l->data;
+ if (driver->resume)
+ driver->resume(adapter);
+ }
+}
+
+static void controller_resume_callback(uint16_t index, uint16_t length,
+ const void *param, void *user_data)
+{
+ const struct mgmt_ev_controller_resume *ev = param;
+ struct btd_adapter *adapter = user_data;
+
+ if (length < sizeof(*ev)) {
+ btd_error(adapter->dev_id, "Too small device resume event");
+ return;
+ }
+
+ info("Controller resume with wake event 0x%x", ev->wake_reason);
+
+ controller_resume_notify(adapter);
+}
+
static void device_blocked_callback(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
@@ -9394,6 +9421,11 @@ static void read_info_complete(uint8_t status, uint16_t length,
user_passkey_notify_callback,
adapter, NULL);
+ mgmt_register(adapter->mgmt, MGMT_EV_CONTROLLER_RESUME,
+ adapter->dev_id,
+ controller_resume_callback,
+ adapter, NULL);
+
set_dev_class(adapter);
set_name(adapter, btd_adapter_get_name(adapter));
@@ -9604,6 +9636,17 @@ static void read_commands_complete(uint8_t status, uint16_t length,
break;
}
}
+
+ for (i = 0; i < num_events; i++) {
+ uint16_t ev = get_le16(rp->opcodes + num_commands + i);
+
+ switch(ev) {
+ case MGMT_EV_CONTROLLER_RESUME:
+ DBG("kernel supports suspend/resume events");
+ kernel_features |= KERNEL_HAS_RESUME_EVT;
+ break;
+ }
+ }
}
static void read_version_complete(uint8_t status, uint16_t length,
diff --git a/src/adapter.h b/src/adapter.h
index b0ed4915f..fae2e9d3d 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -113,6 +113,7 @@ struct btd_adapter_driver {
const char *name;
int (*probe) (struct btd_adapter *adapter);
void (*remove) (struct btd_adapter *adapter);
+ void (*resume) (struct btd_adapter *adapter);
};
typedef void (*service_auth_cb) (DBusError *derr, void *user_data);
@@ -239,6 +240,7 @@ enum kernel_features {
KERNEL_BLOCKED_KEYS_SUPPORTED = 1 << 1,
KERNEL_SET_SYSTEM_CONFIG = 1 << 2,
KERNEL_EXP_FEATURES = 1 << 3,
+ KERNEL_HAS_RESUME_EVT = 1 << 4,
};
bool has_kernel_features(uint32_t feature);
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Bluez PATCH v5 3/4] policy: Enable reconnect for a2dp-sink in defaults
2020-09-15 17:41 [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Abhishek Pandit-Subedi
2020-09-15 17:41 ` [Bluez PATCH v5 1/4] adapter: Refactor kernel feature globals Abhishek Pandit-Subedi
2020-09-15 17:41 ` [Bluez PATCH v5 2/4] adapter: Handle controller resume and notify drivers Abhishek Pandit-Subedi
@ 2020-09-15 17:41 ` Abhishek Pandit-Subedi
2020-09-15 17:41 ` [Bluez PATCH v5 4/4] policy: Reconnect audio on controller resume Abhishek Pandit-Subedi
2020-09-15 18:05 ` [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Luiz Augusto von Dentz
4 siblings, 0 replies; 7+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-09-15 17:41 UTC (permalink / raw)
To: luiz.dentz, marcel
Cc: chromeos-bluetooth-upstreaming, linux-bluetooth, Abhishek Pandit-Subedi
Add a2dp-sink to default reconnects list.
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
plugins/policy.c | 3 ++-
src/main.conf | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/plugins/policy.c b/plugins/policy.c
index de51e58b9..c18ca8d1f 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -65,7 +65,8 @@ struct reconnect_data {
};
static const char *default_reconnect[] = {
- HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID, NULL };
+ HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID,
+ A2DP_SINK_UUID, NULL };
static char **reconnect_uuids = NULL;
static const size_t default_attempts = 7;
diff --git a/src/main.conf b/src/main.conf
index 42f7e41c5..e1d77cc47 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -186,7 +186,7 @@
# timeout). The policy plugin should contain a sane set of values by
# default, but this list can be overridden here. By setting the list to
# empty the reconnection feature gets disabled.
-#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
+#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb,0000110b-0000-1000-8000-00805f9b34fb
# ReconnectAttempts define the number of attempts to reconnect after a link
# lost. Setting the value to 0 disables reconnecting feature.
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Bluez PATCH v5 4/4] policy: Reconnect audio on controller resume
2020-09-15 17:41 [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Abhishek Pandit-Subedi
` (2 preceding siblings ...)
2020-09-15 17:41 ` [Bluez PATCH v5 3/4] policy: Enable reconnect for a2dp-sink in defaults Abhishek Pandit-Subedi
@ 2020-09-15 17:41 ` Abhishek Pandit-Subedi
2020-09-15 18:05 ` [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Luiz Augusto von Dentz
4 siblings, 0 replies; 7+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-09-15 17:41 UTC (permalink / raw)
To: luiz.dentz, marcel
Cc: chromeos-bluetooth-upstreaming, linux-bluetooth, Abhishek Pandit-Subedi
During system suspend, all peer devices are disconnected. On resume, HID
devices will reconnect but audio devices stay disconnected. As a quality
of life improvement, mark audio devices that were disconnected due to
suspend and attempt to reconnect them when the controller resumes (after
a delay for better co-existence with Wi-Fi).
---
Changes in v5: None
Changes in v4:
- Set reconnect timer in disconnect if resume events aren't supported
- Only set reconnect timer if adapter matches current notification
- Refactor changes in src/adapter to its own commit
- Refactor enabling A2DP_SINK_UUID into its own commit
Changes in v3:
- Refactored resume notification to use btd_adapter_driver
- Renamed ReconnectAudioDelay to ResumeDelay and set default to 2
- Added A2DP_SINK_UUID to default reconnect list
Changes in v2:
- Refactored to use policy instead of connecting directly in adapter
plugins/policy.c | 84 ++++++++++++++++++++++++++++++++++++++++--------
src/main.c | 1 +
src/main.conf | 9 ++++++
3 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/plugins/policy.c b/plugins/policy.c
index c18ca8d1f..6bd389518 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -62,6 +62,7 @@ struct reconnect_data {
guint timer;
bool active;
unsigned int attempt;
+ bool on_resume;
};
static const char *default_reconnect[] = {
@@ -76,6 +77,9 @@ static const int default_intervals[] = { 1, 2, 4, 8, 16, 32, 64 };
static int *reconnect_intervals = NULL;
static size_t reconnect_intervals_len = 0;
+static const int default_resume_delay = 2;
+static int resume_delay;
+
static GSList *reconnects = NULL;
static unsigned int service_id = 0;
@@ -712,6 +716,9 @@ static gboolean reconnect_timeout(gpointer data)
/* Mark the GSource as invalid */
reconnect->timer = 0;
+ /* Mark any reconnect on resume as handled */
+ reconnect->on_resume = false;
+
err = btd_device_connect_services(reconnect->dev, reconnect->services);
if (err < 0) {
error("Reconnecting services failed: %s (%d)",
@@ -725,14 +732,17 @@ static gboolean reconnect_timeout(gpointer data)
return FALSE;
}
-static void reconnect_set_timer(struct reconnect_data *reconnect)
+static void reconnect_set_timer(struct reconnect_data *reconnect, int timeout)
{
- static int timeout = 0;
+ static int interval_timeout = 0;
reconnect->active = true;
if (reconnect->attempt < reconnect_intervals_len)
- timeout = reconnect_intervals[reconnect->attempt];
+ interval_timeout = reconnect_intervals[reconnect->attempt];
+
+ if (timeout < 0)
+ timeout = interval_timeout;
DBG("attempt %u/%zu %d seconds", reconnect->attempt + 1,
reconnect_attempts, timeout);
@@ -744,10 +754,14 @@ static void reconnect_set_timer(struct reconnect_data *reconnect)
static void disconnect_cb(struct btd_device *dev, uint8_t reason)
{
struct reconnect_data *reconnect;
+ struct btd_service *service;
+ struct policy_data *data;
DBG("reason %u", reason);
- if (reason != MGMT_DEV_DISCONN_TIMEOUT)
+ /* Only attempt reconnect for the following reasons */
+ if (reason != MGMT_DEV_DISCONN_TIMEOUT &&
+ reason != MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND)
return;
reconnect = reconnect_find(dev);
@@ -756,10 +770,47 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
reconnect_reset(reconnect);
- DBG("Device %s identified for auto-reconnection",
- device_get_path(dev));
+ DBG("Device %s identified for auto-reconnection", device_get_path(dev));
+
+ switch(reason)
+ {
+ case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
+ if (btd_device_get_service(dev, A2DP_SINK_UUID)) {
+ DBG("%s configured to reconnect on resume",
+ device_get_path(dev));
- reconnect_set_timer(reconnect);
+ reconnect->on_resume = true;
+
+ /* If the kernel supports resume events, it is
+ * preferable to set the reconnect timer there as it is
+ * a more predictable delay.
+ */
+ if (!has_kernel_features(KERNEL_HAS_RESUME_EVT))
+ reconnect_set_timer(reconnect, resume_delay);
+ }
+ break;
+ case MGMT_DEV_DISCONN_TIMEOUT:
+ reconnect_set_timer(reconnect, -1);
+ break;
+ default:
+ DBG("Developer error. Reason = %d", reason);
+ break;
+ }
+}
+
+static void policy_adapter_resume(struct btd_adapter *adapter)
+{
+ GSList *l;
+
+ /* Check if devices on this adapter need to be reconnected on resume */
+ for (l = reconnects; l; l = g_slist_next(l)) {
+ struct reconnect_data *reconnect = l->data;
+
+ if (reconnect->on_resume &&
+ device_get_adapter(reconnect->dev) == adapter) {
+ reconnect_set_timer(reconnect, resume_delay);
+ }
+ }
}
static void conn_fail_cb(struct btd_device *dev, uint8_t status)
@@ -787,14 +838,15 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
return;
}
- reconnect_set_timer(reconnect);
+ reconnect_set_timer(reconnect, -1);
}
static int policy_adapter_probe(struct btd_adapter *adapter)
{
DBG("");
- btd_adapter_restore_powered(adapter);
+ if (auto_enable)
+ btd_adapter_restore_powered(adapter);
return 0;
}
@@ -802,6 +854,7 @@ static int policy_adapter_probe(struct btd_adapter *adapter)
static struct btd_adapter_driver policy_driver = {
.name = "policy",
.probe = policy_adapter_probe,
+ .resume = policy_adapter_resume,
};
static int policy_init(void)
@@ -855,14 +908,20 @@ static int policy_init(void)
auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
NULL);
+ resume_delay = g_key_file_get_integer(
+ conf, "Policy", "ResumeDelay", &gerr);
+
+ if (gerr) {
+ g_clear_error(&gerr);
+ resume_delay = default_resume_delay;
+ }
done:
if (reconnect_uuids && reconnect_uuids[0] && reconnect_attempts) {
btd_add_disconnect_cb(disconnect_cb);
btd_add_conn_fail_cb(conn_fail_cb);
}
- if (auto_enable)
- btd_register_adapter_driver(&policy_driver);
+ btd_register_adapter_driver(&policy_driver);
return 0;
}
@@ -883,8 +942,7 @@ static void policy_exit(void)
btd_service_remove_state_cb(service_id);
- if (auto_enable)
- btd_unregister_adapter_driver(&policy_driver);
+ btd_unregister_adapter_driver(&policy_driver);
}
BLUETOOTH_PLUGIN_DEFINE(policy, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
diff --git a/src/main.c b/src/main.c
index b37c32948..038f867b5 100644
--- a/src/main.c
+++ b/src/main.c
@@ -131,6 +131,7 @@ static const char *policy_options[] = {
"ReconnectAttempts",
"ReconnectIntervals",
"AutoEnable",
+ "ResumeDelay",
NULL
};
diff --git a/src/main.conf b/src/main.conf
index e1d77cc47..9f882e65a 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -202,3 +202,12 @@
# This includes adapters present on start as well as adapters that are plugged
# in later on. Defaults to 'false'.
#AutoEnable=false
+
+# Audio devices that were disconnected due to suspend will be reconnected on
+# resume. ResumeDelay determines the delay between when the controller
+# resumes from suspend and a connection attempt is made. A longer delay is
+# better for better co-existence with Wi-Fi.
+# The value is in seconds.
+# Default: 2
+#ResumeDelay = 2
+
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend
2020-09-15 17:41 [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Abhishek Pandit-Subedi
` (3 preceding siblings ...)
2020-09-15 17:41 ` [Bluez PATCH v5 4/4] policy: Reconnect audio on controller resume Abhishek Pandit-Subedi
@ 2020-09-15 18:05 ` Luiz Augusto von Dentz
2020-09-15 18:13 ` Abhishek Pandit-Subedi
4 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2020-09-15 18:05 UTC (permalink / raw)
To: Abhishek Pandit-Subedi
Cc: Marcel Holtmann, ChromeOS Bluetooth Upstreaming, linux-bluetooth
Hi Abhishek,
On Tue, Sep 15, 2020 at 10:41 AM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
>
> Hi Luiz and Marcel,
>
> This is a quality of life improvement for the behavior of audio devices
> during system suspend. This depends on a kernel change that emits
> suspend/resume events:
>
> https://patchwork.kernel.org/project/bluetooth/list/?series=325771
>
> Right now, audio devices will be disconnected as part of suspend but
> won't be reconnected when the system resumes without user interaction.
> This is annoying to some users as it causes an interruption to their
> normal work flow.
>
> This change reconnects audio devices that were disconnected for suspend
> using the following logic:
>
> * In the device disconnect callback, mark any devices with the A2DP
> service uuid for reconnect. The reconnect will not be queued until
> resume.
> * In the controller resume callback, queue any policy items that are
> marked to reconnect on resume for connection with the ResumeDelay
> value (default = 2s).
>
> A reconnect is queued after the controller resumes and the delay
> between resume and reconnect is configurable via the ResumeDelay key in
> the Policy settings. The 2s delay was chosen arbitrarily and I think
> anywhere up to 10s is probably ok. A longer delay is better to account
> for spurious wakeups and Wi-Fi reconnection time (avoiding any co-ex
> issues) at the downside of reconnection speed.
>
> Here are the tests I have done with this:
> - Single suspend and verified the headphones reconnect
> - Suspend stress test for 25 iterations and verify both Wi-Fi and
> Bluetooth audio reconnect on resume. (Ran with wake minimum time of
> 10s)
> - Suspend test with wake time = 1s to verify that BT reconnect isn't
> attempted. Ran 5 iterations with low wake time and then let it stay
> awake to confirm reconnect finally completed on last resume.
> - Suspend test with wake time between 1s - 4s. Ran with 5 iterations and
> verified it connected several times in the middle and finally at the
> end.
>
> I've tested this on a Pixelbook Go (AC-9260 controller) and HP
> Chromebook 14a (RTL8822CE controller) with GID6B headset.
>
> I've also tested this with the Pixel Buds 2. These earbuds actually
> reconnect automatically to the Chromebook (even without this policy
> change) and I verified that the new changes don't break the reconnection
> mechanism.
>
> Thanks
> Abhishek
>
>
> Changes in v5:
> - Remove use of !! in has_kernel_features
It seems I end up merging the old version by mistake, but I fixed this
one change myself and added btd_ prefix to it since that is required
to be exported for plugins that are not built-in.
> Changes in v4:
> - Set reconnect timer in disconnect if resume events aren't supported
> - Only set reconnect timer if adapter matches current notification
> - Refactor changes in src/adapter to its own commit
> - Refactor enabling A2DP_SINK_UUID into its own commit
>
> Changes in v3:
> - Refactored resume notification to use btd_adapter_driver
> - Renamed ReconnectAudioDelay to ResumeDelay and set default to 2
> - Added A2DP_SINK_UUID to default reconnect list
>
> Changes in v2:
> - Refactored to use policy instead of connecting directly in adapter
>
> Abhishek Pandit-Subedi (4):
> adapter: Refactor kernel feature globals
> adapter: Handle controller resume and notify drivers
> policy: Enable reconnect for a2dp-sink in defaults
> policy: Reconnect audio on controller resume
>
> plugins/policy.c | 87 +++++++++++++++++++++++++++++++++-------
> src/adapter.c | 102 +++++++++++++++++++++++++++++++++--------------
> src/adapter.h | 11 +++++
> src/main.c | 1 +
> src/main.conf | 11 ++++-
> 5 files changed, 168 insertions(+), 44 deletions(-)
>
> --
> 2.28.0.618.gf4bc123cb7-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend
2020-09-15 18:05 ` [Bluez PATCH v5 0/4] adapter: Reconnect audio when resuming from suspend Luiz Augusto von Dentz
@ 2020-09-15 18:13 ` Abhishek Pandit-Subedi
0 siblings, 0 replies; 7+ messages in thread
From: Abhishek Pandit-Subedi @ 2020-09-15 18:13 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Marcel Holtmann, ChromeOS Bluetooth Upstreaming, linux-bluetooth
Awesome, thanks!
On Tue, Sep 15, 2020 at 11:05 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Abhishek,
>
> On Tue, Sep 15, 2020 at 10:41 AM Abhishek Pandit-Subedi
> <abhishekpandit@chromium.org> wrote:
> >
> >
> > Hi Luiz and Marcel,
> >
> > This is a quality of life improvement for the behavior of audio devices
> > during system suspend. This depends on a kernel change that emits
> > suspend/resume events:
> >
> > https://patchwork.kernel.org/project/bluetooth/list/?series=325771
> >
> > Right now, audio devices will be disconnected as part of suspend but
> > won't be reconnected when the system resumes without user interaction.
> > This is annoying to some users as it causes an interruption to their
> > normal work flow.
> >
> > This change reconnects audio devices that were disconnected for suspend
> > using the following logic:
> >
> > * In the device disconnect callback, mark any devices with the A2DP
> > service uuid for reconnect. The reconnect will not be queued until
> > resume.
> > * In the controller resume callback, queue any policy items that are
> > marked to reconnect on resume for connection with the ResumeDelay
> > value (default = 2s).
> >
> > A reconnect is queued after the controller resumes and the delay
> > between resume and reconnect is configurable via the ResumeDelay key in
> > the Policy settings. The 2s delay was chosen arbitrarily and I think
> > anywhere up to 10s is probably ok. A longer delay is better to account
> > for spurious wakeups and Wi-Fi reconnection time (avoiding any co-ex
> > issues) at the downside of reconnection speed.
> >
> > Here are the tests I have done with this:
> > - Single suspend and verified the headphones reconnect
> > - Suspend stress test for 25 iterations and verify both Wi-Fi and
> > Bluetooth audio reconnect on resume. (Ran with wake minimum time of
> > 10s)
> > - Suspend test with wake time = 1s to verify that BT reconnect isn't
> > attempted. Ran 5 iterations with low wake time and then let it stay
> > awake to confirm reconnect finally completed on last resume.
> > - Suspend test with wake time between 1s - 4s. Ran with 5 iterations and
> > verified it connected several times in the middle and finally at the
> > end.
> >
> > I've tested this on a Pixelbook Go (AC-9260 controller) and HP
> > Chromebook 14a (RTL8822CE controller) with GID6B headset.
> >
> > I've also tested this with the Pixel Buds 2. These earbuds actually
> > reconnect automatically to the Chromebook (even without this policy
> > change) and I verified that the new changes don't break the reconnection
> > mechanism.
> >
> > Thanks
> > Abhishek
> >
> >
> > Changes in v5:
> > - Remove use of !! in has_kernel_features
>
> It seems I end up merging the old version by mistake, but I fixed this
> one change myself and added btd_ prefix to it since that is required
> to be exported for plugins that are not built-in.
>
> > Changes in v4:
> > - Set reconnect timer in disconnect if resume events aren't supported
> > - Only set reconnect timer if adapter matches current notification
> > - Refactor changes in src/adapter to its own commit
> > - Refactor enabling A2DP_SINK_UUID into its own commit
> >
> > Changes in v3:
> > - Refactored resume notification to use btd_adapter_driver
> > - Renamed ReconnectAudioDelay to ResumeDelay and set default to 2
> > - Added A2DP_SINK_UUID to default reconnect list
> >
> > Changes in v2:
> > - Refactored to use policy instead of connecting directly in adapter
> >
> > Abhishek Pandit-Subedi (4):
> > adapter: Refactor kernel feature globals
> > adapter: Handle controller resume and notify drivers
> > policy: Enable reconnect for a2dp-sink in defaults
> > policy: Reconnect audio on controller resume
> >
> > plugins/policy.c | 87 +++++++++++++++++++++++++++++++++-------
> > src/adapter.c | 102 +++++++++++++++++++++++++++++++++--------------
> > src/adapter.h | 11 +++++
> > src/main.c | 1 +
> > src/main.conf | 11 ++++-
> > 5 files changed, 168 insertions(+), 44 deletions(-)
> >
> > --
> > 2.28.0.618.gf4bc123cb7-goog
> >
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread