fix(settings): band plan Off selection reverts to Small on restart (#3358)#3410
Conversation
…ethersdr#3358) Selecting Off writes BandPlanFontSize=0 to AppSettings. On the next launch, AppSettings::value("BandPlanFontSize","").toInt() returns 0 for both an absent key and an explicitly stored zero — they are indistinguishable. The legacy ShowBandPlan→int migration then always promotes 0 to 6 (Small) because ShowBandPlan defaults to "True". Fix: gate the migration on AppSettings::contains() so an explicit zero is preserved and "Off" survives a restart. Mirrors the correct pattern already used in SpectrumWidget.cpp:724. Verified locally: set Band Plan→Off, restart, View→Band Plan shows ✓ Off. Fixes aethersdr#3358
There was a problem hiding this comment.
Thanks for the careful diagnosis and fix, @M7HNF-Ian — the root-cause analysis is spot-on. QString::toInt() returning 0 for both an absent key and a stored "0" is exactly the ambiguity, and gating the migration on AppSettings::contains() is the right way to disambiguate (and AppSettings does expose contains() — src/core/AppSettings.h:37).
A couple of small notes — nothing blocking:
- The PR description references
SpectrumWidget.cpp:724as the prior-art pattern, but the equivalent migration there is actually a few lines down atSpectrumWidget.cpp:727, and it uses.toString().isEmpty()rather thancontains(). The newcontains()check here is arguably the cleaner of the two; worth knowing the two sites now check slightly different things if anyone ever has to reason about migration parity (an explicitly-stored empty string would behave differently between them — unlikely in practice, but not impossible). - Behaviour preserved for fresh installs and legacy
ShowBandPlan=True/Falseusers; only the previously-broken explicit-Off case changes, which is exactly the intent.
Local Linux verification on main reproduces the bug before, fixed after — matches your macOS report. Nice clean fix to a real, user-visible regression.
🤖 aethersdr-agent · cost: $10.7667 · model: claude-opus-4-7
NF0T
left a comment
There was a problem hiding this comment.
Correct diagnosis and clean fix, @M7HNF-Ian. QString::toInt() returning 0 for both an absent key and a stored "0" is the exact ambiguity, and AppSettings::contains() is the right tool to resolve it — it's the more semantically precise approach compared to .toString().isEmpty() used in the analogous SpectrumWidget migration on the same key (.isEmpty() also catches an explicitly-stored empty string as "absent"; contains() doesn't, which is the more correct behaviour here).
Walked through all four cases — fresh install, legacy ShowBandPlan=False, any explicit size, explicit Off — each resolves correctly after the fix.
All 5 CI checks green.
73,
Ryan NF0T
…ethersdr#3358) (aethersdr#3410) ## Problem Selecting **View → Band Plan → Off** writes \`BandPlanFontSize=0\` to \`AppSettings\`. On the next launch: \`\`\`cpp int savedBpSize = AppSettings::instance().value("BandPlanFontSize", "").toInt(); if (savedBpSize == 0 && AppSettings::instance().value("ShowBandPlan", "True").toString() == "True") savedBpSize = 6; // migrate old boolean setting \`\`\` \`.toInt()\` returns \`0\` for both an **absent key** and an **explicitly stored zero** — they are indistinguishable. The legacy \`ShowBandPlan→int\` migration then always promotes \`0\` to \`6\` (Small) because \`ShowBandPlan\` defaults to \`"True"\`. The user's Off choice is silently discarded every restart. ## Fix Gate the migration on \`AppSettings::contains()\` so an explicit \`0\` is preserved. Mirrors the correct pattern already used in \`SpectrumWidget.cpp:724\`. ## Testing Verified locally on macOS (Apple Silicon, dev build off \`main\`): - **Before fix:** set Band Plan→Off, restart → View→Band Plan shows **✓ Small** (bug reproduced) - **After fix:** set Band Plan→Off, restart → View→Band Plan shows **✓ Off** ✅ Fixes aethersdr#3358
Problem
Selecting View → Band Plan → Off writes `BandPlanFontSize=0` to `AppSettings`. On the next launch:
```cpp
int savedBpSize = AppSettings::instance().value("BandPlanFontSize", "").toInt();
if (savedBpSize == 0 && AppSettings::instance().value("ShowBandPlan", "True").toString() == "True")
savedBpSize = 6; // migrate old boolean setting
```
`.toInt()` returns `0` for both an absent key and an explicitly stored zero — they are indistinguishable. The legacy `ShowBandPlan→int` migration then always promotes `0` to `6` (Small) because `ShowBandPlan` defaults to `"True"`. The user's Off choice is silently discarded every restart.
Fix
Gate the migration on `AppSettings::contains()` so an explicit `0` is preserved. Mirrors the correct pattern already used in `SpectrumWidget.cpp:724`.
Testing
Verified locally on macOS (Apple Silicon, dev build off `main`):
Fixes #3358