CRAS: Remove codec negotiation triggered before slc init

Fully rely on the codec negotiation triggered when configure device.
This makes sure CRAS does start a codec negotiation procedure and won't
skip it because it has been done when the service connection ended.
Without this change, some PTS tests failed because they'll never get the
codec negotiation procedure they expected to start.

There is also a potential issue that this change might introduce when
the HF doesn't reply the codec negotiation request sent by AG.  This CL
covers the case if it failed at the first trial. However, if HF replied
correctly at first but fails after changing its codec capability, this
is not defined in the spec and our current implementation will then keep
retrying the codec negotiation.

This fixes the PTS tests:
HFP/AG/ACC/BV-13-I
HFP/AG/ACC/BV-14-I

BUG=chromium:980454, b:149188379, b:155282782, b:155279823
TEST=deploy and test QC35-ii and airpod2 can pair with NB and WB mode
Also run all related PTS tests

Change-Id: Ibec50256dae547e356261208a081ecd820bb46c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/2195867
Tested-by: En-Shuo Hsu <enshuo@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: En-Shuo Hsu <enshuo@chromium.org>

5 files changed, 50 insertions, 77 deletions

diff --git a/cras/src/server/cras_hfp_iodev.c b/cras/src/server/cras_hfp_iodev.c
index 1fc9a0ff..852a968c 100644
--- a/cras/src/server/cras_hfp_iodev.c
+++ b/cras/src/server/cras_hfp_iodev.c
@@ -329,10 +329,10 @@ struct cras_iodev *hfp_iodev_create(enum CRAS_STREAM_DIRECTION dir,
 	strcpy(node->name, iodev->info.name);
 
 	node->plugged = 1;
-	/* If headset mic uses legacy narrow band, i.e CVSD codec, report a
+	/* If headset mic doesn't support the wideband speech, report a
 	 * different node type so UI can set different plug priority. */
 	node->type = CRAS_NODE_TYPE_BLUETOOTH;
-	if ((hfp_slc_get_selected_codec(hfpio->slc) == HFP_CODEC_ID_CVSD) &&
+	if (!hfp_slc_get_wideband_speech_supported(hfpio->slc) &&
 	    (dir == CRAS_STREAM_INPUT))
 		node->type = CRAS_NODE_TYPE_BLUETOOTH_NB_MIC;
 
diff --git a/cras/src/server/cras_hfp_slc.c b/cras/src/server/cras_hfp_slc.c
index 5c16d5b9..1cfb0ffa 100644
--- a/cras/src/server/cras_hfp_slc.c
+++ b/cras/src/server/cras_hfp_slc.c
@@ -23,9 +23,6 @@
 
 /* The timeout between event reporting and HF indicator commands */
 #define HF_INDICATORS_TIMEOUT_MS 2000
-/* The timeout between service level initialized and codec negotiation
- * completed. */
-#define CODEC_NEGOTIATION_TIMEOUT_MS 10000
 /* The sleep time before reading and processing the following AT commands during
  * codec connection setup.
  */
@@ -260,25 +257,6 @@ static void initialize_slc_handle(struct cras_timer *timer, void *arg)
 	if (timer)
 		handle->timer = NULL;
 
-	/*
-	 * Catch the case if codec negotiation never complete or even
-	 * failed. AG side falls back to use codec CVSD and also tells
-	 * HF to select CVSD again.
-	 */
-	if (handle->selected_codec != handle->preferred_codec) {
-		if (handle->preferred_codec == HFP_CODEC_ID_MSBC) {
-			syslog(LOG_ERR,
-			       "Failed to enable mSBC, fallback to CVSD");
-			handle->preferred_codec = HFP_CODEC_ID_CVSD;
-		}
-		select_preferred_codec(handle);
-	}
-
-	/*
-	 * Codec negotiation is considered to be ended at this point.
-	 * The owner of init_cb may use hfp_slc_get_selected_codec() to
-	 * query the final codec to use for this connection.
-	 */
 	if (handle->init_cb) {
 		handle->init_cb(handle);
 		handle->init_cb = NULL;
@@ -315,38 +293,6 @@ static int bluetooth_codec_selection(struct hfp_slc_handle *handle,
 }
 
 /*
- * Delay the initialization of SLC if codecs negotiation is supported and not
- * down yet. Otherwise just initialize the SLC.
- */
-static void choose_codec_and_init_slc(struct cras_timer *timer, void *arg)
-{
-	struct hfp_slc_handle *handle = (struct hfp_slc_handle *)arg;
-	if (timer)
-		handle->timer = NULL;
-	/*
-	 * We should postpone the initialize call after codec selection,
-	 * otherwise iodev could be open immediately while the headset is still
-	 * communicating about which of CVSD or mSBC codec to use.
-	 * selected_codec != preferred_codec means that either the codec
-	 * negotiation is not done, selected_codec == HFP_CODEC_UNUSED, or
-	 */
-	if (hfp_slc_get_ag_codec_negotiation_supported(handle) &&
-	    hfp_slc_get_hf_codec_negotiation_supported(handle) &&
-	    handle->selected_codec != handle->preferred_codec) {
-		select_preferred_codec(handle);
-		/* Delay init to give headset some time to confirm
-		 * codec selection. */
-		handle->timer =
-			cras_tm_create_timer(cras_system_state_get_tm(),
-					     CODEC_NEGOTIATION_TIMEOUT_MS,
-					     initialize_slc_handle, handle);
-	} else {
-		handle->selected_codec = HFP_CODEC_ID_CVSD;
-		initialize_slc_handle(NULL, (void *)handle);
-	}
-}
-
-/*
  * AT+IPHONEACCEV command from HF to report state change.You can find details
  * of this command in the Accessory Design Guidelines for Apple Devices R11
  * section 16.1.
@@ -533,13 +479,13 @@ static int event_reporting(struct hfp_slc_handle *handle, const char *cmd)
 		handle->timer =
 			cras_tm_create_timer(cras_system_state_get_tm(),
 					     HF_INDICATORS_TIMEOUT_MS,
-					     choose_codec_and_init_slc, handle);
+					     initialize_slc_handle, handle);
 	/*
 	 * Otherwise, regard the Service Level Connection to be fully
 	 * initialized and ready for the potential codec negotiation.
 	 */
 	else
-		choose_codec_and_init_slc(NULL, (void *)handle);
+		initialize_slc_handle(NULL, (void *)handle);
 
 event_reporting_done:
 	free(tokens);
@@ -757,7 +703,7 @@ static int indicator_support(struct hfp_slc_handle *handle, const char *cmd)
 		 * Consider the Service Level Connection to be fully initialized
 		 * and thereby established, after successfully responded with OK
 		 */
-		choose_codec_and_init_slc(NULL, (void *)handle);
+		initialize_slc_handle(NULL, (void *)handle);
 		return 0;
 	} else {
 		goto error_out;
@@ -1289,6 +1235,13 @@ int hfp_slc_get_hf_hf_indicators_supported(struct hfp_slc_handle *handle)
 	return handle->hf_supported_features & HF_HF_INDICATORS;
 }
 
+bool hfp_slc_get_wideband_speech_supported(struct hfp_slc_handle *handle)
+{
+	return hfp_slc_get_ag_codec_negotiation_supported(handle) &&
+	       hfp_slc_get_hf_codec_negotiation_supported(handle) &&
+	       handle->hf_codec_supported[HFP_CODEC_ID_MSBC];
+}
+
 int hfp_slc_get_hf_supports_battery_indicator(struct hfp_slc_handle *handle)
 {
 	return handle->hf_supports_battery_indicator;
diff --git a/cras/src/server/cras_hfp_slc.h b/cras/src/server/cras_hfp_slc.h
index a30af50a..290820a7 100644
--- a/cras/src/server/cras_hfp_slc.h
+++ b/cras/src/server/cras_hfp_slc.h
@@ -135,6 +135,9 @@ int hfp_slc_get_hf_codec_negotiation_supported(struct hfp_slc_handle *handle);
 /* Gets if the remote HF supports HF indicator. */
 int hfp_slc_get_hf_hf_indicators_supported(struct hfp_slc_handle *handle);
 
+/* Gets if the HF side supports wideband speech. */
+bool hfp_slc_get_wideband_speech_supported(struct hfp_slc_handle *handle);
+
 /* Gets if the AG side supports codec negotiation. */
 int hfp_slc_get_ag_codec_negotiation_supported(struct hfp_slc_handle *handle);
 
diff --git a/cras/src/tests/hfp_iodev_unittest.cc b/cras/src/tests/hfp_iodev_unittest.cc
index 600495cf..170f4f17 100644
--- a/cras/src/tests/hfp_iodev_unittest.cc
+++ b/cras/src/tests/hfp_iodev_unittest.cc
@@ -382,6 +382,10 @@ int hfp_slc_get_selected_codec(struct hfp_slc_handle* handle) {
   return HFP_CODEC_ID_CVSD;
 }
 
+bool hfp_slc_get_wideband_speech_supported(struct hfp_slc_handle* handle) {
+  return false;
+}
+
 int hfp_slc_codec_connection_setup(struct hfp_slc_handle* handle) {
   return 0;
 }
diff --git a/cras/src/tests/hfp_slc_unittest.cc b/cras/src/tests/hfp_slc_unittest.cc
index 723089c6..8e78727e 100644
--- a/cras/src/tests/hfp_slc_unittest.cc
+++ b/cras/src/tests/hfp_slc_unittest.cc
@@ -271,6 +271,11 @@ TEST(HfpSlc, CodecNegotiation) {
   codec = hfp_slc_get_selected_codec(handle);
   EXPECT_EQ(HFP_CODEC_ID_MSBC, codec);
 
+  /* Fake HF selects mSBC codec. */
+  err = write(sock[1], "AT+BCS=2\r", 9);
+  ASSERT_EQ(err, 9);
+
+  err = hfp_slc_codec_connection_setup(handle);
   /* Assert CRAS initiates codec selection to mSBC. */
   memset(buf, 0, 256);
   err = read(sock[1], buf, 256);
@@ -281,21 +286,11 @@ TEST(HfpSlc, CodecNegotiation) {
   ASSERT_EQ(err, 9);
   slc_cb(slc_cb_data);
 
-  /* Fake that receiving codec selection from HF. */
-  err = write(sock[1], "AT+BCS=2\r", 9);
-  ASSERT_EQ(err, 9);
-  slc_cb(slc_cb_data);
-
-  memset(buf, 0, 256);
-  err = read(sock[1], buf, 256);
-  pos = strstr(buf, "\r\n+BCS:2\r\n");
-  ASSERT_EQ((void*)NULL, pos);
-
   hfp_slc_destroy(handle);
   cras_bt_event_log_deinit(btlog);
 }
 
-TEST(HfpSlc, CodecNegotiationTimeout) {
+TEST(HfpSlc, CodecNegotiationCapabilityChanged) {
   int codec;
   int err;
   int sock[2];
@@ -329,30 +324,41 @@ TEST(HfpSlc, CodecNegotiationTimeout) {
   ASSERT_EQ(err, 16);
   slc_cb(slc_cb_data);
 
-  ASSERT_NE((void*)NULL, cras_tm_timer_cb);
-
   /* Assert that AG side prefers mSBC codec. */
   codec = hfp_slc_get_selected_codec(handle);
   EXPECT_EQ(HFP_CODEC_ID_MSBC, codec);
 
+  /* Fake HF selects mSBC codec. */
+  err = write(sock[1], "AT+BCS=2\r", 9);
+  ASSERT_EQ(err, 9);
+
+  err = hfp_slc_codec_connection_setup(handle);
   /* Assert CRAS initiates codec selection to mSBC. */
   memset(buf, 0, 256);
   err = read(sock[1], buf, 256);
   pos = strstr(buf, "\r\n+BCS:2\r\n");
   ASSERT_NE((void*)NULL, pos);
 
-  /* Assume codec negotiation failed. so timeout is reached. */
-  cras_tm_timer_cb(NULL, cras_tm_timer_cb_data);
+  /* Fake that HF changes supported codecs. */
+  err = write(sock[1], "AT+BAC=1\r", 9);
+  ASSERT_EQ(err, 9);
+  slc_cb(slc_cb_data);
+  err = read(sock[1], buf, 256);
 
-  codec = hfp_slc_get_selected_codec(handle);
-  EXPECT_EQ(HFP_CODEC_ID_CVSD, codec);
+  /* Fake HF selects CVSD codec. */
+  err = write(sock[1], "AT+BCS=1\r", 9);
+  ASSERT_EQ(err, 9);
 
-  /* Expects CRAS fallback and selects to CVSD codec. */
+  err = hfp_slc_codec_connection_setup(handle);
+  /* Assert CRAS initiates codec selection to CVSD. */
   memset(buf, 0, 256);
   err = read(sock[1], buf, 256);
   pos = strstr(buf, "\r\n+BCS:1\r\n");
   ASSERT_NE((void*)NULL, pos);
 
+  codec = hfp_slc_get_selected_codec(handle);
+  EXPECT_EQ(HFP_CODEC_ID_CVSD, codec);
+
   hfp_slc_destroy(handle);
   cras_bt_event_log_deinit(btlog);
 }
@@ -417,6 +423,13 @@ struct cras_timer* cras_tm_create_timer(struct cras_tm* tm,
   return reinterpret_cast<struct cras_timer*>(0x404);
 }
 
+int cras_poll(struct pollfd* fds,
+              nfds_t nfds,
+              struct timespec* timeout,
+              const sigset_t* sigmask) {
+  return 1;
+}
+
 void cras_tm_cancel_timer(struct cras_tm* tm, struct cras_timer* t) {}
 }