hl2_tx_loopback_test depends on a hardcoded IP on a foreign subnet (192.168.1.12) — passes or fails by accident of network

[nigelfenton]

Raised because the review on #4799 asked for it — hl2_tx_loopback_test was reported failing identically on a clean tree, "so it stops shadowing other work."

I could not reproduce that failure, and the reason is itself the bug: whether this test runs at all depends on what happens to answer at a hardcoded address on a subnet the developer may not even be on.

What it does

const QString simHost = QStringLiteral("192.168.1.12");
if (!simPresent(simHost)) {
    std::fprintf(stderr, "hl2_tx_loopback_test: SKIPPED — no simulator answering at %s\n", ...);
    return 0;
}

192.168.1.12 is hardcoded, on 192.168.1.0/24. There is no environment override and no way to point it elsewhere without a rebuild.

So the test has three possible behaviours on a given machine, and nothing in the name or the output distinguishes the second from the third:

Situation	Result
Nothing answers (my box — different subnet)	SKIPPED, exit 0 — looks like a pass
A real HL2 simulator answers	genuinely runs
Something else entirely answers	runs against the wrong device and fails

On this machine (10.0.0.0/24) it exits 0 every time, having tested nothing.

The probe is looser than it looks

static bool simPresent(const QString& host)
{
    QUdpSocket s;
    ...
    s.writeDatagram(..., QHostAddress(host), kMetisPort);
    return s.waitForReadyRead(1500);
}

waitForReadyRead() is true if any datagram arrives on that unbound-port socket within 1.5 s. It does not check the sender address and does not parse the reply as a Metis discovery response. An ICMP-adjacent stray, a broadcast, or an unrelated device on 192.168.1.12 all read as "the simulator is present" — and then the test proceeds to key a transmit path against it.

That last part matters: past the gate the test does qunsetenv("AETHER_AUTOMATION") deliberately to open the TX gate, then asserts canTransmit. It is a transmit test being pointed at whatever answered.

Why it shadows other work

A test that is green because it silently skipped is indistinguishable, from CI or a terminal, from one that is green because it passed. And when it does fail, the failure is about the reviewer's LAN rather than the code under review — which is exactly what happened on #4799, where it had to be explained away before the real findings could be discussed.

Suggested fix

1. Make the target configurable — an env var (AETHER_HL2_SIM_HOST or similar) with the current IP as a documented default, so it can be pointed at a real simulator or deliberately disabled.
2. Verify the reply, not merely that a packet arrived: check the sender is the host asked, and that the payload parses as a Metis discovery response.
3. Make the skip visible as a skip. Exit 0 with a message on stderr reads as a pass to every automated consumer. If the suite has a skip convention, use it; if not, this is a good reason to have one.
4. Consider whether a test that needs a live device on a specific subnet belongs in the default target at all, or should be opt-in like the other hardware-dependent checks.

Point 3 is the one that costs least and buys most — it turns a silent no-op into an honest one.

Environment

• Windows / MSVC, Qt 6.10.3, 10.0.0.0/24 — exits 0 (SKIPPED) on every run
• Reported failing on a clean tree elsewhere, per the feat(icom): IC-705 bring-up — controls, UX, CI-V stall, audio transport, and transmit-meter staleness #4799 review

[aethersdr-agent]

Confirmed, all four points, and the tree corroborates it more strongly than the report itself does. Triaged to maintainer-review.

Root cause

tests/hl2_tx_loopback_test.cpp:70 is exactly as quoted — 192.168.1.12 is a literal with no override path, and CMakeLists.txt:5030 registers it in the default ctest set with no opt-in guard:

add_test(NAME hl2_tx_loopback_test COMMAND hl2_tx_loopback_test)

The three-outcome table in the report is accurate. Two things worth adding:

1. That address is a specific developer's workstation, and the repo already documents the collision. HERMES.md:3005 and docs/hl2-multi-ddc-test-matrix.md:39:

Do not run the simulator on this Mac while testing the real radio — this machine is 192.168.1.12, which is the address hl2_tx_loopback_test probes, and a locally-running hpsdrsim makes that test run when it should skip.

So "something else entirely answers" is not hypothetical; it's a documented operating hazard with a written workaround, which is the shape of a bug that should have been fixed in code.

2. The #4799 failure is probably real and separate. HERMES.md:2999 lists under Still open:

hl2_tx_loopback_test fails against the simulator on transmit-sideband checks, non-deterministically. Commit 256142a6 — the pre-multi-DDC base — fails identically, so it is pre-existing.

That is a pre-existing flake in the sideband assertions (hl2_tx_loopback_test.cpp:237-245), tracked separately. The reporter couldn't reproduce it because on 10.0.0.0/24 the gate skips before reaching those lines — both observations are correct, they just come from different branches of the gate. Worth splitting out as its own issue rather than folding into this one.

On the probe (:48-58) — one precision on the report: the socket does bind() to an ephemeral port, so a stray subnet broadcast to :1024 won't reach it. The defect is narrower but still real: waitForReadyRead(1500) returns true on any datagram arriving at that ephemeral port from any sender, and the code never calls readDatagram(), never compares the sender to host, and never parses the payload. Any UDP responder at that address — including a colliding hpsdrsim already driven by another app — reads as "simulator present", and the test then does qunsetenv("AETHER_AUTOMATION") (:80) to open the TX gate and keys. That's why I've tagged this safety: the file's own header says "pointing it at real hardware would radiate."

Proposed fix

All in tests/hl2_tx_loopback_test.cpp plus one line of CMakeLists.txt. Notably, both hard parts already exist in the tree:

Verify the reply — AetherSDR::hl2::parseDiscoveryReply() is already declared in MetisProtocol.h:451 and already validates EF FE, board id and gateware; the test includes MetisProtocol.h and simply doesn't call it. Change simPresent() to readDatagram() into a buffer with a sender QHostAddress, require the sender to equal the requested host, require parseDiscoveryReply() to return a value, and require isHermesLite2() (board 0x06). Loop until the 1.5 s deadline rather than returning on the first packet, so one unrelated datagram doesn't consume the window.

Honest skip — the suite already has a skip convention: CMakeLists.txt:1947 uses SKIP_RETURN_CODE 77 for crdv_quarantined_test. Return 77 instead of 0 on the no-simulator path and add:

set_tests_properties(hl2_tx_loopback_test PROPERTIES SKIP_RETURN_CODE 77)

ctest then reports it as Skipped, not Passed. This is point 3 and it's a two-line change.

Configurable target — read AETHER_HL2_SIM_HOST via qEnvironmentVariable(), defaulting to 192.168.1.12, and treat an explicitly empty value as "deliberately disabled" → skip 77. Print the resolved host in the skip message.

Point 4 is the maintainer's call and I'd not bundle it. Once the skip is honest and the probe is verified, leaving it in the default target is defensible — it's self-disabling and now says so. If you'd rather it be opt-in, a LABELS hardware property plus a CI -LE hardware is the smaller lever than removing the add_test.

Handoff summary

Known: hardcoded host confirmed at :70, no override; probe at :48-58 validates nothing; skip masquerades as pass via return 0 at :75; collision hazard already documented in HERMES.md. Fix: env override + parseDiscoveryReply() verification + SKIP_RETURN_CODE 77, entirely in tests/hl2_tx_loopback_test.cpp and one set_tests_properties line. No production code changes. Recommend a separate issue for the pre-existing non-deterministic sideband failure noted in HERMES.md:2999, which is what #4799 actually hit.

---

_{🤖 aethersdr-agent · cost: $1.3702 · model: claude-opus-5}

---

_{📍 Citation health check (Foundry Constitution Principle I): 6 resolved, 2 unresolved against /Users/aetherclaude/workspace/AetherSDR at origin/main.}

Unresolved citations (2)

Citation	Reason
MetisProtocol.h:451	file not found in workspace
hl2_tx_loopback_test.cpp:237-245	file not found in workspace