feat(automation): slice/VFO verbs, DAX/TCI a11y, and a log observability suite#3738
Conversation
There was a problem hiding this comment.
Reviewed the full diff (not just the truncated 500 lines) against the PR branch. Nicely done — this is a careful, well-documented extension of the automation bridge, and CI is fully green (build, check-macos, check-windows, CodeQL, Qt a11y static analysis). Thanks @jensenpat.
What's solid
- Tap threading is correct.
m_tapMutexguards the tap registry; the tap fires synchronously on logging threads but only touches the mutex-guarded ring, never a socket. Crucially, teardown ordering holds:~AutomationServer → stop()callsremoveTap()first, which blocks onm_tapMutexuntil any in-flight tap completes, so no logging thread can touch a half-destroyed server. The "taps must be cheap and MUST NOT log" contract is documented and honored. - Thread confinement of
m_logSubscribersand the drain to the main thread, withm_logSeq/m_logRingreads consistently underm_logMutex, is right. - PII redacted on egress (
redactPiiinlogEventToJson) keeps the in-memory ring cheap while nothing sensitive leaves the process. Good call. - Boundary validation is present everywhere it matters (
doTune/doSlice/doLogreject empty/zero/invalid args), andsetFrequency(double mhz)confirms thetuneverb's MHz unit is correct. - JSON and bare-line request paths both map cleanly to the new verbs, so the
automation_logwatch.pyhelper's JSON requests line up with the parser. - a11y annotations (objectName +
tr()-wrapped accessibleName/Description) are clean and conventional. All files are within the stated scope.
Two minor observations (non-blocking)
-
doMarkseq/mono_us can belong to a later event.qCInfo()runs the tap synchronously and pushes the MARK before returning, butm_logSeq/m_logRing.back()are read on the next line after re-acquiringm_logMutex. A concurrent logging thread can push between those two points, so the returnedseq/mono_usmay be a later event's — andlog tail since=<seq>would then skip whatever was logged immediately after the mark. Narrow window, but bracketing accuracy is the whole point ofmark, so it's the one spot worth tightening (e.g. have the tap hand back the seq it assigned to this specific message rather than re-readingm_logSeq). -
log tail since=<seq>can silently under-report after ring eviction. WithkLogRingMax = 8000, atail since=<old_seq>returns only what's still resident with no signal that earlier matching events were evicted. The response already carries the currentseq; consider also returning the oldest retained seq so a driver can detect a gap rather than mistake a truncated window for a complete one.
Neither blocks merge. Solid work.
🤖 aethersdr-agent · cost: $8.0584 · model: claude-opus-4-8
…ns, slot occupancy Addresses the prioritized blind spots from the bug-scrub: the bridge could drive applet sliders but not the radio's core surfaces (slices, tuning, DAX/TCI). Bridge verbs (src/core/AutomationServer.*): - slice add [freqMhz] | remove <id> | select <id> — slice lifecycle via RadioModel. `add` pre-checks radio-wide slot capacity and refuses with a clear message when full (slices are a MultiFlex-shared resource); `remove` guards the last slice like the GUI does. - tune <freqMhz> — sets the active slice frequency (the VfoWidget is custom- painted, so this was previously undriveable); honors the VFO lock guard. - get radio now reports `maxSlices` + `slots[]` occupancy (ours / foreign / empty + owner station) — diagnoses why a create is refused and is a hook for MultiFlex slice debugging. A11y annotations (now driveable + screen-reader accessible): - DaxApplet "DAX enable"; TciApplet "TCI port" + "TCI server enable". Verified live (FLEX-8400M, ANT1 dummy load): VFO tune tracks (3.72/3.6); DAX/TCI controls resolve and respond to invoke; slot occupancy correctly shows the Maestro client (KI6BCJ) holding slot 0, so slice add honestly refuses (radio at its 2-slice limit). Multi-slice open/close/reopen could not be exercised because the radio is full with the monitoring Maestro connected — needs a free slot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…annel, markers
Adds a high-resolution, push/pull observability layer to the automation bridge
so timer-, meter-, and protocol-reconcile classes of issues can be diagnosed
with the app's own event stream instead of poll-and-reconstruct.
LogManager: a diagnostic tap hook (addTap/removeTap) fanned out from the single
enqueueMessage funnel — every log message is offered to registered taps,
synchronously on the logging thread, after it is handed to the writer. The hash
is empty in normal runs so it is near-free; taps are documented cheap/non-logging.
AutomationServer verbs (all diagnostic — no keying, no TX gating):
- log categories / get / set <cat> <on|off> / set all <on|off> / reset
Runtime QLoggingCategory control via LogManager (no restart): raise verbosity
for a scrub, drop it after. `reset` restores the operator's persisted prefs.
- log tail [n] [since=<seq>]
Pull a snapshot of the recent event ring (default newest 100).
- log subscribe / unsubscribe
Push: stream {"type":"log",...} events to the client as they occur, via a
50 ms main-thread drain so socket writes stay thread-confined.
- mark <text>
Inject an annotation into the same timeline and return its seq + mono_us, so
a driver can bracket actions: mark "START" -> act -> log tail since=<seq>.
Each event carries a process-monotonic microsecond timestamp (QElapsedTimer,
jitter-grade, NTP-immune) plus the wall HH:mm:ss.zzz to line up with the log
file. The ring holds raw text; PII is redacted on egress (redactPii) so the tap
stays cheap and nothing sensitive leaves the process. Tap + drain are torn down
in stop() before the rest of the bridge, so no logging thread can touch a
half-destroyed server.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ream)
Companion to automation_probe.py for the observability suite: LogClient wraps
the log/mark verbs (categories/get/set/reset/tail/mark) and LogStream runs a
dedicated subscribe connection with a background reader thread so streamed
events never collide with request/response traffic. CLI: `automation_logwatch.py
{categories,set,mark,tail,watch}`. Supports both correlation patterns —
pull (mark -> tail since=<seq>) and push (LogStream).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6f4d604 to
acb87c6
Compare
ten9876
left a comment
There was a problem hiding this comment.
Rebased onto current main and pushed (force-with-lease) — head acb87c64, now MERGEABLE.
The conflict was again only AutomationServer.{cpp,h}, against the since-merged #3727 — both add verbs/members/slots to the same regions. Resolved by keeping both throughout (they're complementary):
- Command dispatch + handlers:
txtest/atu(#3727) andslice/tune/log/mark(this PR) coexist. - Header:
onTxWatchdog+TX-rail members (#3727) andonLogDrain+log-channel members (this PR) coexist. start(): both the TX-watchdog arm and the log-tap/drain setup run; the two "listening on (verbs: …)" log lines were merged into one with the union of verbs (that's the only reason the diff is +613 vs the PR's +617 — no functional content dropped).stop(): both the force-unkey teardown and the log-tap detach run.
Verified locally: full app builds clean (AutomationServer.cpp + LogManager.cpp compile, link OK), and meter_model_test + async_log_writer_test (the redactPii path) pass.
Re-approving (force-push reset the prior approval). Verdict unchanged: single redactPii egress chokepoint (reusing the GHSA-audited redactor), correct tap threading, no new keying surface. The two minor observability nits (doMark seq race; tail since under-report after eviction) are worth a fast-follow. I'll merge once CI is green.
(Commits are committer-signed by me from the rebase; the squash-merge will be GitHub-signed.)
… tail eviction-gap signal) (#3756) (#3793) ## Summary Fast-follow to #3738 fixing two accuracy edges in the **agent automation bridge**'s log-correlation helpers (`mark` / `tail`). Both live in `src/core/AutomationServer.cpp` and only affect the *precision* of the `mark → tail` bracketing pattern — neither touches the radio, TX safety, or the captured log data. Severity: low. ## Bug 1 — `doMark` could return a `seq`/`mono_us` belonging to a *later* event (race) `doMark` injected the marker via `qCInfo()` — which runs the log tap synchronously and assigns the marker its sequence — then re-read `m_logSeq` / `m_logRing.back()` on the next line after re-acquiring `m_logMutex`. A concurrent logging thread pushing in that unlocked gap bumps `m_logSeq` and appends a newer event, so **both** the returned `seq` and `mono_us` could belong to an event logged *after* the mark. `log tail since=<seq>` would then **skip whatever was logged immediately after the mark** — exactly what the bracket is meant to capture. **Fix:** capture the marker's own seq/mono *inside* the tap via a `thread_local` sink (`g_markSink`). Because `qCInfo()` runs the tap synchronously on `doMark`'s thread, only that thread's own tap call populates the sink; loggers on other threads see a null sink and leave it untouched (no member-flag race). Falls back to the resident tail only if the tap never fired (i.e. `lcAutomation` info logging disabled). ## Bug 2 — `tail since=<seq>` under-reported silently after ring eviction The ring caps at `kLogRingMax = 8000`. A `tail since=<old_seq>` whose seq had already been evicted returned only the resident suffix with **no signal** that earlier matching events were dropped — a driver could mistake a truncated window for a complete one. **Fix:** include the **oldest retained seq** (`oldest`) in the `tail` response, computed under the same lock. A driver detects a gap whenever `since < oldest`. Additive field, no behavior change for existing callers. `tools/automation_logwatch.py` exposes it via `tail_window()` + `gap_after()` (`tail()` unchanged for compatibility) and documents it. ## How the agent automation bridge proved it Built `RelWithDebInfo`, launched with `AETHER_AUTOMATION=1` (RX-only, no radio, zero TX), and drove the `mark`/`tail` path through `tools/`: **Bug 1 — bracket integrity** ``` mark('FB3756-A') -> seq 10616 emit 25 post-mark log lines tail(since=10616) captured 25 / 25 -> PASS first captured seq 10617 > mark seq 10616 event at seq==mark_seq is the MARK line itself ('MARK FB3756-BRACKET') ``` **Bug 2 — real eviction signal** ``` hold since0 = 10642 generate 8600 marks (overflow the 8000-deep ring) tail() -> seq 20790, oldest 12791 since0(10642) < oldest(12791) -> gap_after = True driver can now DETECT the eviction; before this fix it was silent ``` Before this PR the `tail` response had only `{ok, events, seq}` — `oldest` was absent, so the gap was undetectable. Fixes #3756. Refs #3738, #3646. 💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Extends the agent automation bridge (#3646, gated behind
AETHER_AUTOMATION)with slice/VFO control, DAX/TCI accessibility, and a full
observability suite — runtime log control, a high-resolution event channel,
and timeline markers. Together these let an agent drive the radio's core
surfaces and diagnose timer-, meter-, and protocol-reconcile issues with the
app's own event stream instead of poll-and-reconstruct.
Branches off
main; independent of the other in-flight bridge PRs. The sliceDAX-recall crash fix is filed separately (#3733) and is not included here.
Slice / VFO / accessibility
slice add [freqMhz] | remove <id> | select <id>— slice lifecycle viaRadioModel.addpre-checks radio-wide slot capacity and refuses with aclear message when full (slices are a MultiFlex-shared resource);
removeguards the last slice like the GUI does.
tune <freqMhz>— sets the active slice's frequency. TheVfoWidgetiscustom-painted and was previously undriveable; honors the VFO lock.
get radionow reportsmaxSlices+slots[]— per-slot occupancy(ours / foreign / empty, with the foreign owner's station), so a refused
create is self-explaining and MultiFlex slice contention is observable.
DaxApplet "DAX enable"; TciApplet "TCI port" + "TCI server enable".
Observability suite
LogManagergains a diagnostic tap fanned out from its singleenqueueMessagefunnel — every log message is offered to registered taps,synchronously on the logging thread, after it is handed to the writer. The hash
is empty in normal runs, so it is near-free.
New bridge verbs (all diagnostic — no keying):
log categories | get <cat> | set <cat> <on|off> | set all <on|off> | reset— runtime
QLoggingCategorycontrol with no restart: raise verbosity for ascrub, drop it after.
resetrestores the operator's persisted prefs.log tail [n] [since=<seq>]— pull a snapshot of the recent event ring.log subscribe/unsubscribe— push: stream{"type":"log",...}eventsto the client as they occur, via a 50 ms main-thread drain so socket writes
stay thread-confined while the tap fills the ring from logging threads.
mark <text>— inject a timeline annotation and return itsseq+mono_us, so a driver can bracket actions:mark "START"→ act →log tail since=<seq>.Each event carries a process-monotonic microsecond timestamp
(
QElapsedTimer, jitter-grade, NTP-immune) plus the wallHH:mm:ss.zzzto lineup with the log file. The ring holds raw text; PII is redacted on egress
(
redactPii) so the tap stays cheap and nothing sensitive leaves the process.Tap + drain are torn down first in
stop()so no logging thread can touch ahalf-destroyed server.
A
tools/automation_logwatch.pyhelper (LogClient+LogStream) wraps bothcorrelation patterns: pull (
mark→tail since) on one connection, and push(
LogStream, a dedicated subscribe socket with a reader thread) so streamedevents never collide with request/response traffic.
Why it matters
The event stream surfaces TX→RX protocol round-trip timing at microsecond
resolution — e.g. a
slice tunecommand and itsapd/atu/transmitacks,~7.5 ms apart — which a poll loop structurally cannot resolve. That is exactly
the visibility needed for the timer/meter/protocol-reconcile bug classes.
Testing
Built clean off
main(RelWithDebInfo, Qt 6.11, macOS arm64; full 1214-targetbuild) and validated live on a FLEX-8400M (radio config / RX only here — no
keying):
set/get/reset;mark→tail sincereturned the bracketed window;subscribestreamed events on adedicated connection with µs timestamps.
slots[]occupancy correct;add→ 2 slices;tunetracked;removeclean; per-slice S-meter (
SLC/LEVEL) present on both indices when a secondslice opens.
A broader sweep on the integration build (meters resolution/cadence, control
set/confirm/restore, and a two-tone TX into a dummy load on ANT1 — 32 W, SWR
1.18, PA current 7.67 A) exercised the same verbs end to end; those TX results
live with the TX-hardening work, not this PR.
💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat