Skip to content

Auracast#546

Open
expliyh wants to merge 3 commits into
open-vela:devfrom
expliyh:auracast
Open

Auracast#546
expliyh wants to merge 3 commits into
open-vela:devfrom
expliyh:auracast

Conversation

@expliyh
Copy link
Copy Markdown
Contributor

@expliyh expliyh commented Mar 25, 2026

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Auracast Sink support (including LE Periodic Advertising Sync + BIG Sync) across the stack: Zephyr SAL, service layer, IPC/socket transport, framework APIs (sync + async), and bt_tools/feature integration.

Changes:

  • Introduce PA Sync service + Zephyr SAL periodic advertising sync interface and related scan result enhancements (SID/interval/tx power/flags).
  • Add Auracast Sink role end-to-end: Zephyr BIG sync SAL, service/profile state machine + IPC commands/callbacks + framework APIs.
  • Extend tools and feature async bindings to expose Auracast sink operations (CLI + JIDL).

Reviewed changes

Copilot reviewed 67 out of 67 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tools/utils.h Adds BTTOOL_STRCAT helper macro for bounded string appends.
tools/bt_tools.h Declares auracast sink CLI command entrypoints (sync + async).
tools/bt_tools.c Registers auracast sink command; adapts init/uninit logic for BLE-only adapter states.
tools/async/gap.c Adds auracast sink async command; adapts init/uninit for BLE-only adapter states.
service/stacks/zephyr/sal_pa_sync_interface.c New Zephyr SAL implementation for periodic advertising sync + events.
service/stacks/zephyr/sal_le_scan_interface.c Updates scan receive path to use Zephyr scan callbacks and populate extended fields.
service/stacks/zephyr/sal_auracast_sink_interface.c New Zephyr SAL for Auracast sink BIG sync and ISO data delivery.
service/stacks/include/sal_pa_sync_interface.h New SAL header defining PA sync options/params and public SAL APIs.
service/stacks/include/sal_auracast_sink_interface.h New SAL header defining Auracast sink BIG sync APIs/params.
service/src/scan_manager.h Updates scan result adv_data pointer type to const uint8_t*.
service/src/scan_manager.c Updates scan result handling; flags periodic advertising based on interval.
service/src/pa_sync_service.h New service header for PA sync service and SAL→service callbacks.
service/src/pa_sync_service.c New PA sync service: lifecycle, message dispatch, caching, auracast readiness detection.
service/src/pa_sync_event.h New PA sync internal event/message definitions.
service/src/btservice.c Registers auracast sink profile service when enabled.
service/src/adapter_service.c Initializes/cleans up pa_sync service on LE enable/disable.
service/profiles/service_manager.h Adds <stddef.h> include (size_t dependency).
service/profiles/include/auracast_sink_service.h New auracast sink service interface + SAL callbacks for BIG/ISO data.
service/profiles/auracast/sink/auracast_sink_state_machine.h New auracast sink state machine public API.
service/profiles/auracast/sink/auracast_sink_state_machine.c Implements auracast sink state machine (sync lifecycle + data path placeholder).
service/profiles/auracast/sink/auracast_sink_service.c New profile service: device tracking, callback fanout, startup/shutdown, message loop.
service/profiles/auracast/sink/auracast_sink_event.h New message/event structs for auracast sink internal messaging.
service/profiles/auracast/sink/auracast_sink_event.c Implements auracast sink message allocation/free helpers.
service/ipc/socket/src/bt_socket_server.c Routes PA sync and auracast sink IPC code ranges to new handlers.
service/ipc/socket/src/bt_socket_scan.c Adjusts scan result callback subcodes and dispatch for enum stability.
service/ipc/socket/src/bt_socket_pa_sync.c New socket IPC server/client callbacks for PA sync service.
service/ipc/socket/src/bt_socket_client.c Registers PA sync + auracast sink callbacks; widens async message code type to uint32_t.
service/ipc/socket/src/bt_socket_auracast_sink.c New socket IPC server/client callbacks for auracast sink profile.
service/ipc/socket/include/bt_socket.h Declares PA sync + auracast sink socket APIs; extends async send_with_reply signature.
service/ipc/socket/include/bt_message_scan.h Adds stable scan callback IPC codes/subcodes and defines BT_LE_ON_SCAN_RESULT IPC macro.
service/ipc/socket/include/bt_message_pa_sync.h New IPC codes and payload structs for PA sync commands/callbacks.
service/ipc/socket/include/bt_message_auracast_sink.h New IPC codes and payload structs for auracast sink commands/callbacks.
service/ipc/socket/include/bt_message.h Wires PA sync + auracast sink payloads/results into the unified message packet.
service/ipc/socket/include/bt_ipc_code.h Adds IPC groups for PA sync and auracast profiles.
framework/socket/bt_pa_sync.c New framework socket client API for PA sync create/terminate.
framework/socket/bt_auracast_sink.c New framework socket client API for auracast sink callbacks and sync control.
framework/socket/async/bt_pa_sync_async.c New async framework APIs for PA sync create/terminate.
framework/socket/async/bt_le_scan_async.c Adjusts scan async reply behavior and callback invocation.
framework/socket/async/bt_le_advertiser_async.c Fixes async callback macro usage typo.
framework/socket/async/bt_gattc_async.c Fixes async callback macro usage; updates handle assignment type.
framework/socket/async/bt_device_async.c Fixes async callback macro usage typo.
framework/socket/async/bt_auracast_sink_async.c New async framework APIs for auracast sink callbacks and sync control.
framework/socket/async/bt_adapter_async.c Fixes async callback macro usage typo and removes duplicate struct definition.
framework/include/bt_uuid.h Adds UUIDs for (Broadcast) Audio Announcement services.
framework/include/bt_utils.h Adds popcount helpers (bt_utils_count_ones).
framework/include/bt_profile.h Adds auracast source/sink profile IDs and names.
framework/include/bt_pa_sync.h New public framework API for PA sync + parsing helpers + async APIs.
framework/include/bt_le_scan.h Extends scan result struct with SID/interval/tx power/flags; adds constants.
framework/include/bt_auracast_sink.h New public framework API for auracast sink + parsing helpers + async APIs.
framework/include/bt_audio_numbers.h New assigned-number constants and helpers for codec/context/location decoding.
framework/include/bt_async.h Renames async callback macro and adds shared register-callback types.
framework/include/bt_adapter.h Removes duplicate bt_register_callback_cb_t typedef (now in bt_async.h).
framework/include/bluetooth.h Adds BT_POWER_UNAVAILABLE, broadcast constants, and auracast sink callback storage.
framework/include/advertiser_data.h Adds generic advertiser_data_parse() API + parse callback typedef.
framework/common/bt_audio_numbers.c Implements stringification helpers for codec/context/location (log-gated).
framework/common/advertiser_data_helper.c Implements advertiser_data_parse() and PA sync / auracast advertising parsing.
framework/api/bt_pa_sync.c Adds non-socket framework API passthrough to service for PA sync.
framework/api/bt_auracast_sink.c Adds non-socket framework API passthrough to auracast sink profile interface.
feature/feature_async/src/feature_bluetooth_util.c Adds aurasnk list initialization/cleanup to feature async context.
feature/feature_async/jidl/bluetooth_ble.jidl Adds AuracastSink interface + data structs and callbacks to JIDL.
feature/feature_async/include/feature_bluetooth.h Adds auracast sink feature structs/state and includes bt_auracast_sink.h.
Makefile Adds build wiring for PA sync + auracast sink (service/SAL/tools/framework/socket/api).
Kconfig Adds CONFIG_BLUETOOTH_PA_SYNC and CONFIG_BLUETOOTH_AURACAST_SINK options.
CMakeLists.txt Adds build wiring for PA sync + auracast sink (mirrors Makefile).

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

Comment on lines +392 to +396
bt_list_remove(g_auracast_sink_service.sink_list, device);

if (g_auracast_sink_service.terminating)
auracast_sink_shutdown(g_auracast_sink_service.terminating);
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auracast_sink_service_notify_sync_terminated() calls auracast_sink_shutdown() when a shutdown is pending. This enqueues a new SHUTDOWN message each time a device terminates; with multiple devices it can resend terminate requests and potentially invoke the shutdown callback multiple times. Consider instead checking whether the sink_list is empty after removal and, if so, complete the pending shutdown once (without re-enqueueing another shutdown request).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is safe by design. All messages are processed serially in the service loop. When notify_sync_terminated calls auracast_sink_shutdown(cb), it only enqueues a new SHUTDOWN message. When service_shutdown processes it, it checks bt_list_length(sink_list) > 0 — if devices remain, it re-sends terminate (idempotent); if the list is empty, it performs cleanup and calls cb once. After cleanup, sink_list is set to NULL and terminating is cleared, so subsequent SHUTDOWN messages are no-ops. No fix needed.

Comment on lines 61 to 69
scan = (bt_scan_remote_t*)data->scan;
if (!packet->scan_r.remote) {
status = BT_STATUS_FAIL;
goto error;
}

scan->remote = packet->scan_r.remote;
ret_cb(ins, packet->scan_r.status, data->scan, data->userdata);
ret_cb(ins, BT_STATUS_SUCCESS, data->scan, data->userdata);

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

le_start_scan_reply() always calls the user callback with BT_STATUS_SUCCESS when packet->scan_r.remote is non-null, ignoring packet->scan_r.status. This can report success to callers even when the server returned an error. Consider passing packet->scan_r.status through to ret_cb (and only treating it as success when it is BT_STATUS_SUCCESS).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional. At this point, packet->scan_r.remote is confirmed non-null, meaning the scanner object was successfully created at the IPC level. The actual scan start status is delivered asynchronously via the on_scan_start_status callback. Passing BT_STATUS_SUCCESS here correctly indicates that scanner creation (not scan start) succeeded. No fix needed.

Comment on lines 155 to 159
static void STACK_CALL(stop_scan)(void* args)
{
bt_le_scan_cb_unregister(&scan_cbs);
SAL_CHECK(bt_le_scan_stop(), 0);
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In stop_scan(), the scan callback is unregistered before bt_le_scan_stop() is attempted. If bt_le_scan_stop() fails, scanning may continue but results will no longer be delivered to scan_recv_cb. Consider unregistering the callback only after bt_le_scan_stop() succeeds (or re-registering on failure).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Fixed — reordered to call bt_le_scan_stop() first, then bt_le_scan_cb_unregister(). This ensures the callback remains registered to handle any final events if stop fails.

Comment on lines 120 to +123
result_info.dev_type = BT_DEVICE_DEVTYPE_BLE;
result_info.addr_type = addr->type;
memcpy(&result_info.addr, &addr->a, sizeof(result_info.addr));

scan_on_result_data_update(&result_info, eir.value);
result_info.adv_type = parse_adv_type(info);
result_info.length = ad->len;
result_info.rssi = info->rssi;
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scan_recv_cb assigns result_info.length = ad->len, but ble_scan_result_t.length is a uint8_t. If Zephyr delivers extended advertising data >255 bytes, this will truncate silently and may break downstream AD parsing. Consider clamping/rejecting lengths > UINT8_MAX (and/or updating the public struct to carry a 16-bit length if extended payloads are intended).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per BLE spec, a single advertising data payload is at most 254 bytes (extended advertising), which fits within uint8_t. Zephyr's net_buf_simple.len is uint16_t for generality, but in the scan receive path it never exceeds 254. No fix needed.

Comment on lines +413 to +417
err = bt_le_per_adv_sync_delete(device->sync);
if (err) {
BT_LOGE("failed to terminate sync, err = %d", err);
bt_list_remove(g_sal_pa_sync_info->sync_list, device);
return;
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If bt_le_per_adv_sync_delete() fails, the code removes the device from sync_list anyway, which triggers sync_removed()->device_delete()->pa_sync_on_terminated(). That reports termination even though the delete operation failed and the sync may still be active. Consider keeping the device in the list on delete failure (and report an error via a dedicated callback/status), or retry/handle the failure explicitly.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional. If bt_le_per_adv_sync_delete() fails, we still remove the device to prevent a stale entry from blocking subsequent operations (shutdown, re-sync). Keeping a failed-to-delete device in the list would be worse — it could cause the service to hang during shutdown waiting for a termination that never comes. Force-cleanup is the safer strategy. No fix needed.

Comment on lines +352 to +356
err = bt_iso_big_terminate(device->sink);
if (err) {
BT_LOGE("failed to terminate sync, err = %d", err);
bt_list_remove(g_sal_auracast_sink_info->sink_list, device);
return;
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If bt_iso_big_terminate() fails, the code removes the device from sink_list anyway, which triggers sink_removed()->device_delete()->auracast_sink_on_terminated(). That reports termination even though termination failed and the BIG may still be active. Consider not removing from the list on failure; instead propagate the error and keep state consistent (or retry/force cleanup only when termination is confirmed).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same rationale as the PA sync case above. If bt_iso_big_terminate() fails, force-removing the device prevents residual state from interfering with other operations (shutdown, new sync creation, etc.). A lingering device entry with a failed BIG terminate would be a worse outcome than a clean removal with a terminated notification. No fix needed.

@expliyh expliyh force-pushed the auracast branch 3 times, most recently from 565ac82 to c430220 Compare March 25, 2026 07:33
Copy link
Copy Markdown
Contributor

@chengkai15 chengkai15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #546 Review — Auracast (LE Audio Broadcast Sink)

规模: 67 文件,9817+/60−,全新 feature

🔴 阻断级问题

1. PR 描述为空
9817 行新增代码,body 为空。必须补充:架构设计、测试方案、已知限制、回退路径。

2. 代码中残留 FIXME/TODO

  • sal_auracast_sink_interface.c:68 — FIXME: 不理解 QoS 参数用途
  • auracast_sink_state_machine.c — TODO: derive_mse 未检查 iso interval、derive_sync_timeout 硬编码
  • FIXME/TODO 必须在合并前解决或转为 issue 跟踪

3. PR 规模过大
67 文件应拆分 PA sync / Auracast profile / tools / async 封装,分别评审。

⚠️ 改进建议

4. broadcast_code 缺少 IPC 边界校验(High)
IPC 层传入的 broadcast_code 应校验长度 == 16 字节。

5. PA sync 全局单例无线程保护(Medium)
g_pa_sync_info 无 mutex,多线程场景有竞态风险。

6. Releasing 状态无超时保护(Medium)
Stack 不回调 SYNC_TERMINATED 会卡死,建议加 releasing timeout。

7. iso_recv 无丢帧统计(Low)
建议加 rx_dropped 计数器。

📋 结论

架构合理(PA sync + sink + SAL 分层),但必须补充描述、清理 FIXME/TODO、拆分 PR。REQUEST_CHANGES

@expliyh expliyh force-pushed the auracast branch 2 times, most recently from e5b83c0 to b6c93c5 Compare March 25, 2026 09:04
Copy link
Copy Markdown
Contributor Author

@expliyh expliyh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all 6 Copilot comments. 1 issue accepted and fixed (stop_scan ordering), 5 dismissed with rationale.

@chengkai15
Copy link
Copy Markdown
Contributor

感谢逐条回复,force-cleanup 策略的理由可以接受。

但我评审中的阻断级问题仍未解决:

1. PR 描述为空 — 9817 行代码 body 为空字符串,这是合并阻断项。请补充:架构设计、测试方案、已知限制。

2. FIXME/TODO 残留sal_auracast_sink_interface.c:68 的 FIXME 和 state machine 中的 TODO(derive_mse/derive_sync_timeout)需在合并前解决或转为 issue。

请更新 PR 描述并清理 FIXME/TODO,否则我无法 Approve。

@expliyh
Copy link
Copy Markdown
Contributor Author

expliyh commented Mar 26, 2026

感谢逐条回复,force-cleanup 策略的理由可以接受。

但我评审中的阻断级问题仍未解决:

1. PR 描述为空 — 9817 行代码 body 为空字符串,这是合并阻断项。请补充:架构设计、测试方案、已知限制。

2. FIXME/TODO 残留sal_auracast_sink_interface.c:68 的 FIXME 和 state machine 中的 TODO(derive_mse/derive_sync_timeout)需在合并前解决或转为 issue。

请更新 PR 描述并清理 FIXME/TODO,否则我无法 Approve。

Check commit message instead of PR description

TODO and FIXME are by design and will be implemented in the future.

gzh-terry
gzh-terry previously approved these changes Mar 27, 2026
zhongzhijie1
zhongzhijie1 previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Contributor

@zhongzhijie1 zhongzhijie1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean layered architecture for Auracast Sink: PA Sync → Auracast Sink profile → SAL → Feature/IPC/Tools. Proper Kconfig gating and error handling throughout.

@expliyh expliyh force-pushed the auracast branch 2 times, most recently from fa71575 to 85f1818 Compare April 29, 2026 02:43
gzh-terry
gzh-terry previously approved these changes Apr 29, 2026
chengkai15
chengkai15 previously approved these changes Apr 30, 2026
gzh-terry
gzh-terry previously approved these changes May 8, 2026
@expliyh expliyh force-pushed the auracast branch 2 times, most recently from 1d87b94 to 7a1af25 Compare May 8, 2026 02:40
return msg;
}

void auracast_sink_msg_destory(auracast_sink_msg_t* msg)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destroy

@expliyh expliyh force-pushed the auracast branch 2 times, most recently from 2b93bc5 to 4cf3538 Compare May 8, 2026 08:03
chengkai15
chengkai15 previously approved these changes May 8, 2026
gzh-terry
gzh-terry previously approved these changes May 8, 2026
@expliyh expliyh force-pushed the auracast branch 2 times, most recently from ac7190f to fd9fafd Compare May 8, 2026 13:05
@liujinye-sys liujinye-sys dismissed stale reviews from chengkai15 and gzh-terry May 9, 2026 06:44

The merge-base changed after approval.

expliyh added 3 commits May 12, 2026 10:34
bug: v/82275

Rootcause: Add LE Periodic Advertising (PA) sync functionality including:
- PA sync service with create/terminate/transfer operations
- PA sync report callback for receiving periodic advertising data
- SAL PA sync interface with zephyr stack adaptation
- Framework API, IPC socket layer, and async API support
- Advertiser data parser helper (advertiser_data_parse)
- Kconfig options: BLUETOOTH_PA_SYNC, BLUETOOTH_HCI_FILTER,
  BLUETOOTH_CONNECTION_MANAGER, BLUETOOTH_GATTS_CACHE_SUPPORT,
  BLUETOOTH_LOG_PRIVACY, BLUETOOTH_HFP_AG_LOCAL_TELEPHONY,
  ZBLUE_A2DP_SBC_MAX_BIT_POOL, ZBLUE_A2DP_SOURCE_BUF_SIZE
- Build system integration (CMakeLists.txt, Makefile)

Signed-off-by: liyuheng <liyuheng@xiaomi.com>
bug: v/82275

Implement Auracast Broadcast Sink profile across the full Bluetooth stack,
enabling reception and playback of Auracast broadcast audio streams.

Framework layer:
- Public API (bt_auracast_sink.h) with LC3 codec config, BIS/subgroup
  info structures, and broadcast audio metadata definitions
- Async socket wrappers (bt_auracast_sink_async.c) for non-blocking calls
- Socket IPC transport (bt_socket_auracast_sink.c, bt_message_auracast_sink.h)
- Audio codec number definitions and lookup tables (bt_audio_numbers.h/c)

Service layer:
- Auracast sink service with device lifecycle management, supporting
  multiple concurrent broadcast sources (auracast_sink_service.c)
- State machine with Idle/Enabling/Streaming/Releasing states and
  BIS synchronization tracking (auracast_sink_state_machine.c)
- Event definitions for stack-to-service callbacks (auracast_sink_event.h)

Stack Adaptation Layer (SAL):
- Zephyr-based BIG sync create/terminate and BIS data path setup
  (sal_auracast_sink_interface.c)

Signed-off-by: Zihao Gao <gaozihao@xiaomi.com>
bug: v/82275

Add bttool CLI commands for Auracast Sink testing and debugging,
registered under the "aurasnk" command group in both sync and async modes.

Sync mode (tools/auracast_sink.c):
- PA scan start/stop with nearby broadcast source discovery and aging
- PA sync create/terminate with BASE info reporting
- BIG sync (startReceive/stopReceive) with broadcast code support
- Subgroup and BIS index selection via interactive commands

Async mode (tools/async/auracast_sink.c):
- Mirrors sync mode commands using the async API wrappers
- Adds pending pointer sentinel (AURACAST_SINK_PTR_PENDING) for
  tracking in-flight async operations

Common changes:
- Adapt bt_tools.c adapter state callbacks to support BLE-only builds
  (BT_ADAPTER_STATE_BLE_ON/OFF) when CONFIG_BLUETOOTH_BREDR_SUPPORT
  is disabled
- Register aurasnk command group in bt_tools.h and build system

Signed-off-by: Zihao Gao <gaozihao@xiaomi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants