aethersdr / aethersdr/AetherSDR
CW decoder: three pre-existing items adjacent to the #5641 race fix
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 221
- Forks
- 117
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 299
Description
Split out of the #5645 review so they do not grow that PR. All three are pre-existing on main and none is introduced by #5645 — they were each flagged by more than one reviewer as "adjacent, follow-up".
1. estimatedPitch() can hand Zero Beat a dead pitch after stop-then-unlock
lockPitch(true) at 600 Hz → stop() keeps m_pitch = 600 because the value is locked operator state (CwDecoder.cpp:59-60, deliberate, #5213) → the operator un-toggles the lock button → lockPitch(false) clears m_pendingParameters.pitchHz but never m_pitch. The decoder is now stopped and unlocked, yet estimatedPitch() still returns 600 and MainWindow_Wiring.cpp:5826 will retune the slice on it. That is the #5213 symptom the stop() comment guards against, reached by a different door.
2. Vendored ggmorse prints decoded characters straight to stdout
third_party/ggmorse/src/ggmorse.cpp:991-1004 calls printf("%c", …) / printf("?") / fflush(stdout) unconditionally in the decode path, right beside the rxData.push_back() that is the real output channel. So the application writes every decoded CW character to its own stdout, and cw_decoder_parameters_test now sprinkles VVV VVV VVV... through CI logs. Upstream code; wants a vendored patch in the same style as the WDSP ones rather than an in-tree workaround.
3. Upstream setParametersDecode() re-inits the filters from the pre-update range
// third_party/ggmorse/src/ggmorse.cpp:258-263
if (m_impl->parametersDecode.frequencyRangeMin_hz != parameters.frequencyRangeMin_hz) {
m_impl->filterHighPass.init(Filter::FirstOrderHighPass,
m_impl->parametersDecode.frequencyRangeMin_hz, // <- old cutoff
kBaseSampleRate);
}
Both branches pass m_impl->parametersDecode.* (the values about to be overwritten) instead of parameters.*, so a range change updates the struct but leaves the HPF/LPF on the previous cutoff until the next call. The estimation range takes effect immediately; only the filters lag.
Deliberately out of scope for #5645: that PR does not change the number or ordering of setParametersDecode() calls relative to range edits, so the lag behaves there exactly as it does on main. Fixing it is a DSP behaviour change and wants its own before/after.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the referenced locations in CwDecoder.cpp, MainWindow_Wiring.cpp, and third_party/ggmorse/src/ggmorse.cpp, then run cw_decoder_parameters_test to observe the current behavior. Check the stop/unlock pitch path, ggmorse's stdout calls, and filter initialization during range changes. Done means each regression is covered and decoded output no longer pollutes CI logs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- audio-video-rtc, desktop
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100