zblue: hfp: hf: avoid call hci_send_sync in bluetoothd by expliyh · Pull Request #392 · open-vela/frameworks_bluetooth

expliyh · 2026-01-22T04:31:20Z

bug: v/84390

Rootcause: should not call hci_send_sync in bluetoothd

chengkai15 · 2026-01-27T08:30:09Z

service/stacks/zephyr/sal_hfp_hf_interface.c

@@ -52,6 +52,14 @@ typedef struct _hf_connect_params {
    uint8_t channel;
 } hf_connect_params_t;



What are the advantages of defining a new structure? It's recommended to reuse the original pointers.

chengkai15 · 2026-01-27T08:31:35Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+    if (!params->sco_conn) {
+        BT_LOGE("%s, Invalid sco_conn parameter", __func__);
+        free(params);


It seems that the judgment here is duplicated. Could find_connection_by_sco handle this by checking for null pointers?

Logs are printed only in the first function called in SAL to avoid duplicate log output.

Copilot

Pull request overview

This PR updates the Zephyr HFP HF SAL implementation to avoid executing potentially blocking/synchronous HCI operations (e.g., those that may invoke hci_send_sync) directly in bluetoothd, by deferring SCO audio connect/disconnect operations to the service loop worker.

Changes:

Added dedicated parameter structs for deferred HFP HF SCO connect/disconnect operations.
Introduced do_hf_sco_connect() / do_hf_sco_disconnect() worker callbacks.
Updated bt_sal_hfp_hf_connect_audio() and bt_sal_hfp_hf_disconnect_audio() to queue work via service_loop_work() instead of calling stack APIs inline.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

service/stacks/zephyr/sal_hfp_hf_interface.c

chengkai15 · 2026-01-27T08:32:16Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+    hf = params->hf;
+    free(params);
+
+    sal_conn = find_connection_by_hf(hf);


Logs are printed only in the first function called in SAL to avoid duplicate log output.

service/stacks/zephyr/sal_hfp_hf_interface.c

zhangyuan376 · 2026-02-04T02:19:20Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+        BT_LOGE("%s, failed to allocate calls list", __func__);
+        free(sal_conn);
+        return NULL;
+    }


zhangyuan376 · 2026-02-04T02:32:59Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+        }
+    }
+
+    if (!sal_conn) {


It is a duplicate of line 541.

Remove the address check, since the state machine is responsible for avoiding duplicate connections.

service/stacks/zephyr/sal_hfp_hf_interface.c

huangyulong3 · 2026-02-04T03:04:54Z

service/stacks/zephyr/sal_hfp_hf_interface.c

    }

+    bt_hfp_hf_connection_t* sal_conn = find_connection_by_addr(addr);
+


move the

if (!sal_conn) { BT_LOGE("%s, No initiating connection", __func__); bt_conn_unref(conn); return BT_STATUS_NOMEM; }

here would be better.

Won't change: This function is invoked twice, and sal_conn == NULL is expected on the first call. Therefore, the check must not be moved.

gzh-terry · 2026-02-05T07:32:17Z

service/stacks/zephyr/sal_hfp_hf_interface.c

-    if (do_hf_connect(0 /* bt_controller_id_t */, &bd_addr, NULL) != BT_STATUS_SUCCESS) {
-        free(g_conn_params);
-        g_conn_params = NULL;
+    if (do_hf_connect(0 /* bt_controller_id_t */, &bd_addr, &channel) != BT_STATUS_SUCCESS) {


Add service_loop_work for do_hf_connect as well, so we can ensure that do_hf_connect always happend in worker thread.

Then move these after error:

hfp_hf_on_connection_state_changed(&bd_addr, PROFILE_STATE_DISCONNECTED, 0, 0); bt_list_remove(g_sal_hf_conn_list, sal_conn);

if do_hf_connect fails, it will report disconnected state. we may report disconnected state twice if we move it after error

gzh-terry · 2026-02-05T07:36:04Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+    bt_address_t addr;
+    bt_hfp_hf_connection_t* sal_conn;

+    if (bt_sal_get_remote_address(conn, &addr) != BT_STATUS_SUCCESS) {


Remove addr that is never used.

service/stacks/zephyr/sal_hfp_hf_interface.c

gzh-terry · 2026-02-05T07:51:07Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+    err = Z_API(bt_hfp_hf_audio_connect)(hf);
+    if (err) {
+        BT_LOGE("%s, Failed to connect HFP HF SCO, err=%d", __func__, err);
+        hfp_hf_on_audio_connection_state_changed(&sal_conn->addr, HFP_AUDIO_STATE_DISCONNECTED, 0);


Check if sal_conn->sco_conn == NULL before this audio connection.
At least, we can report HFP_AUDIO_STATE_CONNECTED when err = -ECONNREFUSED

gzh-terry · 2026-02-06T10:03:06Z

service/stacks/zephyr/sal_hfp_hf_interface.c

-        BT_LOGE("%s, Previous connection ongoing", __func__);
+    bt_hfp_hf_connection_t* sal_conn = find_connection_by_addr(addr);
+    if (sal_conn) {
+        BT_LOGW("%s, Connection already exists or in progress", __func__);


Caution:
Before this change, BT_STATUS_BUSY indicated an HFP connection was being established.
After this change, it indicates an HFP connection is either being established or has already been established.

Please double-check that this matches the expected logic.

gzh-terry · 2026-02-06T10:07:17Z

service/stacks/zephyr/sal_hfp_hf_interface.c

-        }
+    bt_hfp_hf_connection_t* sal_conn = find_connection_by_addr(addr);

+    BT_LOGD("%s, SDP not discovered", __func__);


This log is now incorrect.
We can only say do_hf_sdp_discover is called at this stage.
Nobody knows if SDP is already discovered now.

gzh-terry · 2026-02-06T10:09:21Z

service/stacks/zephyr/sal_hfp_hf_interface.c

-            bt_conn_unref(conn);
-            return BT_STATUS_FAIL;
-        }
+    bt_hfp_hf_connection_t* sal_conn = find_connection_by_addr(addr);


Put this check before bt_conn_lookup_addr_br

gzh-terry · 2026-02-06T10:19:01Z

service/stacks/zephyr/sal_hfp_hf_interface.c

@@ -325,44 +336,32 @@ static bt_status_t do_hf_connect(bt_controller_id_t id, bt_address_t* addr, void
        return BT_STATUS_NOT_FOUND;


Report PROFILE_STATE_DISCONNECTED here, since the service is now stay in PROFILE_STATE_CONNECTING

service/stacks/zephyr/sal_hfp_hf_interface.c

gzh-terry · 2026-02-06T10:28:34Z

service/stacks/zephyr/sal_hfp_hf_interface.c

    int err;
    uint16_t port;
-
    bt_address_t bd_addr;


Remove bd_addr, use sal_conn->addr

gzh-terry · 2026-02-06T10:30:17Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+        BT_LOGE("%s, could not find sal_conn", __func__);
+        return BT_SDP_DISCOVER_UUID_STOP;
+    }
    if (bt_sal_get_remote_address(conn, &bd_addr) != BT_STATUS_SUCCESS) {


Remove bt_sal_get_remote_address, use sal_conn->addr

gzh-terry · 2026-02-06T10:33:14Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+
+static void do_hf_slc_connect(service_work_t* work, void* userdata)
+{
+    uint8_t* channel = (uint8_t*)userdata;


Remove (uint8_t*)
Force cast is not required for void*

gzh-terry · 2026-02-06T10:38:32Z

service/stacks/zephyr/sal_hfp_hf_interface.c

+        return;
+    }
+
+    /* Get the address from the first connection in the list that has no hf */


Don't do this.
Use userdata to transfer any necessary information, for example, conn

bug: v/84390 Rootcause: should not call hci_send_sync in bluetoothd Signed-off-by: YuhengLi <liyuheng@xiaomi.com>

expliyh requested review from chengkai15, gzh-terry, huangyulong3, hyson710, zhangyuan376 and zhongzhijie1 as code owners January 22, 2026 04:31

gzh-terry previously approved these changes Jan 22, 2026

View reviewed changes

expliyh force-pushed the 84390 branch from b6134be to b6f5a5a Compare January 27, 2026 07:35

zhongzhijie1 requested a review from Copilot January 27, 2026 08:24

Copilot started reviewing on behalf of zhongzhijie1 January 27, 2026 08:24 View session

chengkai15 reviewed Jan 27, 2026

View reviewed changes

Copilot AI reviewed Jan 27, 2026

View reviewed changes

service/stacks/zephyr/sal_hfp_hf_interface.c Outdated Show resolved Hide resolved

service/stacks/zephyr/sal_hfp_hf_interface.c Show resolved Hide resolved

chengkai15 reviewed Jan 27, 2026

View reviewed changes

expliyh force-pushed the 84390 branch 2 times, most recently from 1a16b88 to 55c5320 Compare January 30, 2026 03:39

expliyh dismissed gzh-terry’s stale review via d2cdd23 January 30, 2026 08:44

expliyh force-pushed the 84390 branch 4 times, most recently from 6ddddf5 to 4ed9b55 Compare February 2, 2026 03:51

chengkai15 reviewed Feb 2, 2026

View reviewed changes

service/stacks/zephyr/sal_hfp_hf_interface.c Outdated Show resolved Hide resolved

expliyh force-pushed the 84390 branch 8 times, most recently from b57c1ab to 571ff98 Compare February 3, 2026 03:39

expliyh force-pushed the 84390 branch from c4bdc54 to 67ddcce Compare February 3, 2026 13:19

chengkai15 previously approved these changes Feb 3, 2026

View reviewed changes

zhangyuan376 reviewed Feb 4, 2026

View reviewed changes

huangyulong3 reviewed Feb 4, 2026

View reviewed changes

expliyh dismissed chengkai15’s stale review via 9f8415f February 4, 2026 06:51

expliyh force-pushed the 84390 branch from 67ddcce to 9f8415f Compare February 4, 2026 06:51

chengkai15 previously approved these changes Feb 4, 2026

View reviewed changes

zhangyuan376 previously approved these changes Feb 4, 2026

View reviewed changes

expliyh force-pushed the 84390 branch from 9f8415f to 043ee8f Compare February 5, 2026 06:44

gzh-terry reviewed Feb 5, 2026

View reviewed changes

service/stacks/zephyr/sal_hfp_hf_interface.c Show resolved Hide resolved

gzh-terry reviewed Feb 5, 2026

View reviewed changes

expliyh dismissed stale reviews from chengkai15 and zhangyuan376 via b138dcc February 6, 2026 08:04

expliyh force-pushed the 84390 branch 3 times, most recently from c669567 to a1d9da6 Compare February 6, 2026 08:48

gzh-terry reviewed Feb 6, 2026

View reviewed changes

expliyh commented Feb 6, 2026

View reviewed changes

service/stacks/zephyr/sal_hfp_hf_interface.c Show resolved Hide resolved

gzh-terry reviewed Feb 6, 2026

View reviewed changes

expliyh force-pushed the 84390 branch from a1d9da6 to ab6cf3d Compare February 8, 2026 10:05

zblue: hfp: hf: avoid call hci_send_sync in bluetoothd

8b90cd9

bug: v/84390 Rootcause: should not call hci_send_sync in bluetoothd Signed-off-by: YuhengLi <liyuheng@xiaomi.com>

expliyh force-pushed the 84390 branch from ab6cf3d to 8b90cd9 Compare February 8, 2026 10:22

		@@ -52,6 +52,14 @@ typedef struct _hf_connect_params {
		uint8_t channel;
		} hf_connect_params_t;

		}

		bt_hfp_hf_connection_t* sal_conn = find_connection_by_addr(addr);

		@@ -325,44 +336,32 @@ static bt_status_t do_hf_connect(bt_controller_id_t id, bt_address_t* addr, void
		return BT_STATUS_NOT_FOUND;

Conversation

expliyh commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chengkai15 Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gzh-terry Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chengkai15 Jan 27, 2026 •

edited

Loading

gzh-terry Feb 5, 2026 •

edited

Loading