HL2: setBandFilter() queues a redundant CONFIG one-shot that can put a stale sample rate on the wire

Open Beginner friendly
#4,579 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
networking

Research direction

Start at MetisClient::setBandFilter() and trace buildNextControlPacket() to confirm that bank A already sends the live configuration. Inspect start() and the m_oneShot lifecycle, then remove the redundant queued snapshot. Done means a band change still reaches the control packet promptly without replaying a stale sample rate, with the relevant C++ tests or build passing.

Written by the indexing model from the issue text.

Description

bug maintainer-review priority: low protocol

Summary

MetisClient::setBandFilter() queues a copy of m_ccConfig into m_oneShot. That one-shot is redundant, and because it is a snapshot, it can put a stale config on the wire.

Found during review of #4503 (not a regression from it — the one-shot is added by that PR, but the effect is bounded and was not worth blocking on).

Why it is redundant

buildNextControlPacket() builds every EP2 frame as:

auto pkt = ep2Packet(m_txSeq++, withMox(m_ccConfig, keyed), withMox(b, keyed));

Bank A is live m_ccConfig on every single frame. The one-shot only ever populates bank B. So the stated purpose — "a band change moves the NCO and the filter in the same gesture, don't wait for the round robin" — is already satisfied by bank A without the push.

Why it can be stale

Bank B is applied after bank A, so when the two disagree the snapshot wins for that frame:

  1. setBandFilter(X) pushes snapshot S = ccConfig(rate_old, numRx, X).
  2. setSampleRate(rate_new) rebuilds m_ccConfig = ccConfig(rate_new, numRx, X).
  3. Next frame: bank A = rate_new, bank B = S = rate_old. The radio ends on rate_old.

Bank A re-asserts the truth on the following frame, so the visible effect is one EP2 frame (~1 ms) of stale DDC rate. Small, but it is a real transient the code does not intend.

Additionally, m_oneShot is not cleared in start() (it is cleared nowhere). Hl2Backend::applyBandFilter() has no m_connected guard and m_metis outlives a disconnect, so a snapshot queued while disconnected can replay on the next session's first frames.

Suggested fix

Delete the m_oneShot.push_back(m_ccConfig) in setBandFilter(). Bank A already does the job, and removing the push eliminates the staleness with it.

If the push is kept for some reason not visible here, then m_oneShot.clear() belongs in start() alongside the other per-session resets (m_txSeq, m_roundRobin, m_haveRxSeq, m_drops, m_linkUp).

Dominant language
C++
Stars
221
Forks
117
Avg merge
1d 23h
Merged PRs (30d)
300

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from aethersdr/AetherSDR

All issues in aethersdr/AetherSDR

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.