Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <5f20d725.1c69fb81.6d882.1d23@mx.google.com>
From:   Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
To:     luiz.dentz@gmail.com, marcel@holtmann.org
Cc:     chromeos-bluetooth-upstreaming@chromium.org,
        linux-bluetooth@vger.kernel.org,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>,
        Sonny Sasaka <sonnysasaka@chromium.org>
Subject: [RFC Bluez PATCH 3/3] adapter: Reconnect audio on controller resume
Date:   Tue, 28 Jul 2020 18:55:40 -0700
In-Reply-To: <20200729015540.1848987-1-abhishekpandit@chromium.org>
References: <20200729015540.1848987-1-abhishekpandit@chromium.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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 <sonnysasaka@chromium.org>
---
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