applications: nrf5340_audio: Delete unicast_group if all devices disc…#26975
applications: nrf5340_audio: Delete unicast_group if all devices disc…#26975alexsven wants to merge 2 commits intonrfconnect:mainfrom
Conversation
NordicBuilder
added
the
changelog-entry-required
CI InformationTo view the history of this post, click the 'edited' button above Inputs:Sources:more detailsGithub labels
List of changed files detected by CI (0)Outputs:ToolchainVersion: Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
alexsven
force-pushed
the
OCT-3615-failure-to-switch-to-a-new-unicast-group
branch
from
566b1ec to
73f09af
Compare
There was a problem hiding this comment.
Pull request overview
Updates unicast group lifecycle in the nrf5340_audio unicast client so stale groups are deleted once all devices disconnect, allowing clean group recreation on subsequent connections.
Changes:
- Add additional debug logging around group creation/fullness.
- Skip start parameters for servers that are no longer connected.
- When stopping with no streams, detect whether any servers are still connected; if none are, delete the unicast group and signal caller to recreate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (param.count == 0) { | ||
| LOG_DBG("No streams to stop"); | ||
| k_sem_give(&sem_cap_procedure_proceed); | ||
|
|
||
| /* No streams found. Check if devices are connected, if no, delete the group */ | ||
| bool connected_server_found = false; | ||
|
|
||
| ret = bt_cap_unicast_group_foreach_stream(unicast_group, server_connected_check, | ||
| &connected_server_found); | ||
| if (ret) { | ||
| LOG_ERR("Failed to check if servers are connected: %d", ret); | ||
| return ret; | ||
| } | ||
|
|
||
| if (!connected_server_found) { | ||
| LOG_DBG("No connected servers found, deleting unicast group"); | ||
| ret = bt_cap_unicast_group_delete(unicast_group); | ||
| if (ret) { | ||
| LOG_ERR("Failed to delete unicast group: %d", ret); | ||
| } | ||
|
|
||
| unicast_group = NULL; | ||
| return -EALREADY; | ||
| } | ||
|
|
||
| return -EIO; | ||
| } |
There was a problem hiding this comment.
The semaphore is released before performing additional CAP/group operations (foreach_stream and potential group delete). This can allow other procedures to run concurrently while the group is being inspected/deleted, leading to races. Keep the semaphore held until after the connected-server check and any group deletion completes (and ensure it’s released on all return paths in this branch).
| LOG_ERR("Failed to delete unicast group: %d", ret); | ||
| } | ||
|
|
||
| unicast_group = NULL; |
There was a problem hiding this comment.
This branch clears unicast_group but does not update the separate unicast_group_created state. That can leave the system in an inconsistent state (e.g., unicast_group_created == true while unicast_group == NULL). Update the flag when the group is deleted (ideally in the same place where unicast_group is cleared), or refactor callers to derive ‘created’ from the pointer instead of maintaining two sources of truth.
| unicast_group = NULL; | |
| unicast_group = NULL; | |
| unicast_group_created = false; |
| if (ret == -EALREADY) { | ||
| srv_store_unlock(); | ||
| unicast_group_create(); | ||
| goto start_streams; | ||
| } |
There was a problem hiding this comment.
Using -EALREADY as a control signal meaning 'group was deleted, recreate it now' is non-obvious and can easily collide with an existing/expected 'already stopped/already in desired state' meaning. Consider returning a more appropriate errno (e.g., -EAGAIN to indicate retry/recreate) or using an explicit out-parameter/flag to signal 'group_deleted' so callers don’t have to overload -EALREADY semantics.
|
|
||
| ret = bt_conn_get_info(server->conn, &info); | ||
| if (ret) { | ||
| LOG_ERR("Failed to get connection info for conn: %p", (void *)server->conn); |
There was a problem hiding this comment.
The error log drops the actual error code, making diagnosis harder. Include ret (and optionally the server name) in the message so failures can be correlated and debugged more effectively.
| LOG_ERR("Failed to get connection info for conn: %p", (void *)server->conn); | |
| LOG_ERR("Failed to get connection info for server %s, conn %p: %d", | |
| server->name, (void *)server->conn, ret); |
…onnected - If all devices in a unicast_group has disconnected and a new device connects, we want to delete the old group and create a new one. - OCT-3615 Signed-off-by: Alexander Svensen <alexander.svensen@nordicsemi.no>
alexsven
force-pushed
the
OCT-3615-failure-to-switch-to-a-new-unicast-group
branch
from
73f09af to
0cd277a
Compare
- Fixes bug where an encoder was not initialized before use. - OCT-3616 Signed-off-by: Alexander Svensen <alexander.svensen@nordicsemi.no>
2 participants
…onnected