Skip to content

test(hl2): pin the TXA transmit passband and onset. Principle VIII. - #6347

Merged
jensenpat merged 2 commits into
aethersdr:mainfrom
on8st:test/hl2-tx-passband-onset
Oct 10, 2026
Merged

jensenpat merged 2 commits into
aethersdr:mainfrom
on8st:test/hl2-tx-passband-onset

Conversation

@on8st

@on8st on8st commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Refs #5911

Correction to #5911 first. The issue put its onset figures (a 14 ms rise, about 12 ms of pre-ringing) down to the 2048-tap bandpass. That is wrong. At current main the key-up onset comes from WdspChannel's mute envelope. WDSP holds the first 10 ms of an over's first non-zero audio at zero, then fades it in over 25 ms (raised cosine, muteDelayUpSec / muteSlewUpSec). This test measures a 14.77 ms 10-90 % rise; that ramp predicts 14.76 ms. The shape does not change with TXA's bp0 at 8192 taps or with the TXA ALC switched off. Only the delay moves. The bandpass's own onset, measured once the envelope has run, is a 0.73 ms rise.

Limits

  • The test reads the IQ Hl2TxDsp emits, and nothing else. No radio, no simulator, no socket, no keying.
  • Times are on the IQ stream's sample clock: output sample n is n / 48 kHz, counted from the tone's first audio sample. Block accumulation on the wall clock, the EP2 queue and the gateware are not in them.
  • TXA build only. The phasing build exits 77, because no CI job builds it. Nothing here compares TXA with the phasing modulator, and HL2 TX: v26.9.4's TXA default narrowed every operator's transmitted audio to the configured passband and added ~12 ms of pre-ringing, and hl2_txdsp_test cannot see either #5911's phasing column is still not re-taken.
  • The ALC is off and the levels are fixed: 0.015 per tone in the multitone, 0.1 for the onset tone. Each passband leg runs once and each onset leg twice. Repeated runs (serial and under -j8 load) gave the same figures to 0.01.
  • Measured on one Mac (macOS, Qt 6.12.0). A response 142 dB or more down sits on float32's numerical floor and is unresolved, not a measurement. The checks stop at 100 dB.

What it checks

hl2_tx_passband_onset_test is a separate target and takes about 7 s; hl2_txdsp_test already takes about 180 s.

  • Passband. A 25-tone multitone (50..3500 Hz on a 50 Hz grid, Schroeder phases) goes through the real TXA chain. The test reads it on whole 960-sample periods, so every tone's bin and image bin are orthogonal. Each wanted bin is compared with the 1 kHz one, for USB and LSB (300..2700) and DIGU (150..3000), taking the edges from defaultTxPassbandForModeName(). The expected edges are written out in the test, so a narrower shipped passband fails. The checks: flat to 0.1 dB from edge+100 to edge-100 Hz, -6 ± 1 dB at each edge, at least 50 dB down 100 Hz outside, at least 100 dB down from 150 Hz outside.
  • Onset. A 1 kHz tone as the over's first audio (key-up), and the same tone after a 200 ms burst and 300 ms of silence (mid-over, with the mute envelope already spent). Key-up: the 50 % point comes no later than 106 ms, and the 10-90 % rise is 10..20 ms. It is the anti-click ramp, so it is bounded from below as well. Mid-over: the 50 % point comes no later than 84 ms, and the envelope crosses -60 dB no more than 12 ms before the 50 % point. Each latency bound is today's figure plus half a 1024-sample DSP block.

Measured at 4e547347

Every figure below is the same as at 25de97c3, before #6245 moved the HL2 ALC into Hl2TxAlc.h. The test runs with that ALC off.

Passband (the same for USB and LSB; DIGU has the same shape at its own edges):

tone 100 150 200 250 300 350 400 500..2500 2600 2650 2700 2750 2800 2900..3500
dB re 1 kHz floor floor -59.38 -22.97 -6.02 -0.64 -0.009 0.000 -0.009 -0.64 -6.02 -22.97 -59.38 floor

The -59.38 dB at 200 and 2800 Hz matches the figure the withdrawn table in #5911 gave for TXA.

Onset:

key-up mid-over
first IQ at -60 dB 83.56 ms 64.08 ms
50 % point 95.54 ms 72.92 ms
10-90 % rise 14.77 ms 0.73 ms
envelope 1 / 4 / 10 ms before the 50 % point -7.2 / -11.7 / -32.2 dB -26.7 / -41.6 / -64.8 dB

The two columns differ by 22.6 ms. The mute envelope accounts for that: 10 ms held at zero plus half of the 25 ms slew. Two probes, never pushed, taken at 25de97c3 and not re-taken since, account for part of the 72.92 ms. With the TXA ALC off, the 50 % point comes 6.0 ms earlier (its 6-τ look-ahead at 1 ms). With bp0 at 8192 taps it comes 64.0 ms later, which is (8192 - 2048) / 2 samples at 48 kHz, and the mid-over -60 dB lead grows from 8.84 to 25.04 ms. #5911's "+96 ms" observation therefore matches the key-up 50 % point here (95.54 ms).

Red on mutation (scratch commits on 25de97c3, never pushed, not re-taken; the full suite each time)

mutation this test hl2_txdsp_test
voice default 300..2400 in defaultTxPassbandForModeName RED: flatness and edge, USB and LSB RED: its baseline pins the pair
high edge 100 Hz lower inside Hl2TxDsp only (mapping unchanged) RED: flatness and edge, USB, LSB and DIGU green
TXA bp0 at 8192 taps RED: key-up and mid-over 50 % point, mid-over lead green

hl2_dsp_readback_test also fails on the second mutation, because it reads back the edges the channel was given. A narrowing inside WDSP itself would get past it. At 9c715b47, 755 of 757 tests pass on this Mac, and this test ran inside that -j8 suite (6.79 s, not skipped). PR CI only compiles this test: it is in no ctest -R list in ci.yml, so its first run upstream is full-suite.yml after merge. The two that fail are known failures here, and this commit touches neither: peripheral_auth_dialog_interaction_test (the #6008 clipped-page check, in the per-case test #6156 introduced) and unified_title_bar_test (its two #6224 checks).

What this does not change

The chain stays as it is. The tap count, the 300..2700 default and the mute envelope are untouched. Whether 300..2700 is the intended shipped voice passband is still #5911's ask 1, and it is yours to decide. If the tap count comes into it: TX's 2048 taps are create_txa's own max(2048, dsp_size). WdspChannel::Config::filterTaps reaches receive channels only (the "Receive only" clause in WdspChannel.cpp), so setting it on the TX config would change nothing. If you would rather the 10 ms hold at the start of every over were shorter, that is a separate decision: it drops the first 10 ms of the first syllable, and the test pins only the ramp's 10..20 ms rise, not the hold.

73, ON8ST

🤖 Generated with Claude Code

Refs aethersdr#5911. hl2_txdsp_test asserts sideband handedness and suppression
ratios, never the wanted bin's amplitude, the band edges or the onset, so
most changes to what operators transmit are invisible to it.

hl2_tx_passband_onset_test drives Hl2TxDsp (ALC off, real-time paced) and
reads the IQ it emits:

- passband: a 24-tone multitone, 100..3500 Hz on a 50 Hz grid, read on
  whole 960-sample periods so every bin is orthogonal; each wanted bin
  against the 1 kHz one, for USB, LSB (300..2700) and DIGU (150..3000)
  as defaultTxPassbandForModeName() gives them. The edges are written out
  in the test, so a narrower shipped passband fails.
- onset: a 1 kHz tone as the over's first audio (key-up), and after a
  200 ms burst and 300 ms of silence (mid-over).

Measured at 4e54734 (after aethersdr#6245; every figure as at 25de97c), on the
IQ stream's sample clock: flat within
0.009 dB from edge+100 to edge-100 Hz, -6.02 dB at each edge, -0.64 dB
50 Hz inside, -22.97 dB 50 Hz outside, -59.38 dB 100 Hz outside, 142 dB
and more beyond. Key-up: 50 % point 95.54 ms after the first audio,
10-90 % rise 14.77 ms. Mid-over: 50 % point 72.92 ms, rise 0.73 ms,
-60 dB reached 8.84 ms before the 50 % point.

The key-up shape is WdspChannel's mute envelope, not the bandpass: WDSP
holds the first 10 ms at zero and then slews over 25 ms (raised cosine)
from the over's first non-zero input sample. The rise matches that ramp
to 0.01 ms. Two probes at 25de97c, not re-taken: it is unchanged with
TXA's bp0 at 8192 taps or the TXA ALC off; only the 50 % point moves
(+64.0 ms, -6.0 ms).

Nothing in the chain changes: the tap count and the default passband are
a ruling (aethersdr#5911 ask 1). Separate target, about 7 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st
on8st requested a review from a team as a code owner October 10, 2026 17:11

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against the test's own header comment and issue #5911 — the PR body was not available to me (the harness handed me title, file list and diff only), so if it discloses local run results or tolerance provenance, some questions below may already be answered there.

1. Issue fit

Partially — and the unaddressed half is a maintainer question, not a code gap.

#5911 makes two asks. Ask 2 is delivered well: "a transient and multi-tone leg in the TX test, so the next change of this class is visible before a release rather than after one." This is exactly that — a 24-tone stimulus through the real Hl2TxDsp chain with wanted-bin amplitude assertions (the thing #5911 identified as structurally missing: hl2_txdsp_test asserts only opposite-sideband ratios, so "a tone being filtered away entirely fails nothing"), plus edge probes at 300/2700/150/3000 and an onset/rise/pre-ring leg. I checked the anti-tautology claim in the header comment and it holds: cfg takes the passband from defaultTxPassbandForModeName(), but checkShape() is handed literal 300, 2700 / 150, 3000 (tests/hl2_tx_passband_onset_test.cpp:323-328), so a shipped mapping that narrowed to e.g. 400..2600 would put the 300 Hz and 2700 Hz edge tones ~50 dB down and fail edgesOk. It is not reading the implementation back to itself.

Ask 1 — "is the 300–2700 box the intended shipped voice bandwidth for HL2 SSB?" — is pre-empted, not answered. That is reasonable for a test PR, but worth naming for the maintainer: this test now pins the status quo in both directions (−6 ±1 dB at the edge, ≤ −50 dB at edge+100), so a later ruling to widen the voice default will require editing main()'s literals. The issue carries maintainer-review and that question is still open.

#5911's third, smaller item — Hl2Backend::setTxFilter's comment still justifying eSSB by "the 255-tap Blackman prototype keeps a usable skirt", describing a non-default modulator — is untouched. Out of scope for a test-only PR; it stays open.

2. Scope

File What it changes Claimed by the title? Verdict
tests/hl2_tx_passband_onset_test.cpp New socket-free CTest: 24-tone passband response per mode (USB/LSB/DIGU) + key-up and mid-over onset measurement Yes — "pin the TXA transmit passband and onset" In scope
tests/tests.cmake:5927-5934 Registers the new target, SKIP_RETURN_CODE 77 Yes In scope

Everything in the diff is explained by the issue. No production code, no new public surface, no capability, no settings key, no CHANGELOG.md entry (correct). No deleted guards — this is a pure addition. The added target is genuinely registered (add_executable + add_test), not a dead addition. tests/hl2_twotone_probe.cpp — the orphan instrument #5911 names as the source of the stimulus generator — is already gone from both trees, so there is nothing left to retire here.

Socket-test surface: none. I read the added test source directly (not the working tree): no QTcpServer/QTcpSocket/QUdpSocket/QLocalServer/QWebSocketServer, no bind/listen/connectToHost, no peer process, no Fake* radio class. The only Qt plumbing is QCoreApplication::processEvents and a signal/slot connection to an in-process Hl2TxDsp. It is socket-free, which is the right layer: AGENTS.md routes DSP to a socket-free CTest.

3. Blockers

None.

4. Nits (non-blocking)

(a) tests/hl2_tx_passband_onset_test.cpp:90,101-116,336-343 — a real defect can exit 77 (SKIP) instead of 1. This is the one I would most like fixed before merge.

Three paths converge on it:

  1. runOnce() reads only the aggregate counter:

        c.faults = tx.modulatorFaultBlocks();

    But src/core/backends/hl2/Hl2TxDsp.h:103-107 draws exactly the distinction this collapses: "The part of modulatorFaultBlocks() dropped unexchanged because the worker had not produced the previous block's output within a block period… A drop means the worker was starved; any other fault means the exchange gate failed." An exchange-gate failure is the #5910 class — a defect, not load. The sibling test reads both (tests/hl2_txdsp_capture_burst_test.cpp:103-105) and tests/tests.cmake:5923 states the convention outright: "An underrun still fails."

  2. The exit-code order is inverted relative to that sibling. hl2_txdsp_capture_burst_test.cpp:349-356 checks failures first:

        if (g_failures) { … return 1; }
        if (g_inconclusive) { … return 77; }

    The new test does the opposite (:336-343): g_starved → 77 is tested before g_failures → 1. So a run where three passband checks genuinely failed and one onset capture also starved reports SKIP.

  3. The configure() failure path takes the same exit. :79-84 does ++g_failures and then c.faults = ~0ull, which run() reads as starvation, retries three times, and converts to g_starved → 77. A hard modulator configure failure is reported as inconclusive.

Drop-in for (1) and (3) — add the counter to Capture, read it, and fail hard on the non-drop remainder:

struct Capture {
    std::vector<std::complex<float>> iq;
    unsigned long long faults = 0;
    unsigned long long stallDrops = 0;
};
    c.faults = tx.modulatorFaultBlocks();
    c.stallDrops = tx.modulatorStallDrops();
    return c;
Capture run(const char* what, const std::vector<float>& audio, WdspChannel::Mode mode,
            std::pair<int, int> passband)
{
    Capture c;
    for (int attempt = 0; attempt < 3; ++attempt) {
        c = runOnce(audio, mode, passband);
        if (c.faults == 0) {
            return c;
        }
        if (c.faults > c.stallDrops) {
            // Not load: the exchange gate failed, which is the defect #5910 names.
            check(false, (std::string(what) + ": every block reached the wire").c_str());
            return c;
        }
        std::fprintf(stderr, "starved in %s: %llu blocks missed the wire (attempt %d of 3)\n",
                     what, c.faults, attempt + 1);
    }
    g_starved = true;
    return c;
}

and for (2), swap the two blocks in main() so g_failures is tested first, matching the sibling.

(b) tests/hl2_tx_passband_onset_test.cpp:148-157 — DIGU's low edge has no stopband probe, and nothing notices. checkShape matches f == lo - 100 and f <= lo - 150. For DIGU lo = 150, that is 50 Hz and ≤ 0 Hz, and kTones starts at 100 — so for the DIGU row, "100 Hz outside an edge is 50 dB down" is evaluated against the 3100 Hz tone alone, and the low-side 150+ Hz check never fires. The edges == 2 guard (:164) catches this class for the edge tones but there is no counterpart for the stopband points, so the gap is silent. Either add 50 to kTones (still a multiple of 50, so it stays orthogonal in the 960-sample window) or count the stop100 hits and assert the count the way edges is asserted.

(c) tests/hl2_tx_passband_onset_test.cpp:139,152-153 — kFlatDb = 0.1 is applied at edge ∓ 100 Hz, where I can't find supporting data. WdspChannel.h's own figure, quoted in #5911, is a "200 Hz [transition] at 2048 taps", and #5911's measured flatness figure is "passband ripple, 600–2400 Hz: 0.00 dB". The tones this test requires flat within 0.1 dB include 400, 2500 and 2600 (USB/LSB) and 250, 2650, 2700, 2750, 2800, 2900 (DIGU) — several of them inside one transition width of an edge, i.e. on the skirt knee rather than the flat top. If those were measured at 0.1 dB in your runs, saying so in the body would settle it; if the figures came from the 600–2400 region, this is the tolerance most likely to go red on correct code.

(d) tests/hl2_tx_passband_onset_test.cpp:68-75 — declaration order of tx and c. Hl2TxDsp tx; is declared before Capture c;, and the iqReady lambda captures c by reference, so reverse-order destruction tears down c while tx is still alive. I checked and there is no live path today: both emit sites (Hl2TxDsp.cpp:333,658) are synchronous inside processAudioBlock, which this test calls on the main thread, and ~Hl2TxDsp is = default. It is one line of hygiene against a future teardown-flush, not a bug — move Capture c; above Hl2TxDsp tx;.

(e) Coverage reality, worth one line in the body. PR CI never runs this test: every ctest in ci.yml is -R-filtered to a named handful (lines 819-820, 832, 1174, 1189) and this target is in none of them. The five green checks prove it compiles on all three lanes — which is real value, and the file defining its own kPi rather than M_PI is why check-windows is green. First actual execution is post-merge in full-suite.yml. On the sanitizer lanes the real-time pacing will almost certainly starve the worker and exit 77 every run, so it guards nothing there (the same trade the burst test makes by design). On the normal lane, a load spike during the full -j suite also lands on 77 — which is the reason nit (a) matters more than it looks. Was it run concurrently with the rest of the suite, rather than alone?

5. What I tried to break

  • CodeGuard's two CRITICALs — both refuted. CS-VLN-CMDI-BACKTICK at :50 and :66 are backtick-quoted parameter names inside // comments (`ms`, `audio`). The file contains no system/popen/exec*/QProcess and no shell string anywhere, and every fprintf uses a literal format string with what/err.c_str() as arguments, so there is no format-string sink either. Credit to security-audit (CodeGuard) for the sweep. Heads-up: :225 and :227 carry the identical pattern (`startIn`, `warm`) and will refire on a rerun.
  • Tried to make the test pass against the unfixed behaviour. It can't: checkShape's edges are literals, not read back from defaultTxPassbandForModeName(), and the wanted-bin magnitudes are compared to the 1 kHz reference rather than to a suppression floor — which is precisely the blindness #5911 documents in hl2_txdsp_test.
  • Index bounds in every raw read. bin() reads [kSettle, kSettle+count) with count = ((size-kSettle)/kPeriod)*kPeriod, guarded by the size < kSettle + 8*kPeriod early return; onset() guards size < settledFrom + 4*kPeriod; envDb's index is bounded above by halfAt and below by the explicit at < t0 test. halfAt is always in range because steady is the mean of |iq[n]| over a non-empty tail, so a sample at ≥ half the mean necessarily exists. No out-of-range read found.
  • DFT orthogonality and capture-length margins. All 24 tones are multiples of 50 Hz and the window is whole 960-sample periods at 48 kHz, so the bins and their images are orthogonal as claimed. Passband capture: 0.75 s × 24 kHz → ~36000 output samples vs the 19968 the guard needs. Warm onset: 0.9 s → 43200 vs 16032 needed. Both have slack.
  • The mid-over premise. The claim that "WDSP starts [the mute envelope] at the over's first non-zero input sample" and does not re-arm across 300 ms of silence held up: WdspChannel::Config's muteDelayUpSec = 0.010 / muteSlewUpSec = 0.025 (src/core/dsp/WdspChannel.h:130-131, un-overridden by Hl2TxDsp) are open-time config driven by the running state, and I found no input-silence detector that would re-arm them mid-over. That also reconciles kMinKeyUpRiseMs/kMaxKeyUpRiseMs: the 10–90 % portion of a 25 ms raised cosine is ≈ 15 ms, comfortably inside [10, 20]. And kMaxKeyUpHalfMs = 106 sits about half a 1024-sample block above #5911's measured +96 ms, as the comment claims.
  • "mic gain unity" (:66-67) — confirmed: Hl2TxDsp.h:172 defaults m_micGain = 1.0 and the test never calls setMicGain().
  • Couldn't check: the shipped figures themselves — 0.1 dB in-band, −6 ±1 dB at the edges, 50 dB at edge±100, 100 dB beyond. I'm headless with no build and no run, so every finding above is reasoned from code rather than reproduced; nits (a) and (b) are source-level and don't depend on a run, nit (c) explicitly does.

6. Recommendation

Approve with nits. This is careful, well-motivated work that closes the real gap #5911 identified, and the anti-tautology design (literal edges, amplitude assertions, retry-then-inconclusive) shows the author thought about how the test could lie. No blockers: nothing here breaks users or violates canon.

Concrete next step: fix (a) — split stall drops from gate faults and put g_failures ahead of g_starved in main(), matching hl2_txdsp_capture_burst_test's precedent — since a test whose failure mode is SKIP is the one way this instrument could quietly stop guarding. Then (b), a one-tone addition. (c) is a question rather than a change: confirm the edge∓100 Hz flatness figures came from a real run, because that tolerance is where a correct build is most likely to go red once this starts executing in full-suite.yml. Thanks for picking this up — the sideband-ratio blindness in the old test was a genuinely non-obvious find and this is the right shape of fix.


🤖 aethersdr-agent · cost: $9.3513 · model: claude-opus-5

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Cisco CodeGuard — static analysis of this PR (2 finding(s))

  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in tests/hl2_tx_passband_onset_test.cpp /tmp/aetherclaude/pr-6347/tests/hl2_tx_passband_onset_test.cpp:50
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in tests/hl2_tx_passband_onset_test.cpp /tmp/aetherclaude/pr-6347/tests/hl2_tx_passband_onset_test.cpp:66

Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.


🤖 aethersdr-agent · cost: $9.7111 · model: claude-opus-5

Review of aethersdr#6347.

(a) A defect exits 1, not 77. Capture reads modulatorStallDrops() beside
modulatorFaultBlocks(): only a stall drop is retried and can end
inconclusive; any other fault is a failed check at once. A configure()
failure is a failed check and is not retried. main() tests g_failures
before g_starved, as hl2_txdsp_capture_burst_test does.

(b) DIGU's low edge has a stopband probe: kTones gains 50 Hz, and each
row asserts that both edges are probed 100 Hz outside, as it does for
the edge tones. 50 Hz reads -59.38 dB under DIGU 150..3000. The 25-tone
sum peaks at 0.164 of full scale (0.163 with 24). No bound is changed.

(d) Capture is declared before Hl2TxDsp, so the iqReady slot's target
outlives the emitter.

Refs aethersdr#5911

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st

on8st commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Fixed in 9c715b47, a new commit on top (no rebase).

(a) A defect now exits 1, not 77. All three paths, as you described them:

  • Capture carries stallDrops (modulatorStallDrops()). A fault that is not a stall drop fails a check at once and is not retried.
  • A configure() failure fails a check and is not retried as starvation.
  • main() tests g_failures before g_starved, as hl2_txdsp_capture_burst_test does.

Forced on the old head and on the new one: configure() made to fail, exit 77 → 1. A fake non-stall fault, 77 → 1. A failing check together with a starved capture, 77 → 1. A fake stall drop alone still exits 77.

(b) kTones gains 50 Hz, and each row now also asserts that both edges are probed 100 Hz outside. Measured at 50 Hz: −59.38 dB under DIGU (150..3000), against the −50 dB bound. No bound changed. The 25-tone sum peaks at 0.164 of full scale.

(c) The 0.1 dB figure is measured at the tones you named, not taken from the 600–2400 Hz region. In dB relative to 1 kHz, five runs with byte-identical output:

mode tone level
USB, LSB (300..2700) 400, 2600 Hz −0.009
500, 2500 Hz 0.000
DIGU (150..3000) 250, 2900 Hz −0.009
300, 350, 2800 Hz 0.000

The worst flat-band tone is 0.009 dB down, in every mode: the one 100 Hz inside an edge. 50 Hz inside an edge reads −0.64 dB and is not in the flat check.

(d) Capture c; is declared above Hl2TxDsp tx;.

(e) Yes, it ran inside the full ctest -j8 suite here: passed in 6.79 s, not skipped. The suite: 755 of 757, the two that fail being the clean-main failures #6008 and #6224. You are right that PR CI only compiles it; the body says so now.

Not done: #5911's ask 1 and the setTxFilter comment, which are outside a test-only PR.

73, ON8ST

@jensenpat jensenpat self-assigned this Oct 10, 2026

@jensenpat jensenpat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9c715b470911696fe586f4443a9685b751d67c01.

Issue fit: matches #5911's remaining ask. The withdrawn measurement table stays withdrawn; this test is the re-take, on the TXA chain, with the edges written as literals. It does not decide whether 300..2700 is the intended voice passband (ask 1). Refs #5911 is the right link. No production code changes, so the RadioModel seam is untouched.

Verdict: Approve with nits. No blockers.

Path What changed
tests/hl2_tx_passband_onset_test.cpp Socket-free TXA passband and onset measurement. Configures Hl2TxDsp from defaultTxPassbandForModeName(), asserts literal edges.
tests/tests.cmake Registers the target next to the other HL2 TX DSP tests. Exit 77 is the phasing build or a capture that stayed starved.

The second commit addresses the prior review: a non-stall fault fails instead of skipping, g_failures is tested before g_starved, DIGU's low edge has a 50 Hz stopband probe (stop100 == 2), and Capture is declared before Hl2TxDsp.

Verified on this SHA, TXA, Qt 6.12, QT_QPA_PLATFORM=offscreen. One run, exit 0. USB and LSB: worst in-band 0.009 dB, 100 Hz outside −59.38 dB, 150 Hz and beyond −142 dB. DIGU: 50 Hz and 3100 Hz at −59.38 dB, edges −6.02 dB, 250..2900 Hz within 0.009 dB. Key-up 50% at 95.54 ms, 10–90% rise 14.77 ms. Mid-over 50% at 72.92 ms, −60 dB lead 8.84 ms. Repeated legs matched. Narrowing defaultTxPassbandForModeName() to 300..2400 in a scratch tree made USB and LSB flatness and edge checks fail (exit 1); DIGU stayed green. Restored, rebuilt, exit 0 again.

CI on this SHA: build, check-windows, check-macos, Static checks, and Sanitizer option configures are green. Those jobs compile this target. They do not run it; ci.yml's per-PR gate is frozen, and the body says so. First upstream execution is full-suite.yml.

Nits (not merge-blocking): the non-stall failure string is the opposite of the condition it guards, and envDb indexes the capture with no upper bound when the 50% point is never found.

Non-blocking follow-up: the phasing modulator still exits 77, so #5911's phasing column is not re-taken here. A sanitizer or heavily loaded full-suite run can exit 77 when the worker is starved; that run is inconclusive, not a pass. DIGU has no deep-stop tone below 0 Hz; the low edge is covered at 50 Hz (−59.4 dB) and the deep stop is the 3500 Hz tone.

Comment thread tests/hl2_tx_passband_onset_test.cpp
Comment thread tests/hl2_tx_passband_onset_test.cpp
@jensenpat
jensenpat merged commit b3e0302 into aethersdr:main Oct 10, 2026
5 checks passed
@jensenpat jensenpat removed their assignment Oct 10, 2026
rfoust pushed a commit that referenced this pull request Oct 12, 2026
Refs #6347

Answers the two non-blocking notes left on the approving review of
#6347.
`tests/hl2_tx_passband_onset_test.cpp` only.

## Limits

- Test only. No product code, no tolerance and no bound changes; the
  measured figures are the same before and after.
- Neither path occurs on a healthy run. Both were reached by scratch
  variants of the test that are not in this PR.
- AddressSanitizer instrumented the test's own translation unit only,
not
  the library it links. The out-of-range read it reports is a BUS on a
  wild address, not a redzone hit.
- Built and run on one Mac (macOS, Qt 6.12.0), TXA build.

## Change

1. `run()`: when the modulator reports a fault that is not a stall drop,
the check failed with ": no fault but a stall drop". The FAIL line now
   states the expectation that broke: "every modulator fault is a stall
   drop", as the comment above `run()` describes it.
2. `onset()`: `firstAbove()` returns 1e9 when a level is never reached.
   `halfAt` is then far past the capture, and `envDb` indexed `c.iq` out
of range before `checkKeyUp` or `checkMidOver` could fail. `envDb` now
   returns the -400 dB floor for any index outside `[t0, c.iq.size())`.
3. `onset()`, from this PR's own review: when the 50 % point is never
reached, `leadMs` is the 1e9 sentinel too, not `1e9 - 1e9 = 0`, and the
short-capture return sets it as well. So the mid-over lead check fails
   with the others, where it passed for the wrong reason.

## Evidence

Four scratch variants, the test file compiled with `-fsanitize=address`
and linked against the tree's library at `5a8a6eb3`:

| variant | result |
|---|---|
| before this PR, 50 % level made unreachable | `AddressSanitizer: BUS
on unknown address`, in `envDb` called from `onset()`; no check ran |
| this PR, 50 % level made unreachable | no sanitizer report; envelope
printed as -400.0 dB; 6 checks failed cleanly, exit 1 |
| this PR, one non-stall fault forced | `FAIL: USB: every modulator
fault is a stall drop` (and per leg), exit 1 |
| this PR, unmodified | all checks passed, no sanitizer report; key-up
50 % at 95.54 ms, rise 14.77 ms; mid-over 50 % at 72.92 ms |

Full ctest at `e726c404`, `-j8`: 755 of 758 passed, 3 failed, all three
also on the serial rerun. Besides the two below, `vkamp_connection_test`
failed with only one of its two known checks (#5254).
- `peripheral_auth_dialog_interaction_test`: fails on clean main on this
  Mac (#6008); this diff touches no dialog file.
- `unified_title_bar_test`: fails on clean main on this Mac (#6224);
only
  its two known checks failed; this diff touches no title-bar file.

`hl2_tx_passband_onset_test` passed in 6.69 s.
`check_test_registration.py
--strict`, `check_wdsp_test_links.py --strict` and
`gen_touchpoint_manifest.py --check` pass.

73, ON8ST

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants