feat(radio): remote reboot button in Radio Setup (#3334)#3335
Conversation
Add a Reboot Radio button to the Radio Information group in the Radio Setup dialog, directly beneath the Options row. Clicking it pops a confirmation; on accept it sends `radio reboot` (FlexLib Radio.cs:2575), closes the dialog so the panadapter is visible, then drops into the unexpected-disconnect path so the existing 3-second reconnect timer brings the link back when the radio finishes booting. Connection-error toasts are suppressed for the duration of the reboot via a new `m_rebootInProgress` flag — without it, each TCP retry while the radio is still booting fires its own `Connection refused` toast (typically 5–6 over the reboot window). The flag is cleared on the next successful `onConnected()` or on user-initiated `disconnectFromRadio()`. Issue aethersdr#3334 also asked for a `radio reboot initialize` (factory reset) variant; that subcommand does not exist in the FlexLib v4.2.18 API surface, so only the plain reboot is wired up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks @jensenpat — nice, tightly scoped change. The reasoning in the PR description is unusually thorough and saved me from re-deriving most of the flag-lifecycle invariants. Especially appreciated calling out that radio reboot initialize was hallucinated rather than just shipping it.
A few non-blocking notes for consideration:
1. m_rebootInProgress can latch indefinitely if the radio never comes back. onConnectionError is the only place errors surface, and it's gated by the flag — but onConnected() is the only clear path. If the reboot wedges the radio's network stack (the risk the PR description already calls out), the 5 s reconnect timer (RadioModel.cpp:523 — minor: PR description says "3-second", code is setInterval(5000)) keeps firing forever with every refused connect silently swallowed. Operator sees no toasts at all and may not realise the radio is stuck.
A simple fix: time-bound the suppression with a one-shot from rebootRadio():
QTimer::singleShot(60'000, this, [this] {
if (m_rebootInProgress) {
m_rebootInProgress = false; // fail open — surface real errors again
}
});60 s comfortably covers a healthy 6000/8600 boot but flips the UX from "silent forever" to "silent for one minute, then you find out something's wrong."
2. Button not disabled when disconnected. You already flagged this in the unchecked test-plan items — worth mentioning that RadioModel::connectionStateChanged(bool) already exists and is used elsewhere in this dialog, so wiring rebootBtn->setEnabled(isConnected) would be a one-liner. The early-return in rebootRadio() makes it safe-but-silent today; the disabled state would make it discoverable.
3. Race-free by construction — but worth a sanity check. rebootRadio() schedules QTimer::singleShot(250, this, &RadioModel::forceDisconnect) with this (the RadioModel) as receiver. The dialog's synchronous close() happens immediately after; the timer is parented to the model, not the dialog, so it survives. ✅ I traced this just to make sure the close-then-fire ordering didn't open a window where the dialog could be re-shown and confuse anything. It does not.
Nothing here is a merge blocker — (1) is the only one I'd be slightly nervous to ship without, because it changes a "minor inconvenience on success" into a "permanent silence on failure." But happy to defer to your judgement on whether to add it now or as a follow-up.
🤖 aethersdr-agent · cost: $12.9655 · model: claude-opus-4-7
rfoust
left a comment
There was a problem hiding this comment.
Built locally from PR head 3b38b628 on macOS arm64:
git diff --check main..HEADpassedcmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfopassedcmake --build build --parallel 8passed
I found one merge blocker before I would be comfortable landing this.
RadioModel::rebootRadio() currently guards on the LAN socket only:
if (!m_connection || !m_connection->isConnected()) {
return;
}That excludes SmartLink/WAN sessions even though RadioModel::isConnected() already treats m_wanConn as connected, and sendCmd() already routes through m_wanConn. Result: in a WAN session the user can confirm Reboot Radio, the setup dialog closes, but no radio reboot command is sent.
I think this needs one of two explicit choices before merge:
- Support WAN command send by guarding with
isConnected()and keeping the existingsendCommand("radio reboot")path, then adjust the confirmation text because WAN reconnect is manual today. - Make this intentionally LAN-only by disabling/hiding the button for WAN and saying so in the UI/PR description.
Related smaller issue: the button is also enabled when fully disconnected. In that case rebootRadio() returns and the dialog still closes. It should probably initialize from m_model->isConnected() and subscribe to connectionStateChanged(bool).
Protocol scope looks right: local FlexLib v4.2.18 confirms Radio.RebootRadio() sends radio reboot, and I found no reboot initialize, FactoryReset, or Reinitialize command in the FlexLib zip.
Four small fix-ups derived directly from the open review comments on PR aethersdr#3335; intentionally on top of @jensenpat's branch so the original attribution stays clean: 1. WAN guard (rfoust, blocker) — rebootRadio() guarded on m_connection->isConnected() (LAN socket only), but isConnected() and sendCmd() already treat WAN/SmartLink sessions as connected. A SmartLink user clicking Reboot Radio confirmed the dialog and saw nothing happen. Now gates on isConnected() so the command path matches what the rest of the model already supports. 2. Confirmation text (rfoust) — distinguishes WAN from LAN: WAN/SmartLink sessions do not auto-reconnect today, so the warning now tells those users they will need to reconnect manually. 3. Button enabled-state (rfoust + AetherClaude) — initial state reads m_model->isConnected() and the button subscribes to RadioModel::connectionStateChanged so disconnect/reconnect transitions disable/re-enable it without the operator having to reopen the dialog. The rebootRadio() early-return remains as a defensive backstop. 4. 60s fail-open safety timeout (AetherClaude) — if the reboot wedges the radio's network stack the reconnect timer keeps firing "connection refused" forever and the user sees nothing because m_rebootInProgress is gating all the toasts. After 60s the flag flips itself off so real errors surface. 60s comfortably covers a healthy 6000/8600 boot. Local build (RelWithDebInfo, Arch/Linux) clean. Principle XI.
|
@jensenpat — pushed a fix-up commit (8b16b3f) addressing the four open review items, following the same pattern we landed on for #3286. Intentionally on top of your branch so the original feature attribution stays clean. Happy to revert if you'd rather take a swing yourself. Summary:
Local build clean. @rfoust — when you have a moment, this is ready for a re-look against your earlier CHANGES_REQUESTED. I think all four of your items are addressed but you're the closest to it. |
rfoust
left a comment
There was a problem hiding this comment.
Retested after the fix-up commit and after updating this branch onto current upstream/main. Local current-main merge build passed on macOS arm64, and manual test confirms the reboot flow works. The previous blocker is addressed: rebootRadio() now gates on RadioModel::isConnected(), which covers both LAN and SmartLink/WAN, and sendCmd() already routes WAN commands through m_wanConn. The button enabled-state and WAN-specific reconnect wording also look correct. Good to merge once CI is green.
…Mode Addresses NF0T's four CHANGES_REQUESTED items on aethersdr#3286 plus the three items raised in my 2026-06-05 coordination comment. NF0T blockers: 1. Per-widget MeterSmoother repaint gate (was shared-static). The previous shared-static gate let whichever meter ticked first each ~83 ms window through and starved the other N−1 meters that window. With 5 active meters (S-Meter + 3 client-side dynamics applets + comp strip) the effective per-meter cadence dropped from the intended 12 Hz to ~2.4 Hz — visibly stuttery on GR bars and the S-meter needle. MeterSmoother::shouldRepaint() is now an instance method carrying its own QElapsedTimer + lastMs; every active meter independently hits kLeanRepaintHz. SMeterWidget runs its own custom needle animation and never owned a MeterSmoother — it now holds one solely as the gate source so the throttle policy stays uniform across every meter. 2. Principle V — nested-JSON settings for Lean Mode. The flat AppSettings key "LeanMode" is replaced by a "Display" root object containing { "leanMode": "..." }. New header-only helper DisplaySettings (mirrors CwDecodeSettings.h's pattern) wraps the getter/setter and runs a one-shot migration on first launch that reads any legacy flat "LeanMode" key, writes it into the JSON root, and removes the flat key. MainWindow now reads via DisplaySettings::leanMode() and writes via DisplaySettings::setLeanMode(bool). 3. 60 Hz / 30 Hz comment inconsistency. The repaint cap was changed to ~30 Hz (kLeanFrameMs = 33) during the final tuning pass but four comments still said "60 Hz" — updated in MainWindow.cpp/.h, SpectrumWidget.cpp, and SpectrumWidget.h with the constant cross-referenced so future readers see one number. 4. WaveApplet activation round-trip respects pre-Lean state. Previously toggling Lean on disabled the WAVE scope and toggling Lean off unconditionally re-enabled it — clobbering a user who had already disabled WAVE manually. We now snapshot wave->isActive() into m_preLeanWaveActive on Lean entry and restore that value on Lean exit. WaveApplet exposes a new isActive() getter for the snapshot; its setActive() doc is expanded to clarify that the "drop sample feed" framing means appendScopeSamples short-circuits — the upstream AudioEngine::{tx,rx}PostChainScopeReady signal still fires per audio callback, so the savings are no-m_waveform-work + no-repaint, not signal suppression. This follows the same maintainer-fix-up pattern used on aethersdr#3279 (KISS TNC), aethersdr#3335 (Reboot Radio), aethersdr#3350, and aethersdr#3391 (perf connect): stale community PR, NF0T review with concrete blockers, maintainer pushes the addressing commits back to the author's fork to unblock. Principle XI.
Summary
Closes part of #3334.
Adds a Reboot Radio button to the Radio Information group of the Radio Setup dialog, directly beneath the Options row. Confirmation dialog gates the destructive action; on accept, AetherSDR sends
radio rebootto the connected FlexRadio, closes the setup dialog so the panadapter is visible, then drops into the unexpected-disconnect path so the existing 3-second reconnect timer brings the link back when the radio finishes booting.What's in scope vs. out of scope
Issue #3334 requested two commands:
radio rebootFlexLib/Radio.cs:2575(Radio.RebootRadio()→SendCommand("radio reboot"))radio reboot initialize(factory reset)SendCommand("radio reboot initialize"), noFactoryReset/Reinitializemethod anywhere in the FlexLib v4.2.18 source tree. SmartSDR factory reset is a front-panel / utility-menu operation, not exposed on the TCP ASCII API. The issue author's AI assistant appears to have hallucinated it.If FlexRadio adds a reinitialize subcommand to a future firmware revision, we can wire the second variant behind the same button machinery — but shipping a command the radio doesn't understand would silently fail and mislead operators into thinking they triggered a factory reset.
Implementation
RadioModel::rebootRadio()(new,src/models/RadioModel.h/.cpp):m_rebootInProgressflagradio rebootvia the standardsendCommand()pathQTimer::singleShot(250ms, forceDisconnect)— lets the TCP write flush before tearing the socket downforceDisconnect()deliberately does not setm_intentionalDisconnect, so the existing 3-secondm_reconnectTimerengages and brings us back once the radio finishes bootingToast suppression (
m_rebootInProgress):Connection refusedonce every ~3 s while the radio is still booting. Without suppression that produces ~5–6 toasts before the radio comes back. The flag gates theemit connectionError(...)inRadioModel::onConnectionError()so the user gets a clean reconnect experience.RadioModel::onConnected()(next successful handshake) and indisconnectFromRadio()(user-initiated disconnect, so future errors aren't silently suppressed across an unrelated reconnect).connectToRadio()— the reconnect timer calls that on every retry, and the flag must stay on across those retries until the handshake actually completes.RadioSetupDialog(src/gui/RadioSetupDialog.cpp):QPushButtonadded at row 3, column 0 of the Radio Information grid (directly under the Options row at row 2)#3a1a1abg,#ffb080text,#6e3030border) to mark it as destructiveQMessageBox::warningconfirmation with Ok / Cancel — defaults to Cancelm_model->rebootRadio()followed byclose()so the dialog dismisses immediately and the reconnect overlay is visible against the panadapterWhy no separate "Actions" top-level menu
The issue suggested a top-level Actions menu. The Radio Information group is the natural home for radio-level lifecycle controls (it already houses
Remote On,multiFLEX,FlexControltoggles, and lives alongside firmware-update controls in the same dialog), and a dedicated menu for a single command would be premature surface area. If future work adds more remote-management commands (e.g. profile management, network reconfig), an Actions menu can absorb them and the reboot button can move with them — but a one-item menu isn't worth the discoverability cost today.Risks / caveats
Test plan
Connection refusedtoasts during the reboot window are suppressed (issue feat: remote Reboot and Reboot+Reinitialize commands for connected FlexRadio #3334 follow-up)Cancelis selected on the confirmation (no command sent, dialog stays open)rebootRadio()early-returns; the button itself is not greyed — could be tightened)m_rebootInProgressso future errors surface)💻 Generated with Claude Code (Opus 4.7) with architecture by @jensenpat