Pull request overview

This PR centralizes profile-load “radio-state write hold” timing into the shared profile-load command helper so that dependent recovery timers (deferred pan-dimension flush and post-hold recovery) stay derived from the same source of truth, reducing duplicated magic delays across model/UI code.

Changes:

• Hoists kProfileLoadStateWriteHoldMs (and derived recovery delays) into ProfileLoadCommand.h and updates call sites to use the shared constants.
• Replaces the hard-coded suppressed-write callback code with a named sentinel constant.
• Extends profile_load_command_test to assert the derived recovery delays are ordered after the hold window.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Reviewed the diff against the branch — this is a clean, well-scoped consolidation. Thanks @rfoust. 🙏

What's good

• The hold window is now a single source of truth. kProfileLoadStateWriteHoldMs moves into ProfileLoadCommand.h, and the local copies in both RadioModel.cpp and MainWindow_Wiring.cpp (two functions) are removed. Both consumers already #include "ProfileLoadCommand.h", so the shared constant resolves cleanly.
• Behavior is preserved exactly: the derived timers reproduce the old magic delays (10000 + 1000 = 11000, 10000 + 1250 = 11250), so the two QTimer::singleShot calls are unchanged at runtime while no longer being free-floating literals.
• The static_cast<int> on the qint64 base is safe at these magnitudes (~11k, far under INT_MAX), and QTimer::singleShot takes int, so the narrowing is correct.
• Naming kProfileLoadSuppressedCommandCode and documenting that it is not a SmartSDR protocol code is a genuine readability win at the suppression site.
• The two new test invariants (deferred pan flush > hold, post-hold recovery > deferred flush) lock in the ordering relationship, so a future edit to the base hold can't silently invert the sequence. Nice touch.

On the Copilot finding (RadioModel.cpp:3654) — the concern that the suppressed-write path invokes cb(...) with a non-zero code while returning 0, which callbacks may misread as a real radio error, is a legitimate observation. But it's pre-existing behavior: this PR only renames the literal and adds a clarifying comment; it doesn't alter the callback contract. So it's not a blocker for this consolidation. If you agree it's worth addressing, it'd be cleaner as a focused follow-up — either report suppression as a benign completion (cb(0, ...)) or skip the callback entirely — after confirming what the registered callbacks actually do with code != 0.

No scope creep, no AppSettings/RAII concerns, includes are correct. Looks good to me.

_{🤖 aethersdr-agent · cost: $2.4616 · model: claude-opus-4-8}

Approving — clean, behavior-preserving consolidation with good test coverage. Verified the values are exactly preserved and nothing else needed touching.

Correctness — verified equivalent

• Values preserved exactly: kProfileLoadDeferredPanFlushDelayMs = hold+1000 = 11000 and kProfileLoadPostHoldRecoveryDelayMs = hold+1250 = 11250 reproduce the old literals precisely; the sentinel 0x50000061 is unchanged. No timing/behavior drift.
• inline constexpr in the header is the right tool — ODR-safe across TUs (C++17 inline variables), correctly replacing the three duplicated local constexpr defs. #include <QtGlobal> correctly added for qint64, and static_cast<int>(10000+1000) is well within int range for QTimer::singleShot.
• Include dependency satisfied: both consuming TUs (RadioModel.cpp, MainWindow_Wiring.cpp) already include ProfileLoadCommand.h, so removing the local defs is safe.
• Sentinel scope: 0x50000061 is only ever produced (passed to a callback in sendCmd), never compared against elsewhere — so naming it is a pure readability win at the one producer, with no missed consumer. The "not a SmartSDR protocol response code" comment is genuinely useful, since 0x5… reads like a protocol code.

Tests

The two new checks lock the ordering invariant (deferredPanFlush > writeHold, postHoldRecovery > deferredPanFlush) — exactly the right thing to pin, so future edits to the hold window keep the derived timers correctly ordered. 👍

Minor / optional (non-blocking)

• The 3500 ms early-flush literal in scheduleProfileLoadRecovery is left un-consolidated — defensible, since it's a best-effort early flush with different semantics from the hold-derived timers, not a copy of the hold value. Only noting it given the PR's consolidation theme.

Good centralization around the radio-authoritative profile-load path (Principle I). 🚀