-
-
Notifications
You must be signed in to change notification settings - Fork 118
fix(hl2): preserve per-band gain across startup and overrides #5402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
922e9d5
fix(hl2): rate-limit the repeated ADC-overload warning
on8st 8e20af6
docs(hermes): mark 6a done and correct 15.7's stale severity
on8st 776a70d
fix(hl2): correct the flush comment, and stop QDebug spacing the paren
on8st 1b9b647
fix(hl2): a pinned connect gain must not consume the band's stored LNA
on8st b1f0576
fix(hl2): the snapshot path must preserve the entry too (#5402 review)
on8st e3d28da
fix(hl2): start() not restart() on the overload clock, behind a polic…
on8st 038a4b0
test(hl2): cover the real snapshot writer. Principle XI.
on8st 971cd05
Merge remote-tracking branch 'origin/main' into aether/pr5402-hl2-gain
jensenpat 7be1edd
fix(hl2): preserve per-band gain on startup. Principle V.
jensenpat dbbf86d
Merge current main into HL2 overload fix. Principle X.
Ozy311 079e169
Merge released memory-tab fix into overload branch. Principle X.
Ozy311 9722108
Merge main into HL2 gain restore. Principle X.
Ozy311 875f415
Merge current main for release validation. Principle X.
Ozy311 54c737e
Integrate reviewed HL2 overload fix. Principle X.
Ozy311 a71edf6
Merge decided overload squash into release branch. Principle X.
Ozy311 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| #pragma once | ||
|
|
||
| // Per-band LNA memory: which value a session comes up on, and which value the | ||
| // band memory records when the operator leaves that band. | ||
| // | ||
| // These are two separate questions and the backend previously answered only the | ||
| // first. The second is where the defect lives: a connect that pins the LNA via | ||
| // the namespaced lnaGainDb param diverges the live value from the start band's | ||
| // stored entry, and the FIRST band change then writes the live value back over | ||
| // that entry (Hl2Backend::rememberCurrentBandState). The operator's calibration | ||
| // for that band is gone, replaced by a number that was only ever meant to hold | ||
| // for one session. | ||
| // | ||
| // Symptom, and why it is worth a header: the loss is silent and it is delayed. | ||
| // Nothing is wrong at connect — the pinned value is what was asked for. The | ||
| // stored entry dies later, on an unrelated action, and the next session comes | ||
| // up on the pinned value as though the operator had chosen it. By the time a | ||
| // band sounds wrong there is nothing left on disk that says what it used to be. | ||
| // | ||
| // They live in a header, evaluated by Hl2Backend rather than copied into it, so | ||
| // the suite exercises the SAME expressions the backend runs — the reasoning | ||
| // Hl2TxLevelPolicy.h states, and the same reason it applies here: a test | ||
| // against a re-typed copy of this decision would agree with itself while the | ||
| // backend kept the bug. | ||
| // | ||
| // See Hl2Backend::connectRadio and ::applyPerBandStateFor for the surrounding | ||
| // ordering; this header is the decision only. | ||
|
|
||
| namespace AetherSDR::hl2 { | ||
|
|
||
| // A clamp local to this header so the decision is testable without pulling in | ||
| // the backend's translation unit. Mirrors qBound's argument order. | ||
| constexpr int clampDb(int minDb, int v, int maxDb) | ||
| { | ||
| return v < minDb ? minDb : (v > maxDb ? maxDb : v); | ||
| } | ||
|
|
||
| // What a session comes up on for the start band. | ||
| struct ConnectLna { | ||
| int liveDb = 0; | ||
| // TRUE when liveDb came from the connect param while the start band ALSO | ||
| // had a stored entry — i.e. the live value is a session pin that the | ||
| // operator never chose for this band. Purely informational to the caller; | ||
| // it is bandMemoryWriteback below that decides what it costs. | ||
| bool sessionPin = false; | ||
| }; | ||
|
|
||
| inline ConnectLna connectLna(bool haveRestoredState, | ||
| bool hasStoredEntry, int storedDb, | ||
| bool paramPresent, int paramDb, | ||
| int defaultDb, int minDb, int maxDb) | ||
| { | ||
| ConnectLna out; | ||
| // The explicit param still wins the LIVE value. That precedence is | ||
| // deliberate and documented at the call site: an automation or test caller | ||
| // pins the gain outright, and a stored entry must not silently ignore what | ||
| // the caller asked for. This header does not reverse it. | ||
| if (paramPresent) { | ||
|
Ozy311 marked this conversation as resolved.
|
||
| // Preserve the pre-existing explicit-parameter behavior; this PR | ||
| // changes persistence, not the connect parameter's range handling. | ||
| out.liveDb = paramDb; | ||
| out.sessionPin = haveRestoredState && hasStoredEntry && paramDb != storedDb; | ||
| return out; | ||
| } | ||
| if (haveRestoredState) { | ||
| out.liveDb = clampDb(minDb, hasStoredEntry ? storedDb : defaultDb, maxDb); | ||
| return out; | ||
| } | ||
| out.liveDb = defaultDb; | ||
| return out; | ||
| } | ||
|
|
||
| // What rememberCurrentBandState() should record for the band being left. | ||
| // | ||
| // Normally the live value: leaving a band records what the operator set while | ||
| // they were on it, which is the whole point of the memory. | ||
| // | ||
| // The exception is a session pin. That value came from the connect param, not | ||
| // from the operator acting on this band, and the band already had an entry of | ||
| // its own — so recording it would overwrite a calibration with a number nobody | ||
| // chose for this band. The stored entry is kept instead. | ||
| // | ||
| // Note what this deliberately does NOT do: it does not make the pin invisible. | ||
| // The live gain stays pinned, the radio runs at the requested value, and every | ||
| // pan is told about it. Only the persistence is refused, because persistence is | ||
| // the part that outlives the session that asked for it. | ||
| inline int bandMemoryWriteback(int liveDb, bool sessionPin, | ||
| bool hasStoredEntry, int storedDb) | ||
| { | ||
| if (sessionPin && hasStoredEntry) { | ||
| return storedDb; | ||
| } | ||
| return liveDb; | ||
| } | ||
|
|
||
| } // namespace AetherSDR::hl2 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #pragma once | ||
|
|
||
| #include <QStringView> | ||
| #include <optional> | ||
|
|
||
| namespace AetherSDR { | ||
|
|
||
| // HL2 restores its per-band gain through RadioStateMemory. Its legacy display | ||
| // value has no band identity and must not be replayed as an operator change | ||
| // (#5400). Keep the existing replay rules for every other family. | ||
| template<typename SetGain> | ||
| int restoreLegacyRfGain(QStringView family, bool clientOwnsGain, | ||
| std::optional<int> savedGain, int currentGain, | ||
| SetGain setGain) | ||
| { | ||
| if (family != u"hl2" && clientOwnsGain && savedGain.has_value()) { | ||
| setGain(*savedGain); | ||
| return *savedGain; | ||
| } | ||
| return currentGain; | ||
| } | ||
|
|
||
| } // namespace AetherSDR |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.