[ux] Fix Flex compression meter derivation#1992
Conversation
jensenpat
changed the title
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nice work on this, @jensenpat. The investigation is thorough, the documentation is excellent, and the dual-family meter derivation approach is well-reasoned. A few observations:
Looks good
- Keying on
source+nameinstead of numeric IDs is the right call. This has been a latent fragility. - Freshness guard (
compressionSamplesFresh) for the mixed-cadence 6600 path (10 fps SC_MIC vs 20 fps COMPPEAK) is a nice detail —kCompressionReferenceMaxSkewMs = 250gives ~2.5x the slower meter's period, which is reasonable. packetUpdatedMscaptured once perupdateValuescall avoids clock skew within a single VITA-49 packet. Good.- Test coverage for the 8000-series path, the 6600 fallback, meter removal, and freshness rejection is solid.
- Documentation in
flex-meter-learnings.mdis capture-backed and clearly separates what's confirmed vs. what's still open. The FPS table will save future contributors a lot of time.
Items worth a look
-
Stale consumer comments:
SMeterWidget.cpp:99andPhoneCwApplet.cpp:828both say"compPeak is raw dBFS from COMPPEAK"with a silence gate at-30 dBFS. With this PR,compPeak()now returns values in[-25, 0](derived compression reduction), so the gate is harmless but the comments are misleading. Since the PR intentionally doesn't touch those files, this is fine to defer — but worth a follow-up cleanup so the next person reading those consumers isn't confused about the semantic change. -
Redundant
m_compPeak = 0.0finremoveMeterform_afterEqIdx: Lines setm_compPeak = 0.0fand then immediately callupdateCompressionReduction(), which will also setm_compPeak. Not a bug, just dead code —updateCompressionReduction()handles all paths. -
Removed EMA smoothing: The old code applied
kAlpha = 0.3fexponential smoothing to both COMPPEAK and AFTEREQ before computing the delta. The new code uses raw converted values. Since both sides of the subtraction now use raw samples from the same packet timestamp, the subtraction itself should cancel correlated noise. But if the gauge visually jitters more on-air than the old path did, re-adding symmetric smoothing to both inputs (or smoothing the output) would be a straightforward follow-up. Just flagging in case testing shows this. -
hasCompressionMeterValue()is public but unused in this PR: The accessor is added to the header, which is fine for future use. If any consumer (e.g., PhoneCwApplet) should use it to show an "unavailable" state instead of0 dB, that would be a separate UI change.
No issues found
- No null pointer risks, resource leaks, or RAII concerns.
- CMake test target follows existing patterns (
cw_sidetone_test). - All changed files are within the stated scope — docs are directly relevant to the code changes.
- No use of QSettings (AppSettings convention respected).
- No Copilot comments to verify.
Thank you for the detailed investigation notes in the PR body and for the shout-out to Randy AG4Q. This is a solid community contribution. 73!
updateCompressionReduction() returned a bool that no caller used; drop it in favor of void. The idempotent setCompressionMeterAvailable() body collapses to a single assignment, so inline it at the call sites. Co-Authored-By: jensenpat <[email protected]> Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
2 participants
Summary
This PR updates the Phone/CW compression meter data path to derive SmartSDR-style compression from radio TX meter pairs instead of displaying raw
COMPPEAKlevel data.display_db = -clamp(max(0, COMPPEAK - AFTEREQ), 0, 25)AFTEREQis absent:display_db = -clamp(max(0, COMPPEAK - SC_MIC), 0, 25)source+name, not numeric IDs, because IDs are dynamic across sessions and radio families.COMPPEAKfallback are used; if the required radio-side pair is missing or stale,MeterModelmarks the compression value unavailable and emits0 dBto preserve the existing gauge presentation.Investigation Notes
The 8000-series captures and SmartSDR comparison showed that
COMPPEAKis a dBFS signal-level tap near the speech processor/clipper stage, not the value SmartSDR renders directly as compression. ComparingCOMPPEAKagainst the post-EQ reference meterAFTEREQproduced the matching compression scale on the 8400M/8000-series path.The FLEX-6600 meter manifest supplied by testing did not expose
AFTEREQ. Its relevant TX meters wereSC_MICandCOMPPEAK; usingCOMPPEAK - SC_MICis the best-supported 6000-series model from the captures.SC_MICadvertises 10 fps whileCOMPPEAKadvertises 20 fps, so the implementation includes a freshness guard to avoid comparing a fresh processor sample against a stale reference sample.The two-tone testing also confirmed why the compression meter must be radio-meter derived: radio-generated two-tone RF can be active while local PC microphone meters still move, so local mic activity is not a reliable indicator of transmitted compression.
I'd like to recognize Randy AG4Q for his valuable assistance in testing, calibrating and debugging meters on the 6600. Thank you, Randy.
Documentation
docs/flex-meter-learnings.mdwith the capture-backed meter rules, family-specific formulas, FPS table, two-tone notes.docs/tx-audio-signal-path.mdto link the meter findings and document howAFTEREQ,SC_MIC, andCOMPPEAKare used.Validation
git diff --check origin/maincmake --build build --target meter_model_test --parallel 10./build/meter_model_testcmake --build build --target AetherSDR --parallel 10Built app:
/Users/patj/.codex/worktrees/be95/AetherSDR/build/AetherSDR.app👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat