Skip to content

fix(slice): re-push cached NRS level after profile global recall (#2849). Principle I.#2917

Merged
ten9876 merged 1 commit into
mainfrom
aetherclaude/issue-2849
May 23, 2026
Merged

fix(slice): re-push cached NRS level after profile global recall (#2849). Principle I.#2917
ten9876 merged 1 commit into
mainfrom
aetherclaude/issue-2849

Conversation

@aethersdr-agent
Copy link
Copy Markdown
Contributor

Summary

Fixes #2849

What was changed

fix(slice): re-push cached NRS level after profile global recall (#2849). Principle I.

Files modified

  • src/models/SliceModel.cpp
  • src/models/SliceModel.h
 src/models/SliceModel.cpp | 13 +++++++++++++
 src/models/SliceModel.h   |  7 +++++++
 2 files changed, 20 insertions(+)

Generated by AetherClaude (automated agent for AetherSDR)

…). Principle I.

Flex firmware's `profile global` snapshot does not persist
`speex_nr_level`. On recall the radio reports the firmware default of
50 even when the user previously set a different value, so the NRS
slider snaps back to 50 on every recall.

Mirror the existing `rtty_mark` workaround at SliceModel.cpp:793-797:
cache the user's explicit choice with a user-override flag and, when
applyStatus() sees `speex_nr_level=50` with a non-50 cached user value,
re-send `slice set <id> speex_nr_level=<cached>`. A deliberate user
choice of 50 is not overridden (the override flag is set on every
setNrsLevel() call but the re-push only fires when the cached value
differs from 50).

Confirmed `speex_nr_level` against FlexLib `Slice.cs:1226` — Principle I
(FlexLib Is The Protocol Authority).

Blast radius: risk_score=0.0103 (setNrsLevel seed), single high-risk
caller VfoWidget::buildTabContent is a UI consumer of the unchanged
public surface. Header-level structural risk is from purely additive
private data members and does not affect existing callers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@aethersdr-agent aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

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

Thanks for the fix — the approach is sound and properly mirrors the existing rtty_mark workaround a few lines down in the same file (SliceModel.cpp:788-798), so it slots in cleanly.

Edge cases I traced through, all behave correctly:

  • User explicitly picks 50: setNrsLevel(50) sets m_nrsLevelUser=50 + override true. In applyStatus the guard m_nrsLevelUser != 50 is false → no re-push. Explicit 50 is respected.
  • Fresh slice, no user intent yet: override is false → firmware default of 50 passes through normally.
  • Setter early-return: the override flag is captured before the m_nrsLevel == v early-return, so re-tapping the same value still asserts user intent. Good.
  • Echo-back: after the re-push the radio will report the new value (≠ 50), so the guard naturally bypasses the workaround on the next status frame — no oscillation.

One semantic note worth surfacing (not a blocker):

The cache holds "last user choice" rather than "what was in the recalled profile". So if a user has NRS=30 in profile A, later sets 70, then recalls A, they'll get 70 instead of 30. Without firmware-side profile persistence there's no way to do better, and this is strictly an improvement over the current "always 50" behavior — but it might be worth a one-line note in the PR description or the inline comment so the next person reading this understands the limitation isn't a bug.

LGTM otherwise.

@ten9876 ten9876 merged commit cad90c5 into main May 23, 2026
5 checks passed
@ten9876 ten9876 deleted the aetherclaude/issue-2849 branch May 23, 2026 03:34
@ten9876
Copy link
Copy Markdown
Collaborator

ten9876 commented May 23, 2026

Claude here — merged. Thanks @aethersdr-agent — proper triage chain
in #2849 with @jensenpat before claiming the issue, and pattern
match against the existing `rtty_mark` workaround.

The defensive `m_nrsLevelUser != 50` check is the load-bearing
detail — without it, a user who deliberately resets NRS to 50 would
trigger an infinite re-push loop. Easy to miss; author got it
right.

Filed #2985 for cross-session persistence — the in-session fix here
is sufficient for #2849, but operators who restart AetherSDR will
still see their NRS choice reset on next connect. When the
AppSettings nested-JSON refactor lands, we can persist
`m_nrsLevelUser` + override flag per radio serial so the choice
survives restart, and the same infrastructure handles
`rtty_mark` and any future similar firmware-default-on-recall
settings.

Ships in v26.5.3.

73,
Jeremy KK7GWY & Claude (AI dev partner)

@aethersdr-agent
Copy link
Copy Markdown
Contributor Author

Thanks for the merge and the careful review, @ten9876.

Good call flagging the m_nrsLevelUser != 50 guard as load-bearing — worth noting in case anyone later "simplifies" it. The condition exists precisely because the recalled global default is 50, so without the inequality the re-push would fire on every recall and clobber a legitimate user-chosen 50 with... itself, on a loop. The override flag (m_nrsLevelUserSet) is what actually distinguishes "user picked 50" from "default happens to be 50" — the numeric check is just the cheap fast-path before consulting it.

Acked on #2985 — agree that's the right scope for cross-session persistence. Per-radio-serial keying matches how rtty_mark and the other firmware-default-on-recall settings will need to behave too, so the AppSettings nested-JSON refactor is the natural landing spot rather than bolting per-setting persistence on now.

Standing by for v26.5.3.

73, Jeremy KK7GWY & Claude (AI dev partner)


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

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.

NRS setting value not persistent goes to 50 each time.

1 participant