Hi Luiz,
When a2dp-sink was added to the reconnection policy, it exposed a bug in
Airpods which were incorrectly emitting Connection Timeout when
connecting to a new device. As a result, Chromebooks started
reconnecting immediately (link loss) and inadvertently "stole"
connections. See hci trace below:
> HCI Event: Disconnect Complete (0x05) plen 4 #1680 [hci0] 62.656436
Status: Success (0x00)
Handle: 256
Reason: Connection Timeout (0x08)
@ MGMT Event: Device Disconnected (0x000c) plen 8 {0x0004} [hci0] 62.656510
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
@ MGMT Event: Device Disconnected (0x000c) plen 8 {0x0002} [hci0] 62.656510
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
@ MGMT Event: Device Disconnected (0x000c) plen 8 {0x0001} [hci0] 62.656510
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
@ MGMT Event: Device Disconnected (0x000c) plen 8 {0x0003} [hci0] 62.656510
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
< HCI Command: Write Scan Enable (0x03|0x001a) plen 1 #1681 [hci0] 62.668156
Scan enable: Page Scan (0x02)
> HCI Event: Command Complete (0x0e) plen 4 #1682 [hci0] 62.670442
Write Scan Enable (0x03|0x001a) ncmd 2
Status: Success (0x00)
< HCI Command: Create Connection (0x01|0x0005) plen 13 #1683 [hci0] 64.090171
Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Packet type: 0xcc18
DM1 may be used
DH1 may be used
DM3 may be used
DH3 may be used
DM5 may be used
DH5 may be used
Page scan repetition mode: R2 (0x02)
Page scan mode: Mandatory (0x00)
Clock offset: 0x0000
Role switch: Allow slave (0x01)
> HCI Event: Command Status (0x0f) plen 4 #1684 [hci0] 64.090446
Create Connection (0x01|0x0005) ncmd 1
This patch separates the ReconnectUUIDs by adding a ReconnectResumeUUIDs
and distinguishes reconnections that occur on timeout vs. resume.
I've run the following tests:
- Check that Airpods changing to a different device doesn't cause
a reconnection with this change.
- Check that adding a2dp-sink to ReconnectUUIDs will cause the original
issue to be seen.
- Make sure Airpods are reconnected on resume.
- Make sure other headphones are reconnected on resume (tested Anker
Soundcore Life Q20 and ATH-M50xBT)
- Run bluetooth_AdapterSRHealth.sr_reconnect_a2dp (ChromeOS end-to-end
test)
Thanks
Abhishek
Abhishek Pandit-Subedi (1):
policy: Refactor reconnect policy for resume
plugins/policy.c | 72 ++++++++++++++++++++++++++++++++++++------------
src/main.conf | 9 +++++-
2 files changed, 62 insertions(+), 19 deletions(-)
--
2.29.2.576.ga3fc446d84-goog
Airpods incorrectly emit a connection timeout as the disconnect reason
when a different device connects to it and triggers the automatic
reconnection by mistake. See hci trace below:
> HCI Event: Disconnect Complete (0x05) plen 4
Status: Success (0x00)
Handle: 256
Reason: Connection Timeout (0x08)
@ MGMT Event: Device Disconnected (0x000c) plen 8
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
@ MGMT Event: Device Disconnected (0x000c) plen 8
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
@ MGMT Event: Device Disconnected (0x000c) plen 8
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
@ MGMT Event: Device Disconnected (0x000c) plen 8
BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Reason: Connection timeout (0x01)
< HCI Command: Write Scan Enable (0x03|0x001a) plen 1
Scan enable: Page Scan (0x02)
> HCI Event: Command Complete (0x0e) plen 4
Write Scan Enable (0x03|0x001a) ncmd 2
Status: Success (0x00)
< HCI Command: Create Connection (0x01|0x0005) plen 13
Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
Packet type: 0xcc18
DM1 may be used
DH1 may be used
DM3 may be used
DH3 may be used
DM5 may be used
DH5 may be used
Page scan repetition mode: R2 (0x02)
Page scan mode: Mandatory (0x00)
Clock offset: 0x0000
Role switch: Allow slave (0x01)
> HCI Event: Command Status (0x0f) plen 4
Create Connection (0x01|0x0005) ncmd 1
To fix this behavior, separate the ReconnectUUIDs into services that
reconnect on timeout and on resume. This ensures that the a2dp-sink
profile reconnects are correctly applied only on resume from suspend and
adds the option to reconnect other profiles automatically on resume.
Reviewed-by: Alain Michaud <[email protected]>
---
plugins/policy.c | 72 ++++++++++++++++++++++++++++++++++++------------
src/main.conf | 9 +++++-
2 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/plugins/policy.c b/plugins/policy.c
index ba9e1be02..a3cc3445e 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -44,19 +44,22 @@
struct reconnect_data {
struct btd_device *dev;
- bool reconnect;
+ bool on_timeout;
+ bool on_resume;
GSList *services;
guint timer;
bool active;
unsigned int attempt;
- bool on_resume;
+ bool mark_on_suspend;
};
static const char *default_reconnect[] = {
- HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID,
- A2DP_SINK_UUID, NULL };
+ HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID, NULL };
static char **reconnect_uuids = NULL;
+static const char *default_reconnect_resume[] = { A2DP_SINK_UUID, NULL };
+static char **reconnect_resume_uuids = NULL;
+
static const size_t default_attempts = 7;
static size_t reconnect_attempts = 0;
@@ -551,14 +554,14 @@ static void reconnect_reset(struct reconnect_data *reconnect)
}
}
-static bool reconnect_match(const char *uuid)
+static bool reconnect_match_internal(const char *uuid, char **matchlist)
{
char **str;
- if (!reconnect_uuids)
+ if (!matchlist)
return false;
- for (str = reconnect_uuids; *str; str++) {
+ for (str = matchlist; *str; str++) {
if (!bt_uuid_strcmp(uuid, *str))
return true;
}
@@ -566,6 +569,16 @@ static bool reconnect_match(const char *uuid)
return false;
}
+static bool reconnect_match(const char *uuid)
+{
+ return reconnect_match_internal(uuid, reconnect_uuids);
+}
+
+static bool reconnect_resume_match(const char *uuid)
+{
+ return reconnect_match_internal(uuid, reconnect_resume_uuids);
+}
+
static struct reconnect_data *reconnect_add(struct btd_service *service)
{
struct btd_device *dev = btd_service_get_device(service);
@@ -687,10 +700,15 @@ static void service_cb(struct btd_service *service,
* be the first profile that's connected so we might have an
* entry but with the reconnect flag set to false.
*/
- if (!reconnect->reconnect)
- reconnect->reconnect = reconnect_match(profile->remote_uuid);
+ if (!reconnect->on_timeout)
+ reconnect->on_timeout = reconnect_match(profile->remote_uuid);
+
+ if (!reconnect->on_resume)
+ reconnect->on_resume =
+ reconnect_resume_match(profile->remote_uuid);
- DBG("Added %s reconnect %u", profile->name, reconnect->reconnect);
+ DBG("Added %s reconnect on timeout(%u), resume(%u)",
+ profile->name, reconnect->on_timeout, reconnect->on_resume);
}
static gboolean reconnect_timeout(gpointer data)
@@ -704,7 +722,7 @@ static gboolean reconnect_timeout(gpointer data)
reconnect->timer = 0;
/* Mark any reconnect on resume as handled */
- reconnect->on_resume = false;
+ reconnect->mark_on_suspend = false;
err = btd_device_connect_services(reconnect->dev, reconnect->services);
if (err < 0) {
@@ -750,7 +768,7 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
return;
reconnect = reconnect_find(dev);
- if (!reconnect || !reconnect->reconnect)
+ if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
return;
reconnect_reset(reconnect);
@@ -759,11 +777,11 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
switch (reason) {
case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
- if (btd_device_get_service(dev, A2DP_SINK_UUID)) {
+ if (reconnect->on_resume) {
DBG("%s configured to reconnect on resume",
device_get_path(dev));
- reconnect->on_resume = true;
+ reconnect->mark_on_suspend = true;
/* If the kernel supports resume events, it is
* preferable to set the reconnect timer there as it is
@@ -774,7 +792,8 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
}
break;
case MGMT_DEV_DISCONN_TIMEOUT:
- reconnect_set_timer(reconnect, -1);
+ if (reconnect->on_timeout)
+ reconnect_set_timer(reconnect, -1);
break;
default:
DBG("Developer error. Reason = %d", reason);
@@ -790,7 +809,7 @@ static void policy_adapter_resume(struct btd_adapter *adapter)
for (l = reconnects; l; l = g_slist_next(l)) {
struct reconnect_data *reconnect = l->data;
- if (reconnect->on_resume &&
+ if (reconnect->mark_on_suspend &&
device_get_adapter(reconnect->dev) == adapter) {
reconnect_set_timer(reconnect, resume_delay);
}
@@ -804,7 +823,7 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
DBG("status %u", status);
reconnect = reconnect_find(dev);
- if (!reconnect || !reconnect->reconnect)
+ if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
return;
if (!reconnect->active)
@@ -845,6 +864,7 @@ static int policy_init(void)
{
GError *gerr = NULL;
GKeyFile *conf;
+ bool has_reconnect_uuids;
service_id = btd_service_add_state_cb(service_cb, NULL);
@@ -892,6 +912,15 @@ static int policy_init(void)
auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
NULL);
+ reconnect_resume_uuids = g_key_file_get_string_list(
+ conf, "Policy", "ReconnectResumeUUIDs", NULL, &gerr);
+
+ if (gerr) {
+ g_clear_error(&gerr);
+ reconnect_resume_uuids =
+ g_strdupv((char **)default_reconnect_resume);
+ }
+
resume_delay = g_key_file_get_integer(
conf, "Policy", "ResumeDelay", &gerr);
@@ -900,7 +929,11 @@ static int policy_init(void)
resume_delay = default_resume_delay;
}
done:
- if (reconnect_uuids && reconnect_uuids[0] && reconnect_attempts) {
+ has_reconnect_uuids =
+ (reconnect_uuids && reconnect_uuids[0]) ||
+ (reconnect_resume_uuids && reconnect_resume_uuids[0]);
+
+ if (reconnect_attempts && has_reconnect_uuids) {
btd_add_disconnect_cb(disconnect_cb);
btd_add_conn_fail_cb(conn_fail_cb);
}
@@ -918,6 +951,9 @@ static void policy_exit(void)
if (reconnect_uuids)
g_strfreev(reconnect_uuids);
+ if (reconnect_resume_uuids)
+ g_strfreev(reconnect_resume_uuids);
+
g_free(reconnect_intervals);
g_slist_free_full(reconnects, reconnect_destroy);
diff --git a/src/main.conf b/src/main.conf
index ad36638b7..6f66acb14 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -220,7 +220,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,0000110b-0000-1000-8000-00805f9b34fb
+#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
# ReconnectAttempts define the number of attempts to reconnect after a link
# lost. Setting the value to 0 disables reconnecting feature.
@@ -237,6 +237,13 @@
# in later on. Defaults to 'false'.
#AutoEnable=false
+# The ReconnectResumeUUIDs defines the set of remote services that should try
+# to be reconnected after resume from suspend if they were connected before
+# suspend. If a device is disconnected due to suspend, it will be queued for
+# reconnect on resume first using the ResumeDelay value and then the remaining
+# values in ReconnectIntervals.
+#ReconnectResumeUUIDs=0000110b-0000-1000-8000-00805f9b34fb
+
# 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
--
2.29.2.576.ga3fc446d84-goog
Hi Abhishek,
On Mon, Dec 7, 2020 at 3:49 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> Airpods incorrectly emit a connection timeout as the disconnect reason
> when a different device connects to it and triggers the automatic
> reconnection by mistake. See hci trace below:
>
> > HCI Event: Disconnect Complete (0x05) plen 4
> Status: Success (0x00)
> Handle: 256
> Reason: Connection Timeout (0x08)
> @ MGMT Event: Device Disconnected (0x000c) plen 8
> BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> Reason: Connection timeout (0x01)
> @ MGMT Event: Device Disconnected (0x000c) plen 8
> BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> Reason: Connection timeout (0x01)
> @ MGMT Event: Device Disconnected (0x000c) plen 8
> BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> Reason: Connection timeout (0x01)
> @ MGMT Event: Device Disconnected (0x000c) plen 8
> BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> Reason: Connection timeout (0x01)
> < HCI Command: Write Scan Enable (0x03|0x001a) plen 1
> Scan enable: Page Scan (0x02)
> > HCI Event: Command Complete (0x0e) plen 4
> Write Scan Enable (0x03|0x001a) ncmd 2
> Status: Success (0x00)
> < HCI Command: Create Connection (0x01|0x0005) plen 13
> Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> Packet type: 0xcc18
> DM1 may be used
> DH1 may be used
> DM3 may be used
> DH3 may be used
> DM5 may be used
> DH5 may be used
> Page scan repetition mode: R2 (0x02)
> Page scan mode: Mandatory (0x00)
> Clock offset: 0x0000
> Role switch: Allow slave (0x01)
> > HCI Event: Command Status (0x0f) plen 4
> Create Connection (0x01|0x0005) ncmd 1
>
> To fix this behavior, separate the ReconnectUUIDs into services that
> reconnect on timeout and on resume. This ensures that the a2dp-sink
> profile reconnects are correctly applied only on resume from suspend and
> adds the option to reconnect other profiles automatically on resume.
>
> Reviewed-by: Alain Michaud <[email protected]>
> ---
>
> plugins/policy.c | 72 ++++++++++++++++++++++++++++++++++++------------
> src/main.conf | 9 +++++-
> 2 files changed, 62 insertions(+), 19 deletions(-)
>
> diff --git a/plugins/policy.c b/plugins/policy.c
> index ba9e1be02..a3cc3445e 100644
> --- a/plugins/policy.c
> +++ b/plugins/policy.c
> @@ -44,19 +44,22 @@
>
> struct reconnect_data {
> struct btd_device *dev;
> - bool reconnect;
> + bool on_timeout;
> + bool on_resume;
> GSList *services;
> guint timer;
> bool active;
> unsigned int attempt;
> - bool on_resume;
> + bool mark_on_suspend;
> };
>
> static const char *default_reconnect[] = {
> - HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID,
> - A2DP_SINK_UUID, NULL };
> + HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID, NULL };
> static char **reconnect_uuids = NULL;
>
> +static const char *default_reconnect_resume[] = { A2DP_SINK_UUID, NULL };
> +static char **reconnect_resume_uuids = NULL;
> +
> static const size_t default_attempts = 7;
> static size_t reconnect_attempts = 0;
>
> @@ -551,14 +554,14 @@ static void reconnect_reset(struct reconnect_data *reconnect)
> }
> }
>
> -static bool reconnect_match(const char *uuid)
> +static bool reconnect_match_internal(const char *uuid, char **matchlist)
> {
> char **str;
>
> - if (!reconnect_uuids)
> + if (!matchlist)
> return false;
>
> - for (str = reconnect_uuids; *str; str++) {
> + for (str = matchlist; *str; str++) {
> if (!bt_uuid_strcmp(uuid, *str))
> return true;
> }
> @@ -566,6 +569,16 @@ static bool reconnect_match(const char *uuid)
> return false;
> }
>
> +static bool reconnect_match(const char *uuid)
> +{
> + return reconnect_match_internal(uuid, reconnect_uuids);
> +}
> +
> +static bool reconnect_resume_match(const char *uuid)
> +{
> + return reconnect_match_internal(uuid, reconnect_resume_uuids);
> +}
> +
> static struct reconnect_data *reconnect_add(struct btd_service *service)
> {
> struct btd_device *dev = btd_service_get_device(service);
> @@ -687,10 +700,15 @@ static void service_cb(struct btd_service *service,
> * be the first profile that's connected so we might have an
> * entry but with the reconnect flag set to false.
> */
> - if (!reconnect->reconnect)
> - reconnect->reconnect = reconnect_match(profile->remote_uuid);
> + if (!reconnect->on_timeout)
> + reconnect->on_timeout = reconnect_match(profile->remote_uuid);
> +
> + if (!reconnect->on_resume)
> + reconnect->on_resume =
> + reconnect_resume_match(profile->remote_uuid);
>
> - DBG("Added %s reconnect %u", profile->name, reconnect->reconnect);
> + DBG("Added %s reconnect on timeout(%u), resume(%u)",
> + profile->name, reconnect->on_timeout, reconnect->on_resume);
> }
>
> static gboolean reconnect_timeout(gpointer data)
> @@ -704,7 +722,7 @@ static gboolean reconnect_timeout(gpointer data)
> reconnect->timer = 0;
>
> /* Mark any reconnect on resume as handled */
> - reconnect->on_resume = false;
> + reconnect->mark_on_suspend = false;
>
> err = btd_device_connect_services(reconnect->dev, reconnect->services);
> if (err < 0) {
> @@ -750,7 +768,7 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> return;
>
> reconnect = reconnect_find(dev);
> - if (!reconnect || !reconnect->reconnect)
> + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> return;
>
> reconnect_reset(reconnect);
> @@ -759,11 +777,11 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
>
> switch (reason) {
> case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
> - if (btd_device_get_service(dev, A2DP_SINK_UUID)) {
> + if (reconnect->on_resume) {
> DBG("%s configured to reconnect on resume",
> device_get_path(dev));
>
> - reconnect->on_resume = true;
> + reconnect->mark_on_suspend = true;
>
> /* If the kernel supports resume events, it is
> * preferable to set the reconnect timer there as it is
> @@ -774,7 +792,8 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> }
> break;
> case MGMT_DEV_DISCONN_TIMEOUT:
> - reconnect_set_timer(reconnect, -1);
> + if (reconnect->on_timeout)
> + reconnect_set_timer(reconnect, -1);
> break;
> default:
> DBG("Developer error. Reason = %d", reason);
> @@ -790,7 +809,7 @@ static void policy_adapter_resume(struct btd_adapter *adapter)
> for (l = reconnects; l; l = g_slist_next(l)) {
> struct reconnect_data *reconnect = l->data;
>
> - if (reconnect->on_resume &&
> + if (reconnect->mark_on_suspend &&
> device_get_adapter(reconnect->dev) == adapter) {
> reconnect_set_timer(reconnect, resume_delay);
> }
> @@ -804,7 +823,7 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
> DBG("status %u", status);
>
> reconnect = reconnect_find(dev);
> - if (!reconnect || !reconnect->reconnect)
> + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> return;
>
> if (!reconnect->active)
> @@ -845,6 +864,7 @@ static int policy_init(void)
> {
> GError *gerr = NULL;
> GKeyFile *conf;
> + bool has_reconnect_uuids;
>
> service_id = btd_service_add_state_cb(service_cb, NULL);
>
> @@ -892,6 +912,15 @@ static int policy_init(void)
> auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
> NULL);
>
> + reconnect_resume_uuids = g_key_file_get_string_list(
> + conf, "Policy", "ReconnectResumeUUIDs", NULL, &gerr);
> +
> + if (gerr) {
> + g_clear_error(&gerr);
> + reconnect_resume_uuids =
> + g_strdupv((char **)default_reconnect_resume);
> + }
> +
> resume_delay = g_key_file_get_integer(
> conf, "Policy", "ResumeDelay", &gerr);
>
> @@ -900,7 +929,11 @@ static int policy_init(void)
> resume_delay = default_resume_delay;
> }
> done:
> - if (reconnect_uuids && reconnect_uuids[0] && reconnect_attempts) {
> + has_reconnect_uuids =
> + (reconnect_uuids && reconnect_uuids[0]) ||
> + (reconnect_resume_uuids && reconnect_resume_uuids[0]);
> +
> + if (reconnect_attempts && has_reconnect_uuids) {
> btd_add_disconnect_cb(disconnect_cb);
> btd_add_conn_fail_cb(conn_fail_cb);
> }
> @@ -918,6 +951,9 @@ static void policy_exit(void)
> if (reconnect_uuids)
> g_strfreev(reconnect_uuids);
>
> + if (reconnect_resume_uuids)
> + g_strfreev(reconnect_resume_uuids);
> +
> g_free(reconnect_intervals);
>
> g_slist_free_full(reconnects, reconnect_destroy);
> diff --git a/src/main.conf b/src/main.conf
> index ad36638b7..6f66acb14 100644
> --- a/src/main.conf
> +++ b/src/main.conf
> @@ -220,7 +220,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,0000110b-0000-1000-8000-00805f9b34fb
> +#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
>
> # ReconnectAttempts define the number of attempts to reconnect after a link
> # lost. Setting the value to 0 disables reconnecting feature.
> @@ -237,6 +237,13 @@
> # in later on. Defaults to 'false'.
> #AutoEnable=false
>
> +# The ReconnectResumeUUIDs defines the set of remote services that should try
> +# to be reconnected after resume from suspend if they were connected before
> +# suspend. If a device is disconnected due to suspend, it will be queued for
> +# reconnect on resume first using the ResumeDelay value and then the remaining
> +# values in ReconnectIntervals.
> +#ReconnectResumeUUIDs=0000110b-0000-1000-8000-00805f9b34fb
I don't follow the reasoning here really, we need to be consistent
with regards to policy so how about the other profiles listed to be
reconnected does it means they no longer are connected on resume? From
the looks of you it we cannot really tell if the device was
disconnected due to link supervision timeout or not so we can't really
assume the Connection timeout (0x01) is in fact a second device
connecting, IMO if it is a mistake on the manufacturer then this
should be made device specific even though I think you will have to
disable the reconnection logic altogether if we cannot trust the
reason for the disconnection or does it cleanup the A2DP stream before
disconnecting, in that case we should have consider a clean
disconnection.
> # 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
> --
> 2.29.2.576.ga3fc446d84-goog
>
--
Luiz Augusto von Dentz
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=397867
---Test result---
##############################
Test: CheckPatch - FAIL
Output:
policy: Refactor reconnect policy for resume
WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
#79: FILE: plugins/policy.c:60:
+static const char *default_reconnect_resume[] = { A2DP_SINK_UUID, NULL };
ERROR:INITIALISED_STATIC: do not initialise statics to NULL
#80: FILE: plugins/policy.c:61:
+static char **reconnect_resume_uuids = NULL;
- total: 1 errors, 1 warnings, 195 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] policy: Refactor reconnect policy for resume" has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
---
Regards,
Linux Bluetooth
Hi Luiz,
On Mon, Dec 7, 2020 at 4:36 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Abhishek,
>
> On Mon, Dec 7, 2020 at 3:49 PM Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >
> > Airpods incorrectly emit a connection timeout as the disconnect reason
> > when a different device connects to it and triggers the automatic
> > reconnection by mistake. See hci trace below:
> >
> > > HCI Event: Disconnect Complete (0x05) plen 4
> > Status: Success (0x00)
> > Handle: 256
> > Reason: Connection Timeout (0x08)
> > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > Reason: Connection timeout (0x01)
> > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > Reason: Connection timeout (0x01)
> > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > Reason: Connection timeout (0x01)
> > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > Reason: Connection timeout (0x01)
> > < HCI Command: Write Scan Enable (0x03|0x001a) plen 1
> > Scan enable: Page Scan (0x02)
> > > HCI Event: Command Complete (0x0e) plen 4
> > Write Scan Enable (0x03|0x001a) ncmd 2
> > Status: Success (0x00)
> > < HCI Command: Create Connection (0x01|0x0005) plen 13
> > Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > Packet type: 0xcc18
> > DM1 may be used
> > DH1 may be used
> > DM3 may be used
> > DH3 may be used
> > DM5 may be used
> > DH5 may be used
> > Page scan repetition mode: R2 (0x02)
> > Page scan mode: Mandatory (0x00)
> > Clock offset: 0x0000
> > Role switch: Allow slave (0x01)
> > > HCI Event: Command Status (0x0f) plen 4
> > Create Connection (0x01|0x0005) ncmd 1
> >
> > To fix this behavior, separate the ReconnectUUIDs into services that
> > reconnect on timeout and on resume. This ensures that the a2dp-sink
> > profile reconnects are correctly applied only on resume from suspend and
> > adds the option to reconnect other profiles automatically on resume.
> >
> > Reviewed-by: Alain Michaud <[email protected]>
> > ---
> >
> > plugins/policy.c | 72 ++++++++++++++++++++++++++++++++++++------------
> > src/main.conf | 9 +++++-
> > 2 files changed, 62 insertions(+), 19 deletions(-)
> >
> > diff --git a/plugins/policy.c b/plugins/policy.c
> > index ba9e1be02..a3cc3445e 100644
> > --- a/plugins/policy.c
> > +++ b/plugins/policy.c
> > @@ -44,19 +44,22 @@
> >
> > struct reconnect_data {
> > struct btd_device *dev;
> > - bool reconnect;
> > + bool on_timeout;
> > + bool on_resume;
> > GSList *services;
> > guint timer;
> > bool active;
> > unsigned int attempt;
> > - bool on_resume;
> > + bool mark_on_suspend;
> > };
> >
> > static const char *default_reconnect[] = {
> > - HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID,
> > - A2DP_SINK_UUID, NULL };
> > + HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID, NULL };
> > static char **reconnect_uuids = NULL;
> >
> > +static const char *default_reconnect_resume[] = { A2DP_SINK_UUID, NULL };
> > +static char **reconnect_resume_uuids = NULL;
> > +
> > static const size_t default_attempts = 7;
> > static size_t reconnect_attempts = 0;
> >
> > @@ -551,14 +554,14 @@ static void reconnect_reset(struct reconnect_data *reconnect)
> > }
> > }
> >
> > -static bool reconnect_match(const char *uuid)
> > +static bool reconnect_match_internal(const char *uuid, char **matchlist)
> > {
> > char **str;
> >
> > - if (!reconnect_uuids)
> > + if (!matchlist)
> > return false;
> >
> > - for (str = reconnect_uuids; *str; str++) {
> > + for (str = matchlist; *str; str++) {
> > if (!bt_uuid_strcmp(uuid, *str))
> > return true;
> > }
> > @@ -566,6 +569,16 @@ static bool reconnect_match(const char *uuid)
> > return false;
> > }
> >
> > +static bool reconnect_match(const char *uuid)
> > +{
> > + return reconnect_match_internal(uuid, reconnect_uuids);
> > +}
> > +
> > +static bool reconnect_resume_match(const char *uuid)
> > +{
> > + return reconnect_match_internal(uuid, reconnect_resume_uuids);
> > +}
> > +
> > static struct reconnect_data *reconnect_add(struct btd_service *service)
> > {
> > struct btd_device *dev = btd_service_get_device(service);
> > @@ -687,10 +700,15 @@ static void service_cb(struct btd_service *service,
> > * be the first profile that's connected so we might have an
> > * entry but with the reconnect flag set to false.
> > */
> > - if (!reconnect->reconnect)
> > - reconnect->reconnect = reconnect_match(profile->remote_uuid);
> > + if (!reconnect->on_timeout)
> > + reconnect->on_timeout = reconnect_match(profile->remote_uuid);
> > +
> > + if (!reconnect->on_resume)
> > + reconnect->on_resume =
> > + reconnect_resume_match(profile->remote_uuid);
> >
> > - DBG("Added %s reconnect %u", profile->name, reconnect->reconnect);
> > + DBG("Added %s reconnect on timeout(%u), resume(%u)",
> > + profile->name, reconnect->on_timeout, reconnect->on_resume);
> > }
> >
> > static gboolean reconnect_timeout(gpointer data)
> > @@ -704,7 +722,7 @@ static gboolean reconnect_timeout(gpointer data)
> > reconnect->timer = 0;
> >
> > /* Mark any reconnect on resume as handled */
> > - reconnect->on_resume = false;
> > + reconnect->mark_on_suspend = false;
> >
> > err = btd_device_connect_services(reconnect->dev, reconnect->services);
> > if (err < 0) {
> > @@ -750,7 +768,7 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > return;
> >
> > reconnect = reconnect_find(dev);
> > - if (!reconnect || !reconnect->reconnect)
> > + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> > return;
> >
> > reconnect_reset(reconnect);
> > @@ -759,11 +777,11 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> >
> > switch (reason) {
> > case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
> > - if (btd_device_get_service(dev, A2DP_SINK_UUID)) {
> > + if (reconnect->on_resume) {
Note: Previously, only A2DP-SINK would reconnect if disconnected during suspend.
> > DBG("%s configured to reconnect on resume",
> > device_get_path(dev));
> >
> > - reconnect->on_resume = true;
> > + reconnect->mark_on_suspend = true;
> >
> > /* If the kernel supports resume events, it is
> > * preferable to set the reconnect timer there as it is
> > @@ -774,7 +792,8 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > }
> > break;
> > case MGMT_DEV_DISCONN_TIMEOUT:
> > - reconnect_set_timer(reconnect, -1);
> > + if (reconnect->on_timeout)
> > + reconnect_set_timer(reconnect, -1);
> > break;
> > default:
> > DBG("Developer error. Reason = %d", reason);
> > @@ -790,7 +809,7 @@ static void policy_adapter_resume(struct btd_adapter *adapter)
> > for (l = reconnects; l; l = g_slist_next(l)) {
> > struct reconnect_data *reconnect = l->data;
> >
> > - if (reconnect->on_resume &&
> > + if (reconnect->mark_on_suspend &&
> > device_get_adapter(reconnect->dev) == adapter) {
> > reconnect_set_timer(reconnect, resume_delay);
> > }
> > @@ -804,7 +823,7 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
> > DBG("status %u", status);
> >
> > reconnect = reconnect_find(dev);
> > - if (!reconnect || !reconnect->reconnect)
> > + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> > return;
> >
> > if (!reconnect->active)
> > @@ -845,6 +864,7 @@ static int policy_init(void)
> > {
> > GError *gerr = NULL;
> > GKeyFile *conf;
> > + bool has_reconnect_uuids;
> >
> > service_id = btd_service_add_state_cb(service_cb, NULL);
> >
> > @@ -892,6 +912,15 @@ static int policy_init(void)
> > auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
> > NULL);
> >
> > + reconnect_resume_uuids = g_key_file_get_string_list(
> > + conf, "Policy", "ReconnectResumeUUIDs", NULL, &gerr);
> > +
> > + if (gerr) {
> > + g_clear_error(&gerr);
> > + reconnect_resume_uuids =
> > + g_strdupv((char **)default_reconnect_resume);
> > + }
> > +
> > resume_delay = g_key_file_get_integer(
> > conf, "Policy", "ResumeDelay", &gerr);
> >
> > @@ -900,7 +929,11 @@ static int policy_init(void)
> > resume_delay = default_resume_delay;
> > }
> > done:
> > - if (reconnect_uuids && reconnect_uuids[0] && reconnect_attempts) {
> > + has_reconnect_uuids =
> > + (reconnect_uuids && reconnect_uuids[0]) ||
> > + (reconnect_resume_uuids && reconnect_resume_uuids[0]);
> > +
> > + if (reconnect_attempts && has_reconnect_uuids) {
> > btd_add_disconnect_cb(disconnect_cb);
> > btd_add_conn_fail_cb(conn_fail_cb);
> > }
> > @@ -918,6 +951,9 @@ static void policy_exit(void)
> > if (reconnect_uuids)
> > g_strfreev(reconnect_uuids);
> >
> > + if (reconnect_resume_uuids)
> > + g_strfreev(reconnect_resume_uuids);
> > +
> > g_free(reconnect_intervals);
> >
> > g_slist_free_full(reconnects, reconnect_destroy);
> > diff --git a/src/main.conf b/src/main.conf
> > index ad36638b7..6f66acb14 100644
> > --- a/src/main.conf
> > +++ b/src/main.conf
> > @@ -220,7 +220,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,0000110b-0000-1000-8000-00805f9b34fb
> > +#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
> >
> > # ReconnectAttempts define the number of attempts to reconnect after a link
> > # lost. Setting the value to 0 disables reconnecting feature.
> > @@ -237,6 +237,13 @@
> > # in later on. Defaults to 'false'.
> > #AutoEnable=false
> >
> > +# The ReconnectResumeUUIDs defines the set of remote services that should try
> > +# to be reconnected after resume from suspend if they were connected before
> > +# suspend. If a device is disconnected due to suspend, it will be queued for
> > +# reconnect on resume first using the ResumeDelay value and then the remaining
> > +# values in ReconnectIntervals.
> > +#ReconnectResumeUUIDs=0000110b-0000-1000-8000-00805f9b34fb
>
> I don't follow the reasoning here really, we need to be consistent
> with regards to policy so how about the other profiles listed to be
> reconnected does it means they no longer are connected on resume? From
> the looks of you it we cannot really tell if the device was
> disconnected due to link supervision timeout or not so we can't really
> assume the Connection timeout (0x01) is in fact a second device
> connecting, IMO if it is a mistake on the manufacturer then this
> should be made device specific even though I think you will have to
> disable the reconnection logic altogether if we cannot trust the
> reason for the disconnection or does it cleanup the A2DP stream before
> disconnecting, in that case we should have consider a clean
> disconnection.
ReconnectUUIDs was overloaded to mean both "reconnect on link loss"
and "reconnect only a2dp-sink if disconnected due to suspend". This
patch removes that overloading and ReconnectUUIDs simply always means,
"Reconnect if link loss". ReconnectResumeUUIDs means simply, "If this
device was disconnected during suspend, attempt to reconnect it on
resume." It's adding a Reconnect policy for disconnect reason =
"Disconnect due to Suspend". I think this is more generic and enables
adding other uuids to this policy.
If we really want link loss reconnects for a2dp-sink, I agree that we
should be treating the Airpods differently because it is specific to
that device.
This is what I'm seeing on the Airpods when I connect from an Android
phone (while already connected to Chromebook). There is no profile
disconnect (no l2cap disconnection requests either).
> HCI Event: Number of Completed Packets (0x13) plen 5 #1674 [hci0] 38.810693
Num handles: 1
Handle: 256
Count: 1
< ACL Data TX: Handle 256 flags 0x02 dlen 969
#1675 [hci0]
38.820328
Channel: 11011 len 965 [PSM 25 mode Basic (0x00)] {chan 2}
< ACL Data TX: Handle 256 flags 0x00 dlen 7
#1676 [hci0]
38.821054
Channel: 10756 len 3 [PSM 25 mode Basic (0x00)] {chan 3}
AVDTP: Suspend (0x09) Command (0x00) type 0x00 label 4 nosp 0
ACP SEID: 1
> HCI Event: Number of Completed Packets (0x13) plen 5 #1677 [hci0] 38.835673
Num handles: 1
Handle: 256
Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5 #1678 [hci0] 38.836347
Num handles: 1
Handle: 256
Count: 1
> ACL Data RX: Handle 256 flags 0x02 dlen 6 #1679 [hci0] 38.861708
Channel: 67 len 2 [PSM 25 mode Basic (0x00)] {chan 3}
AVDTP: Suspend (0x09) Response Accept (0x02) type 0x00 label 4
nosp 0
> HCI Event: Disconnect Complete (0x05) plen 4 #1680 [hci0] 62.656436
Status: Success (0x00)
Handle: 256
Reason: Connection Timeout (0x08)
>
> > # 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
> > --
> > 2.29.2.576.ga3fc446d84-goog
> >
>
>
> --
> Luiz Augusto von Dentz
Hi Abhishek,
On Mon, Dec 7, 2020 at 5:19 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> Hi Luiz,
>
> On Mon, Dec 7, 2020 at 4:36 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Abhishek,
> >
> > On Mon, Dec 7, 2020 at 3:49 PM Abhishek Pandit-Subedi
> > <[email protected]> wrote:
> > >
> > > Airpods incorrectly emit a connection timeout as the disconnect reason
> > > when a different device connects to it and triggers the automatic
> > > reconnection by mistake. See hci trace below:
> > >
> > > > HCI Event: Disconnect Complete (0x05) plen 4
> > > Status: Success (0x00)
> > > Handle: 256
> > > Reason: Connection Timeout (0x08)
> > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > Reason: Connection timeout (0x01)
> > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > Reason: Connection timeout (0x01)
> > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > Reason: Connection timeout (0x01)
> > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > Reason: Connection timeout (0x01)
> > > < HCI Command: Write Scan Enable (0x03|0x001a) plen 1
> > > Scan enable: Page Scan (0x02)
> > > > HCI Event: Command Complete (0x0e) plen 4
> > > Write Scan Enable (0x03|0x001a) ncmd 2
> > > Status: Success (0x00)
> > > < HCI Command: Create Connection (0x01|0x0005) plen 13
> > > Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > Packet type: 0xcc18
> > > DM1 may be used
> > > DH1 may be used
> > > DM3 may be used
> > > DH3 may be used
> > > DM5 may be used
> > > DH5 may be used
> > > Page scan repetition mode: R2 (0x02)
> > > Page scan mode: Mandatory (0x00)
> > > Clock offset: 0x0000
> > > Role switch: Allow slave (0x01)
> > > > HCI Event: Command Status (0x0f) plen 4
> > > Create Connection (0x01|0x0005) ncmd 1
> > >
> > > To fix this behavior, separate the ReconnectUUIDs into services that
> > > reconnect on timeout and on resume. This ensures that the a2dp-sink
> > > profile reconnects are correctly applied only on resume from suspend and
> > > adds the option to reconnect other profiles automatically on resume.
> > >
> > > Reviewed-by: Alain Michaud <[email protected]>
> > > ---
> > >
> > > plugins/policy.c | 72 ++++++++++++++++++++++++++++++++++++------------
> > > src/main.conf | 9 +++++-
> > > 2 files changed, 62 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/plugins/policy.c b/plugins/policy.c
> > > index ba9e1be02..a3cc3445e 100644
> > > --- a/plugins/policy.c
> > > +++ b/plugins/policy.c
> > > @@ -44,19 +44,22 @@
> > >
> > > struct reconnect_data {
> > > struct btd_device *dev;
> > > - bool reconnect;
> > > + bool on_timeout;
> > > + bool on_resume;
> > > GSList *services;
> > > guint timer;
> > > bool active;
> > > unsigned int attempt;
> > > - bool on_resume;
> > > + bool mark_on_suspend;
> > > };
> > >
> > > static const char *default_reconnect[] = {
> > > - HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID,
> > > - A2DP_SINK_UUID, NULL };
> > > + HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID, NULL };
> > > static char **reconnect_uuids = NULL;
> > >
> > > +static const char *default_reconnect_resume[] = { A2DP_SINK_UUID, NULL };
> > > +static char **reconnect_resume_uuids = NULL;
> > > +
> > > static const size_t default_attempts = 7;
> > > static size_t reconnect_attempts = 0;
> > >
> > > @@ -551,14 +554,14 @@ static void reconnect_reset(struct reconnect_data *reconnect)
> > > }
> > > }
> > >
> > > -static bool reconnect_match(const char *uuid)
> > > +static bool reconnect_match_internal(const char *uuid, char **matchlist)
> > > {
> > > char **str;
> > >
> > > - if (!reconnect_uuids)
> > > + if (!matchlist)
> > > return false;
> > >
> > > - for (str = reconnect_uuids; *str; str++) {
> > > + for (str = matchlist; *str; str++) {
> > > if (!bt_uuid_strcmp(uuid, *str))
> > > return true;
> > > }
> > > @@ -566,6 +569,16 @@ static bool reconnect_match(const char *uuid)
> > > return false;
> > > }
> > >
> > > +static bool reconnect_match(const char *uuid)
> > > +{
> > > + return reconnect_match_internal(uuid, reconnect_uuids);
> > > +}
> > > +
> > > +static bool reconnect_resume_match(const char *uuid)
> > > +{
> > > + return reconnect_match_internal(uuid, reconnect_resume_uuids);
> > > +}
> > > +
> > > static struct reconnect_data *reconnect_add(struct btd_service *service)
> > > {
> > > struct btd_device *dev = btd_service_get_device(service);
> > > @@ -687,10 +700,15 @@ static void service_cb(struct btd_service *service,
> > > * be the first profile that's connected so we might have an
> > > * entry but with the reconnect flag set to false.
> > > */
> > > - if (!reconnect->reconnect)
> > > - reconnect->reconnect = reconnect_match(profile->remote_uuid);
> > > + if (!reconnect->on_timeout)
> > > + reconnect->on_timeout = reconnect_match(profile->remote_uuid);
> > > +
> > > + if (!reconnect->on_resume)
> > > + reconnect->on_resume =
> > > + reconnect_resume_match(profile->remote_uuid);
> > >
> > > - DBG("Added %s reconnect %u", profile->name, reconnect->reconnect);
> > > + DBG("Added %s reconnect on timeout(%u), resume(%u)",
> > > + profile->name, reconnect->on_timeout, reconnect->on_resume);
> > > }
> > >
> > > static gboolean reconnect_timeout(gpointer data)
> > > @@ -704,7 +722,7 @@ static gboolean reconnect_timeout(gpointer data)
> > > reconnect->timer = 0;
> > >
> > > /* Mark any reconnect on resume as handled */
> > > - reconnect->on_resume = false;
> > > + reconnect->mark_on_suspend = false;
> > >
> > > err = btd_device_connect_services(reconnect->dev, reconnect->services);
> > > if (err < 0) {
> > > @@ -750,7 +768,7 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > > return;
> > >
> > > reconnect = reconnect_find(dev);
> > > - if (!reconnect || !reconnect->reconnect)
> > > + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> > > return;
> > >
> > > reconnect_reset(reconnect);
> > > @@ -759,11 +777,11 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > >
> > > switch (reason) {
> > > case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
> > > - if (btd_device_get_service(dev, A2DP_SINK_UUID)) {
> > > + if (reconnect->on_resume) {
>
> Note: Previously, only A2DP-SINK would reconnect if disconnected during suspend.
>
> > > DBG("%s configured to reconnect on resume",
> > > device_get_path(dev));
> > >
> > > - reconnect->on_resume = true;
> > > + reconnect->mark_on_suspend = true;
> > >
> > > /* If the kernel supports resume events, it is
> > > * preferable to set the reconnect timer there as it is
> > > @@ -774,7 +792,8 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > > }
> > > break;
> > > case MGMT_DEV_DISCONN_TIMEOUT:
> > > - reconnect_set_timer(reconnect, -1);
> > > + if (reconnect->on_timeout)
> > > + reconnect_set_timer(reconnect, -1);
> > > break;
> > > default:
> > > DBG("Developer error. Reason = %d", reason);
> > > @@ -790,7 +809,7 @@ static void policy_adapter_resume(struct btd_adapter *adapter)
> > > for (l = reconnects; l; l = g_slist_next(l)) {
> > > struct reconnect_data *reconnect = l->data;
> > >
> > > - if (reconnect->on_resume &&
> > > + if (reconnect->mark_on_suspend &&
> > > device_get_adapter(reconnect->dev) == adapter) {
> > > reconnect_set_timer(reconnect, resume_delay);
> > > }
> > > @@ -804,7 +823,7 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
> > > DBG("status %u", status);
> > >
> > > reconnect = reconnect_find(dev);
> > > - if (!reconnect || !reconnect->reconnect)
> > > + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> > > return;
> > >
> > > if (!reconnect->active)
> > > @@ -845,6 +864,7 @@ static int policy_init(void)
> > > {
> > > GError *gerr = NULL;
> > > GKeyFile *conf;
> > > + bool has_reconnect_uuids;
> > >
> > > service_id = btd_service_add_state_cb(service_cb, NULL);
> > >
> > > @@ -892,6 +912,15 @@ static int policy_init(void)
> > > auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
> > > NULL);
> > >
> > > + reconnect_resume_uuids = g_key_file_get_string_list(
> > > + conf, "Policy", "ReconnectResumeUUIDs", NULL, &gerr);
> > > +
> > > + if (gerr) {
> > > + g_clear_error(&gerr);
> > > + reconnect_resume_uuids =
> > > + g_strdupv((char **)default_reconnect_resume);
> > > + }
> > > +
> > > resume_delay = g_key_file_get_integer(
> > > conf, "Policy", "ResumeDelay", &gerr);
> > >
> > > @@ -900,7 +929,11 @@ static int policy_init(void)
> > > resume_delay = default_resume_delay;
> > > }
> > > done:
> > > - if (reconnect_uuids && reconnect_uuids[0] && reconnect_attempts) {
> > > + has_reconnect_uuids =
> > > + (reconnect_uuids && reconnect_uuids[0]) ||
> > > + (reconnect_resume_uuids && reconnect_resume_uuids[0]);
> > > +
> > > + if (reconnect_attempts && has_reconnect_uuids) {
> > > btd_add_disconnect_cb(disconnect_cb);
> > > btd_add_conn_fail_cb(conn_fail_cb);
> > > }
> > > @@ -918,6 +951,9 @@ static void policy_exit(void)
> > > if (reconnect_uuids)
> > > g_strfreev(reconnect_uuids);
> > >
> > > + if (reconnect_resume_uuids)
> > > + g_strfreev(reconnect_resume_uuids);
> > > +
> > > g_free(reconnect_intervals);
> > >
> > > g_slist_free_full(reconnects, reconnect_destroy);
> > > diff --git a/src/main.conf b/src/main.conf
> > > index ad36638b7..6f66acb14 100644
> > > --- a/src/main.conf
> > > +++ b/src/main.conf
> > > @@ -220,7 +220,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,0000110b-0000-1000-8000-00805f9b34fb
> > > +#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
> > >
> > > # ReconnectAttempts define the number of attempts to reconnect after a link
> > > # lost. Setting the value to 0 disables reconnecting feature.
> > > @@ -237,6 +237,13 @@
> > > # in later on. Defaults to 'false'.
> > > #AutoEnable=false
> > >
> > > +# The ReconnectResumeUUIDs defines the set of remote services that should try
> > > +# to be reconnected after resume from suspend if they were connected before
> > > +# suspend. If a device is disconnected due to suspend, it will be queued for
> > > +# reconnect on resume first using the ResumeDelay value and then the remaining
> > > +# values in ReconnectIntervals.
> > > +#ReconnectResumeUUIDs=0000110b-0000-1000-8000-00805f9b34fb
> >
> > I don't follow the reasoning here really, we need to be consistent
> > with regards to policy so how about the other profiles listed to be
> > reconnected does it means they no longer are connected on resume? From
> > the looks of you it we cannot really tell if the device was
> > disconnected due to link supervision timeout or not so we can't really
> > assume the Connection timeout (0x01) is in fact a second device
> > connecting, IMO if it is a mistake on the manufacturer then this
> > should be made device specific even though I think you will have to
> > disable the reconnection logic altogether if we cannot trust the
> > reason for the disconnection or does it cleanup the A2DP stream before
> > disconnecting, in that case we should have consider a clean
> > disconnection.
>
> ReconnectUUIDs was overloaded to mean both "reconnect on link loss"
> and "reconnect only a2dp-sink if disconnected due to suspend". This
> patch removes that overloading and ReconnectUUIDs simply always means,
> "Reconnect if link loss". ReconnectResumeUUIDs means simply, "If this
> device was disconnected during suspend, attempt to reconnect it on
> resume." It's adding a Reconnect policy for disconnect reason =
> "Disconnect due to Suspend". I think this is more generic and enables
> adding other uuids to this policy.
To my understanding disconnected due to suspend was actually
considered a link loss due to suspend, if we were to carried out
different policies for these conditions the whole thing becomes a lot
more complicated, and to make matters worse this so far has been a
problem to a single manufacturer which in the future might actually
change the disconnect reason. How about the other profiles, if it were
connected to HFP shouldn't it be reconnected as well? Also would the
problem of stealing the connection still manifest when the laptop
resumes when one attempts to connect to the phone?
> If we really want link loss reconnects for a2dp-sink, I agree that we
> should be treating the Airpods differently because it is specific to
> that device.
>
> This is what I'm seeing on the Airpods when I connect from an Android
> phone (while already connected to Chromebook). There is no profile
> disconnect (no l2cap disconnection requests either).
>
> > HCI Event: Number of Completed Packets (0x13) plen 5 #1674 [hci0] 38.810693
> Num handles: 1
> Handle: 256
> Count: 1
> < ACL Data TX: Handle 256 flags 0x02 dlen 969
> #1675 [hci0]
> 38.820328
> Channel: 11011 len 965 [PSM 25 mode Basic (0x00)] {chan 2}
> < ACL Data TX: Handle 256 flags 0x00 dlen 7
> #1676 [hci0]
> 38.821054
> Channel: 10756 len 3 [PSM 25 mode Basic (0x00)] {chan 3}
> AVDTP: Suspend (0x09) Command (0x00) type 0x00 label 4 nosp 0
> ACP SEID: 1
> > HCI Event: Number of Completed Packets (0x13) plen 5 #1677 [hci0] 38.835673
> Num handles: 1
> Handle: 256
> Count: 1
> > HCI Event: Number of Completed Packets (0x13) plen 5 #1678 [hci0] 38.836347
> Num handles: 1
> Handle: 256
> Count: 1
> > ACL Data RX: Handle 256 flags 0x02 dlen 6 #1679 [hci0] 38.861708
> Channel: 67 len 2 [PSM 25 mode Basic (0x00)] {chan 3}
> AVDTP: Suspend (0x09) Response Accept (0x02) type 0x00 label 4
> nosp 0
> > HCI Event: Disconnect Complete (0x05) plen 4 #1680 [hci0] 62.656436
> Status: Success (0x00)
> Handle: 256
> Reason: Connection Timeout (0x08)
This is a link loss in my books, so if any profiles on ReconnectUUIDs
were connected this still would trigger a reconnection and we can't
really remove everything from there, perhaps for Chrome OS only A2DP
is connected but in other cases HFP might be connected as well. Imo a
better way to handle this is with a quirk for broken reason, or
perhaps expose an interface where the upper layer can disable policies
on per-device, in which case the user can disable the UUIDs to be
reconnected.
>
> >
> > > # 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
> > > --
> > > 2.29.2.576.ga3fc446d84-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
Hi Luiz,
Alain is investigating a generic quirks implementation since we have
more than one use-case for it.
For now, I've merged this into ChromeOS to resolve the immediate
problems with Airpods. Once we have a generic quirks library, we can
apply it here so that Link Loss for this specific generation of
Airpods is treated specially (not triggering reconnect).
Abhishek
On Mon, Dec 7, 2020 at 7:08 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Abhishek,
>
> On Mon, Dec 7, 2020 at 5:19 PM Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > On Mon, Dec 7, 2020 at 4:36 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Abhishek,
> > >
> > > On Mon, Dec 7, 2020 at 3:49 PM Abhishek Pandit-Subedi
> > > <[email protected]> wrote:
> > > >
> > > > Airpods incorrectly emit a connection timeout as the disconnect reason
> > > > when a different device connects to it and triggers the automatic
> > > > reconnection by mistake. See hci trace below:
> > > >
> > > > > HCI Event: Disconnect Complete (0x05) plen 4
> > > > Status: Success (0x00)
> > > > Handle: 256
> > > > Reason: Connection Timeout (0x08)
> > > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > > Reason: Connection timeout (0x01)
> > > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > > Reason: Connection timeout (0x01)
> > > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > > Reason: Connection timeout (0x01)
> > > > @ MGMT Event: Device Disconnected (0x000c) plen 8
> > > > BR/EDR Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > > Reason: Connection timeout (0x01)
> > > > < HCI Command: Write Scan Enable (0x03|0x001a) plen 1
> > > > Scan enable: Page Scan (0x02)
> > > > > HCI Event: Command Complete (0x0e) plen 4
> > > > Write Scan Enable (0x03|0x001a) ncmd 2
> > > > Status: Success (0x00)
> > > > < HCI Command: Create Connection (0x01|0x0005) plen 13
> > > > Address: E4:90:FD:7D:E7:5F (OUI E4-90-FD)
> > > > Packet type: 0xcc18
> > > > DM1 may be used
> > > > DH1 may be used
> > > > DM3 may be used
> > > > DH3 may be used
> > > > DM5 may be used
> > > > DH5 may be used
> > > > Page scan repetition mode: R2 (0x02)
> > > > Page scan mode: Mandatory (0x00)
> > > > Clock offset: 0x0000
> > > > Role switch: Allow slave (0x01)
> > > > > HCI Event: Command Status (0x0f) plen 4
> > > > Create Connection (0x01|0x0005) ncmd 1
> > > >
> > > > To fix this behavior, separate the ReconnectUUIDs into services that
> > > > reconnect on timeout and on resume. This ensures that the a2dp-sink
> > > > profile reconnects are correctly applied only on resume from suspend and
> > > > adds the option to reconnect other profiles automatically on resume.
> > > >
> > > > Reviewed-by: Alain Michaud <[email protected]>
> > > > ---
> > > >
> > > > plugins/policy.c | 72 ++++++++++++++++++++++++++++++++++++------------
> > > > src/main.conf | 9 +++++-
> > > > 2 files changed, 62 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/plugins/policy.c b/plugins/policy.c
> > > > index ba9e1be02..a3cc3445e 100644
> > > > --- a/plugins/policy.c
> > > > +++ b/plugins/policy.c
> > > > @@ -44,19 +44,22 @@
> > > >
> > > > struct reconnect_data {
> > > > struct btd_device *dev;
> > > > - bool reconnect;
> > > > + bool on_timeout;
> > > > + bool on_resume;
> > > > GSList *services;
> > > > guint timer;
> > > > bool active;
> > > > unsigned int attempt;
> > > > - bool on_resume;
> > > > + bool mark_on_suspend;
> > > > };
> > > >
> > > > static const char *default_reconnect[] = {
> > > > - HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID,
> > > > - A2DP_SINK_UUID, NULL };
> > > > + HSP_AG_UUID, HFP_AG_UUID, A2DP_SOURCE_UUID, NULL };
> > > > static char **reconnect_uuids = NULL;
> > > >
> > > > +static const char *default_reconnect_resume[] = { A2DP_SINK_UUID, NULL };
> > > > +static char **reconnect_resume_uuids = NULL;
> > > > +
> > > > static const size_t default_attempts = 7;
> > > > static size_t reconnect_attempts = 0;
> > > >
> > > > @@ -551,14 +554,14 @@ static void reconnect_reset(struct reconnect_data *reconnect)
> > > > }
> > > > }
> > > >
> > > > -static bool reconnect_match(const char *uuid)
> > > > +static bool reconnect_match_internal(const char *uuid, char **matchlist)
> > > > {
> > > > char **str;
> > > >
> > > > - if (!reconnect_uuids)
> > > > + if (!matchlist)
> > > > return false;
> > > >
> > > > - for (str = reconnect_uuids; *str; str++) {
> > > > + for (str = matchlist; *str; str++) {
> > > > if (!bt_uuid_strcmp(uuid, *str))
> > > > return true;
> > > > }
> > > > @@ -566,6 +569,16 @@ static bool reconnect_match(const char *uuid)
> > > > return false;
> > > > }
> > > >
> > > > +static bool reconnect_match(const char *uuid)
> > > > +{
> > > > + return reconnect_match_internal(uuid, reconnect_uuids);
> > > > +}
> > > > +
> > > > +static bool reconnect_resume_match(const char *uuid)
> > > > +{
> > > > + return reconnect_match_internal(uuid, reconnect_resume_uuids);
> > > > +}
> > > > +
> > > > static struct reconnect_data *reconnect_add(struct btd_service *service)
> > > > {
> > > > struct btd_device *dev = btd_service_get_device(service);
> > > > @@ -687,10 +700,15 @@ static void service_cb(struct btd_service *service,
> > > > * be the first profile that's connected so we might have an
> > > > * entry but with the reconnect flag set to false.
> > > > */
> > > > - if (!reconnect->reconnect)
> > > > - reconnect->reconnect = reconnect_match(profile->remote_uuid);
> > > > + if (!reconnect->on_timeout)
> > > > + reconnect->on_timeout = reconnect_match(profile->remote_uuid);
> > > > +
> > > > + if (!reconnect->on_resume)
> > > > + reconnect->on_resume =
> > > > + reconnect_resume_match(profile->remote_uuid);
> > > >
> > > > - DBG("Added %s reconnect %u", profile->name, reconnect->reconnect);
> > > > + DBG("Added %s reconnect on timeout(%u), resume(%u)",
> > > > + profile->name, reconnect->on_timeout, reconnect->on_resume);
> > > > }
> > > >
> > > > static gboolean reconnect_timeout(gpointer data)
> > > > @@ -704,7 +722,7 @@ static gboolean reconnect_timeout(gpointer data)
> > > > reconnect->timer = 0;
> > > >
> > > > /* Mark any reconnect on resume as handled */
> > > > - reconnect->on_resume = false;
> > > > + reconnect->mark_on_suspend = false;
> > > >
> > > > err = btd_device_connect_services(reconnect->dev, reconnect->services);
> > > > if (err < 0) {
> > > > @@ -750,7 +768,7 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > > > return;
> > > >
> > > > reconnect = reconnect_find(dev);
> > > > - if (!reconnect || !reconnect->reconnect)
> > > > + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> > > > return;
> > > >
> > > > reconnect_reset(reconnect);
> > > > @@ -759,11 +777,11 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > > >
> > > > switch (reason) {
> > > > case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
> > > > - if (btd_device_get_service(dev, A2DP_SINK_UUID)) {
> > > > + if (reconnect->on_resume) {
> >
> > Note: Previously, only A2DP-SINK would reconnect if disconnected during suspend.
> >
> > > > DBG("%s configured to reconnect on resume",
> > > > device_get_path(dev));
> > > >
> > > > - reconnect->on_resume = true;
> > > > + reconnect->mark_on_suspend = true;
> > > >
> > > > /* If the kernel supports resume events, it is
> > > > * preferable to set the reconnect timer there as it is
> > > > @@ -774,7 +792,8 @@ static void disconnect_cb(struct btd_device *dev, uint8_t reason)
> > > > }
> > > > break;
> > > > case MGMT_DEV_DISCONN_TIMEOUT:
> > > > - reconnect_set_timer(reconnect, -1);
> > > > + if (reconnect->on_timeout)
> > > > + reconnect_set_timer(reconnect, -1);
> > > > break;
> > > > default:
> > > > DBG("Developer error. Reason = %d", reason);
> > > > @@ -790,7 +809,7 @@ static void policy_adapter_resume(struct btd_adapter *adapter)
> > > > for (l = reconnects; l; l = g_slist_next(l)) {
> > > > struct reconnect_data *reconnect = l->data;
> > > >
> > > > - if (reconnect->on_resume &&
> > > > + if (reconnect->mark_on_suspend &&
> > > > device_get_adapter(reconnect->dev) == adapter) {
> > > > reconnect_set_timer(reconnect, resume_delay);
> > > > }
> > > > @@ -804,7 +823,7 @@ static void conn_fail_cb(struct btd_device *dev, uint8_t status)
> > > > DBG("status %u", status);
> > > >
> > > > reconnect = reconnect_find(dev);
> > > > - if (!reconnect || !reconnect->reconnect)
> > > > + if (!reconnect || !(reconnect->on_timeout || reconnect->on_resume))
> > > > return;
> > > >
> > > > if (!reconnect->active)
> > > > @@ -845,6 +864,7 @@ static int policy_init(void)
> > > > {
> > > > GError *gerr = NULL;
> > > > GKeyFile *conf;
> > > > + bool has_reconnect_uuids;
> > > >
> > > > service_id = btd_service_add_state_cb(service_cb, NULL);
> > > >
> > > > @@ -892,6 +912,15 @@ static int policy_init(void)
> > > > auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
> > > > NULL);
> > > >
> > > > + reconnect_resume_uuids = g_key_file_get_string_list(
> > > > + conf, "Policy", "ReconnectResumeUUIDs", NULL, &gerr);
> > > > +
> > > > + if (gerr) {
> > > > + g_clear_error(&gerr);
> > > > + reconnect_resume_uuids =
> > > > + g_strdupv((char **)default_reconnect_resume);
> > > > + }
> > > > +
> > > > resume_delay = g_key_file_get_integer(
> > > > conf, "Policy", "ResumeDelay", &gerr);
> > > >
> > > > @@ -900,7 +929,11 @@ static int policy_init(void)
> > > > resume_delay = default_resume_delay;
> > > > }
> > > > done:
> > > > - if (reconnect_uuids && reconnect_uuids[0] && reconnect_attempts) {
> > > > + has_reconnect_uuids =
> > > > + (reconnect_uuids && reconnect_uuids[0]) ||
> > > > + (reconnect_resume_uuids && reconnect_resume_uuids[0]);
> > > > +
> > > > + if (reconnect_attempts && has_reconnect_uuids) {
> > > > btd_add_disconnect_cb(disconnect_cb);
> > > > btd_add_conn_fail_cb(conn_fail_cb);
> > > > }
> > > > @@ -918,6 +951,9 @@ static void policy_exit(void)
> > > > if (reconnect_uuids)
> > > > g_strfreev(reconnect_uuids);
> > > >
> > > > + if (reconnect_resume_uuids)
> > > > + g_strfreev(reconnect_resume_uuids);
> > > > +
> > > > g_free(reconnect_intervals);
> > > >
> > > > g_slist_free_full(reconnects, reconnect_destroy);
> > > > diff --git a/src/main.conf b/src/main.conf
> > > > index ad36638b7..6f66acb14 100644
> > > > --- a/src/main.conf
> > > > +++ b/src/main.conf
> > > > @@ -220,7 +220,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,0000110b-0000-1000-8000-00805f9b34fb
> > > > +#ReconnectUUIDs=00001112-0000-1000-8000-00805f9b34fb,0000111f-0000-1000-8000-00805f9b34fb,0000110a-0000-1000-8000-00805f9b34fb
> > > >
> > > > # ReconnectAttempts define the number of attempts to reconnect after a link
> > > > # lost. Setting the value to 0 disables reconnecting feature.
> > > > @@ -237,6 +237,13 @@
> > > > # in later on. Defaults to 'false'.
> > > > #AutoEnable=false
> > > >
> > > > +# The ReconnectResumeUUIDs defines the set of remote services that should try
> > > > +# to be reconnected after resume from suspend if they were connected before
> > > > +# suspend. If a device is disconnected due to suspend, it will be queued for
> > > > +# reconnect on resume first using the ResumeDelay value and then the remaining
> > > > +# values in ReconnectIntervals.
> > > > +#ReconnectResumeUUIDs=0000110b-0000-1000-8000-00805f9b34fb
> > >
> > > I don't follow the reasoning here really, we need to be consistent
> > > with regards to policy so how about the other profiles listed to be
> > > reconnected does it means they no longer are connected on resume? From
> > > the looks of you it we cannot really tell if the device was
> > > disconnected due to link supervision timeout or not so we can't really
> > > assume the Connection timeout (0x01) is in fact a second device
> > > connecting, IMO if it is a mistake on the manufacturer then this
> > > should be made device specific even though I think you will have to
> > > disable the reconnection logic altogether if we cannot trust the
> > > reason for the disconnection or does it cleanup the A2DP stream before
> > > disconnecting, in that case we should have consider a clean
> > > disconnection.
> >
> > ReconnectUUIDs was overloaded to mean both "reconnect on link loss"
> > and "reconnect only a2dp-sink if disconnected due to suspend". This
> > patch removes that overloading and ReconnectUUIDs simply always means,
> > "Reconnect if link loss". ReconnectResumeUUIDs means simply, "If this
> > device was disconnected during suspend, attempt to reconnect it on
> > resume." It's adding a Reconnect policy for disconnect reason =
> > "Disconnect due to Suspend". I think this is more generic and enables
> > adding other uuids to this policy.
>
> To my understanding disconnected due to suspend was actually
> considered a link loss due to suspend, if we were to carried out
> different policies for these conditions the whole thing becomes a lot
> more complicated, and to make matters worse this so far has been a
> problem to a single manufacturer which in the future might actually
> change the disconnect reason. How about the other profiles, if it were
> connected to HFP shouldn't it be reconnected as well? Also would the
> problem of stealing the connection still manifest when the laptop
> resumes when one attempts to connect to the phone?
>
> > If we really want link loss reconnects for a2dp-sink, I agree that we
> > should be treating the Airpods differently because it is specific to
> > that device.
> >
> > This is what I'm seeing on the Airpods when I connect from an Android
> > phone (while already connected to Chromebook). There is no profile
> > disconnect (no l2cap disconnection requests either).
> >
> > > HCI Event: Number of Completed Packets (0x13) plen 5 #1674 [hci0] 38.810693
> > Num handles: 1
> > Handle: 256
> > Count: 1
> > < ACL Data TX: Handle 256 flags 0x02 dlen 969
> > #1675 [hci0]
> > 38.820328
> > Channel: 11011 len 965 [PSM 25 mode Basic (0x00)] {chan 2}
> > < ACL Data TX: Handle 256 flags 0x00 dlen 7
> > #1676 [hci0]
> > 38.821054
> > Channel: 10756 len 3 [PSM 25 mode Basic (0x00)] {chan 3}
> > AVDTP: Suspend (0x09) Command (0x00) type 0x00 label 4 nosp 0
> > ACP SEID: 1
> > > HCI Event: Number of Completed Packets (0x13) plen 5 #1677 [hci0] 38.835673
> > Num handles: 1
> > Handle: 256
> > Count: 1
> > > HCI Event: Number of Completed Packets (0x13) plen 5 #1678 [hci0] 38.836347
> > Num handles: 1
> > Handle: 256
> > Count: 1
> > > ACL Data RX: Handle 256 flags 0x02 dlen 6 #1679 [hci0] 38.861708
> > Channel: 67 len 2 [PSM 25 mode Basic (0x00)] {chan 3}
> > AVDTP: Suspend (0x09) Response Accept (0x02) type 0x00 label 4
> > nosp 0
> > > HCI Event: Disconnect Complete (0x05) plen 4 #1680 [hci0] 62.656436
> > Status: Success (0x00)
> > Handle: 256
> > Reason: Connection Timeout (0x08)
>
> This is a link loss in my books, so if any profiles on ReconnectUUIDs
> were connected this still would trigger a reconnection and we can't
> really remove everything from there, perhaps for Chrome OS only A2DP
> is connected but in other cases HFP might be connected as well. Imo a
> better way to handle this is with a quirk for broken reason, or
> perhaps expose an interface where the upper layer can disable policies
> on per-device, in which case the user can disable the UUIDs to be
> reconnected.
>
> >
> > >
> > > > # 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
> > > > --
> > > > 2.29.2.576.ga3fc446d84-goog
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz