Skip to content

fix(gui): SpotHub clear buttons persist, fix counter desync, add console clears (#2022)#3823

Merged
NF0T merged 1 commit into
aethersdr:mainfrom
jensenpat:fix/2022-spothub-clear
Jun 26, 2026
Merged

fix(gui): SpotHub clear buttons persist, fix counter desync, add console clears (#2022)#3823
NF0T merged 1 commit into
aethersdr:mainfrom
jensenpat:fix/2022-spothub-clear

Conversation

@jensenpat

Copy link
Copy Markdown
Collaborator

Issue

Fixes #2022 — SpotHub's Clear buttons didn't remove spots permanently, the
Spot List counter went stale after "Clear All", and none of the six source
console tabs had a Clear button.

Root cause

The Spot List comes from a dialog-local SpotTableModel, but the dialog is
WA_DeleteOnClose. On every open the constructor schedules loadLogFiles(),
which tailFile()s the on-disk source logs
(~/…/AetherSDR/spothub/dxcluster.log, rbn.log, …) and repopulates the
model and consoles. Both Clear buttons only wiped the in-memory model, never
the log files — so spots and console text always came back on reopen.

Two further defects: the Spot List counter (countLabel) was a local
captured only by the Spot List "Clear" lambda, so the Display tab's "Clear
All" — which resets the model via m_totalSpotsLabel only — left it stale.
And the six source consoles had no Clear control at all.

Fix (src/gui/DxClusterDialog.{cpp,h} only)

  • Capture the cluster/RBN/WSJT-X/POTA/SpotCollector/FreeDV log paths as
    members and add truncateLogFile(). The Spot List Clear zeroes the
    cluster + RBN spot logs; the Display tab Clear All zeroes every source
    log and empties every console. Spots and console text no longer return
    after close/reopen.
  • Promote the counter to m_spotCountLabel and drive it from
    rowsInserted / rowsRemoved / modelReset, so it self-corrects no
    matter which path clears the model (fixes the stale count after
    "Clear All").
  • Add a Clear button to each of the six source console tabs that empties
    the console and zeroes its log file.
  • Add tooltips to both spot Clear buttons explaining their differing scope,
    and stable objectNames for automation.

SpotSettingsDialog dead-code removal (mentioned in the issue's "additional
finding") is intentionally out of scope here — it's unrelated to the
Clear-button behavior and belongs in its own change.

How the agent automation bridge proved it

Built with the bridge fidelity improvements applied locally for the proof
only
(PR #3819 is not part of this branch — this commit touches only
DxClusterDialog.{cpp,h}). Seeded dxcluster.log (4 spots) + rbn.log
(3 spots) on disk, launched AETHER_AUTOMATION=1, and opened SpotHub from
the closed Settings menu via the bridge's menu-reachability verb
(invoke "SpotHub..." trigger).

Step Bridge action Reading
Open dialog, spots reload from disk dumpTreespotListCountLabel 7 spots
Six console Clear buttons exist (Bug 3) dumpTree objectNames clusterClearBtn, rbnClearBtn, wsjtxClearBtn, scClearBtn, potaClearBtn, freedvClearBtn
Display Clear All (Bug 2) invoke displayClearAllBtn clickdumpTree spotListCountLabel 7 spots0 spots
Logs truncated (Bug 1) on-disk wc -c dxcluster.log / rbn.log 146/118 B → 0/0 B
Persistence — restart app + reopen relaunch, invoke "SpotHub..." trigger, dumpTree spotListCountLabel 0 spots (spots did not return)
Spot List Clear (other button) re-seed 5 spots, invoke spotListClearBtn click 5 spots0 spots, logs → 0 B

Radio never keyed — RX-only proof; transmitting == false confirmed at
teardown.

Agent automation bridge — gaps found

  • Tooltip not observabledumpTree serializes objectName /
    accessibleName / value but not toolTip, so Bug 5 (the two Clear
    tooltips) can't be asserted through the bridge; verified by code only.
    Proposed: add toolTip to the dumpTree node serialization.
  • No window-close verb — the dialog's title-bar close is a clickable
    QLabel (accessibleName "Close window"), which invoke … click can't
    target, so the reopen/persistence proof required a full app restart.
    Proposed: a close <target> verb (or synthetic-click support for the
    custom title-bar close QLabel).

Otherwise the bridge was sufficient: menu reachability opened the dialog,
invoke click drove both Clear buttons, and dumpTree read the counter and
button inventory directly.

💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat

…ole clears (aethersdr#2022)

SpotHub's two Clear buttons wiped only the in-memory SpotTableModel, so on
the next dialog open loadLogFiles() re-tailed the on-disk source logs and
every spot (and console line) came back. The Spot List counter also went
stale after the Display tab's "Clear All", which never touched it, and none
of the six source consoles had a Clear button at all.

- Capture the cluster/RBN/WSJT-X/POTA/SpotCollector/FreeDV log paths as
  members and add truncateLogFile() so the Clear handlers can zero them.
  The Spot List "Clear" zeroes the cluster+RBN spot logs; the Display tab
  "Clear All" zeroes every source log and empties every console. Spots and
  console text no longer return after close/reopen.
- Promote the Spot List counter to m_spotCountLabel and drive it from
  rowsInserted/rowsRemoved/modelReset, so it self-corrects no matter which
  path clears the model (fixes the stale count after "Clear All").
- Add a "Clear" button to each of the six source console tabs that empties
  the console and zeroes its log file.
- Add tooltips to both spot Clear buttons explaining their differing scope,
  and stable objectNames for automation.

Fixes aethersdr#2022
@jensenpat jensenpat marked this pull request as ready for review June 26, 2026 00:39
@jensenpat jensenpat requested a review from a team as a code owner June 26, 2026 00:39

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jensenpat — this is a well-diagnosed, tightly-scoped fix. The root-cause writeup matches what's in the code, and I verified the key invariants:

  • Spot List "Clear" truncating exactly the cluster + RBN logs is correct. loadLogFiles() only feeds m_spotModel from clusterSpots + rbnSpots (DxClusterDialog.cpp:654-660); WSJT-X/POTA/SC/FreeDV are console-display-only. So zeroing those two logs is precisely the set that backs the spot list — nice.
  • Counter fix is the right shape. Promoting to m_spotCountLabel and driving it from rowsInserted/rowsRemoved/modelReset makes it self-correct regardless of which path mutates the model, which is exactly the "Clear All" desync source. Good.
  • Conventions are clean (Qt parent-ownership for the buttons, no flat-key AppSettings, RAII via layout reparenting). CI is green across all six checks.

A couple of notes, neither blocking:

  1. Truncating a log while its client is actively connected. The clients keep m_logFile open and append each line at the current offset (DxClusterClient.cpp:257-259; opened once with Truncate on connect at :125). An external QFile::resize(0) doesn't move the open writer's file offset, so if a Clear fires mid-session the next write lands at the old offset and leaves a leading NUL hole rather than a truly empty file. The user-visible outcome is mostly benign — the pre-clear bytes become a zero-filled hole that parseSpots() won't match on reopen, so old spots still don't return — but you may see stray blank/NUL lines in the reloaded console and some wasted space until the next reconnect re-truncates. Worth a comment acknowledging it, or (if you want it airtight) routing truncation through the owning client so it can also rewind its open handle. Fine to leave as-is given the persistence goal is still met.

  2. Per-tab console Clear vs. the spot list. Clicking e.g. the Cluster tab's Clear truncates the cluster log (which backs the spot list) but doesn't touch the in-memory m_spotModel/counter, so the count only changes on the next reopen. That's a reasonable scope boundary for a "clear this console" control — just flagging the slight asymmetry in case it surprises anyone.

The destructive aspect (these Clears now delete on-disk logs) is well-communicated through the tooltips and matches the intent of #2022, so no concern there. Holding scope to the Clear-button behavior and deferring the SpotSettingsDialog dead-code removal is the right call.

Nice work — looks good to me.


🤖 aethersdr-agent · cost: $5.1047 · model: claude-opus-4-8

@NF0T

NF0T commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

@NF0T review — PR #3823

Two files, three bugs, one commit. Well-diagnosed and well-scoped.

All three root causes are correct. The dialog is WA_DeleteOnClose, so loadLogFiles() re-tails the on-disk source logs on every open — Clear buttons that only wipe the in-memory model can never win. The counter desync is a direct consequence of countLabel being a local lambda capture connected only to rowsInserted, invisible to modelReset (which "Clear All" triggers). The missing per-console Clears are a straightforward gap.

The implementation is the right shape on all three fixes:

  • truncateLogFile() via QFile::resize(0) is correct. Empty-path and exists guards are present; m_*LogPath members are assigned in the constructor before any click is possible, so the const QString* logPath pointer-to-member in makeConsoleClearButton is stable for the dialog's lifetime.
  • Counter promotion to m_spotCountLabel connected to rowsInserted / rowsRemoved / modelReset covers every mutation path — live inserts, Spot List Clear, and Display "Clear All" (m_spotModel->clear()modelReset). The // modelReset → m_spotCountLabel updates itself comment at the clear call site is the right kind of WHY annotation.
  • HAVE_WEBSOCKETS guard is correct: FreeDV log path capture and FreeDV console clear are both guarded; the unconditional truncateLogFile(m_freedvLogPath) in "Clear All" is safe because the path is empty when the guard is absent and truncateLogFile returns early on empty path.
  • makeConsoleClearButton() factory is clean — button gets Qt parent-ownership via the layout; lambda captures are all stable for dialog lifetime.

Two non-blocking notes worth carrying forward (already flagged by aethersdr-agent):

  1. Mid-session truncation leaves a NUL hole. QFile::resize(0) while a DxClusterClient has the same file open for appending doesn't rewind the writer's file offset — the next write lands at the pre-truncation offset, leaving a zero-filled prefix. User-visible impact is benign (old spots don't return; parseSpots() ignores NUL bytes), but the reloaded console may show stray blank lines until the next reconnect. The persistence goal is met as-is. A full fix would route truncation through the owning client so it can also seek(0) its open handle — appropriate as a follow-up if it surfaces in the field.

  2. Per-tab console Clear vs. in-memory spot list. Clearing the Cluster console truncates the cluster log (which backs the spot list) but doesn't touch m_spotModel or the counter in the current session — the count only drops on next open. Reasonable scope for a "clear this console" control; just flagging the asymmetry.

For future reference (not blocking): Several inline comments carry (#2022) tracker citations in the source (e.g., // so the Clear handlers can zero them. (#2022)). AGENTS.md asks that source comments not reference the task/issue — those go in the PR description and commit message, not the source, where they rot. The WHY content in those comments is solid; just drop the tracker suffix next time.

CI: All six checks green. Pure widget/file logic — no live-radio dependency, fully exercisable by CI. The bridge proof is thorough supplemental verification on top.

Constitution: Principle IX (Surface Only What Survives — dead local-capture counter replaced by properly-wired member) and Principle XI (Fixes Are Demonstrated — per-bug verification table with on-disk size checks and persistence-after-restart proof) both honored.

Fixes #2022 is in both the PR body and commit body — issue auto-closes on merge. Tier 3 only (DxClusterDialog.{cpp,h}* catch-all). Merging now.

@NF0T NF0T self-assigned this Jun 26, 2026

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Root cause correct on all three bugs; fix is correctly scoped to DxClusterDialog.{cpp,h} only. HAVE_WEBSOCKETS guards verified correct; truncateLogFile empty-path guard confirmed; m_spotCountLabel wired to rowsInserted/rowsRemoved/modelReset — covers all mutation paths including the Clear All modelReset that the old local-capture missed. Fixes #2022 in both PR body and commit body. All six CI checks green. Tier 3 only.

Squash subject: fix(gui): SpotHub clear buttons persist, fix counter desync, console clears — Principle XI. (#3823)

@NF0T NF0T merged commit c27dd06 into aethersdr:main Jun 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SpotHub: Spots and console text return after close/reopen; consoles lack Clear buttons; Clear buttons lack tooltips

2 participants