bluetooth: ble: add on_advertising_terminated callback#562
Conversation
bug: v/80268 Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/80258 Rootcause: ARRAY_SIZE redefined Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/80268 Rootcause: profile direct connect api has changed Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/80268 Rootcause: should not call unref at this time Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/80269 Rootcause: add logs to print call number when sync Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81504 return -EINPROGRESS dial callback and send OK after dial response Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81567 remove unused functions Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81589 remove global var g_sal_ag_sync_conn and sync call by addr Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81504 Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/80268 Rootcause: Not unregister callbacks when cleanup. Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81522 When event_id == BT_AVRCP_EVT_VOLUME_CHANGED, flag = true is set only if both CONFIG_BLUETOOTH_AVRCP_ABSOLUTE_VOLUME and CONFIG_BLUETOOTH_AVRCP_CONTROL are enabled. Otherwise in the else branch, flag = true is set only if CONFIG_BLUETOOTH_AVRCP_TARGET is enabled. All configurations are enabled, treating flag as always true, making the if (!flag) condition unreachable. Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81702 Fixed memory leak caused by premature return. Signed-off-by: liuxiang18 <liuxiang18@xiaomi.com>
bug: v/80258 Rootcause: attributes in bt_sdp_discover_params may be modified by ZBlue SDP. Using const could cause a crash in some cases Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81752 Rootcause: audio_connect should not be called in bluetoothd task Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81968 Rootcause: disconnected_callback not called caused connect info not cleared in connection manager module Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
… an asynchronous API. bug: v/81682 When priv dynamically allocates memory successfully but fails later due to other reasons before reaching the assignment ins->priv = priv;, the memory allocated to priv cannot be freed in bt_socket_async_client_deinit, leading to a resource leak. Signed-off-by: jialu <jialu@xiaomi.com>
…se functions bug: v/80811 Rootcause: In certain scenarios, users of `euv_pipe` must ensure all UV requests have completed execution before releasing resources. Consequently, it is necessary to notify users that `euv_pipe` has been fully released after its close operation is completed, thereby permitting subsequent operational procedures to proceed. Support for the close callback has therefore been added. Signed-off-by: chejinxian1 <chejinxian1@xiaomi.com>
…e callback bug: v/80808 Rootcause: In high-throughput reception scenarios, situations may arise where the `write_cb` for SPP data transmission to the application has not yet completed, yet the SPP device is released due to an abrupt disconnection, thereby preventing notification to the protocol stack that data reception has concluded. To circumvent this issue, it is imperative to ensure all write operations are finalised before releasing the SPP device. Consequently, an `euv_pipe` close callback implementation has been introduced to guarantee that all `write_cb` operations execute successfully prior to severing the data pathway. Signed-off-by: chejinxian1 <chejinxian1@xiaomi.com>
bug: v/74709
only open CONFIG_BLUETOOTH_AVRCP_CONTROL or CONFIG_BLUETOOTH_AVRCP_ABSOLUTE_VOLUME can build in bt_avrcp_control_notification_cb.
error: 'bt_avrcp_info_find_by_ct' undeclared (first use in this function); did you mean 'bt_avrcp_info_find_by_tg'?
1501 | avrcp_info = bt_list_find(bt_avrcp_conn, bt_avrcp_info_find_by_ct, ct);
| ^~~~~~~~~~~~~~~~~~~~~~~~
| bt_avrcp_info_find_by_tg
Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/82095 The spp_connect_handler was attempting to look up the SPP connection by rfcomm_dlc before it was added to the connection list, causing "SPP connection not found for rfcomm_dlc" error. Root Cause: The connection object wasn't in the global connection list at the time of lookup, making spp_find_connection_by_dlc() always fail. Fix: Pass the spp_conn pointer directly as user_data to avoid the lookup, and add it to the connection list after successful initialization. Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/81925 Add a generic descriptor allocation path, matching alloc_characteristic() style. Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/81958 Some services are marked with GATT_PROP_EXPOSED_OVER_BREDR, but current not implement gatt over bredr. As a temporary workaround, clear this flag when calling add_service() so the service is exposed over BLE. Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/82081 `bt_le_scan.h` is a globally exposed header file. Then, Zephyr's `#include <zephyr/bluetooth/bluetooth.h>` is declared as a private inclusion of Zephyr in CMake. However, the problem is that when third-party apps use my global `bt_le_scan.h`, the CMake system doesn't know where `zephyr/bluetooth/bluetooth.h` is and throws an error. One solution is for the third-party app to also declare a private inclusion of Zephyr in CMake, but this doesn't conform to design principles. The app only needs to be concerned with my framework layer. If the app also needs to include Zephyr's header files, then the framework layer is not properly configured. Therefore, `zephyr/bluetooth/bluetooth.h` must not be explicitly included in `bt_le_scan`. Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/82104 The number of gatts sal DB attributes was insufficient for miwear's needs, so it was increased to a margin of 60. Future memory optimization projects will no longer maintain static arrays. Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/81701 Rootcause: dereference null. Signed-off-by: Yuheng Li <liyuheng@xiaomi.com>
bug: v/81701 Rootcause: unnecessary malloc Signed-off-by: YuhengLi <liyuheng@xiaomi.com>
bug: v/81924 By default, just working (no I/O) should automatically accept user confirmation, but for compatibility with the watch app, app confirmation is required. Signed-off-by: zhongzhijie1 <zhongzhijie1@xiaomi.com>
bug: v/59050 Rootcause:When reconnect to the headset during a call, the headset will obtain the call status through the cind command. Since Vela does not have modem, the status will be error. So, get cind from Android. Signed-off-by: zhangyuan20 <zhangyuan20@xiaomi.com>
bug: v/61170 Signed-off-by: zhangyuan20 <zhangyuan20@xiaomi.com>
bug: v/81520 In `spp_find_connection_by_sdp_param` and `spp_connect_with_uuid`, the pointer `spp_conn` was dereferenced before the NULL check. This patch ensures the pointer is validated before access to avoid potential crashes. Signed-off-by: v-yichenxi <v-yichenxi@xiaomi.com>
bug: v/88931 Narrow each SBC codec IE entry to a single channel mode and sampling frequency combination, ordered by preference: 44.1kHz Joint Stereo > 48kHz Joint Stereo > 44.1kHz Stereo > 48kHz Stereo > 44.1kHz Dual > 48kHz Dual > 44.1kHz Mono > 48kHz Mono. Fix block length and allocation method comments. Signed-off-by: jialu <jialu@xiaomi.com>
There was a problem hiding this comment.
Pull request overview
Adds a new BLE advertiser callback to notify clients when the controller auto-terminates an advertising set (including the peer address when termination is due to connection establishment), and propagates this event through the service layer and socket IPC.
Changes:
- Extend
advertiser_callback_twithon_advertising_terminatedand document the new callback semantics. - Add
advertising_on_terminated()plumbing and update Zephyr extended advertising callbacks to report termination reasons/address. - Add a new socket IPC callback code + packet payload to deliver the terminated event to clients; update tooling to print termination details.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/adv.c | Adds a terminated callback implementation that prints peer address or timeout/max-events. |
| service/stacks/zephyr/sal_le_advertise_interface.c | Splits ext adv callbacks and extracts peer address on connection termination. |
| service/src/advertising.h | Declares advertising_on_terminated() hook. |
| service/src/advertising.c | Stores termination metadata and dispatches to the new callback (with fallback). |
| service/ipc/socket/include/bt_message_advertiser.h | Introduces a new BT_IPC_CODE subcode and callback payload for termination. |
| service/ipc/socket/src/bt_socket_advertiser.c | Server sends terminated event; client dispatches it via subcode parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void advertising_on_state_changed(uint8_t adv_id, uint8_t state) | ||
| { | ||
| adv_event_t* advstate = malloc(sizeof(adv_event_t)); | ||
|
|
||
| if (!advstate) { | ||
| BT_LOGE("adv_id: %d state malloc failed", adv_id); | ||
| return; | ||
| } | ||
|
|
||
| advstate->adv_id = adv_id; | ||
| advstate->state = state; | ||
| do_in_service_loop(advertiser_notify_state, advstate); | ||
| } |
There was a problem hiding this comment.
advertising_on_state_changed() allocates adv_event_t with malloc() but doesn’t initialize the newly added has_addr/addr fields. When state == LE_ADVERTISING_STOPPED, advertiser_notify_state() reads has_addr, which can be garbage and may incorrectly log a peer address and/or invoke on_advertising_terminated with uninitialized data. Initialize adv_event_t (e.g., calloc/memset or explicitly set has_addr = false).
| if (advstate->has_addr | ||
| && adver->callbacks.on_advertising_terminated) { | ||
| adver->callbacks.on_advertising_terminated( | ||
| get_adver(adver), advstate->adv_id, | ||
| &advstate->addr); |
There was a problem hiding this comment.
The new on_advertising_terminated callback is only invoked when advstate->has_addr is true. This prevents delivering the “duration/max events” termination case described in the API (where addr == NULL) and instead always falls back to on_advertising_stopped. Call on_advertising_terminated whenever it is registered, passing NULL when has_addr is false; only fall back to on_advertising_stopped when the terminated callback is NULL.
| if (advstate->has_addr | |
| && adver->callbacks.on_advertising_terminated) { | |
| adver->callbacks.on_advertising_terminated( | |
| get_adver(adver), advstate->adv_id, | |
| &advstate->addr); | |
| if (adver->callbacks.on_advertising_terminated) { | |
| adver->callbacks.on_advertising_terminated( | |
| get_adver(adver), advstate->adv_id, | |
| advstate->has_addr ? &advstate->addr : NULL); |
| if (advstate->has_addr | ||
| && adver->callbacks.on_advertising_terminated) { | ||
| adver->callbacks.on_advertising_terminated( | ||
| get_adver(adver), advstate->adv_id, | ||
| &advstate->addr); |
There was a problem hiding this comment.
advertiser_t.callbacks is populated via memcpy(&adver->callbacks, cbs, sizeof(advertiser_callback_t)) (see alloc_new_advertiser). After adding on_advertising_terminated to the struct, callers built against an older header may pass a smaller callbacks struct; this memcpy would read past the caller’s allocation and the subsequent adver->callbacks.on_advertising_terminated access can crash. Use the cbs->size field to copy only the provided bytes (bounded by sizeof(advertiser_callback_t)) and zero-initialize the remainder.
| * **Example:** | ||
| * @code | ||
| void on_advertising_terminated(bt_advertiser_t* adv, uint8_t adv_id, bt_le_address_t* addr) | ||
| { | ||
| if (addr) { | ||
| printf("adv_id:%d terminated by connection from %02X:%02X:%02X:%02X:%02X:%02X type:%d\n", | ||
| adv_id, addr->addr[5], addr->addr[4], addr->addr[3], | ||
| addr->addr[2], addr->addr[1], addr->addr[0], addr->addr_type); | ||
| } else { | ||
| printf("adv_id:%d terminated by timeout or max events\n", adv_id); | ||
| } | ||
| } | ||
| * @endcode |
There was a problem hiding this comment.
The Doxygen example block is missing leading * comment prefixes on the code lines inside @code (e.g., the void on_advertising_terminated... line). This can break generated documentation formatting. Prefix the example lines with * (or move them into a fenced Doxygen block style used elsewhere in the repo).
| } | ||
|
|
||
| /* Extract peer address from the new connection */ | ||
| dst = bt_conn_get_dst(info->conn); |
There was a problem hiding this comment.
Double check if we can receive the ACL connected callback before advertising is terminated.
If yes, we may use bt_conn_get_addr, which is faster, rather than bt_conn_get_dst
There was a problem hiding this comment.
Logs show that ext_adv_connected is received before the ACL connected callback, so bt_conn_get_dst is the safer choice here.
| } | ||
|
|
||
| /* Extract peer address from the new connection */ | ||
| dst = bt_conn_get_dst(info->conn); |
There was a problem hiding this comment.
Actally I would recommand to use get_le_addr_from_conn
BTW, better to check info->conn != NULL inadvance since bt_conn_get_dst is from external repository and we can not always say it's safe on null pointers.
There was a problem hiding this comment.
Done. Using get_le_addr_from_conn now, with info->conn != NULL guard.
bug: v/88196 Rootcause: Concurrent bt_sal_connect calls send multiple HCI Create Connection commands before the first completes, causing controller to reject with Command Disallowed. Add a pending queue to serialize ACL connect requests, dispatching the next via sal_send_req only after zblue_on_connected callback fires. Signed-off-by: Kai Cheng <chengkai@xiaomi.com>
3d9d0d1 to
16524ed
Compare
zhongzhijie1
left a comment
There was a problem hiding this comment.
LGTM. advertising_on_terminated 区分 connection/timeout/max_events,ext_adv_connected 正确 NULL check bt_conn_get_dst,IPC 层 fallback 到 stopped 兼容旧客户端,cbs->size 拷贝保护向后兼容。
bug: v/87939 Move all synchronous zblue HID API calls from service_loop/sysworkq context to worker thread via service_loop_work() to avoid blocking. The affected functions are: - bt_sal_hid_device_send_report (bt_hid_device_send_intr_data) - bt_sal_hid_device_get_report_response (bt_hid_device_send_ctrl_data) - bt_sal_hid_device_report_error (bt_hid_device_report_error) - bt_sal_hid_device_virtual_unplug (bt_hid_device_virtual_unplug) - hid_get_protocol_callback (bt_hid_device_send_ctrl_data) These calls internally invoke bt_conn_create_pdu_timeout which may block, causing service_loop stalls. Signed-off-by: v-yichenxi <v-yichenxi@xiaomi.com>
bug: v/87673 Rootcause: bt_sal_hfp_hf_set_volume and bt_sal_hfp_ag_set_volume directly call zblue API (bt_hfp_hf_vgm/vgs, bt_hfp_ag_vgm/vgs) in the caller's context. During volume adjustment stress testing on iOS calls, rapid consecutive invocations can cause deadlock. Move the actual zblue API calls into service_loop_work to serialize execution in the service loop, preventing concurrent access issues. Signed-off-by: liyuheng <liyuheng@xiaomi.com>
bug: v/87673 Rootcause: Multiple bt_sal_hfp_ag_* functions directly call zblue Z_API() in the caller's context, which can cause concurrent access issues similar to the set_volume deadlock. Move all remaining AG interfaces (voice_recognition, cind_response, dial_response, cops_response, notify_device_status_changed, set_inband_ring_enable, send_at_cmd, error_response) into service_loop_work to serialize execution in the service loop. Signed-off-by: liyuheng <liyuheng@xiaomi.com>
bug: v/87673 Rootcause: Multiple bt_sal_hfp_hf_* functions directly call zblue Z_API() in the caller's context, which can cause concurrent access issues. Move all remaining HF interfaces (answer_call, reject_call, hold_call, hangup_call, dial_number, dial_memory, call_control, get_current_calls, voice_recognition, send_battery_level, send_at_cmd, send_dtmf, get_subscriber_number) into service_loop_work to serialize execution in the service loop. Signed-off-by: liyuheng <liyuheng@xiaomi.com>
bug: v/87935 Rootcause: The global connection list g_sal_ag_conn_list in sal_hfp_ag_interface.c is accessed by multiple threads (zblue callback thread, service work thread, upper layer API thread) without any synchronization, which may cause data race, list corruption, use-after-free or crash. Add a pthread recursive mutex (g_sal_ag_conn_lock) with wrapper functions conn_list_lock()/conn_list_unlock() to protect all accesses to g_sal_ag_conn_list and calls sub-lists. The mutex is initialized in bt_sal_hfp_ag_init() and destroyed in bt_sal_hfp_ag_cleanup(). NULL checks for the list pointer are moved inside the lock scope to avoid TOCTOU races. Signed-off-by: liyuheng <liyuheng@xiaomi.com>
bug: v/87935 Rootcause: The global connection list g_sal_hf_conn_list in sal_hfp_hf_interface.c is accessed by multiple threads (zblue callback thread, service work thread, upper layer API thread) without any synchronization, which may cause data race, list corruption, use-after-free or crash. Add a pthread recursive mutex (g_sal_hf_conn_lock) with wrapper functions conn_list_lock()/conn_list_unlock() to protect all accesses to g_sal_hf_conn_list and calls sub-lists. The mutex is initialized in bt_sal_hfp_hf_init() and destroyed in bt_sal_hfp_hf_cleanup(). NULL checks for the list pointer are moved inside the lock scope to avoid TOCTOU races. Signed-off-by: liyuheng <liyuheng@xiaomi.com>
bug: v/88589 Add on_advertising_terminated callback to advertiser_callback_t, triggered by HCI LE Advertising Set Terminated event (Core Spec v6.2 Section 7.7.65.18) when Controller auto-terminates advertising. - addr != NULL: terminated due to connection establishment - addr == NULL: terminated due to duration/max events Use LE_ADVERTISING_STOPPED state for both host-stop and controller- terminated cases. Distinguish by has_addr field in adv_event_t: when has_addr is set and on_advertising_terminated is registered, call terminated callback; otherwise call stopped callback. Changes: - bt_le_advertiser.h: add on_advertising_terminated_cb_t - advertising.h/c: add advertising_on_terminated() with fallback to on_advertising_stopped, no new state enum - sal_le_advertise_interface.c: split ext_adv_connected/ext_adv_sent, extract peer address via bt_conn_get_dst() - bt_message_advertiser.h: add new IPC code using BT_IPC_CODE macro - bt_socket_advertiser.c: server sends terminated, client dispatches via BT_IPC_GET_SUBCODE - tools/adv.c: add terminated callback with address printing Signed-off-by: Lu Jia <jialu@xiaomi.com>
16524ed to
be0d9d6
Compare
The merge-base changed after approval.
bug: v/88589
Add on_advertising_terminated callback to advertiser_callback_t, triggered by HCI LE Advertising Set Terminated event (Core Spec v6.2 Section 7.7.65.18) when Controller auto-terminates advertising.
Changes: