Review: PC audio remote stream ownership fix

Thanks for this thorough investigation, @jensenpat. The root cause analysis — bare 4000009 parsed as decimal instead of hex, producing 0x003d0909 vs 0x04000009 — is convincing, and the fix in parseCreateResponseStreamId directly addresses it. The test coverage for the exact log-derived case is exactly what we need.

Overall

This is well-structured work. Extracting RadioStatusOwnership as a header-only, testable helper with clear enums and pure-function ownership classification is a good pattern. The state machine in RemoteAudioRxTracking is clean and the test exercises the key transitions. The diagnostic surfacing in SliceTroubleshooter and DeviceDiagnostics will make future debugging much easier.

Items worth a look

1. parseCreateResponseStreamId always parses as hex — is that safe for all stream types?

The parseCreateResponseStreamId inner lambda strips 0x and then unconditionally parses as base 16. This is correct for Flex stream IDs (which are always hex), but worth a comment noting this intentional choice, since parseStatusHandle in StreamStatus.h tries toUInt(&ok, 0) first (auto-detecting base) before falling back to hex. The difference is deliberate here, but someone might later try to "unify" these without understanding why.

2. streamCommandId omits the 0x prefix, but hexId includes it

streamCommandId(0x0400000A) returns "0400000a" while hexId(0x0400000A) returns "0x0400000a". The stream remove command uses hexId() in removeRxAudioStream but streamCommandId() in disconnectFromRadio/handleForcedClientDisconnect (via gracefulDisconnect). Is the radio's command parser consistent about accepting both 0x-prefixed and bare hex? The old code used "0x%1" with m_rxAudioStreamId for remove, and bare m_rxAudioStreamId for graceful disconnect. If the radio accepts both forms, this is fine — just want to confirm the intent.

3. ensureOwnedPanadapter captures raw pan pointer in lambda

In ensureOwnedPanadapter, the rfgain_info callback captures pan (a raw pointer). This mirrors the existing code exactly, but if the panadapter is removed before the async response arrives, the lambda holds a dangling pointer. The existing code had the same pattern, so this isn't a regression — just noting it as pre-existing.

4. 350ms settle delay is a magic number

scheduleRxAudioStreamEnsure uses a 350ms QTimer::singleShot. This is reasonable for letting SmartConnect status settle, but consider defining it as a named constant (e.g., kStreamSettleDelayMs) so it's discoverable and tunable if needed. Minor.

5. enable_testing() placement in CMakeLists.txt

enable_testing() is called inside the new test block, but it's typically called once near the top of the root CMakeLists. If it's already called elsewhere (likely), this is harmless but redundant. If not, it should be moved to a more visible location so all tests benefit.

6. Panadapter changes are part of #2222 base — scope note

The PR description correctly notes this is stacked on #2222. The ensureOwnedPanadapter, normalizePanadapterId, m_pendingPanStatuses, createDefaultSliceOnPan, panadapter fallback from display panafall create to panadapter create, and waterfall ownership changes are substantial and should collapse once #2222 lands. After rebase, worth confirming the diff is purely the audio-specific changes.

Looks good

• RemoteAudioRxTracking state machine with createPending/removeRequested flags handles the async race between create-response and status-arrival correctly.
• The deferred-create-then-remove pattern (if (m_rxAudio.removeRequested) removeRxAudioStream()) at the end of the create callback is the right way to handle toggle-off-during-pending.
• allowUnknownOwner keyed to m_clientInfoMap.size() <= 1 is a clean way to handle legacy single-client firmware without a separate code path.
• The duplicate cleanup in createRxAudioStream (removing old stream only when parsed ID differs) avoids the original bug.
• Logging is thorough without being excessive — the summary line format will be easy to grep in user logs.

Nice work tracking this down. The core fix is solid and the test coverage is on point.