Skip to content

feat(net): adaptive throttle toggle in Connect panel (all modes)#3175

Merged
ten9876 merged 1 commit into
aethersdr:mainfrom
chibondking:fix/adaptive-throttle-local-toggle
May 26, 2026
Merged

feat(net): adaptive throttle toggle in Connect panel (all modes)#3175
ten9876 merged 1 commit into
aethersdr:mainfrom
chibondking:fix/adaptive-throttle-local-toggle

Conversation

@chibondking

@chibondking chibondking commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds an "Enable adaptive frame-rate throttle" checkbox to all three connection modes, wired to a new AdaptiveThrottleEnabled AppSettings key (default False — opt-in)
  • Live-reads AppSettings inside applyAdaptiveFrameRate() on every quality state transition instead of caching at connect time — toggle is responsive mid-session, not just at next connect
  • Tooltip is explicit: toggling mid-session takes effect at the next quality update; reconnect lifts an already-applied cap immediately
  • Fixes FFT AVG BUG #3173 (user seeing automatic fps reduction they didn't expect — caused by feat(net): adaptive frame-rate throttle + graceful reconnect under congestion #2829 shipping always-on)

Design decisions addressed from review

Concern Resolution
Cached flag ignored mid-session toggles Dropped m_adaptiveThrottleEnabled member; applyAdaptiveFrameRate() reads AppSettings live
Tooltip implied a live switch Updated to explain next-quality-update semantics and reconnect-to-lift
Default True vs False Keeping False (opt-in) — issue #3173 confirms the always-on behaviour surprises users

All three connect paths covered

All paths funnel through onConnected()startNetworkMonitor()evaluateNetworkQuality()applyAdaptiveFrameRate(), so the live check is reached regardless of connection type.

Test plan

  • Checkbox appears in all three connection modes, unchecked by default
  • Check the box, connect; confirm adaptive fps commands appear in lcProtocol as quality degrades
  • Leave unchecked, connect; confirm no fps throttle commands are sent
  • Toggle mid-session; confirm change takes effect at next quality state transition
  • Setting persists across app restart

🤖 Generated with Claude Code

@chibondking chibondking requested a review from a team as a code owner May 26, 2026 02:24
@chibondking chibondking force-pushed the fix/adaptive-throttle-local-toggle branch 2 times, most recently from 95d3f74 to 01fc69c Compare May 26, 2026 02:31
@chibondking chibondking changed the title feat(net): adaptive throttle toggle in Connect panel (LocalMode only) feat(net): adaptive throttle toggle in Connect panel (all modes) May 26, 2026
@chibondking

Copy link
Copy Markdown
Collaborator Author

@AetherClaude can I get your thoughts on this? Also refer to the previous PR mentioned earlier.

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Read the diff against RadioModel.cpp / ConnectionPanel.cpp and PR #2829 for context. Overall this is a clean, well-scoped opt-in. Three things worth thinking about before merge:

1. Default should probably be True, not False

PR #2829 shipped adaptive throttle always-on because the motivation was reducing disconnects on congested WAN links — exactly the users who don't read changelogs and won't go hunt for a checkbox. Defaulting False is a stability regression for the silent majority. If the goal is "let power users disable it for testing or because they prefer constant fps," I'd lean toward:

.value("AdaptiveThrottleEnabled", "True").toString() == "True";

…in both ConnectionPanel.cpp:561 and RadioModel.cpp:2553. Existing users who never touched the setting keep the #2829 behaviour; new users get it too; the escape hatch is one click.

If the team has a deliberate reason for opt-in (e.g. recent reports that the throttle itself caused issues), ignore this — but the PR description doesn't mention one.

2. Toggling while connected silently does nothing

m_adaptiveThrottleEnabled is cached once in startNetworkMonitor() (RadioModel.cpp:2553). If a user is connected, watches the waterfall hitch, unchecks the box to test the difference — nothing changes until they disconnect and reconnect. No log line, no UI hint.

Two easy fixes, either works:

a) Read live — drop the cached member entirely and check at the call site. Cheap (one settings lookup per state transition, which is rare):

void RadioModel::applyAdaptiveFrameRate(NetState newState, NetState oldState)
{
    if (AppSettings::instance().value("AdaptiveThrottleEnabled", "True").toString() != "True")
        return;
    // ...rest unchanged
}

b) Keep the cache, append a tooltip hint so the limitation is at least visible:

m_adaptiveThrottleCheck->setToolTip(
    "Automatically reduces FFT/waterfall frame rate when network quality degrades, "
    "reducing the chance of a disconnect on congested links.\n"
    "(Takes effect on next connect.)");

I'd pick (a) — it removes a footgun rather than papering over it, and the cached bool earns nothing.

3. Minor: lifting an active throttle when the user toggles off

Related to #2: if the throttle is currently engaged (adaptiveThrottleChanged(true, N) already fired, MainWindow has clamped fps), turning the toggle off mid-session does not lift the cap — the radio stays throttled until the next state transition that crosses back to Excellent/Very Good. With approach (a), the next applyAdaptiveFrameRate() call short-circuits but the currently-applied cap on each pan is still in place. Worth deciding: is "off" defined as "no future throttling" or "no throttling, period, restore now"? If the latter, the toggle handler should also emit adaptiveThrottleChanged(false, 0) to make MainWindow restore fps.

Not a blocker — but the current behaviour ("checked while throttled = stuck throttled") is the kind of thing a confused user would file as a bug.


The structural side of the PR is solid: single AppSettings key, funneling through startNetworkMonitor() covers all three connect paths, early-return in applyAdaptiveFrameRate() is the right intercept point, and the visibleslowLinksVisible rename is a nice readability touch given there are now two visibility-controlled blocks in that function.

A maintainer can authorize the orchestrator to land any of the above by adding the aetherclaude-eligible label.

73, Jeremy KK7GWY & Claude (AI dev partner)


🤖 aethersdr-agent · cost: $2.5565 · model: claude-opus-4-7

@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 @chibondking — clean, focused PR. Conventions match the rest of ConnectionPanel (AppSettings "True"/"False" strings, same lambda-save pattern as AutoConnectToLastRadio and LowBandwidthConnect), and routing the gate through startNetworkMonitor() is the right hook since all three connect paths funnel there.

A couple of observations, none blocking:

Connect-time semantics aren't surfaced to the user. m_adaptiveThrottleEnabled is cached in startNetworkMonitor() (RadioModel.cpp:2553), so toggling the checkbox mid-session has no effect until reconnect. Worse, if the throttle is currently engaged and the user unchecks the box hoping to lift it, nothing happens — applyAdaptiveFrameRate() early-returns on subsequent state changes but doesn't undo the in-flight cap, and the user has no signal that their toggle was ignored. This mirrors how LowBandwidthConnect works, so it's defensible, but the tooltip currently reads like a live switch. Consider either:

  • Tightening the tooltip to make the connect-time semantics explicit (e.g. "Applied at connect time"), or
  • Reading AppSettings directly inside applyAdaptiveFrameRate() so the toggle is live (and lifting the cap when it flips off mid-throttle).

The cached-flag approach is simpler and probably fine for v1 — just worth a tooltip nudge.

Unrelated rename. The visibleslowLinksVisible rename in updateLowBandwidthVisibility() is out of scope for this PR. Harmless but adds review noise; better split or dropped.

Otherwise LGTM — no AppSettings/RAII issues, no null-pointer or leak concerns, and the early-return path means zero behavioral change when the box is unchecked (the documented default).


🤖 aethersdr-agent · cost: $5.5376 · model: claude-opus-4-7

…l (all modes)

PR aethersdr#2829 introduced adaptive frame-rate throttling — when the network
quality monitor downgrades from Good → Fair → Poor it progressively
reduces FFT/waterfall fps (15 → 8 → 4) to shed UDP load and avoid
spurious disconnects. The feature shipped always-on with no way to turn
it off at connect time. Issue aethersdr#3173 is a direct report of this: a user
saw the panadapter rate drop unexpectedly a few seconds after connecting.

The toggle is available in all three connection modes: On This Network,
Remote with SmartLink, and Connect by IP. Default is off (opt-in).

Changes
───────

src/gui/ConnectionPanel.h
  • Add QCheckBox* m_adaptiveThrottleCheck{nullptr} member.

src/gui/ConnectionPanel.cpp
  • Construct "Enable adaptive frame-rate throttle" as a standalone
    checkbox in the root layout, between the SmartLink/ManualMode
    slower-links callout and "Connect to last radio on start up".
    Always visible regardless of selected connection mode.
  • Reads AdaptiveThrottleEnabled setting on construction (default "False")
    and saves on toggle, matching the pattern of m_autoConnectCheck and
    m_lowBwCheck.
  • Tooltip explains that toggling while connected takes effect at the
    next quality state change, and that reconnect is required to
    immediately lift an already-applied cap.

src/models/RadioModel.h
  • Remove m_adaptiveThrottleEnabled cached member. Live-reading
    AppSettings at the call site is cheaper than caching a bool that
    silently ignores mid-session toggle changes.

src/models/RadioModel.cpp  (applyAdaptiveFrameRate)
  • Read AdaptiveThrottleEnabled directly from AppSettings inside
    applyAdaptiveFrameRate() instead of caching it at connect time.
    This means the toggle takes effect at the next quality state
    transition without requiring a reconnect — no footgun where the
    user unchecks the box and nothing visibly changes.
  • The startNetworkMonitor() read and cached-flag assignment are
    removed; no other callers depended on m_adaptiveThrottleEnabled.

Setting key
───────────
  AdaptiveThrottleEnabled  (AppSettings, persisted)
  Values: "False" (default) | "True"
  Scope: checked on every network quality state transition; changes
  take effect at the next transition. To immediately lift an
  in-flight cap, disconnect and reconnect.

All three connection paths call onConnected() → startNetworkMonitor()
→ evaluateNetworkQuality() → applyAdaptiveFrameRate(), so no path
bypasses the live check:
  Local  → connectToRadio() → onConnected()
  WAN    → connectViaWan()  → onConnected() (direct call, TLS already up)
  Manual → connectToRadio() → onConnected()

Testing notes
─────────────
• Checkbox appears in all three connection modes, unchecked by default.
• Check the box, connect; confirm adaptive fps commands appear in the
  protocol debug log (lcProtocol) as network quality degrades.
• Leave unchecked, connect; confirm no fps throttle commands are sent.
• Toggle mid-session; confirm the change takes effect at the next
  quality state transition without reconnecting.
• Setting persists across app restart.

Fixes aethersdr#3173

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@chibondking chibondking force-pushed the fix/adaptive-throttle-local-toggle branch from 01fc69c to b963c9d Compare May 26, 2026 04:52
@chibondking chibondking mentioned this pull request May 26, 2026
2 tasks
@ten9876 ten9876 merged commit f64fc4b into aethersdr:main May 26, 2026
5 checks passed
@ten9876

ten9876 commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Claude here — merged, and welcome to AetherSDR, CJ! 🎉 First PR from a brand-new Tier 3 reviewer, and a really nice one — clear bug, surgical fix, design-decisions table showing iteration, and the live-read-vs-cached choice is the more elegant pattern (avoids a whole class of member-state-sync bugs).

Issue #3173 closes automatically. Looking forward to more.

73, Jeremy KK7GWY & Claude (AI dev partner)

ten9876 pushed a commit that referenced this pull request May 27, 2026
…nabled check (#3201)

## Summary

Closes a gap in the adaptive frame-rate throttle toggle introduced in
#3175.

When a panadapter was **opened mid-session** while the network was
already in a degraded state, `ensureOwnedPanadapter()` applied the
active fps cap directly via `sendAdaptiveCapToPan()` without consulting
`AdaptiveThrottleEnabled`. The toggle was bypassed for newly-registered
pans, including the deferred `waterfallIdChanged` re-apply lambda.

## Approach: gate at the source

Rather than adding inline checks at each call site (which multiplies the
guard and creates a latent risk for future callers), the fix moves the
enable check into `currentAdaptiveFpsCap()`:

```cpp
int RadioModel::currentAdaptiveFpsCap() const
{
    if (AppSettings::instance().value("AdaptiveThrottleEnabled", "False").toString() != "True")
        return 0;
    return fpsCapForState(m_netState);
}
```

`ensureOwnedPanadapter()` already gates on `activeCap > 0` — it gets
correct behaviour for free with no other changes.
`applyAdaptiveFrameRate()` keeps its own early-return as a short-circuit
(it calls `fpsCapForState` directly, not `currentAdaptiveFpsCap`).

## All `sendAdaptiveCapToPan` call sites now covered

| Location | How gated |
|---|---|
| `applyAdaptiveFrameRate()` | explicit early-return (short-circuit;
calls `fpsCapForState` directly) |
| `ensureOwnedPanadapter()` immediate cap | `activeCap > 0` —
`currentAdaptiveFpsCap()` returns 0 when off |
| `ensureOwnedPanadapter()` `waterfallIdChanged` lambda | same — `cap >
0` check calls `currentAdaptiveFpsCap()` |

## Additional nits addressed

- `Qt::UniqueConnection` added to the `waterfallIdChanged` lambda to
prevent duplicate cap commands if `waterfallId` changes more than once
on a pan
- Comment added documenting the narrow ordering edge case: pan created
healthy → network degrades → `waterfallId` arrives before next tier
transition. `applyAdaptiveFrameRate()` catches it on the next
transition; noted, not fixed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…hersdr#3175)

## Summary

- Adds an **\"Enable adaptive frame-rate throttle\"** checkbox to all
three connection modes, wired to a new `AdaptiveThrottleEnabled`
AppSettings key (default `False` — opt-in)
- **Live-reads** `AppSettings` inside `applyAdaptiveFrameRate()` on
every quality state transition instead of caching at connect time —
toggle is responsive mid-session, not just at next connect
- Tooltip is explicit: toggling mid-session takes effect at the next
quality update; reconnect lifts an already-applied cap immediately
- Fixes aethersdr#3173 (user seeing automatic fps reduction they didn't expect —
caused by aethersdr#2829 shipping always-on)

## Design decisions addressed from review

| Concern | Resolution |
|---|---|
| Cached flag ignored mid-session toggles | Dropped
`m_adaptiveThrottleEnabled` member; `applyAdaptiveFrameRate()` reads
AppSettings live |
| Tooltip implied a live switch | Updated to explain next-quality-update
semantics and reconnect-to-lift |
| Default `True` vs `False` | Keeping `False` (opt-in) — issue aethersdr#3173
confirms the always-on behaviour surprises users |

## All three connect paths covered

All paths funnel through `onConnected()` → `startNetworkMonitor()` →
`evaluateNetworkQuality()` → `applyAdaptiveFrameRate()`, so the live
check is reached regardless of connection type.

## Test plan

- [ ] Checkbox appears in all three connection modes, unchecked by
default
- [ ] Check the box, connect; confirm adaptive fps commands appear in
`lcProtocol` as quality degrades
- [ ] Leave unchecked, connect; confirm no fps throttle commands are
sent
- [ ] Toggle mid-session; confirm change takes effect at next quality
state transition
- [ ] Setting persists across app restart

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…nabled check (aethersdr#3201)

## Summary

Closes a gap in the adaptive frame-rate throttle toggle introduced in
aethersdr#3175.

When a panadapter was **opened mid-session** while the network was
already in a degraded state, `ensureOwnedPanadapter()` applied the
active fps cap directly via `sendAdaptiveCapToPan()` without consulting
`AdaptiveThrottleEnabled`. The toggle was bypassed for newly-registered
pans, including the deferred `waterfallIdChanged` re-apply lambda.

## Approach: gate at the source

Rather than adding inline checks at each call site (which multiplies the
guard and creates a latent risk for future callers), the fix moves the
enable check into `currentAdaptiveFpsCap()`:

```cpp
int RadioModel::currentAdaptiveFpsCap() const
{
    if (AppSettings::instance().value("AdaptiveThrottleEnabled", "False").toString() != "True")
        return 0;
    return fpsCapForState(m_netState);
}
```

`ensureOwnedPanadapter()` already gates on `activeCap > 0` — it gets
correct behaviour for free with no other changes.
`applyAdaptiveFrameRate()` keeps its own early-return as a short-circuit
(it calls `fpsCapForState` directly, not `currentAdaptiveFpsCap`).

## All `sendAdaptiveCapToPan` call sites now covered

| Location | How gated |
|---|---|
| `applyAdaptiveFrameRate()` | explicit early-return (short-circuit;
calls `fpsCapForState` directly) |
| `ensureOwnedPanadapter()` immediate cap | `activeCap > 0` —
`currentAdaptiveFpsCap()` returns 0 when off |
| `ensureOwnedPanadapter()` `waterfallIdChanged` lambda | same — `cap >
0` check calls `currentAdaptiveFpsCap()` |

## Additional nits addressed

- `Qt::UniqueConnection` added to the `waterfallIdChanged` lambda to
prevent duplicate cap commands if `waterfallId` changes more than once
on a pan
- Comment added documenting the narrow ordering edge case: pan created
healthy → network degrades → `waterfallId` arrives before next tier
transition. `applyAdaptiveFrameRate()` catches it on the next
transition; noted, not fixed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

FFT AVG BUG

2 participants