Skip to content

RDKOSS-1008: Wait for A2DP disconnect confirmation in BTRMGR_DeInit - #116

Merged
DamianoBaroneSky merged 7 commits into
developfrom
RDKOSS-1008
Jul 23, 2026
Merged

RDKOSS-1008: Wait for A2DP disconnect confirmation in BTRMGR_DeInit#116
DamianoBaroneSky merged 7 commits into
developfrom
RDKOSS-1008

Conversation

@natrajmuthusamy

@natrajmuthusamy natrajmuthusamy commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Reason for change : Added wait time (max 500ms) for audio device disconnect confirmation before exiting D-Bus. Without this, btmgr exits while AVDTP CLOSE signaling is still in-flight, causing bluetoothd to crash in clear_endpoint when the response arrives after endpoint destruction.

Reason for change : Added wait time (max 1s) for audio device disconnect confirmation before exiting D-Bus. Without this, btmgr exits while AVDTP CLOSE signaling is still in-flight, causing bluetoothd to crash in clear_endpoint when the response arrives after endpoint destruction.

Test Procedure: Regression test cases should be performed.
Risks: High
Signed-off-by: Natraj <Natraj_Muthusamy@comcast.com>
Copilot AI review requested due to automatic review settings July 20, 2026 14:39
@natrajmuthusamy
natrajmuthusamy requested a review from a team as a code owner July 20, 2026 14:39

Copilot AI left a comment

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.

Pull request overview

This PR updates BTRMGR_DeInit to wait (up to 500ms) for A2DP-related disconnect confirmation before tearing down D-Bus, reducing the chance of bluetoothd crashing when late AVDTP CLOSE responses arrive after endpoint destruction.

Changes:

  • After initiating a disconnect for audio devices (speakers/headset), poll for a disconnect-confirmation status for up to 500ms before continuing shutdown.
  • Add logging for disconnect-confirmation success/timeout during deinitialization.

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

Comment thread src/ifce/btrMgr.c Outdated
Copilot AI review requested due to automatic review settings July 21, 2026 09:22

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/ifce/btrMgr.c:4131

  • The disconnect-confirmation polling block has a few issues: (1) it hard-codes a 1000ms max wait (10×100ms) but the PR description says 500ms, (2) it always sleeps 100ms before the first status check (adding an avoidable delay even when already disconnected) and the reported time can be wrong (often 0ms), and (3) the elapsed value is printed with "%d" even though it’s an unsigned expression (format mismatch/UB). Consider restructuring to check-first then sleep until an explicit max-wait is reached, and log success at INFO/DEBUG rather than WARN.
                if (lenBtrCoreDevTy == enBTRCoreSpeakers || lenBtrCoreDevTy == enBTRCoreHeadSet) {
                    unsigned int ui32PollCount = 10;  /* 10 × 100ms = 1000ms max */

                    do {
                        usleep(100000);  /* 100ms */

Copilot AI review requested due to automatic review settings July 22, 2026 07:13

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/ifce/btrMgr.c:4128

  • In BTRMGR_DeInit, the disconnect confirmation loop currently (a) can run even if BTRCore_DisconnectDevice fails (return value is not stored) and (b) waits up to 700ms (7×100ms), which conflicts with the PR description’s “max 500ms”. The elapsed-time log also uses (10 - ui32PollCount) which reports incorrect durations (e.g., 300ms when only 100ms elapsed). Consider storing the disconnect return, polling only on success, using a 500ms max, and computing the elapsed time from the number of polls performed.
                if (BTRCore_DisconnectDevice(ghBTRCoreHdl, lstConnectedDevices.m_deviceProperty[ui16LoopIdx].m_deviceHandle, lenBtrCoreDevTy) != enBTRCoreSuccess) {
                    BTRMGRLOG_ERROR ("Failed to Disconnect - %llu\n", lstConnectedDevices.m_deviceProperty[ui16LoopIdx].m_deviceHandle);
                }
                if (lenBtrCoreDevTy == enBTRCoreSpeakers || lenBtrCoreDevTy == enBTRCoreHeadSet) {
                    unsigned int ui32PollCount = 7;  /* 7 × 100ms = 700ms max */

Copilot AI review requested due to automatic review settings July 22, 2026 12:18

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/ifce/btrMgr.c:4133

  • The new disconnect-confirmation polling has a few correctness issues: it always sleeps 100ms before the first status check (adding avoidable delay even when the device is already disconnected), the loop never decrements the poll counter on success (so the reported elapsed time can be 0ms even though a sleep already occurred), and it currently waits up to 1000ms despite the PR description stating 500ms. Also, successful confirmation is logged as WARN and the elapsed time uses a signed format specifier.

Consider checking the disconnect status immediately, limiting the wait to 500ms, and logging success at DEBUG/INFO level.

                if (lenBtrCoreDevTy == enBTRCoreSpeakers || lenBtrCoreDevTy == enBTRCoreHeadSet) {
                    unsigned int ui32PollCount = 10;  /* 10 × 100ms = 1000ms max */

                    do {
                        usleep(100000);  /* 100ms */
                        lenBtrCoreRet = BTRCore_GetDeviceDisconnected(ghBTRCoreHdl,lstConnectedDevices.m_deviceProperty[ui16LoopIdx].m_deviceHandle,lenBtrCoreDevTy);
                    } while ((lenBtrCoreRet != enBTRCoreSuccess) && (--ui32PollCount));

Comment thread src/ifce/btrMgr.c
@DamianoBaroneSky
DamianoBaroneSky merged commit 3a0000b into develop Jul 23, 2026
13 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants