refactor(gui): split controller methods into MainWindow_Controllers.cpp (#3351 Phase 1a)#3516
Conversation
…rollers.cpp (#3351 Phase 1a) Second PR of the #3351 monolith-decomposition series. Pure code motion — no behavior change, no header changes to MainWindow.h (a C++ class may define its members across any number of translation units). Moves two blocks out of MainWindow.cpp into the new MainWindow_Controllers.cpp (1,892 lines): 1. The contiguous external-controller method block (was lines 6682-7983, ~1,300 lines): - FlexControl: showFlexControlDialog, syncFlexControlDialog, syncFlexControlIndicatorForSettings, setFlexControlHardwareIndicator, handleFlexControlTuneSteps, handleFlexControlButton, handleVirtualFlexControlWheel, applyFlexControlWheelAction - HID encoders: hidEncoderDefaultAction, hidEncoderDefaultPushAction, dispatchHidAction - Icom RC-28: rc28HoldActionActive, updateRC28Leds - TMate 2: the full overlay/display/status/indicator group (9 methods) - StreamDeck: refreshStreamDeckLabels - Support bundle: buildControlDevicesSnapshot 2. registerMidiParams() (was lines 18083-18569, ~485 lines inside #ifdef HAVE_MIDI). Supporting moves, each forced by the split and called out for review: - flexWheelModeForAction / flexControlButtonAction moved from MainWindow.cpp's anonymous namespace into the new TU's anonymous namespace — the moved methods are their only callers. - kTMate2DefaultOverlayDurationMs duplicated-by-move into the new TU's anonymous namespace (its only remaining user). - kCw*ActionId / kCw*ActionName constants promoted from MainWindow.cpp file-statics to inline constexpr in MainWindowHelpers.h — they are now genuinely cross-TU (shortcut registry in MainWindow.cpp, MIDI/HID registries in the new TU). Breadcrumb comments at both excision sites point to the new file. Constructor wiring for these subsystems intentionally still lives in MainWindow.cpp — it moves in the constructor-decomposition phase. MainWindow.cpp: 19,138 → 17,308 lines. Verified: full build clean, all 32 ctest suites pass, a11y lint clean. Refs #3351. Principle XI.
|
Review note (non-blocking): |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed with the code-motion claim verified mechanically: I diffed the lines removed from MainWindow.cpp against the lines added in MainWindow_Controllers.cpp, and they match verbatim except for the documented exceptions (new-TU preamble/includes/namespace wrappers, and the kCw* constants promoted to MainWindowHelpers.h). No includes were removed from MainWindow.cpp, the HAVE_SERIALPORT/HAVE_HIDAPI/HAVE_MIDI guards traveled intact with the moved code, the inline constexpr promotion in the header is the right C++17 idiom for the now-cross-TU constants, and all six CI checks are green. Nice, disciplined split — the breadcrumb comments at the excision sites and the "not-pure-motion exceptions" section made this very reviewable.
One small discrepancy vs. the PR description:
kTMate2DefaultOverlayDurationMs was copied, not moved. The description says it moved to the new TU's anon namespace ("only remaining user"), but the original definition is still at MainWindow.cpp:280 and is now unused there (its single use site went to MainWindow_Controllers.cpp:550). No ODR concern — both copies have internal linkage — but it's dead code that contradicts the stated intent and may draw -Wunused-const-variable on clang. Suggest deleting the MainWindow.cpp copy.
While you're on that line: the adjacent kTMate2DefaultUserInteractionTimeoutMs (MainWindow.cpp:281) is also unused — but that's pre-existing on main, not introduced here, so feel free to leave it or fold it into the same one-line cleanup, your call.
Everything else checks out. Thanks @ten9876 — the per-phase QA checklist for the bench is a nice touch given this touches every physical-controller path.
🤖 aethersdr-agent · cost: $16.1808 · model: claude-fable-5
…ainWindow.cpp The Phase 1a split copied this constant into MainWindow_Controllers.cpp's anonymous namespace (its only user) but left the original behind unused. Delete the leftover so the constant lives only next to its user. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Codex <noreply@openai.com>
jensenpat
left a comment
jensenpat
left a comment
There was a problem hiding this comment.
Reviewed mechanically (deleted-vs-added multiset compare): faithful code motion — every removed line reappears verbatim in MainWindow_Controllers.cpp, except the six kCw* constants which moved to MainWindowHelpers.h as inline constexpr with identical values, as described. No dangling callers of the moved anon-namespace functions remain in MainWindow.cpp.
One fix pushed (fe2f966): kTMate2DefaultOverlayDurationMs was duplicated into the new TU rather than moved, leaving the original unused at the top of MainWindow.cpp — deleted the leftover.
Noting for a later phase: the adjacent kTMate2DefaultUserInteractionTimeoutMs was already dead before this PR (defined, never referenced anywhere) — left untouched here to keep this PR pure motion, but it can go in a future cleanup.
2 participants
Summary
Second PR of the #3351 series. Pure code motion — no behavior change, no MainWindow.h changes (a C++ class may define its members across any number of TUs).
Moves the external-controller surface (~1,800 lines) out of MainWindow.cpp into a new
MainWindow_Controllers.cpp:hidEncoderDefaultAction,hidEncoderDefaultPushAction,dispatchHidAction(292 lines)rc28HoldActionActive,updateRC28LedsrefreshStreamDeckLabelsregisterMidiParams(~485 lines,#ifdef HAVE_MIDI)buildControlDevicesSnapshotNot-pure-motion exceptions (the review surface)
Everything below was forced by the split; everything else is verbatim:
flexWheelModeForAction/flexControlButtonAction— moved from MainWindow.cpp's anon namespace to the new TU's anon namespace. The moved methods are their only callers (verified by grep).kTMate2DefaultOverlayDurationMs— moved into the new TU's anon namespace (only remaining user).kCw*ActionId/kCw*ActionNameconstants — promoted from file-statics toinline constexprinMainWindowHelpers.h; they're now genuinely cross-TU (shortcut registry stays in MainWindow.cpp; MIDI/HID registries moved).constexpr+ include additions in the new TU as needed for standalone compilation.Breadcrumb comments at both excision sites in MainWindow.cpp point to the new file.
Deliberately not moved: constructor wiring for these subsystems — that's the constructor-decomposition phase, later in the series.
Numbers
MainWindow.cpp: 19,138 → 17,308 lines (−1,830)MainWindow_Controllers.cpp: 1,892 linesVerification
QA checklist for the bench (per the per-phase QA gate)
This phase touches every physical-controller path. Suggested hands-on pass:
Refs #3351