2020-07-29 01:56:23

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC Bluez PATCH 0/3] adapter: Reconnect audio when resuming from suspend


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 Disconnected management event, if the disconnect reason
was 0x5 (Disconnected by Local Host for Suspend) and the device is an
audio sink (implements major services Audio + Rendering), then it is
queued for reconnect.
* When the Controller Resumed management event is seen, we check if
an audio device needs to be reconnected. If yes, we queue a delayed
callback to do the reconnection. The delay is 5s by default and is
meant to allow sufficient time for any Wi-Fi activity that may occur
during resume (since Bluetooth connect may adversely affect that).

A reconnect is only attempted once after the controller resumes and the
delay between resume and reconnect is configurable via the
ReconnectAudioDelay key in the General settings. The 5s 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 < 5s 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 after 5s+ wake time.
- Suspend test with wake time between 3s - 6s. Ran with 5 iterations and
verified it wasn't connected at the end. A connection attempt was
made but not completed due to suspend. A reconnect attempt was not
made afterwards, which is by design.

Luiz@ Marcel@: Does this sound ok (give up after an attempt)?

I've tested this on a Pixelbook Go (AC-9260 controller) and HP
Chromebook 14a (RTL8822CE controller) with GID6B headset. I'm hoping to
test this with a few more headsets to make sure this is ok and I'm
looking for some early feedback.

Thanks
Abhishek



Abhishek Pandit-Subedi (3):
mgmt: Add controller suspend and resume events
monitor: Add btmon support for Suspend and Resume events
adapter: Reconnect audio on controller resume

lib/mgmt.h | 14 +++++++++
monitor/packet.c | 55 ++++++++++++++++++++++++++++++++
src/adapter.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
src/device.c | 27 ++++++++++++++++
src/device.h | 2 ++
src/main.conf | 6 ++++
6 files changed, 186 insertions(+)

--
2.28.0.rc0.142.g3c755180ce-goog


2020-07-29 01:56:23

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC Bluez PATCH 2/3] monitor: Add btmon support for Suspend and Resume events

Add support to pretty print Suspend and Resume mgmt events in btmon.

Example:

@ MGMT Event: Controller Suspended (0x002d) plen 1
Suspend state: Page scanning and/or passive scanning (2)

@ MGMT Event: Controller Resumed (0x002e) plen 8
Wake reason: Remote wake due to peer device connection (2)
LE Address: CD:F3:CD:13:C5:9A (OUI CD-F3-CD)

Reviewed-by: Sonny Sasaka <[email protected]>
Reviewed-by: Miao-chen Chou <[email protected]>
---

monitor/packet.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index 431a39b66..451630e04 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -13555,6 +13555,9 @@ static void mgmt_device_disconnected_evt(const void *data, uint16_t size)
case 0x04:
str = "Connection terminated due to authentication failure";
break;
+ case 0x05:
+ str = "Connection terminated by local host for suspend";
+ break;
default:
str = "Reserved";
break;
@@ -13782,6 +13785,54 @@ static void mgmt_device_flags_changed_evt(const void *data, uint16_t size)
mgmt_print_added_device_flags("Current Flags", current_flags);
}

+static void mgmt_controller_suspend_evt(const void *data, uint16_t size)
+{
+ uint8_t state = get_u8(data);
+ char *str;
+
+ switch (state) {
+ case 0x0:
+ str = "Controller running (failed to suspend)";
+ break;
+ case 0x1:
+ str = "Disconnected and not scanning";
+ break;
+ case 0x2:
+ str = "Page scanning and/or passive scanning";
+ break;
+ default:
+ str = "Unknown suspend state";
+ break;
+ }
+
+ print_field("Suspend state: %s (%d)", str, state);
+}
+
+static void mgmt_controller_resume_evt(const void *data, uint16_t size)
+{
+ uint8_t addr_type = get_u8(data + 6);
+ uint8_t wake_reason = get_u8(data + 7);
+ char *str;
+
+ switch (wake_reason) {
+ case 0x0:
+ str = "Resume from non-Bluetooth wake source";
+ break;
+ case 0x1:
+ str = "Wake due to unexpected event";
+ break;
+ case 0x2:
+ str = "Remote wake due to peer device connection";
+ break;
+ default:
+ str = "Unknown wake reason";
+ break;
+ }
+
+ print_field("Wake reason: %s (%d)", str, wake_reason);
+ mgmt_print_address(data, addr_type);
+}
+
static const struct mgmt_data mgmt_event_table[] = {
{ 0x0001, "Command Complete",
mgmt_command_complete_evt, 3, false },
@@ -13863,6 +13914,10 @@ static const struct mgmt_data mgmt_event_table[] = {
mgmt_exp_feature_changed_evt, 20, true },
{ 0x002a, "Device Flags Changed",
mgmt_device_flags_changed_evt, 15, true },
+ { 0x002d, "Controller Suspended",
+ mgmt_controller_suspend_evt, 1, true },
+ { 0x002e, "Controller Resumed",
+ mgmt_controller_resume_evt, 8, true },
{ }
};

--
2.28.0.rc0.142.g3c755180ce-goog

2020-07-29 01:56:23

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC Bluez PATCH 3/3] adapter: Reconnect audio on controller resume

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, keep track of the last audio device disconnected
during suspend and try to reconnect when the controller resumes from
suspend.

Reviewed-by: Sonny Sasaka <[email protected]>
---
Hey Luiz,

On our internal review, two things stood out in this commit that we
weren't able to come to a consensus on internally:

* Is it better to use the audio device class or should we compare to the
A2DP, HFP and HSP uuids?
* Constructing the dbus message internally before calling dev_connect
looks a bit weird. I couldn't figure out how to internally trigger
this function (since it seems to require a msg to respond to on
success/failure). Any thoughts?


src/adapter.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/device.c | 27 +++++++++++++++++
src/device.h | 2 ++
src/main.conf | 6 ++++
4 files changed, 117 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 5e896a9f0..b1073c439 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -90,6 +90,7 @@
#define IDLE_DISCOV_TIMEOUT (5)
#define TEMP_DEV_TIMEOUT (3 * 60)
#define BONDING_TIMEOUT (2 * 60)
+#define RECONNECT_AUDIO_DELAY (5)

#define SCAN_TYPE_BREDR (1 << BDADDR_BREDR)
#define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
@@ -269,6 +270,15 @@ struct btd_adapter {
struct btd_device *connect_le; /* LE device waiting to be connected */
sdp_list_t *services; /* Services associated to adapter */

+ /* audio device to reconnect after resuming from suspend */
+ struct reconnect_audio_info {
+ bdaddr_t addr;
+ uint8_t addr_type;
+ bool reconnect;
+ } reconnect_audio;
+ guint reconnect_audio_timeout; /* timeout for reconnect on resume */
+ uint32_t reconnect_audio_delay; /* delay reconnect after resume */
+
struct btd_gatt_database *database;
struct btd_adv_manager *adv_manager;

@@ -6256,6 +6266,7 @@ static void load_config(struct btd_adapter *adapter)
/* Get discoverable mode */
adapter->stored_discoverable = g_key_file_get_boolean(key_file,
"General", "Discoverable", &gerr);
+
if (gerr) {
adapter->stored_discoverable = false;
g_error_free(gerr);
@@ -6271,6 +6282,16 @@ static void load_config(struct btd_adapter *adapter)
gerr = NULL;
}

+ /* Get audio reconnect delay */
+ adapter->reconnect_audio_delay = g_key_file_get_integer(
+ key_file, "General", "ReconnectAudioDelay", &gerr);
+
+ if (gerr) {
+ adapter->reconnect_audio_delay = RECONNECT_AUDIO_DELAY;
+ g_error_free(gerr);
+ gerr = NULL;
+ }
+
g_key_file_free(key_file);
}

@@ -7820,6 +7841,15 @@ static void dev_disconnected(struct btd_adapter *adapter,
if (device) {
adapter_remove_connection(adapter, device, addr->type);
disconnect_notify(device, reason);
+
+ if (reason == MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND &&
+ device_class_is_audio(device)) {
+ adapter->reconnect_audio.reconnect = true;
+ adapter->reconnect_audio.addr_type =
+ btd_device_get_bdaddr_type(device);
+ bacpy(&adapter->reconnect_audio.addr,
+ device_get_address(device));
+ }
}

bonding_attempt_complete(adapter, &addr->bdaddr, addr->type,
@@ -8766,6 +8796,53 @@ static void connected_callback(uint16_t index, uint16_t length,
eir_data_free(&eir_data);
}

+static gboolean reconnect_audio_timeout(gpointer user_data)
+{
+ struct btd_adapter *adapter = user_data;
+
+ adapter->reconnect_audio_timeout = 0;
+
+ if (adapter->reconnect_audio.reconnect) {
+ struct btd_device *dev = btd_adapter_find_device(
+ adapter, &adapter->reconnect_audio.addr,
+ adapter->reconnect_audio.addr_type);
+
+ adapter->reconnect_audio.reconnect = false;
+
+ if (!dev || btd_device_is_connected(dev))
+ return FALSE;
+
+ device_internal_connect(dev);
+ }
+
+ return FALSE;
+}
+
+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;
+ }
+
+ DBG("Controller resume with wake event 0x%x", ev->wake_reason);
+
+ if (adapter->reconnect_audio_timeout > 0) {
+ g_source_remove(adapter->reconnect_audio_timeout);
+ adapter->reconnect_audio_timeout = 0;
+ }
+
+ if (adapter->reconnect_audio.reconnect) {
+ adapter->reconnect_audio_timeout =
+ g_timeout_add_seconds(adapter->reconnect_audio_delay,
+ reconnect_audio_timeout, adapter);
+ }
+}
+
static void device_blocked_callback(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
@@ -9389,6 +9466,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));
diff --git a/src/device.c b/src/device.c
index bb8e07e8f..8b165ffa4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -747,6 +747,12 @@ gboolean device_is_trusted(struct btd_device *device)
return device->trusted;
}

+bool device_class_is_audio(struct btd_device *device)
+{
+ /* Look for major service classes Audio (0x20) + Rendering (0x4) */
+ return ((device->class >> 16) & 0x24) == 0x24;
+}
+
static gboolean dev_property_get_address(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
@@ -6853,6 +6859,27 @@ struct btd_service *btd_device_get_service(struct btd_device *dev,
return NULL;
}

+/* Internal function to connect to a device. This fakes the dbus message used to
+ * call the "Connect" api on the device so that the same function can be called
+ * by bluez internally.
+ */
+bool device_internal_connect(struct btd_device *dev)
+{
+ DBusMessage *msg;
+
+ if (!device_is_connectable(dev))
+ return false;
+
+ msg = dbus_message_new_method_call("org.bluez",
+ device_get_path(dev),
+ DEVICE_INTERFACE,
+ "Connect");
+ /* Sending the message usually sets serial. Fake it here. */
+ dbus_message_set_serial(msg, 1);
+
+ dev_connect(dbus_conn, msg, dev);
+}
+
void btd_device_init(void)
{
dbus_conn = btd_get_dbus_connection();
diff --git a/src/device.h b/src/device.h
index 956fec1ae..82f97b5bd 100644
--- a/src/device.h
+++ b/src/device.h
@@ -98,6 +98,7 @@ bool device_is_connectable(struct btd_device *device);
bool device_is_paired(struct btd_device *device, uint8_t bdaddr_type);
bool device_is_bonded(struct btd_device *device, uint8_t bdaddr_type);
gboolean device_is_trusted(struct btd_device *device);
+bool device_class_is_audio(struct btd_device *device);
void device_set_paired(struct btd_device *dev, uint8_t bdaddr_type);
void device_set_unpaired(struct btd_device *dev, uint8_t bdaddr_type);
void btd_device_set_temporary(struct btd_device *device, bool temporary);
@@ -186,6 +187,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
uint32_t btd_device_get_current_flags(struct btd_device *dev);
void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags,
uint32_t current_flags);
+bool device_internal_connect(struct btd_device *dev);

void btd_device_init(void);
void btd_device_cleanup(void);
diff --git a/src/main.conf b/src/main.conf
index f41203b96..c6bb78a84 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -82,6 +82,12 @@
# 0 = disable timer, i.e. never keep temporary devices
#TemporaryTimeout = 30

+# How long to wait after controller resume before reconnecting to last used
+# audio device.
+# The value is in seconds.
+# Default: 5
+#ReconnectAudioDelay = 5
+
[Controller]
# The following values are used to load default adapter parameters. BlueZ loads
# the values into the kernel before the adapter is powered if the kernel
--
2.28.0.rc0.142.g3c755180ce-goog

2020-08-04 17:13:49

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: Re: [RFC Bluez PATCH 0/3] adapter: Reconnect audio when resuming from suspend

Hi,

Gentle reminder that this is waiting for feedback.

Thanks
Abhishek

On Tue, Jul 28, 2020 at 6:55 PM Abhishek Pandit-Subedi
<[email protected]> 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 Disconnected management event, if the disconnect reason
> was 0x5 (Disconnected by Local Host for Suspend) and the device is an
> audio sink (implements major services Audio + Rendering), then it is
> queued for reconnect.
> * When the Controller Resumed management event is seen, we check if
> an audio device needs to be reconnected. If yes, we queue a delayed
> callback to do the reconnection. The delay is 5s by default and is
> meant to allow sufficient time for any Wi-Fi activity that may occur
> during resume (since Bluetooth connect may adversely affect that).
>
> A reconnect is only attempted once after the controller resumes and the
> delay between resume and reconnect is configurable via the
> ReconnectAudioDelay key in the General settings. The 5s 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 < 5s 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 after 5s+ wake time.
> - Suspend test with wake time between 3s - 6s. Ran with 5 iterations and
> verified it wasn't connected at the end. A connection attempt was
> made but not completed due to suspend. A reconnect attempt was not
> made afterwards, which is by design.
>
> Luiz@ Marcel@: Does this sound ok (give up after an attempt)?
>
> I've tested this on a Pixelbook Go (AC-9260 controller) and HP
> Chromebook 14a (RTL8822CE controller) with GID6B headset. I'm hoping to
> test this with a few more headsets to make sure this is ok and I'm
> looking for some early feedback.
>
> Thanks
> Abhishek
>
>
>
> Abhishek Pandit-Subedi (3):
> mgmt: Add controller suspend and resume events
> monitor: Add btmon support for Suspend and Resume events
> adapter: Reconnect audio on controller resume
>
> lib/mgmt.h | 14 +++++++++
> monitor/packet.c | 55 ++++++++++++++++++++++++++++++++
> src/adapter.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/device.c | 27 ++++++++++++++++
> src/device.h | 2 ++
> src/main.conf | 6 ++++
> 6 files changed, 186 insertions(+)
>
> --
> 2.28.0.rc0.142.g3c755180ce-goog
>

2020-08-04 18:56:08

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [RFC Bluez PATCH 3/3] adapter: Reconnect audio on controller resume

Hi Abhishek,

On Tue, Jul 28, 2020 at 6:55 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> 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, keep track of the last audio device disconnected
> during suspend and try to reconnect when the controller resumes from
> suspend.
>
> Reviewed-by: Sonny Sasaka <[email protected]>
> ---
> Hey Luiz,
>
> On our internal review, two things stood out in this commit that we
> weren't able to come to a consensus on internally:
>
> * Is it better to use the audio device class or should we compare to the
> A2DP, HFP and HSP uuids?
> * Constructing the dbus message internally before calling dev_connect
> looks a bit weird. I couldn't figure out how to internally trigger
> this function (since it seems to require a msg to respond to on
> success/failure). Any thoughts?
>
>
> src/adapter.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
> src/device.c | 27 +++++++++++++++++
> src/device.h | 2 ++
> src/main.conf | 6 ++++
> 4 files changed, 117 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 5e896a9f0..b1073c439 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -90,6 +90,7 @@
> #define IDLE_DISCOV_TIMEOUT (5)
> #define TEMP_DEV_TIMEOUT (3 * 60)
> #define BONDING_TIMEOUT (2 * 60)
> +#define RECONNECT_AUDIO_DELAY (5)
>
> #define SCAN_TYPE_BREDR (1 << BDADDR_BREDR)
> #define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
> @@ -269,6 +270,15 @@ struct btd_adapter {
> struct btd_device *connect_le; /* LE device waiting to be connected */
> sdp_list_t *services; /* Services associated to adapter */
>
> + /* audio device to reconnect after resuming from suspend */
> + struct reconnect_audio_info {
> + bdaddr_t addr;
> + uint8_t addr_type;
> + bool reconnect;
> + } reconnect_audio;
> + guint reconnect_audio_timeout; /* timeout for reconnect on resume */
> + uint32_t reconnect_audio_delay; /* delay reconnect after resume */
> +
> struct btd_gatt_database *database;
> struct btd_adv_manager *adv_manager;
>
> @@ -6256,6 +6266,7 @@ static void load_config(struct btd_adapter *adapter)
> /* Get discoverable mode */
> adapter->stored_discoverable = g_key_file_get_boolean(key_file,
> "General", "Discoverable", &gerr);
> +
> if (gerr) {
> adapter->stored_discoverable = false;
> g_error_free(gerr);
> @@ -6271,6 +6282,16 @@ static void load_config(struct btd_adapter *adapter)
> gerr = NULL;
> }
>
> + /* Get audio reconnect delay */
> + adapter->reconnect_audio_delay = g_key_file_get_integer(
> + key_file, "General", "ReconnectAudioDelay", &gerr);
> +
> + if (gerr) {
> + adapter->reconnect_audio_delay = RECONNECT_AUDIO_DELAY;
> + g_error_free(gerr);
> + gerr = NULL;
> + }
> +
> g_key_file_free(key_file);
> }
>
> @@ -7820,6 +7841,15 @@ static void dev_disconnected(struct btd_adapter *adapter,
> if (device) {
> adapter_remove_connection(adapter, device, addr->type);
> disconnect_notify(device, reason);
> +
> + if (reason == MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND &&
> + device_class_is_audio(device)) {
> + adapter->reconnect_audio.reconnect = true;
> + adapter->reconnect_audio.addr_type =
> + btd_device_get_bdaddr_type(device);
> + bacpy(&adapter->reconnect_audio.addr,
> + device_get_address(device));
> + }
> }
>
> bonding_attempt_complete(adapter, &addr->bdaddr, addr->type,
> @@ -8766,6 +8796,53 @@ static void connected_callback(uint16_t index, uint16_t length,
> eir_data_free(&eir_data);
> }
>
> +static gboolean reconnect_audio_timeout(gpointer user_data)
> +{
> + struct btd_adapter *adapter = user_data;
> +
> + adapter->reconnect_audio_timeout = 0;
> +
> + if (adapter->reconnect_audio.reconnect) {
> + struct btd_device *dev = btd_adapter_find_device(
> + adapter, &adapter->reconnect_audio.addr,
> + adapter->reconnect_audio.addr_type);
> +
> + adapter->reconnect_audio.reconnect = false;
> +
> + if (!dev || btd_device_is_connected(dev))
> + return FALSE;
> +
> + device_internal_connect(dev);
> + }
> +
> + return FALSE;
> +}
> +
> +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;
> + }
> +
> + DBG("Controller resume with wake event 0x%x", ev->wake_reason);
> +
> + if (adapter->reconnect_audio_timeout > 0) {
> + g_source_remove(adapter->reconnect_audio_timeout);
> + adapter->reconnect_audio_timeout = 0;
> + }
> +
> + if (adapter->reconnect_audio.reconnect) {
> + adapter->reconnect_audio_timeout =
> + g_timeout_add_seconds(adapter->reconnect_audio_delay,
> + reconnect_audio_timeout, adapter);
> + }
> +}
> +
> static void device_blocked_callback(uint16_t index, uint16_t length,
> const void *param, void *user_data)
> {
> @@ -9389,6 +9466,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));
> diff --git a/src/device.c b/src/device.c
> index bb8e07e8f..8b165ffa4 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -747,6 +747,12 @@ gboolean device_is_trusted(struct btd_device *device)
> return device->trusted;
> }
>
> +bool device_class_is_audio(struct btd_device *device)
> +{
> + /* Look for major service classes Audio (0x20) + Rendering (0x4) */
> + return ((device->class >> 16) & 0x24) == 0x24;
> +}
> +
> static gboolean dev_property_get_address(const GDBusPropertyTable *property,
> DBusMessageIter *iter, void *data)
> {
> @@ -6853,6 +6859,27 @@ struct btd_service *btd_device_get_service(struct btd_device *dev,
> return NULL;
> }
>
> +/* Internal function to connect to a device. This fakes the dbus message used to
> + * call the "Connect" api on the device so that the same function can be called
> + * by bluez internally.
> + */
> +bool device_internal_connect(struct btd_device *dev)
> +{
> + DBusMessage *msg;
> +
> + if (!device_is_connectable(dev))
> + return false;
> +
> + msg = dbus_message_new_method_call("org.bluez",
> + device_get_path(dev),
> + DEVICE_INTERFACE,
> + "Connect");
> + /* Sending the message usually sets serial. Fake it here. */
> + dbus_message_set_serial(msg, 1);
> +
> + dev_connect(dbus_conn, msg, dev);
> +}
> +
> void btd_device_init(void)
> {
> dbus_conn = btd_get_dbus_connection();
> diff --git a/src/device.h b/src/device.h
> index 956fec1ae..82f97b5bd 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -98,6 +98,7 @@ bool device_is_connectable(struct btd_device *device);
> bool device_is_paired(struct btd_device *device, uint8_t bdaddr_type);
> bool device_is_bonded(struct btd_device *device, uint8_t bdaddr_type);
> gboolean device_is_trusted(struct btd_device *device);
> +bool device_class_is_audio(struct btd_device *device);
> void device_set_paired(struct btd_device *dev, uint8_t bdaddr_type);
> void device_set_unpaired(struct btd_device *dev, uint8_t bdaddr_type);
> void btd_device_set_temporary(struct btd_device *device, bool temporary);
> @@ -186,6 +187,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
> uint32_t btd_device_get_current_flags(struct btd_device *dev);
> void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags,
> uint32_t current_flags);
> +bool device_internal_connect(struct btd_device *dev);
>
> void btd_device_init(void);
> void btd_device_cleanup(void);
> diff --git a/src/main.conf b/src/main.conf
> index f41203b96..c6bb78a84 100644
> --- a/src/main.conf
> +++ b/src/main.conf
> @@ -82,6 +82,12 @@
> # 0 = disable timer, i.e. never keep temporary devices
> #TemporaryTimeout = 30
>
> +# How long to wait after controller resume before reconnecting to last used
> +# audio device.
> +# The value is in seconds.
> +# Default: 5
> +#ReconnectAudioDelay = 5
> +
> [Controller]
> # The following values are used to load default adapter parameters. BlueZ loads
> # the values into the kernel before the adapter is powered if the kernel
> --
> 2.28.0.rc0.142.g3c755180ce-goog

Usually connection policy is handled by the policy plugin since there
may be platforms that want implement their own connection policies on
top of bluetoothd so they can just disable the policy plugin, iirc we
do have reconnection policies for unexpected disconnect which should
probably be used in the event a suspend actually trigger a
disconnection see:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/plugins/policy.c#n761

We might need a reason code to indicate to the policy when a
disconnect happens due to suspend logic.

--
Luiz Augusto von Dentz

2020-08-06 04:07:55

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: Re: [RFC Bluez PATCH 3/3] adapter: Reconnect audio on controller resume

Hi Luiz,

On Tue, Aug 4, 2020 at 11:54 AM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Abhishek,
>
> On Tue, Jul 28, 2020 at 6:55 PM Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >
> > 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, keep track of the last audio device disconnected
> > during suspend and try to reconnect when the controller resumes from
> > suspend.
> >
> > Reviewed-by: Sonny Sasaka <[email protected]>
> > ---
> > Hey Luiz,
> >
> > On our internal review, two things stood out in this commit that we
> > weren't able to come to a consensus on internally:
> >
> > * Is it better to use the audio device class or should we compare to the
> > A2DP, HFP and HSP uuids?
> > * Constructing the dbus message internally before calling dev_connect
> > looks a bit weird. I couldn't figure out how to internally trigger
> > this function (since it seems to require a msg to respond to on
> > success/failure). Any thoughts?
> >
> >
> > src/adapter.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > src/device.c | 27 +++++++++++++++++
> > src/device.h | 2 ++
> > src/main.conf | 6 ++++
> > 4 files changed, 117 insertions(+)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 5e896a9f0..b1073c439 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -90,6 +90,7 @@
> > #define IDLE_DISCOV_TIMEOUT (5)
> > #define TEMP_DEV_TIMEOUT (3 * 60)
> > #define BONDING_TIMEOUT (2 * 60)
> > +#define RECONNECT_AUDIO_DELAY (5)
> >
> > #define SCAN_TYPE_BREDR (1 << BDADDR_BREDR)
> > #define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
> > @@ -269,6 +270,15 @@ struct btd_adapter {
> > struct btd_device *connect_le; /* LE device waiting to be connected */
> > sdp_list_t *services; /* Services associated to adapter */
> >
> > + /* audio device to reconnect after resuming from suspend */
> > + struct reconnect_audio_info {
> > + bdaddr_t addr;
> > + uint8_t addr_type;
> > + bool reconnect;
> > + } reconnect_audio;
> > + guint reconnect_audio_timeout; /* timeout for reconnect on resume */
> > + uint32_t reconnect_audio_delay; /* delay reconnect after resume */
> > +
> > struct btd_gatt_database *database;
> > struct btd_adv_manager *adv_manager;
> >
> > @@ -6256,6 +6266,7 @@ static void load_config(struct btd_adapter *adapter)
> > /* Get discoverable mode */
> > adapter->stored_discoverable = g_key_file_get_boolean(key_file,
> > "General", "Discoverable", &gerr);
> > +
> > if (gerr) {
> > adapter->stored_discoverable = false;
> > g_error_free(gerr);
> > @@ -6271,6 +6282,16 @@ static void load_config(struct btd_adapter *adapter)
> > gerr = NULL;
> > }
> >
> > + /* Get audio reconnect delay */
> > + adapter->reconnect_audio_delay = g_key_file_get_integer(
> > + key_file, "General", "ReconnectAudioDelay", &gerr);
> > +
> > + if (gerr) {
> > + adapter->reconnect_audio_delay = RECONNECT_AUDIO_DELAY;
> > + g_error_free(gerr);
> > + gerr = NULL;
> > + }
> > +
> > g_key_file_free(key_file);
> > }
> >
> > @@ -7820,6 +7841,15 @@ static void dev_disconnected(struct btd_adapter *adapter,
> > if (device) {
> > adapter_remove_connection(adapter, device, addr->type);
> > disconnect_notify(device, reason);
> > +
> > + if (reason == MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND &&
> > + device_class_is_audio(device)) {
> > + adapter->reconnect_audio.reconnect = true;
> > + adapter->reconnect_audio.addr_type =
> > + btd_device_get_bdaddr_type(device);
> > + bacpy(&adapter->reconnect_audio.addr,
> > + device_get_address(device));
> > + }
> > }
> >
> > bonding_attempt_complete(adapter, &addr->bdaddr, addr->type,
> > @@ -8766,6 +8796,53 @@ static void connected_callback(uint16_t index, uint16_t length,
> > eir_data_free(&eir_data);
> > }
> >
> > +static gboolean reconnect_audio_timeout(gpointer user_data)
> > +{
> > + struct btd_adapter *adapter = user_data;
> > +
> > + adapter->reconnect_audio_timeout = 0;
> > +
> > + if (adapter->reconnect_audio.reconnect) {
> > + struct btd_device *dev = btd_adapter_find_device(
> > + adapter, &adapter->reconnect_audio.addr,
> > + adapter->reconnect_audio.addr_type);
> > +
> > + adapter->reconnect_audio.reconnect = false;
> > +
> > + if (!dev || btd_device_is_connected(dev))
> > + return FALSE;
> > +
> > + device_internal_connect(dev);
> > + }
> > +
> > + return FALSE;
> > +}
> > +
> > +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;
> > + }
> > +
> > + DBG("Controller resume with wake event 0x%x", ev->wake_reason);
> > +
> > + if (adapter->reconnect_audio_timeout > 0) {
> > + g_source_remove(adapter->reconnect_audio_timeout);
> > + adapter->reconnect_audio_timeout = 0;
> > + }
> > +
> > + if (adapter->reconnect_audio.reconnect) {
> > + adapter->reconnect_audio_timeout =
> > + g_timeout_add_seconds(adapter->reconnect_audio_delay,
> > + reconnect_audio_timeout, adapter);
> > + }
> > +}
> > +
> > static void device_blocked_callback(uint16_t index, uint16_t length,
> > const void *param, void *user_data)
> > {
> > @@ -9389,6 +9466,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));
> > diff --git a/src/device.c b/src/device.c
> > index bb8e07e8f..8b165ffa4 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -747,6 +747,12 @@ gboolean device_is_trusted(struct btd_device *device)
> > return device->trusted;
> > }
> >
> > +bool device_class_is_audio(struct btd_device *device)
> > +{
> > + /* Look for major service classes Audio (0x20) + Rendering (0x4) */
> > + return ((device->class >> 16) & 0x24) == 0x24;
> > +}
> > +
> > static gboolean dev_property_get_address(const GDBusPropertyTable *property,
> > DBusMessageIter *iter, void *data)
> > {
> > @@ -6853,6 +6859,27 @@ struct btd_service *btd_device_get_service(struct btd_device *dev,
> > return NULL;
> > }
> >
> > +/* Internal function to connect to a device. This fakes the dbus message used to
> > + * call the "Connect" api on the device so that the same function can be called
> > + * by bluez internally.
> > + */
> > +bool device_internal_connect(struct btd_device *dev)
> > +{
> > + DBusMessage *msg;
> > +
> > + if (!device_is_connectable(dev))
> > + return false;
> > +
> > + msg = dbus_message_new_method_call("org.bluez",
> > + device_get_path(dev),
> > + DEVICE_INTERFACE,
> > + "Connect");
> > + /* Sending the message usually sets serial. Fake it here. */
> > + dbus_message_set_serial(msg, 1);
> > +
> > + dev_connect(dbus_conn, msg, dev);
> > +}
> > +
> > void btd_device_init(void)
> > {
> > dbus_conn = btd_get_dbus_connection();
> > diff --git a/src/device.h b/src/device.h
> > index 956fec1ae..82f97b5bd 100644
> > --- a/src/device.h
> > +++ b/src/device.h
> > @@ -98,6 +98,7 @@ bool device_is_connectable(struct btd_device *device);
> > bool device_is_paired(struct btd_device *device, uint8_t bdaddr_type);
> > bool device_is_bonded(struct btd_device *device, uint8_t bdaddr_type);
> > gboolean device_is_trusted(struct btd_device *device);
> > +bool device_class_is_audio(struct btd_device *device);
> > void device_set_paired(struct btd_device *dev, uint8_t bdaddr_type);
> > void device_set_unpaired(struct btd_device *dev, uint8_t bdaddr_type);
> > void btd_device_set_temporary(struct btd_device *device, bool temporary);
> > @@ -186,6 +187,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
> > uint32_t btd_device_get_current_flags(struct btd_device *dev);
> > void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags,
> > uint32_t current_flags);
> > +bool device_internal_connect(struct btd_device *dev);
> >
> > void btd_device_init(void);
> > void btd_device_cleanup(void);
> > diff --git a/src/main.conf b/src/main.conf
> > index f41203b96..c6bb78a84 100644
> > --- a/src/main.conf
> > +++ b/src/main.conf
> > @@ -82,6 +82,12 @@
> > # 0 = disable timer, i.e. never keep temporary devices
> > #TemporaryTimeout = 30
> >
> > +# How long to wait after controller resume before reconnecting to last used
> > +# audio device.
> > +# The value is in seconds.
> > +# Default: 5
> > +#ReconnectAudioDelay = 5
> > +
> > [Controller]
> > # The following values are used to load default adapter parameters. BlueZ loads
> > # the values into the kernel before the adapter is powered if the kernel
> > --
> > 2.28.0.rc0.142.g3c755180ce-goog
>
> Usually connection policy is handled by the policy plugin since there
> may be platforms that want implement their own connection policies on
> top of bluetoothd so they can just disable the policy plugin, iirc we
> do have reconnection policies for unexpected disconnect which should
> probably be used in the event a suspend actually trigger a
> disconnection see:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/plugins/policy.c#n761
>
> We might need a reason code to indicate to the policy when a
> disconnect happens due to suspend logic.

I am emitting a new reason code for disconnect (local disconnect due
to suspend, 0x5). If this is just passing through the reason from the
mgmt event, I can continue using that.

Will update this patch to use the policy plugin and send up another revision.

Thanks
Abhishek

>
> --
> Luiz Augusto von Dentz

2020-08-06 17:22:24

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [RFC Bluez PATCH 3/3] adapter: Reconnect audio on controller resume

Hi Abhishek,

On Wed, Aug 5, 2020 at 8:48 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> Hi Luiz,
>
> On Tue, Aug 4, 2020 at 11:54 AM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Abhishek,
> >
> > On Tue, Jul 28, 2020 at 6:55 PM Abhishek Pandit-Subedi
> > <[email protected]> wrote:
> > >
> > > 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, keep track of the last audio device disconnected
> > > during suspend and try to reconnect when the controller resumes from
> > > suspend.
> > >
> > > Reviewed-by: Sonny Sasaka <[email protected]>
> > > ---
> > > Hey Luiz,
> > >
> > > On our internal review, two things stood out in this commit that we
> > > weren't able to come to a consensus on internally:
> > >
> > > * Is it better to use the audio device class or should we compare to the
> > > A2DP, HFP and HSP uuids?
> > > * Constructing the dbus message internally before calling dev_connect
> > > looks a bit weird. I couldn't figure out how to internally trigger
> > > this function (since it seems to require a msg to respond to on
> > > success/failure). Any thoughts?
> > >
> > >
> > > src/adapter.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > src/device.c | 27 +++++++++++++++++
> > > src/device.h | 2 ++
> > > src/main.conf | 6 ++++
> > > 4 files changed, 117 insertions(+)
> > >
> > > diff --git a/src/adapter.c b/src/adapter.c
> > > index 5e896a9f0..b1073c439 100644
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -90,6 +90,7 @@
> > > #define IDLE_DISCOV_TIMEOUT (5)
> > > #define TEMP_DEV_TIMEOUT (3 * 60)
> > > #define BONDING_TIMEOUT (2 * 60)
> > > +#define RECONNECT_AUDIO_DELAY (5)
> > >
> > > #define SCAN_TYPE_BREDR (1 << BDADDR_BREDR)
> > > #define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
> > > @@ -269,6 +270,15 @@ struct btd_adapter {
> > > struct btd_device *connect_le; /* LE device waiting to be connected */
> > > sdp_list_t *services; /* Services associated to adapter */
> > >
> > > + /* audio device to reconnect after resuming from suspend */
> > > + struct reconnect_audio_info {
> > > + bdaddr_t addr;
> > > + uint8_t addr_type;
> > > + bool reconnect;
> > > + } reconnect_audio;
> > > + guint reconnect_audio_timeout; /* timeout for reconnect on resume */
> > > + uint32_t reconnect_audio_delay; /* delay reconnect after resume */
> > > +
> > > struct btd_gatt_database *database;
> > > struct btd_adv_manager *adv_manager;
> > >
> > > @@ -6256,6 +6266,7 @@ static void load_config(struct btd_adapter *adapter)
> > > /* Get discoverable mode */
> > > adapter->stored_discoverable = g_key_file_get_boolean(key_file,
> > > "General", "Discoverable", &gerr);
> > > +
> > > if (gerr) {
> > > adapter->stored_discoverable = false;
> > > g_error_free(gerr);
> > > @@ -6271,6 +6282,16 @@ static void load_config(struct btd_adapter *adapter)
> > > gerr = NULL;
> > > }
> > >
> > > + /* Get audio reconnect delay */
> > > + adapter->reconnect_audio_delay = g_key_file_get_integer(
> > > + key_file, "General", "ReconnectAudioDelay", &gerr);
> > > +
> > > + if (gerr) {
> > > + adapter->reconnect_audio_delay = RECONNECT_AUDIO_DELAY;
> > > + g_error_free(gerr);
> > > + gerr = NULL;
> > > + }
> > > +
> > > g_key_file_free(key_file);
> > > }
> > >
> > > @@ -7820,6 +7841,15 @@ static void dev_disconnected(struct btd_adapter *adapter,
> > > if (device) {
> > > adapter_remove_connection(adapter, device, addr->type);
> > > disconnect_notify(device, reason);
> > > +
> > > + if (reason == MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND &&
> > > + device_class_is_audio(device)) {
> > > + adapter->reconnect_audio.reconnect = true;
> > > + adapter->reconnect_audio.addr_type =
> > > + btd_device_get_bdaddr_type(device);
> > > + bacpy(&adapter->reconnect_audio.addr,
> > > + device_get_address(device));
> > > + }
> > > }
> > >
> > > bonding_attempt_complete(adapter, &addr->bdaddr, addr->type,
> > > @@ -8766,6 +8796,53 @@ static void connected_callback(uint16_t index, uint16_t length,
> > > eir_data_free(&eir_data);
> > > }
> > >
> > > +static gboolean reconnect_audio_timeout(gpointer user_data)
> > > +{
> > > + struct btd_adapter *adapter = user_data;
> > > +
> > > + adapter->reconnect_audio_timeout = 0;
> > > +
> > > + if (adapter->reconnect_audio.reconnect) {
> > > + struct btd_device *dev = btd_adapter_find_device(
> > > + adapter, &adapter->reconnect_audio.addr,
> > > + adapter->reconnect_audio.addr_type);
> > > +
> > > + adapter->reconnect_audio.reconnect = false;
> > > +
> > > + if (!dev || btd_device_is_connected(dev))
> > > + return FALSE;
> > > +
> > > + device_internal_connect(dev);
> > > + }
> > > +
> > > + return FALSE;
> > > +}
> > > +
> > > +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;
> > > + }
> > > +
> > > + DBG("Controller resume with wake event 0x%x", ev->wake_reason);
> > > +
> > > + if (adapter->reconnect_audio_timeout > 0) {
> > > + g_source_remove(adapter->reconnect_audio_timeout);
> > > + adapter->reconnect_audio_timeout = 0;
> > > + }
> > > +
> > > + if (adapter->reconnect_audio.reconnect) {
> > > + adapter->reconnect_audio_timeout =
> > > + g_timeout_add_seconds(adapter->reconnect_audio_delay,
> > > + reconnect_audio_timeout, adapter);
> > > + }
> > > +}
> > > +
> > > static void device_blocked_callback(uint16_t index, uint16_t length,
> > > const void *param, void *user_data)
> > > {
> > > @@ -9389,6 +9466,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));
> > > diff --git a/src/device.c b/src/device.c
> > > index bb8e07e8f..8b165ffa4 100644
> > > --- a/src/device.c
> > > +++ b/src/device.c
> > > @@ -747,6 +747,12 @@ gboolean device_is_trusted(struct btd_device *device)
> > > return device->trusted;
> > > }
> > >
> > > +bool device_class_is_audio(struct btd_device *device)
> > > +{
> > > + /* Look for major service classes Audio (0x20) + Rendering (0x4) */
> > > + return ((device->class >> 16) & 0x24) == 0x24;
> > > +}
> > > +
> > > static gboolean dev_property_get_address(const GDBusPropertyTable *property,
> > > DBusMessageIter *iter, void *data)
> > > {
> > > @@ -6853,6 +6859,27 @@ struct btd_service *btd_device_get_service(struct btd_device *dev,
> > > return NULL;
> > > }
> > >
> > > +/* Internal function to connect to a device. This fakes the dbus message used to
> > > + * call the "Connect" api on the device so that the same function can be called
> > > + * by bluez internally.
> > > + */
> > > +bool device_internal_connect(struct btd_device *dev)
> > > +{
> > > + DBusMessage *msg;
> > > +
> > > + if (!device_is_connectable(dev))
> > > + return false;
> > > +
> > > + msg = dbus_message_new_method_call("org.bluez",
> > > + device_get_path(dev),
> > > + DEVICE_INTERFACE,
> > > + "Connect");
> > > + /* Sending the message usually sets serial. Fake it here. */
> > > + dbus_message_set_serial(msg, 1);
> > > +
> > > + dev_connect(dbus_conn, msg, dev);
> > > +}
> > > +
> > > void btd_device_init(void)
> > > {
> > > dbus_conn = btd_get_dbus_connection();
> > > diff --git a/src/device.h b/src/device.h
> > > index 956fec1ae..82f97b5bd 100644
> > > --- a/src/device.h
> > > +++ b/src/device.h
> > > @@ -98,6 +98,7 @@ bool device_is_connectable(struct btd_device *device);
> > > bool device_is_paired(struct btd_device *device, uint8_t bdaddr_type);
> > > bool device_is_bonded(struct btd_device *device, uint8_t bdaddr_type);
> > > gboolean device_is_trusted(struct btd_device *device);
> > > +bool device_class_is_audio(struct btd_device *device);
> > > void device_set_paired(struct btd_device *dev, uint8_t bdaddr_type);
> > > void device_set_unpaired(struct btd_device *dev, uint8_t bdaddr_type);
> > > void btd_device_set_temporary(struct btd_device *device, bool temporary);
> > > @@ -186,6 +187,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
> > > uint32_t btd_device_get_current_flags(struct btd_device *dev);
> > > void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags,
> > > uint32_t current_flags);
> > > +bool device_internal_connect(struct btd_device *dev);
> > >
> > > void btd_device_init(void);
> > > void btd_device_cleanup(void);
> > > diff --git a/src/main.conf b/src/main.conf
> > > index f41203b96..c6bb78a84 100644
> > > --- a/src/main.conf
> > > +++ b/src/main.conf
> > > @@ -82,6 +82,12 @@
> > > # 0 = disable timer, i.e. never keep temporary devices
> > > #TemporaryTimeout = 30
> > >
> > > +# How long to wait after controller resume before reconnecting to last used
> > > +# audio device.
> > > +# The value is in seconds.
> > > +# Default: 5
> > > +#ReconnectAudioDelay = 5
> > > +
> > > [Controller]
> > > # The following values are used to load default adapter parameters. BlueZ loads
> > > # the values into the kernel before the adapter is powered if the kernel
> > > --
> > > 2.28.0.rc0.142.g3c755180ce-goog
> >
> > Usually connection policy is handled by the policy plugin since there
> > may be platforms that want implement their own connection policies on
> > top of bluetoothd so they can just disable the policy plugin, iirc we
> > do have reconnection policies for unexpected disconnect which should
> > probably be used in the event a suspend actually trigger a
> > disconnection see:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/plugins/policy.c#n761
> >
> > We might need a reason code to indicate to the policy when a
> > disconnect happens due to suspend logic.
>
> I am emitting a new reason code for disconnect (local disconnect due
> to suspend, 0x5). If this is just passing through the reason from the
> mgmt event, I can continue using that.

Yep.

> Will update this patch to use the policy plugin and send up another revision.

I'd use a switch to catch all reasons that should trigger a reconnection.

> Thanks
> Abhishek
>
> >
> > --
> > Luiz Augusto von Dentz



--
Luiz Augusto von Dentz