[codex] Add automation radio connect command#3851
Conversation
There was a problem hiding this comment.
Pull request overview
Adds automation-bridge support for connecting/disconnecting via the real Connect to Radio workflow (ConnectionPanel → MainWindow → RadioModel), plus documentation and probe-script updates to exercise the new verbs.
Changes:
- Adds
connect/disconnectverbs to the in-appAutomationServer, includingconnect waitwith a deferred response. - Exposes a small automation facade on
ConnectionPaneland assigns stable object/accessibility names to key connection controls. - Updates the automation probe script and bridge documentation to cover the new commands.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/automation_probe.py | Adds CLI support/examples for connect and disconnect requests to the bridge. |
| src/main.cpp | Wires AutomationServer to ConnectionPanel by locating the panel instance on MainWindow. |
| src/gui/ConnectionPanel.h | Adds an automation-facing API surface for local/IP connect + disconnect triggers. |
| src/gui/ConnectionPanel.cpp | Implements automation connection helpers, adds stable object/accessibility names, and improves hostname probe address handling. |
| src/core/AutomationServer.h | Documents new verbs and adds ConnectionPanel hook + deferred connect-wait tracking state. |
| src/core/AutomationServer.cpp | Implements connect/disconnect verbs, deferred connect wait, and shared JSON response writing. |
| docs/automation-bridge.md | Documents connect/disconnect usage in both JSON and bare-line forms. |
| if (selector.isEmpty() || selectorLower == QLatin1String("first")) { | ||
| QPointer<ConnectionPanel> guardedPanel = panel; | ||
| QTimer::singleShot(0, qApp, [guardedPanel] { | ||
| if (!guardedPanel) { | ||
| return; | ||
| } | ||
| QString error; | ||
| if (!guardedPanel->automationConnectLocalFirst(&error)) { | ||
| qCWarning(lcAutomation).noquote() | ||
| << "connect local first failed after scheduling:" << error; | ||
| } | ||
| }); | ||
| return QJsonObject{ | ||
| {QStringLiteral("ok"), true}, | ||
| {QStringLiteral("connect"), QStringLiteral("local")}, | ||
| {QStringLiteral("selector"), QStringLiteral("first")}, | ||
| {QStringLiteral("requested"), true}, | ||
| {QStringLiteral("deferred"), true}, | ||
| {QStringLiteral("radio"), connectionRadioToJson(radios.first())}, | ||
| }; | ||
| } |
04d469e to
b44f5bc
Compare
|
Follow-up pushed in
Validation after the follow-up:
|
There was a problem hiding this comment.
Thanks for this, @rfoust — this is a well-structured addition. Routing every connect/disconnect through ConnectionPanel → MainWindow → RadioModel instead of bypassing app logic is the right call, and the boundary validation (empty serial, unknown selector, already-/not-connected guards, clamped wait timeout) is thorough. The ConnectWait lifecycle is handled correctly in all three teardown paths (stop(), onDisconnected(), finishConnectWait() — timer stopped + deleteLater, signal disconnected, erased from the vector), and deferring the actual connect onto a singleShot(0, qApp, …) to avoid running modal connection-conflict UI inside the local-socket read callback is a nice touch. I verified the RadioInfo fields used in connectionRadioToJson all exist, and CI is green across all six checks.
A few notes:
Copilot's finding is a false positive. Copilot flagged that connect local first schedules automationConnectLocalFirst(), which re-selects row 0 at execution time and could target a different radio if discovery reorders m_radios. But that's not what the code does — doConnect captures selected.serial synchronously before scheduling, then defers automationConnectLocalSerial(selectedSerial), which re-resolves by serial. That's exactly the deterministic behavior Copilot recommended, and the response's serial/radio fields stay consistent with what actually gets targeted. No change needed here.
Minor — dead code. A consequence of the above: ConnectionPanel::automationConnectLocalFirst() (ConnectionPanel.cpp:766 / .h:34) is now never called — the bridge uses the capture-serial-then-automationConnectLocalSerial path instead. Consider removing it, or keep it only if you intend it as a documented convenience API.
Nice correctness fix in scope. The probeRadio change to fall back to sock->peerAddress() when QHostAddress(trimmedIp) is null correctly handles the connect ip <hostname> case the docs advertise, where the old QHostAddress(trimmedIp) would have produced a null address for a hostname.
Heads-up (not blocking): the PR currently shows as merge-conflicted against main (base is a few commits behind) — it'll need a rebase before it can merge.
Overall this looks solid and ready once the dead-method nit and rebase are sorted.
🤖 aethersdr-agent · cost: $4.4341 · model: claude-opus-4-8
b44f5bc to
b94f3f4
Compare
b94f3f4 to
dccfd18
Compare
|
Update after the latest review pass:
Validation on final head
|
ten9876
left a comment
There was a problem hiding this comment.
Approved. Reviewed the security posture + correctness:
- Opt-in only — the bridge (and these verbs) start only with
AETHER_AUTOMATIONset, over a local socket; no new always-on/remote surface. - No bypass — connect/disconnect route through
ConnectionPanel → MainWindow → RadioModel, respecting multiflex/conflict UI. - TX-safety intact — connecting establishes a session, it doesn't key;
kTxKeyingProperty/ALLOW_TXstill gates every keying verb. - Boundary validation (Principle VII) thorough (empty serial / unknown selector / already-/not-connected / clamped
wait). - Lifecycle clean:
ConnectWaittorn down in all three paths; connect deferred viasingleShot(0)so modal conflict UI never runs in the socket callback. - Copilot's
connect local firstrace is a false positive — serial is captured synchronously then re-resolved by serial.
One non-blocking nit (dead automationConnectLocalFirst()) filed as a fast-follow. CI green on all 6. Thanks @rfoust — clean, and exactly the verb I was missing for bridge-driven debugging.
## Release prep for v26.6.5 Docs + version-string updates for the **50 merges** since v26.6.4. No code change — safe to merge without a rebuild. ### Changes - **CHANGELOG.md** — new `[v26.6.5] — 2026-06-28` section, headlined by **KiwiSDR receive sync** (#3872), **SmartMTR TX meters** (#3776), the **PROF profile-switcher applet** (#3829), and a large **agent automation bridge** expansion (#3851/#3856/#3883/#3842/#3832/#3819); categorized Added/Changed/Fixed/Performance/Removed compiled from the full merge list. Folds in the prior `[Unreleased]` entries and resets `[Unreleased]` to empty. - **CMakeLists.txt / README.md / AGENTS.md** — `26.6.4` → `26.6.5`. - **ROADMAP.md** — current cycle → `post-v26.6.5`; four v26.6.5 highlights added atop *Recently shipped*. ### Not included - Tagging `v26.6.5` is a separate, explicit step (not done here). - The untracked `docs/aetherd-headless-engine-design.md` RFC and `prototypes/` are intentionally left out. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Adds semantic automation-bridge support for showing the Connect to Radio dialog and for connecting/disconnecting through the real Connect to Radio workflow. The bridge now supports
connect list,connect show,connect hide,connect local first,connect local serial <serial>,connect ip <host-or-ip>,connect wait <timeout_ms>, anddisconnect, all routed throughConnectionPanel/MainWindow/RadioModelinstead of bypassing application logic. Also adds stable object/accessibility names to connection-panel controls and documents the new commands in the automation bridge guide.Constitution principle honored
Principle VII — boundary input validation. The new automation commands validate selectors, timeouts, and connection state at the bridge boundary, then route only valid requests through the existing connection UI/model path.
Test plan
cmake --build build --target AetherSDR --parallel)connect local serial <serial>, verifiedconnected:true, then disconnected and verifiedconnected:false. Local serial/IP details intentionally omitted from the public PR body.Additional local validation:
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfocmake --build build --parallelcmake --build build --target AetherSDR --parallelpython3 -m py_compile tools/automation_probe.pypython3 tools/check_a11y.py src/gui/ConnectionPanel.cpp src/gui/MainWindow.cppgit diff --checktools/automation_probe.py pingtools/automation_probe.py connect listtools/automation_probe.py connect showtools/automation_probe.py connect hidetools/automation_probe.py connect wait 5tools/automation_probe.py connect local serial <serial>tools/automation_probe.py connect wait 30000tools/automation_probe.py get radiotools/automation_probe.py disconnectChecklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key(Principle V)
reverse-engineered from a proprietary binary (Principle IV)
MeterSmoother(AGENTS.md convention)