feat(waveform): Docker waveform install via file upload protocol#3585
feat(waveform): Docker waveform install via file upload protocol#3585NF0T wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Thanks for this, @NF0T — the pcap-derived protocol notes are excellent, the WaveformInstaller is a clean line-for-line mirror of the shipping FirmwareUploader, the dual gzip/ustar validation is well-justified, and the theme-token migration (color.accent / color.text.secondary, applyStyleSheet, themeChanged re-wire) is exactly right. All APIs check out (RadioModel::sendCmdPublic, radioAddress(), flexWaveformModel()), no client-side persistence added, and CI is green across all 6 checks. The on-radio verification (install/remove/reinstall on a FLEX-8400) is great evidence.
One real bug and one minor defensive note:
1. (Likely crash on 2nd install) finished connections accumulate on the reused installer → use-after-free of the stale progress dialog.
In onInstallClicked() the installer is created once and reused (if (!m_installer) m_installer = new WaveformInstaller(...)), but each click adds a new finished connection whose context is this (the dialog), capturing that click's progress by value:
connect(m_installer, &WaveformInstaller::finished,
this, [this, progress](bool ok, const QString& msg) {
progress->close();
progress->deleteLater();
...
});The progressChanged→progress and canceled→m_installer connections auto-disconnect when progress dies (it's the sender/context there), but this finished connection does not — its context is the long-lived dialog. So on the second install, the first install's lambda is still connected and fires with a dangling progress (already deleteLater()'d after install #1), calling progress->close() on freed memory.
Suggested fixes (any one):
- Make the connection single-shot:
connect(m_installer, &WaveformInstaller::finished, this, [...]{...}, Qt::SingleShotConnection); - Or create a fresh
WaveformInstallerper install anddeleteLater()it in the finished handler, instead of cachingm_installer. - Or use
progressas the connection's context object (it outlives nothing here, andthisis captured anyway).
2. (Minor, inherited) Guard onError()/onBytesWritten() against post-completion re-emission.
After a successful upload, onBytesWritten() calls disconnectFromHost() and emits finished(true), but the socket signals stay connected. If the radio closes the connection around the same time, errorOccurred can fire onError(), which (since m_bytesSent > 0) falls through to emit finished(false, ...) — surfacing a spurious "Install Failed" box right after "Install Complete". This pattern is inherited verbatim from FirmwareUploader, but there the radio reboots so the dialog context disappears; here the radio stays up, so it's more likely to surface. A cheap guard — if (!m_installing) return; at the top of onError() (and onBytesWritten()) — closes it. Your hardware test didn't hit it, so this is precautionary, not a blocker.
Nice work overall — fix #1 before merge and this is in good shape.
🤖 aethersdr-agent · cost: $3.5072 · model: claude-opus-4-8
27b2bb9 to
ac237c2
Compare
|
Thanks for the thorough review — both issues addressed in the amended commit ( #1 (SingleShotConnection): Added #2 (post-completion guard): Added |
…nciple VIII. Adds Docker waveform container install to the File → Waveforms dialog, mirroring the SmartSDR Waveform Manager install flow. Protocol confirmed by pcap 4.2.18-waveform-install.pcapng (frame 4496): "file upload <size> waveform_docker_image <filename>" Radio responds with a dynamic TCP port; client streams raw archive bytes. Completion signalled by the existing "waveform container name=… version=…" status push, already handled by FlexWaveformModel::handleContainerStatus. New WaveformInstaller (src/core/) — mirrors FirmwareUploader but uses a single command with the filename embedded (no separate "file filename" step per pcap). Validates the archive before loading: accepts gzip magic (0x1F 0x8B) or POSIX/GNU tar ustar magic at byte offset 257. The dual check is required because newer FlexRadio releases ship uncompressed OCI- layout tars despite the .tar.gz extension (confirmed on freedv-waveform-2.4.0-dev-7132.tar.gz). 500 MB ceiling, 64 KB chunk streaming, 10 s connect timeout with fallback to port 42607. onError() and onBytesWritten() guard on !m_installing in addition to m_cancelled so spurious socket signals after a completed upload cannot emit a second finished() (avoids "Install Failed" after "Install Complete" if the radio closes the connection simultaneously). WaveformsDialog now takes RadioModel* (was FlexWaveformModel*) to give it access to installer dependencies. Adds Install… button (enabled only when WFP is powered and ready), QFileDialog filtered to *.tar.gz *.tgz, and a cancelable QProgressDialog. The finished→dialog connection uses Qt::SingleShotConnection so the lambda auto-disconnects after one install, preventing the captured progress pointer from being called on a subsequent install cycle after deleteLater(). Waveform list refreshes automatically on completion via the existing waveformsChanged signal. Status indicator colors and list badges now resolve from ThemeManager tokens (color.accent, color.text.secondary) with themeChanged wired for live re-paint. Adds lcWaveform (aether.waveform) logging category to LogManager. Legacy (non-Docker) waveform install deferred. Verified OTA on FLEX-8400 (NF0T): fresh install, remove, and reinstall of a Docker waveform container all confirmed working. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ac237c2 to
714e9f8
Compare
Summary
Adds Docker waveform container install to the File → Waveforms dialog, mirroring the SmartSDR Waveform Manager's Install flow.
What's new:
*.tar.gz / *.tgz, streams the image to the radio, and shows a cancelable progress dialog.WaveformInstaller(src/core/) — new class implementing thefile upload <size> waveform_docker_image <filename>protocol confirmed by pcap (fw 4.2.18). Validates the archive before loading (gzip magic0x1F 0x8Bor POSIX tarustarmagic at byte 257; accepts both because newer FlexRadio releases ship uncompressed OCI-layout tars despite the.tar.gzextension). 500 MB ceiling, 64 KB chunk streaming, 10 s connect timeout with fallback to port 42607. MirrorsFirmwareUploaderin structure.WaveformsDialognow takesRadioModel*(wasFlexWaveformModel*) to give it access to the installer's dependencies. Existing Remove/Restart per-row buttons are unchanged. Status indicator colors and list badges now resolve from theme tokens (color.accent,color.text.secondary) and reconnect onthemeChanged.lcWaveform(aether.waveform) logging category added to LogManager.The waveform list updates automatically after install via the existing
FlexWaveformModel::handleContainerStatus/waveformsChangedsignal — no extra wiring required.Legacy waveform install (non-Docker) is deferred.
Constitution principle honored
Principle VIII — Evidence Over Assertion. The install protocol —
file upload <size> waveform_docker_image <filename>, dynamic TCP port, ustar/gzip dual-format validation — is derived entirely from pcap4.2.18-waveform-install.pcapng, not from documentation or assumptions. The plain-tar format discovery (OCI layout, no gzip) was confirmed by inspecting the actual bytes offreedv-waveform-2.4.0-dev-7132.tar.gzbefore widening the validator.Principle XI — Fixes Are Demonstrated. Install of a fresh container, remove, and reinstall of the same container verified OTA on a FLEX-8400 (NF0T).
Test plan
cmake --build /c/AetherBuild)wfp_status power=on ready=trueChecklist
docs/COMMIT-SIGNING.md)AppSettingscalls — waveform state is radio-authoritative; no client-side persistence addedMeterSmoothernot applicableWaveformsDialogtheming: status indicators, list badges, and progress dialog resolve via ThemeManager tokens;themeChangedwired for live re-paint