Wake IC-9700 from standby on explicit connect - #5360
Conversation
f546f32 to
edd3d9b
Compare
|
I’m happy to try it on my 7300mk2 if released. |
|
Will also add, the capability to place the radio in standby mode upon shutting the program down or disconnecting is an option in the other programs I use, not an absolute requirement. |
ten9876
left a comment
There was a problem hiding this comment.
Issue fit
#5349 is real and correctly diagnosed: an IC-9700 grants RS-BA1 transport while its CI-V plane is still asleep, and AetherSDR treated that as connected (0.000.000, dead panadapter). Separating transport-up from radio-ready, and holding a model-neutral "Waking the radio…" surface until a CI-V identity reply lands, is the right design, and it was verified on live hardware. But the readiness/wake state machine is spread across an ad-hoc boolean in the GUI and a second connection boolean in the backend, and the interactions produce several ways to wedge or mis-drive a connection. Six blockers, each verified in the PR-head sources; the eight-angle pass converged tightly on the same root causes and I traced the load-bearing ones to their emit sites myself.
Scope
18 files, mostly explained by the issue. Two items are undisclosed, unrelated scope flagged for your ruling under AGENTS.md's autonomous-boundary rules, not waved through: the isInternalNonFlexCommand() denylist (a UX change to the #5263 loud-drop notice) and the SpectrumWidget connection-animation re-anchoring (a visual-layout change affecting every backend) — both belong in their own PRs. Preflight: no sockets, no fake peers.
Blockers
1. Both in-flow session bounces emit nothing, so the promised "fresh session" never opens (inline). The narrowing of onSessionDisconnected's emit guard to was = m_connectionPublished is the root cause: during conditional-wake the session is authenticated (m_connected=true) but unpublished, so post-wake teardown and the RetrySession path both call onSessionDisconnected("") with was==false and an empty reason — emit disconnected() and emit connectionError() are both skipped, RadioModel::onDisconnected/onConnectionError never run, and m_reconnectTimer is never armed. The wake works in live testing only because MainWindow's unrelated AutoConnectToLastRadio default happens to re-drive the connect; with auto-connect off, or a non-matching last serial, the radio wakes and AetherSDR sits on the overlay until the 75 s watchdog fails it. Verified by tracing the guard to emit disconnected().
2. ConnectPowerAction::Stop is a silent dead-end (inline). Wake budget exhausted (two wakes inside the 5-min cooldown) or an unverified model → both the NG-frame branch and the timeout lambda just return: no publish, no error, no disconnect, m_connectReadinessPending left true so spectrum frames stay gated forever. If the FA/timeout precedes any connectionProgress, the GUI watchdog was never armed either — a headless/automation connect hangs indefinitely holding the radio's single client slot. Pre-PR that radio took the pinned-address path and connected.
3. The directed identity probe removed the broadcast auto-detect and its fallback (inline). For any powerOn-capable model the connect edge sends cmdReadId(m_session->civAddress()) — the seeded address — replacing the broadcast 19 00 that let adoptReportedCivAddress() learn a front-panel-changed CI-V address, and the m_civDetectTimer fallback that connected a silent-but-awake radio. Now an awake IC-9700 re-addressed off 0xA2 (a user-settable option) never connects: silence is misread as standby and answered with wake frames. Keep the broadcast probe (or fall back to it) before treating silence as asleep.
4. m_radioWakeInProgress is never cleared on user disconnect (inline). It clears only on empty connectionProgress, onConnectionError, or the watchdog. Connect a sleeping IC-9700 (watchdog armed, generation N), Disconnect at 20 s, connect a Flex (no connectionProgress, generation unchanged) — at 75 s the stale lambda force-disconnects the healthy Flex, sets m_userDisconnected, and shows "did not finish connecting". The stale flag also suppresses the reconnect dialog for real disconnects in the interim. Clear the flag (and bump the generation) on user disconnect.
5. A late identity reply publishes a stateless session (inline, PLAUSIBLE-verified). An identity reply after the 3 s timeout cleared m_connectIdentityPending still runs publishConnectedSession() (completedReadiness true) but skips sendConnectReadBurst()/applyScopeStartup() (completesConnectProbe false, and the burst was skipped for conditionalWake) — Connected with 0.000.000 and no scope, the exact symptom this PR exists to remove. Reachable on a busy LAN or after the Stop dead-end left the session alive.
6. onConnectionError drops the error text during wake (inline). The new wake early-return reopens the dialog but returns before setting panel/label/status-bar text, so every backend-reported terminal wake failure (RS-BA1 auth, session-open) is reasonless. Compose the message before returning.
Two more that sit just under the blocker line and want a maintainer eye: isConnected() returns m_connectionPublished while every command guard (including civ power standby) reads m_connected, so an automation client can power the radio off during the unpublished wake window; and m_lastConnectWakeUtcMs isn't cleared on success, so for 5 minutes after any wake a genuine CI-V stall is silently swallowed as "finishing startup" and pushed into the same 75 s force-disconnect path instead of the ordinary reconnect dialog.
Nits (non-blocking, condensed)
- Layering / altitude: the wake belongs in a
RadioModelconnection-state enum (Disconnected/Connecting/WakingRadio/Connected/Failed) the GUI renders declaratively, not four!m_radioWakeInProgressspecial cases plus an empty-stringconnectionProgresssentinel carrying the "ready" edge — every new edge risks missing a guard. The pre-readiness leak is gated at one emit (spectrumFrameReady) whilepanCenterBandwidthChangedand the rest stay ungated (which is why 0 MHz reaches the widget and forced the SpectrumWidget re-anchor); gate publication once. - Reuse/metadata (RFC #4984):
power.settinggates oncivAddress != 0xA2+ magic register 146 where the PR's ownPowerOnProfile/existingNetworkConfigurationProfileis the seam; the connect-wake budget re-rolls theCivRecoveryProfile{maxAttempts,intervalMs}pattern as backend constants; the quiesce/resume timer sequence is written four times withwakeForConnectomitting the meter/link stop; the FA/timeout wake-decision preamble is copy-pasted; the reconnect-dialog teardown is now the fifth/sixth copy (one using rawdelete). The wake frame's 0xE1 controller address bypassesIcomSession's 0xE0 echo filter, so the radio's own echo counts as inbound liveness. - Efficiency:
onCivFramecopies the wholeStatsstruct (3 std::strings) per frame just to readinFlightKeywhile the probe is pending seconds/connect; the drop-notice filter runs its 8-way scan before the one-shot early-return; the awake path serializes a full identity round-trip before any state read where addressed reads could run concurrently. - Conventions: the
civhelp string gainedpower …without regenerating the bridge verb table (gen_bridge_docs.py --checkfails — though it also fails on main, so deepened not created); an unbracedifatMainWindow_Session.cpp:712;auto x = foo()forstd::optional<PowerOnProfile>in the new test; the dead"identity.post-wake"scheduler key nothing queues; them_connectWakeAttemptedthird flag already implied bym_connectIdentityPending.
What was verified vs read
- Verified by me in the PR head: the
onSessionDisconnectedemit guard (was = m_connectionPublished) and both bounce call sites;isConnected()returning the published flag while command guards readm_connected; the directed-probe replacement of the broadcast detect; them_radioWakeInProgressclear paths. - From the eight-angle automated pass (forked, ~40 candidates deduped to a verified set; several CONFIRMED by its own signal-wiring trace through
IcomSession::stop/onSessionDisconnected/the reconnect timer) — every adopted finding re-checked before inclusion; the pass also flagged its own PLAUSIBLE (blocker 5) as timing-dependent, which I kept labeled. - Not run: no hardware and no bridge session — the author's live IC-9700 covers the happy path, and blocker 1's own analysis explains why live testing passed despite the defect (the auto-connect rescue). Every blocker is a control-flow argument reproducible from the code.
| qCInfo(lcIcomLink) | ||
| << "IC-9700 wake delay complete; opening a fresh RS-BA1 session"; | ||
| m_session->stop(); | ||
| onSessionDisconnected(QString{}); |
There was a problem hiding this comment.
Blocker 1 — this bounce (and the RetrySession bounce at :1123) emits nothing, so the fresh session never opens. onSessionDisconnected guards its emits on was = m_connectionPublished, which is false throughout conditional-wake — so with an empty reason here, both emit disconnected() and emit connectionError() are skipped, RadioModel::onDisconnected/onConnectionError never run, and m_reconnectTimer is never armed. Verified by tracing the guard to its emit sites. The wake works in live testing only because MainWindow's AutoConnectToLastRadio default re-drives the connect; with auto-connect off or a non-matching serial, the radio wakes and the app hangs on the overlay until the 75 s watchdog. Emit an explicit reconnect trigger (or publish-then-disconnect) so the flow doesn't depend on an unrelated default.
| applyScopeStartup(); | ||
| } | ||
| publishIdentity(); | ||
| if (completesConnectProbe && !m_connectBurstSent) { |
There was a problem hiding this comment.
Blocker 5 — a late identity reply publishes a stateless session. After the 3 s timeout clears m_connectIdentityPending, a 19 00 reply still runs publishConnectedSession() (completedReadiness true) and clears the overlay, but this burst/scope block is gated on completesConnectProbe (== the already-cleared pending flag) and onSessionConnected's burst was skipped for conditionalWake — so Connected with 0.000.000 and no scope, the exact symptom this PR removes. Reachable on a busy LAN or after the Stop dead-end left the session alive.
| && profileFor(*m_modelByName).powerOn.has_value(); | ||
| m_connectReadinessPending = conditionalWake; | ||
| if (conditionalWake) { | ||
| queueConnectIdentityProbe("identity.connect-probe"); |
There was a problem hiding this comment.
Blocker 3 — the directed probe drops the broadcast auto-detect and its fallback. For a powerOn model the connect edge sends cmdReadId(m_session->civAddress()) at the seeded address, replacing the broadcast 19 00 that let adoptReportedCivAddress() learn a front-panel-changed CI-V address and the m_civDetectTimer fallback that connected a silent-but-awake radio. An awake IC-9700 re-addressed off 0xA2 now never connects — silence reads as standby and gets answered with wake frames. Keep/fall back to the broadcast probe before treating silence as asleep.
| } | ||
|
|
||
| const bool startingWake = !m_radioWakeInProgress; | ||
| m_radioWakeInProgress = true; |
There was a problem hiding this comment.
Blocker 4 — m_radioWakeInProgress never clears on user disconnect. It clears only on empty connectionProgress, onConnectionError, or the watchdog firing. Connect a sleeping IC-9700 (watchdog armed, generation N) → Disconnect at 20 s → connect a Flex (no connectionProgress, generation unchanged): at 75 s the stale lambda force-disconnects the healthy Flex, sets m_userDisconnected, shows "did not finish connecting", and in the interim suppresses the reconnect dialog for real disconnects. Clear the flag and bump the generation on user disconnect.
| // A wake failure is terminal for the bounded connect attempt. Clear its | ||
| // overlay and return to the connection panel without briefly presenting | ||
| // the ordinary unexpected-disconnect dialog underneath it. | ||
| if (m_radioWakeInProgress) { |
There was a problem hiding this comment.
Blocker 6 — the wake early-return drops the error text. This branch reopens the connection dialog but returns before setting the panel status text, the status label, and the status-bar message, so every backend-reported terminal wake failure (RS-BA1 auth, session-open) is reasonless. Compose "Error: " + msg into the panel/label/status bar before returning.
edd3d9b to
7390930
Compare
|
Addressed the current-head review in signed commit
Validation:
I could not add myself as assignee: GitHub rejected |
|
Removed the unrelated global |
|
We have pushed updates since the latest review: the latest blocker fixes and scope cleanup are now on the branch; static checks are green and the remaining CI jobs are running (current head `e4b30ac9`). When convenient, @aethersdr-agent, could you please review the current head? Thank you. |
Summary
Closes #5349.
An explicit connection to an IC-9700 now wakes the radio from standby and does not report the connection as complete until the CI-V command plane answers. The implementation is deliberately limited to the IC-9700, the model verified on live hardware.
@aethersdr-agent please review the IC-9700-only scope, the readiness boundary, and the ordinary-connect regression behavior.
Problem
An IC-9700 in standby can accept an RS-BA1 network login and grant UDP streams while its CI-V command plane is still unavailable. AetherSDR previously treated that transport event as a completed connection. The result could be a nominally connected application with frequency at
0.000.000, incomplete controls, and no usable panadapter or waterfall.Sending CI-V Power ON (
18 01) after publishing that false connected edge was not enough. The application must distinguish transport availability from radio readiness and keep the user informed during the transition.Implementation
18 01wake command, and retry the readiness probe.connectedonly after a valid CI-V identity reply; scope traffic alone is not treated as proof that radio control is ready.Model scope and radio prerequisite
The IC-9700's radio-side remote-power permission must already be enabled. AetherSDR does not change that setting. If remote power is disabled, the ordinary already-awake connection path remains unchanged; a radio that does not become CI-V-ready reaches the bounded connection failure instead of a false connected state.
The user-facing wake text is model-neutral so the readiness workflow can be reused after another model is separately verified.
Verification
icom_civ_test: passed.icom_family_test: passed.tools/check_test_registration.py --strict: passed.tools/check_engine_boundary.py --strict: passed with existing baseline warnings only.git diff --check: passed.Test boundary
No socket-based tests were added or run. The new deterministic coverage is socket-free.
The separate RS-BA1 session-lifecycle work discovered during investigation is intentionally not included in this PR.