feat(automation): TX automation hardening for the agent automation bridge#3727
Conversation
There was a problem hiding this comment.
Reviewed the C++ closely against the current tree. This is clean, well-scoped, and genuinely additive — the per-meter freshness (age_ms) is the right root fix for the "single-instant read catches a stale/zero value" class of meter bug, and the safety rails (watchdog, power ceiling, force-unkey on stop()) are exactly what an agent-drivable TX bridge needs. All six CI checks are green.
Verification I did locally:
- Every new TransmitModel/RadioModel/MeterModel call resolves against
main(atuStatus,atuBypass,atuStart,startTwoToneTune,stopTune,setMox,isTuning/isMox/isTransmitting,model(),meterModel(),transmitModel()). atuStatusName()covers all 10ATUStatusenum values — no fallthrough gap.packetUpdatedMsreferenced inupdateValues()is the existing local at line 417; the newm_valueUpdatedMswrites/erases are kept in sync withm_valuesinremoveMeter()/clear(). ✅- Null-handling is consistent:
doGetchecksradio, anddoTxTest/doAtu/forceUnkey/onTxWatchdogall guardm_radioModel.
Two small, non-blocking robustness notes:
1. Watchdog re-arms after a force-unkey. forceUnkey() resets m_txKeyedSinceMs = 0, so if the radio somehow stays keyed (stopTune/setMox didn't take), the next watchdog tick sees keyed && m_txKeyedSinceMs == 0, re-arms from now, and grants a fresh full m_txMaxKeyMs window before it would try again — i.e. the backstop fires once every ~20 s rather than continuously while still keyed. In practice the unkey should take effect immediately so the risk is low, but for a true last-resort backstop you might prefer to keep issuing the unkey each tick while still-keyed-past-limit (e.g. latch "limit exceeded" instead of resetting the start timestamp in forceUnkey).
2. Misconfigured env can silently disable TX. qEnvironmentVariableIntValue returns 0 for an empty or non-numeric value, so AETHER_AUTOMATION_TX_MAX_MS= (or a typo) sets m_txMaxKeyMs = 0, which force-unkeys on the first watchdog tick of any transmit; AETHER_AUTOMATION_TX_MAX_POWER=0 likewise clamps all power to 0. Consider only overriding the defaults when the parsed value is > 0:
bool ok = false;
const int ms = qEnvironmentVariableIntValue("AETHER_AUTOMATION_TX_MAX_MS", &ok);
if (ok && ms > 0) m_txMaxKeyMs = ms;so a bad value fails safe to the default rather than to "never transmit."
Neither is a merge blocker. Nice work, and thanks for the detailed live-verification log on the FLEX-8400M into a dummy load — the PACURRENT reliability annotation (scoped to one explicit entry with a SMART-11281 reference rather than a heuristic) is a good call. 👍
🤖 aethersdr-agent · cost: $4.7751 · model: claude-opus-4-8
ten9876
left a comment
There was a problem hiding this comment.
Approving (Tier 1 / automation bridge, per AGENTS.md).
TX-keying gating is correct and complete: txtest twotone and atu start (both key TX) are gated behind AETHER_AUTOMATION_ALLOW_TX; txtest off and atu bypass (relay-only) are correctly ungated; the power ceiling, TX watchdog, and force-unkey-on-stop() rails are all ALLOW_TX-armed. These new verbs call startTwoToneTune()/atuStart() directly (bypassing the widget-level aetherTxKeying guard) but carry their own m_txAllowed gate, so the whole keying surface still requires the explicit opt-in.
The age_ms per-meter freshness is the right root fix for the single-instant-stale-read bug class (timestamps kept in sync on removeMeter/clear). Thoroughly hardware-verified on a FLEX-8400M into a dummy load — watchdog fired ~8.5s, power clamp 80/90W→30W, ATU bypass cleaned SWR 1.83→1.15, two-tone drove ALC to −6.4 dBFS, PACURRENT staleness now detectable. CI green on all six; the bot verified the full API surface.
The two robustness items from review are deferred to fast-follow #3743:
- Watchdog should re-issue the unkey each tick while still keyed-past-limit (today it re-arms a fresh window after
forceUnkeyresets the timestamp) — narrow edge (only if the radio ignores an unkey; the primary hung-script threat is handled), but worth tightening on a last-resort safety rail. - Fail safe to defaults on a misconfigured
TX_MAX_MS/TX_MAX_POWERenv (both already fail toward not-transmitting — usability nit).
Landing the verified-on-hardware improvement now; #3743 tracks the hardening. Thanks @jensenpat — excellent live-verification log.
|
Approved ✅ — and the fast-follow for the two robustness items (watchdog re-unkey latch + fail-safe env parsing) is filed as #3743. One thing before it can land: the branch is 7 commits behind Could you rebase |
…fety rails Hardens the agent automation bridge for hardware-in-the-loop transmit and meter testing, addressing the seven improvements found during live TX runs. Bridge (src/core/AutomationServer.*): - get meters — full meter readout: flat TX convenience values (fwd/inst power, swr, paTemp, supplyVolts, swAlc/hwAlc, mic/comp), each with a freshness age, plus the complete `all` meter table (index/source_index/age_ms per meter). - get transmit gains atuStatus (enum→string). - txtest twotone|off — triggers the radio's two-tone test so ALC/PEP meters can be exercised (a steady carrier can't); gated by AETHER_AUTOMATION_ALLOW_TX. - atu bypass|start — bypass takes the tuner out of circuit (no TX) for clean load readings; start is ALLOW_TX-gated. - TX safety rails: a watchdog that force-unkeys the radio after a max continuous key time (AETHER_AUTOMATION_TX_MAX_MS, default 20s), armed automatically whenever ALLOW_TX is set; an optional power-ceiling clamp on RF/Tune power setpoints (AETHER_AUTOMATION_TX_MAX_POWER); and a force-unkey on bridge stop. Model (src/models/MeterModel.*): - per-meter update timestamps (m_valueUpdatedMs) so consumers can reject stale reads. allMeters()/metersForSource() now emit age_ms per meter. This is the root fix for the meter-read flakiness: PACURRENT is only reported sparsely, so a single read catches a stale/zero value that looks real. Tooling: - tools/tx_meter_test.py — windowed, freshness-gated TX meter harness: ensures ATU bypass, keys short bursts, samples meters across the window (median power/ swr, freshest PA current), runs a two-tone ALC pass, restores, with SWR/temp gates and verified unkey. Verified live on a FLEX-8400M into a 100W dummy load (ANT1, antennas removed): watchdog force-unkeyed a held tune at ~8.5s (8s limit); power ceiling clamped 80/90W→30W; ATU bypass gave clean SWR 1.15; windowed sweep tracked tune power ~1:1 (10→9.2W … 100→93.9W); two-tone drove ALC from −40 dBFS (carrier floor) to −6.4 dBFS; PA current confirmed sparsely/erratically reported (fresh 0s) — now detectable via age. All bursts unkey-verified; radio restored and idle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dr#3729) get meters now annotates known-bad meters for the connected radio with "reliable": false + a note, mirroring the UI's curation. Today that is exactly PACURRENT on the FLEX-8000 series, which clips at its declared 10A cap (SMART-11281) and is omitted from MeterApplet — freshness (age_ms) can't catch a fresh-but-clipped value, so an explicit flag is the only safe signal. The list is a small explicit table (unreliableMeterNote), not a heuristic, kept in sync with the UI. Other meters are untouched. tx_meter_test.py honors the flag: a meter the bridge marks unreliable is reported as "unreliable" rather than a number. Verified live on a FLEX-8400M: PACURRENT carries reliable:false + the SMART-11281 note; FWDPWR and all other meters are unflagged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9480f71 to
a5b5ab6
Compare
ten9876
left a comment
There was a problem hiding this comment.
Rebased onto current main and pushed (force-with-lease) — head is now a5b5ab6d, MERGEABLE.
The only conflict was AutomationServer.cpp, where this PR and the since-merged #3722 (validation hardening) + #3729 (PACURRENT flag) all added to the same regions. Resolved by keeping all three (they're complementary):
getmodel dispatch + the unknown-model error list now includeequalizer/eq(#3722),meters(this PR).equalizerSnapshot()(#3722) andmetersSnapshot()(this PR) coexist;metersSnapshotkept its 2-arg PACURRENT-aware form (unreliableMeterNote(name, radio->model())annotating thealltable withreliable:false+note, from #3729).
Verified locally: full app builds clean (AutomationServer.cpp + MeterModel.cpp compile, link OK), and meter_model_test passes 13/13. Diff vs main is +507/−8 — identical footprint to the pre-rebase PR.
Re-approving (the force-push reset the prior approval). My review verdict is unchanged: TX-keying gating correct and complete, hardware-verified; the two robustness items are tracked in fast-follow #3743. I'll merge once CI goes green.
(Commits are committer-signed by me from the rebase; the squash-merge will be GitHub-signed regardless.)
Hardens the agent automation bridge for hardware-in-the-loop transmit and meter testing. Built and verified on a FLEX-8400M into a 100W dummy load (antennas/transverter removed, ANT1). Branched off current
main; independent of #3722 (validation hardening) and #3717 (password redaction).This is the product of several live TX sessions — each item below traces to a concrete thing that bit us.
Meter readout + freshness (the root meter fix)
Meter bugs have been a recurring cluster, and live testing showed why: single-instant meter reads are unreliable because meters update at different rates.
PACURRENTis only reported sparsely, so a single read catches a stale/zero value that looks real (we briefly "measured" 7W at a 50W setting before realizing it was a sampling artifact).MeterModelnow records a per-meter update timestamp (m_valueUpdatedMs);allMeters()/metersForSource()emitage_msper meter (-1 = never). This is the root enabler — a consumer can now reject stale reads.get metersverb: flat TX convenience values (fwd/instant power, SWR, PA temp, supply volts, swAlc/hwAlc, mic/comp) each with a freshness age, plus the fullallmeter table (index/source_index/age_ms— so duplicate-named meters, one live and one floored, are distinguishable).TX test-signal + ATU control
txtest twotone|off— triggers the radio's two-tone test so ALC/PEP meters can be exercised (a steady carrier can't). Verified: ALC moved from −40 dBFS (carrier floor) to −6.4 dBFS. ALLOW_TX-gated. (Compression still needs mic-audio injection via DAX TX — out of scope here, noted as follow-up.)get transmit.atuStatus+atu bypass|start— the ATU was recalling a stored antenna match against the dummy load (the "abnormal SWR" we chased);atu bypasstakes the tuner out of circuit for clean readings (bypassdoesn't transmit;startis ALLOW_TX-gated).Hardware safety rails (auto-armed when
AETHER_AUTOMATION_ALLOW_TX=1)AETHER_AUTOMATION_TX_MAX_MS, default 20s), so a hung/abandoned script can never leave a live transmitter on. Independent of whatever drives the bridge.AETHER_AUTOMATION_TX_MAX_POWER).stop()— the bridge never leaves the radio keyed on shutdown.Tooling
tools/tx_meter_test.py— windowed, freshness-gated TX meter harness: confirms the TX antenna, ensures ATU bypass, keys short bursts, samples meters across the keyed window (median power/SWR, freshest PA current), runs a two-tone ALC pass, restores, with SWR/temp gates and verified unkey after every burst.Live verification (FLEX-8400M, 100W dummy load)
PACURRENTconfirmed sparsely/erratically reported by the radio (fresh 0s mid-transmit) — now detectable viaage_msrather than mistaken for a real reading. ✅Diff
AutomationServer.{cpp,h},MeterModel.{cpp,h},tools/tx_meter_test.py— +465 / −8, additive.💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat