CRAS: bt_device - modify logic in checking A2DP and HFP conn

When a device supports A2DP or HFP profile, but said profile is not
connected, CRAS would try to connect them.

if (supported(A2DP) && !connected(A2DP))
  send_conn(A2DP)
if (supported(HFP) && !connected(HFP))
  send_conn(HFP)

However this could potentially clash with connection request from
UI, causing UI to stuck in "connecting..." even though the device
is actually connected.

This patch makes CRAS less self-initiative by only connecting to
HFP if A2DP is already connected by bluez, and vice versa. The
new logic could be summarized in pseudocode below.

if (supported(A2DP) && supported(HFP)) {
  if (connected(A2DP) && !connected(HFP))
    send_conn(HFP);
  if (connected(HFP) && !connected(A2DP))
    send_conn(A2DP);
}

BUG=b:147628413
TEST=Using UI, pair to Bose QC35 or Caseflex BT Mini Speaker.
     Observe that the UI will not stuck in "Connecting..."

Change-Id: I54edabd727cba11fac2dffece5736a182e498a56
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/2064029
Tested-by: Archie Pusaka <apusaka@chromium.org>
Commit-Queue: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Archie Pusaka <apusaka@chromium.org>

2 files changed, 48 insertions, 42 deletions

diff --git a/cras/src/server/cras_bt_device.c b/cras/src/server/cras_bt_device.c
index 90bd8755..27897a00 100644
--- a/cras/src/server/cras_bt_device.c
+++ b/cras/src/server/cras_bt_device.c
@@ -52,7 +52,6 @@ static const unsigned int PROFILE_DROP_SUSPEND_DELAY_MS = 5000;
  */
 static const unsigned int CONN_WATCH_PERIOD_MS = 2000;
 static const unsigned int CONN_WATCH_MAX_RETRIES = 30;
-static const unsigned int PROFILE_CONN_RETRIES = 3;
 
 static const unsigned int CRAS_SUPPORTED_PROFILES =
 	CRAS_BT_DEVICE_PROFILE_A2DP_SINK |
@@ -544,6 +543,10 @@ static void bt_device_conn_watch_cb(struct cras_timer *timer, void *arg)
 	struct cras_tm *tm;
 	struct cras_bt_device *device = (struct cras_bt_device *)arg;
 	int rc;
+	bool a2dp_supported;
+	bool a2dp_connected;
+	bool hfp_supported;
+	bool hfp_connected;
 
 	BTLOG(btlog, BT_DEV_CONN_WATCH_CB, device->conn_watch_retries,
 	      device->profiles);
@@ -553,28 +556,35 @@ static void bt_device_conn_watch_cb(struct cras_timer *timer, void *arg)
 	if (!device->profiles)
 		return;
 
-	/* If A2DP is not ready, try connect it after a while. */
-	if (cras_bt_device_supports_profile(device,
-					    CRAS_BT_DEVICE_PROFILE_A2DP_SINK) &&
-	    !cras_bt_device_is_profile_connected(
-		    device, CRAS_BT_DEVICE_PROFILE_A2DP_SINK)) {
-		if (0 == device->conn_watch_retries % PROFILE_CONN_RETRIES)
+	a2dp_supported = cras_bt_device_supports_profile(
+		device, CRAS_BT_DEVICE_PROFILE_A2DP_SINK);
+	a2dp_connected = cras_bt_device_is_profile_connected(
+		device, CRAS_BT_DEVICE_PROFILE_A2DP_SINK);
+	hfp_supported = cras_bt_device_supports_profile(
+		device, CRAS_BT_DEVICE_PROFILE_HFP_HANDSFREE);
+	hfp_connected = cras_bt_device_is_profile_connected(
+		device, CRAS_BT_DEVICE_PROFILE_HFP_HANDSFREE);
+
+	/* If not both A2DP and HFP are supported, simply wait for BlueZ
+	 * to notify us about the new connection.
+	 * Otherwise, when seeing one but not the other profile is connected,
+	 * send message to ask BlueZ to connect the pending one.
+	 */
+	if (a2dp_supported && hfp_supported) {
+		/* If both a2dp and hfp are not connected, do nothing. BlueZ
+		 * should be responsible to notify connection of one profile.
+		 */
+		if (!a2dp_connected && hfp_connected)
 			cras_bt_device_connect_profile(device->conn, device,
 						       A2DP_SINK_UUID);
-		goto arm_retry_timer;
-	}
-
-	/* If HFP is not ready, try connect it after a while. */
-	if (cras_bt_device_supports_profile(
-		    device, CRAS_BT_DEVICE_PROFILE_HFP_HANDSFREE) &&
-	    !cras_bt_device_is_profile_connected(
-		    device, CRAS_BT_DEVICE_PROFILE_HFP_HANDSFREE)) {
-		if (0 == device->conn_watch_retries % PROFILE_CONN_RETRIES)
+		if (a2dp_connected && !hfp_connected)
 			cras_bt_device_connect_profile(device->conn, device,
 						       HFP_HF_UUID);
-		goto arm_retry_timer;
 	}
 
+	if (a2dp_supported != a2dp_connected || hfp_supported != hfp_connected)
+		goto arm_retry_timer;
+
 	/* Expected profiles are all connected, no more connection watch
 	 * callback will be scheduled.
 	 * Base on the decision that we expose only the latest connected
diff --git a/cras/src/tests/bt_device_unittest.cc b/cras/src/tests/bt_device_unittest.cc
index c6e6dcad..ce2526ca 100644
--- a/cras/src/tests/bt_device_unittest.cc
+++ b/cras/src/tests/bt_device_unittest.cc
@@ -15,6 +15,8 @@ extern "C" {
 #define FAKE_OBJ_PATH "/obj/path"
 }
 
+static const unsigned int CONN_WATCH_MAX_RETRIES = 30;
+
 static struct cras_iodev* cras_bt_io_create_profile_ret;
 static struct cras_iodev* cras_bt_io_append_btio_val;
 static struct cras_ionode* cras_bt_io_get_profile_ret;
@@ -229,8 +231,9 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpOnly) {
   /* Schedule another timer, if A2DP not yet configured. */
   cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
   EXPECT_EQ(2, cras_tm_create_timer_called);
-  EXPECT_EQ(1, dbus_message_new_method_call_called);
-  EXPECT_STREQ("ConnectProfile", dbus_message_new_method_call_method);
+
+  /* ConnectProfile must not be called, since this is A2DP only case. */
+  EXPECT_EQ(0, dbus_message_new_method_call_called);
 
   cras_bt_device_a2dp_configured(device);
   cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
@@ -262,8 +265,9 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedHfpHspOnly) {
   /* Schedule another timer, if HFP AG not yet intialized. */
   cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
   EXPECT_EQ(2, cras_tm_create_timer_called);
-  EXPECT_EQ(1, dbus_message_new_method_call_called);
-  EXPECT_STREQ("ConnectProfile", dbus_message_new_method_call_method);
+
+  /* ConnectProfile must not be called, since this is HFP only case. */
+  EXPECT_EQ(0, dbus_message_new_method_call_called);
 
   cras_bt_device_audio_gateway_initialized(device);
 
@@ -298,12 +302,19 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpHfpHsp) {
   cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
   EXPECT_EQ(2, cras_tm_create_timer_called);
 
+  /* ConnectProfile must not be called, since the first profile connection
+   * should be initiated by Bluez.
+   */
+  EXPECT_EQ(0, dbus_message_new_method_call_called);
+
   cras_bt_device_audio_gateway_initialized(device);
 
   /* Schedule another timer, because A2DP is not ready. */
   cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
   EXPECT_EQ(3, cras_tm_create_timer_called);
   EXPECT_EQ(0, cras_hfp_ag_start_called);
+
+  /* ConnectProfile should be called to connect A2DP, since HFP is connected */
   EXPECT_EQ(1, dbus_message_new_method_call_called);
   EXPECT_STREQ("ConnectProfile", dbus_message_new_method_call_method);
 
@@ -372,29 +383,20 @@ TEST_F(BtDeviceTestSuite, A2dpDropped) {
 
   cur = msg_root = NewMockDBusConnectedMessage(1);
   cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
+  cras_bt_device_audio_gateway_initialized(device);
+  cras_bt_device_a2dp_configured(device);
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
-  /* Schedule another timer, if HFP AG not yet intialized. */
-  cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
-  EXPECT_EQ(2, cras_tm_create_timer_called);
-  EXPECT_EQ(1, dbus_message_new_method_call_called);
-  EXPECT_STREQ("ConnectProfile", dbus_message_new_method_call_method);
-
-  cras_bt_device_a2dp_configured(device);
-
-  cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
-  EXPECT_EQ(3, cras_tm_create_timer_called);
-
   cras_bt_device_notify_profile_dropped(device,
                                         CRAS_BT_DEVICE_PROFILE_A2DP_SINK);
-  EXPECT_EQ(4, cras_tm_create_timer_called);
+  EXPECT_EQ(2, cras_tm_create_timer_called);
 
   /* Expect suspend timer is scheduled. */
   cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
   EXPECT_EQ(1, cras_a2dp_suspend_connected_device_called);
   EXPECT_EQ(1, cras_hfp_ag_suspend_connected_device_called);
-  EXPECT_EQ(2, dbus_message_new_method_call_called);
+  EXPECT_EQ(1, dbus_message_new_method_call_called);
   EXPECT_STREQ("Disconnect", dbus_message_new_method_call_method);
 
   cras_bt_device_remove(device);
@@ -486,17 +488,11 @@ TEST_F(BtDeviceTestSuite, ConnectionWatchTimeout) {
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
-  /* Schedule another timer, if HFP AG not yet intialized. */
-  cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
-  EXPECT_EQ(2, cras_tm_create_timer_called);
-  EXPECT_EQ(1, dbus_message_new_method_call_called);
-  EXPECT_STREQ("ConnectProfile", dbus_message_new_method_call_method);
-
   cras_bt_device_a2dp_configured(device);
 
-  for (int i = 0; i < 29; i++) {
+  for (int i = 0; i < CONN_WATCH_MAX_RETRIES; i++) {
     cras_tm_create_timer_cb(NULL, cras_tm_create_timer_cb_data);
-    EXPECT_EQ(i + 3, cras_tm_create_timer_called);
+    EXPECT_EQ(i + 2, cras_tm_create_timer_called);
     EXPECT_EQ(0, cras_a2dp_start_called);
     EXPECT_EQ(0, cras_hfp_ag_start_called);
     EXPECT_EQ(0, cras_hfp_ag_remove_conflict_called);