[dax] [panadapter] Fix multi-client startup panadapter creation#2222
Conversation
There was a problem hiding this comment.
PR #2222 Review — Multi-client startup panadapter creation
Nice work on this one, @jensenpat. The PR description is thorough and the root cause analysis is excellent — understanding how the client_handle filtering interacts with the startup fallback path was the hard part, and you've nailed it.
The overall approach is sound: ensureOwnedPanadapter() centralizes pan creation logic, the display panafall create → panadapter create fallback is the right pattern, and deferring/replaying pan status without client_handle is a clean solution to the ordering problem.
A few things I'd like to flag:
1. m_pendingPanStatuses not cleared on disconnect
onDisconnected() clears m_panadapters, m_activePanId, m_ownedSliceIds, etc., but does not clear m_pendingPanStatuses. If a disconnect occurs while deferred statuses are pending, those stale entries survive into the next connection. On reconnect (possibly to a different radio), ensureOwnedPanadapter() could replay status from the previous session if a pan ID happens to collide.
Suggested fix — add to onDisconnected() alongside the existing cleanup:
m_pendingPanStatuses.clear();2. Waterfall status path — double setWaterfallId risk
In the waterfall status handler, the new code does:
if (ownerPan && ownerPan->waterfallId().isEmpty())
ownerPan->setWaterfallId(wfId);Then a few lines later:
if (activeWfId().isEmpty() && ownerPan == activePanadapter())
ownerPan->setWaterfallId(wfId);The second call is redundant if the first already set it (since activeWfId() delegates to activePanadapter()->waterfallId()). Not a bug — setWaterfallId should be idempotent — but the second block appears to be a leftover from the original setActiveWfId() pattern. Consider removing it to avoid confusion, or adding a comment if there's a subtle ordering reason it needs to stay.
3. handleCreatedPan lambda captures this — consider lifecycle
In createDefaultSlice(), the handleCreatedPan lambda captures this, freqMhz, mode, and antenna by value, then gets copied into the inner legacy-fallback lambda. This is fine as long as the RadioModel outlives the command response, which should be guaranteed by the disconnect cleanup. Just noting it for awareness — the same pattern exists in the current code.
4. parsePanadapterCreateId falls through to bare body
parsePanadapterCreateId() falls back to normalizePanadapterId(body) when neither pan nor id keys are present. If the radio returns an unexpected response body (e.g. an error string that happens to not be caught by the code != 0 check), this could produce a garbage pan ID. The normalizePanadapterId hex parse will return the original text if toUInt fails, which then flows into ensureOwnedPanadapter. The isEmpty() guard catches empty strings but not invalid ones. Low risk in practice — just something to be aware of.
Summary
This is a well-scoped fix that addresses a real multi-client pain point. The deferred-status replay and fallback command pattern are both solid. The one actionable item is clearing m_pendingPanStatuses on disconnect (#1 above) — the rest are minor observations.
Thanks for the contribution and the detailed write-up!
The deferred pan-status queue was added in this PR to hold display pan status that arrives without client_handle until ownership can be confirmed. However, onDisconnected() didn't clear it, which leaks state across reconnects: if AetherSDR disconnects before ownership confirms and reconnects to a different radio, the new radio's pan IDs (typically the same 0x40000000+N range) would replay stale center/bandwidth/min-max values from the previous session through ensureOwnedPanadapter. One-line fix: clear the queue alongside m_panadapters in the disconnect cleanup block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi @jensenpat — Claude here. Pushed a one-line follow-up to your branch directly ( m_pendingPanStatuses.clear();added alongside the existing There's a separate, lower-severity concern with that queue that I filed as a follow-up: #2228 — entries for pans whose owner never emits a Excellent work overall. Capability-based fallback (try-new, retry-legacy) is the right shape, the 73, Jeremy KK7GWY & Claude (AI dev partner) |
Heavy community-contribution release: - AetherSweep — in-panadapter SWR analyzer (jensenpat, 3 PRs) - ShackSwitch antenna switch integration (nigelfenton, 2 PRs) - Multi-client startup hardening: panadapter creation (#2222) + PC audio remote stream ownership (#2226), both jensenpat - macOS DMG dark-theme fix (Chaosuk97) - Windows-without-Qt6::SerialPort build fix (NF0T) - NRL gating correctness on 6000-series (#2219) - CMakeLists.txt / README.md / CLAUDE.md: 0.9.3 → 0.9.4 - CHANGELOG: v0.9.4 entry covers 10 PRs since v0.9.3 - WhatsNewData.cpp: regenerated from CHANGELOG Acknowledgements: - jensenpat — AetherSweep, multi-client startup, PC audio ownership - nigelfenton — ShackSwitch integration - NF0T — Windows build fix - Chaosuk97 — macOS DMG dark theme fix Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Fixes #2194 by making AetherSDR's startup panadapter creation resilient when SmartSDR, DAX, or another GUI/audio client is already connected to the radio.
The primary startup path now matches FlexLib v4.2.18 and creates a display pair with:
If older firmware rejects that command, AetherSDR falls back to the legacy:
The fix also hardens status handling so AetherSDR can recover when panadapter or waterfall ownership status arrives out of order during multi-client startup.
Fixes #2194, #2210, #2203, #2221, #2223, #2224
Root Cause
When SmartSDR DAX is already running, the radio is not starting from a quiet single-client state. On connect, AetherSDR receives status for all current radio objects, including slices, pans, waterfalls, DAX streams, and TX/audio streams associated with other client handles.
AetherSDR correctly filters these objects by
client_handleso it does not accidentally adopt SmartSDR/DAX-owned state. The failure came from the interaction between that filtering and the startup fallback:slice listcan still return non-empty because slices exist globally on the radio.panadapter createcommand and waited too heavily on later pan ownership status.display panstatus was delayed, interleaved, or first arrived withoutclient_handle, AetherSDR could fail to instantiate aPanadapterModel.Without a
PanadapterModel, FFT and waterfall stream IDs are not registered withPanadapterStream, so VITA-49 data never reaches the UI and the startup "getting ready" path can remain stuck.Why DAX Exposed This
The SmartSDR DAX app creates exactly the kind of multi-client environment that stresses this path:
client_handleownershipSo the issue is broader than firmware 4.2.18. The newer firmware and FlexLib API made it more visible, but the underlying bug was that AetherSDR's owned-pan startup path was too dependent on later, perfectly ordered ownership status.
Implementation
This PR changes
RadioModelin a few coordinated ways:pan=0x...,id=0x..., or a bare stream ID.ensureOwnedPanadapter()to create and register aPanadapterModelas soon as AetherSDR has an owned pan ID.display panafall create x=100 y=100first, matching FlexLib v4.2.18.panadapter createifdisplay panafall createfails.display panstatus that lacksclient_handleinstead of dropping it permanently.panadapter=...to recover the parent pan/waterfall association if waterfall status arrives before a complete pan status.Compatibility
Older firmware should continue to work. The new command is tried first because it is what current FlexLib uses, but any rejection falls back to the legacy
panadapter createcommand.This is intentionally capability-based rather than a hard firmware-version gate. The radio/API behavior varies enough across model and firmware combinations that "try current command, fall back to legacy command" is safer than guessing from version strings.
Panadapter and Waterfall Sizing
display panafall create x=100 y=100is only the creation request for the paired display object. It does not set the final UI stream dimensions.AetherSDR still pushes real dimensions after the
PanadapterModelexists with:That matches FlexLib's model: panadapter width/height drive
display pan set ... xpixels/ypixels, while waterfall width/height do not send separate pixel sizing commands. Final FFT/waterfall sizing is therefore still controlled by AetherSDR's widget-dimension path, not by the initialx=100 y=100create command.User Impact
Users should be able to start AetherSDR while SmartSDR DAX or another client is already connected, as long as the radio still has available pan/slice capacity.
If the radio is genuinely out of resources, AetherSDR now routes that failure through the existing status-bar messages instead of silently failing to get a panadapter.
Validation
display panafall create x=100 y=100.CTest completed successfully but reported no registered tests in this build tree.
👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat