Skip to content

refactor(gui): extract pure helpers from MainWindow.cpp (#3351 Phase 0)#3508

Merged
jensenpat merged 1 commit into
mainfrom
refactor/3351-phase0-helpers
Jun 10, 2026
Merged

refactor(gui): extract pure helpers from MainWindow.cpp (#3351 Phase 0)#3508
jensenpat merged 1 commit into
mainfrom
refactor/3351-phase0-helpers

Conversation

@ten9876

@ten9876 ten9876 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

First PR of the #3351 monolith-decomposition series. Pure code motion — no behavior change.

Extracts the ~20 file-scope static helpers that have no MainWindow dependency (no members, no mutable file-scope state) from MainWindow.cpp into a new MainWindowHelpers.{h,cpp}:

Group Functions
Network diagnostics tooltip formatNetworkMs, formatNetworkSeqErrors, buildNetworkTooltip
TNF tooltip tnfFrequencyHz, formatTnfFrequency, formatTnfDepth, buildTnfTooltip
Memory/passive spot-ID math memorySpotId, memoryIndexFromSpotId, isPassiveLocalSpotId, memorySpotLabel, memorySpotComment + the two ID-base constants (used only by these)
Client connection parsing splitClientField, parseClientHandle, buildDisconnectClients ×3, cleanClientDisplayText, clientConnectionStatusMessage
Misc buildBandStackIndicatorPixmap, shortcutSequenceFromKeyEvent, macDaxDriverInstalled

Deliberately left behind: the shortcut-lease helpers (textInputCaptured, shortcutGuard, leaseHolderBusy, …) — they read the mutable statics s_keyboardShortcutsEnabled / s_sliderShortcutLeaseActive and are destined for the Shortcuts TU in Phase 1. The new header documents the rule: state-coupled helpers don't belong in this file.

Notes

  • Functions lose internal linkage (static → namespace-scope external). All live in namespace AetherSDR; no ODR risk.
  • The long-standing Linux -Wunused-function: macDaxDriverInstalled warning at the tail of every build disappears — its only caller is inside #ifdef Q_OS_MAC, and external-linkage functions don't trigger that warning.
  • One transplant fix: the CategoryStats convenience overload of formatNetworkSeqErrors became formatCategorySeqErrors (anon-namespace, private to the new TU). As an anonymous-namespace overload of the same name it would have hidden the namespace-scope two-int overload from unqualified lookup inside the TU.

Numbers

  • MainWindow.cpp: 19,474 → 19,138 lines
  • New files: MainWindowHelpers.h (90), MainWindowHelpers.cpp (379)

Verification

  • Full build clean (Linux x86, RelWithDebInfo)
  • All 32 ctest suites pass
  • Diff is reviewable as: new files = verbatim copies of the deleted statics (modulo the one rename above); MainWindow.cpp = deletions + one #include

Next in the series (separate PRs): Phase 1a MainWindow_Controllers.cpp, then the remaining per-seam TUs per the plan on #3351.

Refs #3351

@ten9876 ten9876 requested review from a team as code owners June 10, 2026 16:32
@aethersdr-agent

Copy link
Copy Markdown
Contributor

Hi @ten9876, thanks for the PR — and don't worry, this CI failure is not caused by your changes. 🎉

What failed

On commit b48c3aa, the check-windows job (run 27290587519) failed at the "Setup hidapi (HID encoder support)" step — an environment-provisioning step that runs before your code is ever configured or compiled. The Configure and Build steps were skipped as a result, so the failure tells us nothing about the code in this PR.

That step runs scripts/setup/setup-hidapi.ps1, which downloads the hidapi 0.14.0 source tarball from GitHub and builds it with MSVC on the runner (see .github/workflows/ci.yml, the step right after "Setup MSVC environment"). Failures here are almost always transient — a flaky download from the GitHub release CDN or a hiccup on the Windows runner — and unlike windows-installer.yml, the check-windows job doesn't cache the hidapi build, so it re-downloads and rebuilds it on every run.

Why we're confident it's infrastructure, not your code

  • Your PR touches CMakeLists.txt, src/gui/MainWindow.cpp, and the new src/gui/MainWindowHelpers.{h,cpp} — nothing related to CI workflows, setup scripts, or hidapi.
  • Every check that actually compiled your code passed: check-macos ✅, build ✅, analyze (cpp) ✅, CodeQL ✅, Qt Accessibility Static Analysis ✅.
  • The Windows job died before reaching the Configure step, so your code was never even handed to the compiler there.

How to fix it

Just re-run the failed job — no code changes needed. If you don't have re-run permissions on this repo, pushing an empty commit works too:

git commit --allow-empty -m "ci: retrigger check-windows" && git push

or a maintainer can re-run it from the Actions page.

Thanks for taking on the MainWindow decomposition work (#3351 Phase 0) — extracting those stateless helpers into their own TU is exactly the right first slice, and the header comment documenting the "no MainWindow state" rule for future additions is a nice touch. 👍


🤖 aethersdr-agent · cost: $7.5456 · model: claude-fable-5

ten9876 added a commit that referenced this pull request Jun 10, 2026
…#3509)

## Summary

**Repo-wide CI blocker.** Every `check-windows` run since 2026-06-10
~14:36 UTC fails at the **Setup hidapi** step, before any AetherSDR code
compiles. Last green run on main: 04:10 UTC. Every PR branch since fails
identically (#3508, the AetherClaude bot's `aetherclaude/issue-3506`
branch).

## Root cause

GitHub's `windows-latest` runner image updated to **CMake 4.x**, which
removed compatibility with `cmake_minimum_required(<3.5)`. hidapi
0.14.0's CMakeLists declares an older minimum, so
`scripts/setup/setup-hidapi.ps1`'s source build now hard-fails at
configure:

```
CMake Error at CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 has been removed from CMake.
  ...
  Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.
```

## Fix

One flag on the configure line — the exact override CMake's own error
message recommends:

```powershell
cmake -B $buildDir -S $srcDir.FullName -G "Ninja" `
    -DCMAKE_BUILD_TYPE=Release `
    -DBUILD_SHARED_LIBS=ON `
    -DCMAKE_POLICY_VERSION_MINIMUM=3.5
```

Configure proceeds with 3.5 policy semantics; the compiled artifact is
unaffected. Inline comment documents when the flag can be dropped
(hidapi version bump with a modern minimum).

## Sibling-script audit

Checked the other Windows setup scripts for the same latent failure:

| Script | cmake source build? | Declared minimum | At risk? |
|---|---|---|---|
| `setup-hidapi.ps1` | yes — hidapi 0.14.0 | < 3.5 | **this PR** |
| `setup-opus.ps1` | yes — opus 1.5.2 | 3.16 | no (verified: builds
locally under CMake 4.3.3) |
| `setup-onnxruntime.ps1` | no (informational echo only) | n/a | no |

## Validation

This PR's own `check-windows` run exercises the fixed script — green CI
on this PR **is** the validation.

Once merged, #3508 (and the bot's PR) just need a CI re-run.
…Helpers (#3351 Phase 0)

First PR of the #3351 monolith-decomposition series. Pure code motion —
no behavior change.

Moves the ~20 file-scope static helper functions that have no MainWindow
dependency (no members, no mutable file-scope state) out of
MainWindow.cpp into a new MainWindowHelpers.{h,cpp}:

- Network diagnostics tooltip: formatNetworkMs, formatNetworkSeqErrors,
  buildNetworkTooltip
- TNF tooltip: tnfFrequencyHz, formatTnfFrequency, formatTnfDepth,
  buildTnfTooltip
- Memory/passive spot-ID math: memorySpotId, memoryIndexFromSpotId,
  isPassiveLocalSpotId, memorySpotLabel, memorySpotComment (plus the
  kMemorySpotIdBase / kPassiveSpotIdBase constants, used only by these)
- Client connection parsing: splitClientField, parseClientHandle,
  buildDisconnectClients ×3, cleanClientDisplayText,
  clientConnectionStatusMessage
- Misc: buildBandStackIndicatorPixmap, shortcutSequenceFromKeyEvent,
  macDaxDriverInstalled

Deliberately left behind: the shortcut-lease helpers (textInputCaptured,
shortcutGuard, leaseHolderBusy, etc.) — they read the mutable file-scope
statics s_keyboardShortcutsEnabled / s_sliderShortcutLeaseActive and are
destined for the Shortcuts TU in Phase 1. The header documents the rule:
state-coupled helpers don't belong in this file.

Two incidental notes:

- The functions lose internal linkage (static → namespace-scope external).
  No ODR risk: names are project-prefixed contextually and live in
  namespace AetherSDR.
- The long-standing Linux "-Wunused-function: macDaxDriverInstalled"
  warning at the tail of every build disappears — the function's only
  caller is inside #ifdef Q_OS_MAC, and external-linkage functions don't
  trigger that warning.

One transplant fix: the CategoryStats convenience overload of
formatNetworkSeqErrors became formatCategorySeqErrors (anon-namespace,
private to the new TU) — as an anonymous-namespace *overload* it would
have hidden the namespace-scope two-int overload from unqualified lookup.

MainWindow.cpp: 19,474 → 19,138 lines.
Verified: full build clean, all 32 ctest suites pass.

Refs #3351. Principle XI.
@ten9876 ten9876 force-pushed the refactor/3351-phase0-helpers branch from b48c3aa to c5a0e4a Compare June 10, 2026 19:54

@jensenpat jensenpat 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.

Reviewed line-by-line: pure code motion as advertised — new files are verbatim copies of the deleted statics, constants preserved, the formatCategorySeqErrors rename is correct and applied at all call sites. All CI green.

@jensenpat jensenpat merged commit 0027834 into main Jun 10, 2026
6 checks passed
@jensenpat jensenpat deleted the refactor/3351-phase0-helpers branch June 10, 2026 23:11
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.

2 participants