Skip to content

feat(anan): subnet discovery and a front-end setup page - #6146

Merged
jensenpat merged 1 commit into
aethersdr:mainfrom
tropo1234:anan-front-end
Oct 17, 2026
Merged

jensenpat merged 1 commit into
aethersdr:mainfrom
tropo1234:anan-front-end

Conversation

@tropo1234

@tropo1234 tropo1234 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

ANAN-G2 receive front-end options live under Settings → Radio Setup → Radio → ANAN Front End, so users connecting from Available radios can configure them too. The seven existing options retain their AnanSettings values and next-connect semantics; Connect by IP shows an ANAN-only pointer to the page.

RFC #6209 is approved, with visibility choice (a): the page is available while disconnected, and capability-gated while connected. Other families' controls, settings and protocol paths are unchanged. The band-filter selector, speaker-amplifier mute and anan/frontend.set remain outside this PR.

ANAN discovery retains the global broadcast and adds one directed broadcast per distinct attached IPv4 subnet. This covers a second NIC on a distinct subnet. It retains the existing single wildcard socket: two NICs sharing a subnet still follow the preferred route, and VPN/router broadcast restrictions still apply. The operator explicitly chose to retain this design. /31 and /32 point-to-point entries are excluded even when Qt synthesizes a broadcast address equal to the local address.

The user guide covers the page location, all seven current labels, next-connect behavior and discovery limitations. The connection hint includes dither and randomization, and both new combos retain accessible descriptions.

Validation on signed head 04400ba31, rebased onto 5813556e0:

  • macOS Qt 6.12 app build passed; 3/3 focused CTests passed: anan_discovery_test, anan_settings_test, anan_backend_test.
  • Removing the prefix-length guard made the discovery test fail at both the /31 and /32 assertions on a8ae12c0a; the discovery source and PR patch are unchanged by the second rebase (git range-diff reports equality). The restored guard passes on the current head.
  • Offscreen bridge, isolated settings, TX disabled, DEMO-0001 only: all seven controls and descriptions; editing and restart persistence; ANAN-only connection hint across ANAN/Flex/HL2/Icom/RTL selections; page unavailable on demo connection and restored on disconnect. Proof instances closed.
  • Four engine/capability/command-plane gates pass on base and head; boundary findings match main. Registration, frozen CI gate, QSettings, network timeout, theme and colour checks pass. Accessibility remains the base's nine warnings.
  • npm ci and user-doc build pass; generated references are current; Log Analyzer passes 707 checks with refreshed source locations.

No live-radio, multi-NIC hardware, RF or TX validation was performed in this landing pass. Cross-platform CI is checked separately before merging. The automated code-review skill was unavailable; the final source pass was performed manually.

Generated with OpenAI Codex (GPT-6 Astra)

@tropo1234
tropo1234 requested a review from a team as a code owner October 4, 2026 05:26

@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 at head c258baa0, against the head checkout. Findings below are reasoned from source, not reproduced at runtime — no build, no test run, no GUI here.

Issue fit

No linked issue. The body cites "a G2 field report" with no issue or RFC number, so there is no thread to build a requirements list from and nothing redefining the ask. Reviewed against the PR's own stated intent instead.

Against that intent the discovery half lands cleanly and is the strongest part of the PR: the root cause (a datagram to 255.255.255.255 leaves by the default route only) is correct, the fix is additive by construction, and the bench result — a G2 that previously needed a typed IP now appearing in the picker — is exactly the evidence GOVERNANCE's "bug fixes with a clear root cause" exemption contemplates. No RFC needed for that item.

The other three items are a different matter; see Scope and the maintainer-decision section.

Scope

The title claims two things. The diff does four. The body discloses all four honestly and at length — the objection is bundling and title coverage, not concealment.

File / group What it changes Claimed by title/body? Verdict
anan/AnanDiscovery.{h,cpp} per-interface directed broadcast + broadcastTargets() title In scope — bug fix, bench-confirmed
gui/ConnectionPanel.{h,cpp}, gui/RadioSetupDialog.{h,cpp} seven ANAN rows removed from the connect form, new "ANAN Front End" page title Changes existing UX; no issue or RFC cites the old placement as a defect — §6 ruling (D3)
anan/P2Protocol.{h,cpp} (AlexRxFilter, alexRxFilterBits, alexRxFilterFor, HighPriorityOptions), P2Client live setter, AnanSettings::bandFilters*, the page combo new wire-level hardware control — energises relay banks from a band-edge ladder 4 of whose 7 selections are unverified body only, not the title New hardware surface, bundled — ruling (D1)
P2Protocol byte 1400, P2Client::setSpeakerAmpMuted, AnanSettings::speakerAmpMuted, page checkbox new wire control — speaker-amp mute body only, not the title New wire surface — ruling (D1)
AnanBackend.cpp anan/frontend.set extension verb, adcFrontEnd declaration body New extension verb — ruling (D2)
RadioCapabilities.h + docs/architecture/radio-capabilities-map.md AdcFrontEnd record + map row body In scope; correct shape per #5262 M2 (record, not five bools)
models/RadioModel.cpp two new anan.* connect params body In scope — inserted inside the pre-existing family == "anan" branch, no new family-string branch
tests/*, tests/tests.cmake 2 new targets, 3 extended body In scope. No sockets — see below
AnanBackend.{h,cpp}, P2Client.{h,cpp}, MainWindow.{h,cpp}, anan_link_telemetry_test.cpp link telemetry + PA-row withdrawal declared as #6137's commit Not reviewed here — belongs to #6137

CHANGELOG.md correctly untouched.

Socket tests: none. Both new targets (anan_front_end_test, anan_discovery_test) are registered in tests/tests.cmake with add_test, and I read both sources — no QUdpSocket/QTcpSocket/bind/listen/connectToHost/Fake* peer, and no fake radio standing in for firmware. anan_front_end_test reaches P2Client::highPriorityOptions() through a friend struct P2ClientTestAccess (P2Client.h:218) on a client that is never start()ed. Neither new target is on the frozen per-PR ctest -R gate — .github/workflows/ci.yml contains no anan match at all — so neither gated the green checks above.

Blockers

1. src/core/backends/anan/P2Client.cpp:456-468 — the body's independence claim is false, and the merge order is load-bearing.

The body states:

The dependency is only on the branch, not on the code: nothing in this commit references anything #6137 adds.

sendRunningHighPriority(), added by this commit, is:

void P2Client::sendRunningHighPriority()
{
    sendTo(*m_socket,
          buildHighPriority(true, sharedFreqWords(),
                            m_bypassAdc0Filters, m_bypassAdc1Filters,
                            m_adc0AttenuationDb, m_adc1AttenuationDb,
                            highPriorityOptions()),
          m_host, kHighPriorityPort, m_txBytes);
}

Both the fifth sendTo() parameter (txAccum, which #6137 made required precisely so no send site can omit it) and the member m_txBytes are added by #6137 — git log -S'm_txBytes' on this branch returns exactly two commits, #6137's and this one, and nothing earlier. So c258baa0 does not compile against main, and five of this commit's own call sites in start(), setDdc0FrequencyHz(), setStepAttenuationDb() and onKeepaliveTick() are rewrites of #6137's lines.

This matters because the sentence is the one a maintainer would rely on to deviate from the stated order — cherry-picking c258baa0, or rebasing this branch onto main expecting it to build. The remedy is a one-sentence body edit, not a code change: delete the claim and keep the "#6137 merges first" instruction, which is correct and already bolded.

Needs maintainer decision

D1. The Alex band-filter selector and the speaker-amp mute are new wire-level hardware controls in a PR titled "subnet discovery and a front-end setup page."

P2Protocol.cpp:142-178 adds a seven-way relay selection and the ladder that drives it; P2Client.cpp:405-454 makes it live on a 100 ms packet; byte 1400 bit 1 newly asserts a hardware mute. None of this is discovery and none of it is a setup page. The author's own disclosure is the reason to take it seriously rather than wave it through: 4 of the 7 selections are untested, the 22 MHz edge sends 17 m and 15 m to a filter proven only at 14 MHz, and the failure mode is silence, not degradation — the relays are in series ahead of the ADC.

The mitigations are real: it ships defaulting to Bypassed, which the body says is byte-for-byte the previous behaviour (consistent with P2Protocol.cpp:203-206, where opts.adc0Filter is nullopt and the else if (bypassAdc0Filters) branch reproduces the old bit), and the tooltip tells the operator to check band by band. So this is not a risk to anyone who leaves it alone.

Worth noting that you have the author's own agreement on the shape: #6129's body (same author, open) lists "the connect-time options moving into Radio Setup" and "the filter-board selector" as separate changes against this backend — the split I am asking for here is the one that PR already announced. #6129 also edits the same populateFamilyParams() and makes a restored span win over anan.ddc0RateKsps, which would falsify this page's new Sample-rate tooltip ("the width of the panadapter span a session starts at"); whichever lands second needs to reconcile that wording.

My recommendation is to split the band-filter selector into its own PR so it gets a ruling on its own evidence rather than inheriting the discovery fix's. At minimum the title should name both new controls. Your call — the disclosure is complete enough that an informed ruling is possible as-is.

D2. New extension verb anan/frontend.set (AnanBackend.cpp:1361).

Consistent with the established per-family pattern (hl2 hw.set, icom, sim), partial-by-design, and reached from the GUI through backendDeclaresExtension() rather than a family string — which is the right shape and worth crediting. Flagging it because it is new surface on the backend seam.

One question against the AGENTS.md #5554 standing notice ("no new capability declared without a verb behind it"): adcFrontEnd has five fields, and only two — hasTunedBandFilters and hasSpeakerAmpMute — have frontend.set behind them. The other three (hasDitherAndRandom, hasAdcSelect, hasFilterBankBypass) are connect-time Params with no verb. I read that as satisfying the notice's intent (the capability does drive real action, just not all of it through a verb) rather than violating it, but it is your ruling, not mine.

D3. The connect form loses seven working controls (§6 preference check).

ConnectionPanel.cpp drops 187 lines of working UI; the controls reappear as a Radio Setup page. The move is well-argued — those rows only ever wrote AnanSettings, the connect path has always read AnanSettings, and the page is reachable with nothing connected so it can configure a first connect.

But it cannot point at an authority. There is no issue and no RFC calling the connect form's placement a defect, and GOVERNANCE.md's no-RFC list covers "New applets or dialogs that don't change existing UX" — this one does change existing UX. That puts it in §6 preference territory: correct code, plausible reasoning, unratified behaviour change. It needs your ruling rather than my approval. I am not calling it a blocker because it is disclosed in the title and the body, and because the two new live controls genuinely need a home that exists while disconnected.

D4. Security — the sweep now reaches every attached subnet, with no opt-out. (security-audit (CodeGuard), confirmed against source)

AnanDiscovery.cpp:52-62 accepts every interface that is merely IsUp|IsRunning; :184-188 sends the probe to each one's subnet broadcast every 5 s (AnanDiscovery.h:35, default intervalMs = 5000) for as long as discovery runs. The point-to-point tun case is handled (broadcast().isNull() at :81) and loopback is dropped by address, but tap-mode VPNs, Docker/libvirt bridges, hotspot and guest networks are not — those have real broadcast addresses.

The payload itself is harmless: buildDiscovery() is 60 zero bytes with pkt[4] = 0x02, no hostname, no MAC. What is newly disclosed to peers on those subnets is that this machine runs AetherSDR and the ephemeral source port of m_socket. On main the single QHostAddress::Broadcast reached only the default-route subnet.

This is deliberate — AnanDiscovery.h:45-50 names "a VPN up" as the motivating case — so it needs you to accept the trade rather than the author to fix a mistake. If you would rather not: restrict directed broadcast to interfaces whose subnet matches a configured radio address, or make the directed sweep opt-in with the unconditional 255.255.255.255 probe as the default.

D5. Security — onReadyRead() applies no sender filter, and this change widens who can reach it. (security-audit (CodeGuard), confirmed against source)

AnanDiscovery.cpp:207-253 accepts any datagram that parses and passes isSaturn(), then sets info.address = dg.senderAddress() (:220). Selecting that picker entry makes P2Client::start() dial the address. Protocol-2 discovery is unauthenticated by design, so the bound available is topological.

This code is pre-existing and not in the diff — I am raising it because D4 changes the attacker set from "a peer on the default-route subnet" to "a peer on any attached subnet, including a corporate VPN or a container bridge." The widening is what this PR does; the missing check is what makes it matter.

The available fix is a subnet containment test, and broadcastTargets() already enumerates exactly the entries needed (QNetworkAddressEntry::ip()/netmask() → QHostAddress::isInSubnet). Hl2TelemetryPoller.cpp:208-212 is the in-tree shape of a sender filter, though a weaker precedent than it first looks — it filters to a known target, which discovery by definition does not have. Routing this to its own issue rather than growing this PR seems right.

Nits

All non-blocking.

  • src/core/backends/anan/AnanBackend.cpp:1395 — stray indentation. The droop branch sits at 8 spaces after the new frontend.set block closes, reading as if it were nested inside it:

      if (ns == QLatin1String("anan") && verb.startsWith(QLatin1String("droop."))) {
  • src/core/backends/hl2/Hl2Discovery.cpp:179-187 — the sibling is left unfixed. Hl2Discovery::sweepNow() still sends only to QHostAddress::Broadcast, so a Hermes-Lite 2 on a second NIC or behind a bridge still never appears in the picker — the same defect, the same root cause, the same protocol family. broadcastTargets() is a static on AnanDiscovery, so HL2 cannot reuse it without reaching into the ANAN class. Since the flag test was already derived from NetworkPathResolver::enumerateIpv4Candidates() (per the comment at AnanDiscovery.cpp:46-51), that looks like the natural home for a shared helper. Completeness note only — the PR does not claim to fix HL2.

  • src/core/backends/anan/P2Protocol.cpp:165-177 — alexRxFilterFor() has no finite guard. Every comparison against NaN is false, so a NaN tuning falls through the whole ladder to Preamp6m — the one selection whose consequence on HF is silence. A negative frequency is safe (< 1.5 → Bypass). I could not find a path that delivers NaN (AnanBackend.cpp:755 passes m_sliceFreqHz), so this is cheap insurance rather than a live defect. The file has no <cmath> today, so the fix is two lines:

    #include <cmath>   // std::isfinite, alexRxFilterFor()'s NaN guard
        const double mhz = tunedHz / 1e6;
        if (!std::isfinite(mhz)) return AlexRxFilter::Bypass;
        if (mhz < 1.5)  return AlexRxFilter::Bypass;      // below the bank's lowest filter

    Separately, the final return sends everything above 35 MHz to Preamp6m, including the span between the 6 m band and the 61.44 MHz Nyquist this radio can reach. Inert at the shipped default, and consistent with the body's "untested" labelling.

  • src/gui/RadioSetupDialog.cpp:4922-4950 — the two bypass checkboxes now have exactly one usable value. When the bank follows the tuned frequency they are superseded and disabled; when it does not, unchecking one leaves that ADC with no signal path at all — which the accessible description says plainly, to its credit. A control whose only non-default setting breaks reception may be better removed than relocated. Raising it because this PR moved them into a new page rather than leaving them alone.

  • src/core/backends/anan/AnanBackend.cpp:1361-1393 — frontend.set deliberately does not write AnanSettings ("the setting is what survives a restart and the verb is what reaches the radio now"). The GUI pairs the two correctly, but the verb is on the public extension surface; a future bridge or automation caller would get a change that silently reverts on the next connect. Worth one line in the verb's own comment telling callers they must persist separately.

What I tried to break

  • The Alex word halves. The body's protocol finding — that 1432–1435 is really two 16-bit words, and ADC0's receive filter is the low half at 1434–1435 — checks out against the code. writeU32be(&pkt[1432], alex0) puts ANT1 (bit 24) in byte 1432 and every filter bit (12, 6, 5, 4, 3, 2, 1) in bytes 1434–1435. The claim and the encoder agree, and a test reads 1434–1435 specifically.
  • Byte-for-byte compatibility at the default. With bandFiltersEnabled false, highPriorityOptions() leaves both filters nullopt, so buildHighPriority() takes the else if (bypassAdc0Filters) branch and emits the old bit. Byte 1400 stays zero. The "default is what this client always sent" claim holds.
  • A stale relay after a retune. setDdc0FrequencyHz() sets m_ddc0FreqHz and m_ddc0FreqWord together and then sends, and all four run=1 senders funnel through sendRunningHighPriority(), so no sender can disagree. start() resets both to 0 (P2Client.cpp:84, :92), so the "reset with m_ddc0FreqWord above" comment is accurate and a reconnect cannot inherit the previous session's band. anan_front_end_test pins the stale case, the both-orders case, and the turn-it-back-off case, and it would fail if highPriorityOptions() returned a default — it is not a test asserting the implementation back to itself.
  • The reseed writing back what it just read. m_ananFrontEndReseed blocks all nine controls with QSignalBlocker before seeding, which it has to: every control persists on change and two of them push live. Unblocked, showEvent would push a mute to the radio as if the operator had asked. Covered.
  • Nothing lost in the UI move. populateFamilyParams() (RadioModel.cpp:669-701) reads all nine values from AnanSettings, never from a widget, and a tree-wide grep for the removed connectionManualAnan* object names across *.cpp/*.h/*.py/*.md/*.json/*.yml returns nothing outside ConnectionPanel.cpp itself — so the body's "nothing was lost" holds, including for automation scripts and docs. The connect form keeps a pointer to the new location (ConnectionPanel.cpp:852).
  • Canon checks that came back clean. The capability is a record, not a bool, so the #5262 M2 freeze is untouched; the map doc has its row and anan_backend_test pins all five fields. The new settings keys land inside the existing nested Anan document (Principle V) with no new flat key. The touchpoint manifest needs no edit — AnanSettings.h keeps exactly one GUI includer as the include moves from ConnectionPanel.cpp to RadioSetupDialog.cpp (manifest row: 1). The page gates on !connected || capability, not a family string; theme tokens are the ones this file already uses 54 times; no CHANGELOG.md entry. On the counters and the 1444-byte builders the security audit walked every AnanBackend→m_client call site and every packet index and found no race and no out-of-bounds write; I spot-checked the bounds and agree, and those counters belong to #6137 regardless.
  • What I could not check. Anything at runtime: no build, no ctest, no GUI, so the 629/629 and the 9/9 mutation claims are unverified here, as are the bench results and the band edges. CI is 5/5 green on c258baa0, but that merge base contains #6137's branch, so it does not show this commit building on main — see Blocker 1 — and the two new test targets are not on the frozen per-PR gate. check_a11y I could not run; by reading, the new combos carry accessible names, the checkboxes rely on their label text as Qt intends, the disabled-state explanation reaches the accessible description rather than only the tooltip, and no interactive QLabel is added.

Recommendation

Needs maintainer decision. The one blocker is in the PR body, not the code — a single false sentence, fixable in an edit — so what actually gates this change is your ruling, not a rework. The discovery fix is the best-evidenced part of this PR and I would take it as-is. What needs your ruling is everything riding with it: a new relay-bank control with four unverified selections and a silence failure mode (D1), a relocation of seven working controls with no issue or RFC behind it (D3), and a discovery sweep that now reaches corporate VPN and container subnets by design (D4/D5).

Concrete next step: fix the one false sentence in the body (Blocker 1) and keep the stated merge order behind #6137; then rule on whether the band-filter selector splits out. If it does, what remains — discovery, the page, the mute, the capability record — is a clean, unusually well-documented change.

Thank you for the disclosure discipline here. The untested-selections table, the "what is not verified" section, and the removed supply-rail readout with its 40.3 V-on-13.8 V measurement are exactly the kind of thing that makes a PR reviewable, and naming the four untested filters cost you a harder review than hiding them would have.


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

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Antares Detector — candidate vulnerable file(s)

  • src/core/backends/anan/P2Client.cpp

onReadyRead() receives QNetworkDatagram datagrams and immediately calls asBytes(dg.data()) (line 132), creating a span over raw network memory without validating the datagram size. handleDatagram() then copies bytes into local spans and calls parseDdcFrame/parseHighPriorityStatus (lines 515-535), which will panic on oversized datagrams. This is a raw-memory buffer-copy vulnerability (CWE-800/memory-safety violation) reachable via UDP datagram transport.

Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting.


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

tropo1234 added a commit to tropo1234/AetherSDR that referenced this pull request Oct 4, 2026
…Principle VIII.

Review follow-ups on aethersdr#6146, all non-blocking, all from the review of c258baa.

- alexRxFilterFor() sent a non-finite tuning to the 6 m preamp. Every
  comparison against NaN is false, so an unguarded ladder falls through to its
  last arm -- and on HF that selection is silence, not a weaker signal. No
  caller delivers NaN today (AnanBackend passes m_sliceFreqHz), so this is
  insurance rather than a fix. Two assertions pin NaN and +inf; -inf is
  deliberately not asserted, because it takes the `< 1.5` branch with or
  without the guard and so would not discriminate. Deleting the guard turns
  anan_p2_protocol_test red.

- frontend.set now states that callers on the extension surface must persist
  to AnanSettings themselves. The GUI pairs the two, but a bridge or
  automation caller that only invokes the verb would get a change that
  silently reverts on the next connect, because connect reads AnanSettings and
  nothing else.

- The droop branch carried a stray 8-space indent that read as if it were
  nested inside the frontend.set block.

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

@ten9876 ten9876 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.

Second-pass review at head 11dda168. It covers only this PR's own two commits (840cf4ff..11dda168); 840cf4ff belongs to #6137 and is reviewed there. Unlike the earlier aethersdr-agent review of c258baa0, which was reasoned from source, this pass builds the head, runs the tests, mutation-tests the claims, drives the new page, and checks the wire claims against p2app's source.

Issue fit

There is no linked issue; the body cites a G2 field report. Against that intent, all four items land, and the protocol claims are verified against p2app (laurencebarker/Saturn), not just consistent with the spec:

  • Byte 1400 bit 1 = speaker-amp mute. InHighPriority.c:154-156: SetXvtrEnable(Byte&1); SetSpkrMute((Byte>>1)&1). Bit 0 is XVTR, as the body says.
  • "1432–1435 is two 16-bit words." InHighPriority.c:182 reads rd_be_u16(+1432) as the TX/antenna word, :204-205 reads +1430 → AlexManualRXFilters(Word, 2) (RX2), and :207-208 reads +1434 → AlexManualRXFilters(Word, 0) (RX1). ADC0's receive filter really is the low half.
  • "p2app writes its register only when the value changed." saturnregisters.c:1106, if(Register != GAlexRXRegister). Manual filter select is forced on at startup (p2app.c:519), so the client's bits are the ones that apply.

Follow-up from the earlier review: its Blocker 1, the false "no code dependency" sentence, is fixed in the current body, which now states the dependency correctly. 11dda168 does what its message says. The NaN/+inf guard is real (mutation below), the frontend.set persist note is present (AnanBackend.cpp:1373-1377), and the stray indent is gone (:1400).

Scope

File / group Change Claimed Verdict
anan/AnanDiscovery.{h,cpp} per-interface directed broadcast, broadcastTargets() title in scope, bench-confirmed
gui/ConnectionPanel.{h,cpp}, gui/RadioSetupDialog.{h,cpp} 7 connect-form rows → "ANAN Front End" page title maintainer decision (D3), as before
anan/P2Protocol.*, P2Client.*, AnanSettings.*: Alex band-filter selector new wire-level relay control body, not title maintainer decision (D1), as before
byte 1400 mute (P2Protocol, P2Client, AnanSettings, page) new wire control body, not title maintainer decision (D1)
AnanBackend.cpp anan/frontend.set new extension verb body maintainer decision (D2); see the new nit on the capability fields
RadioCapabilities.h + caps map AdcFrontEnd record body shape OK (check_capability_records --strict: 70 … ok), but see nit 1
models/RadioModel.cpp two anan.* connect params inside the existing anan branch body in scope
tests/*, tests.cmake anan_front_end_test, anan_discovery_test new; three extended body in scope; socket-free (no bind, listen, start() or fake peer in either new source)

No CHANGELOG.md entry. Engine-boundary findings on this head versus #6137's head differ only by line shifts (102 findings either way, 0 blocking under --strict). The touchpoint manifest is up to date, and check_a11y, check_command_plane, check_test_registration, check_ci_test_gate, check_network_timeouts and check_qsettings are clean.

Blockers

None from this pass. Decisions D1–D5 from the earlier review (the band-filter selector and mute bundled under a discovery/setup-page title, the frontend.set verb, the connect-form relocation without an issue or RFC, the directed sweep reaching VPN/container subnets, and the unchanged absence of a sender filter in AnanDiscovery::onReadyRead()) are still open. They need a maintainer ruling, not author rework, and I've forwarded them to the maintainer.

Nits (non-blocking)

  1. src/core/backends/RadioCapabilities.h:550-581: the five AdcFrontEnd fields have no consumer, and the comment promises one. The header says "A false field means the control is not offered, not that it is offered and disabled". Nothing reads hasDitherAndRandom, hasAdcSelect, hasFilterBankBypass, hasTunedBandFilters or hasSpeakerAmpMute; a tree-wide grep finds them only in AnanBackend.cpp:411-415, anan_backend_test.cpp:534-539 and the caps map. The page gates on adcFrontEnd.has_value() alone (RadioSetupDialog.cpp:4739) and builds all nine controls unconditionally. The same comment also says "these six" and "all six" (:556-563), but there are five fields. This is harmless today, since ANAN is the only declarer and declares all five true. Still, it is capability surface with nothing behind it, which docs/agents/backends.md § Standing notice — #5554 asks PRs not to add ("no new capability declared without a verb behind it"). Either gate each control group on its field when connected, or collapse the record to the one fact the page actually reads.
  2. Three comments describe a supply-rail calibration that is not on the page. RadioSetupDialog.h:140 ("…the radio's speaker amplifier and the supply-rail calibration"), RadioSetupDialog.cpp:5-6 ("the rate list and the supply-rail reference are reached through this header") and ConnectionPanel.cpp:843 ("the filter bank and the supply-rail calibration that joined them there"). The body says that control was built, bench-tested and removed. Per AGENTS.md § "Comments", these should state the current invariant.
  3. The amp-mute hint overclaims. RadioSetupDialog.cpp:5016 says "A disconnect always hands the radio back unmuted, so quitting here cannot leave a radio that seems to have lost its audio." That holds for P2Client::stop(). It does not hold for a crash, a kill, sleep, or a pulled cable. p2app's CheckForActivity() (p2app.c:351-370) reverts to inactive after 1 s of silence and clears MOX/TX/CW, but never touches SetSpkrMute. The radio stays muted until some client's next High Priority packet. Suggested wording: "A clean disconnect hands the radio back unmuted."
  4. The disabled bypass checkboxes don't look disabled. Driven on the PR head: with "Follow the tuned frequency" selected, ananBypassAdc0/1 report "enabled": false, but in the grabbed page they render with the same text colour and check glyph as an enabled box. kCheck sets QCheckBox { color: {{color.text.primary}} } for every state, with no :disabled rule. The accessible description explains the state to a screen reader, but a sighted user gets no cue. AGENTS.md asks for a control the radio can't use to be "dimmed with a stated reason". A QCheckBox:disabled { color: {{color.text.secondary}}; } clause would do it.
  5. Every family sees "ANAN Front End" while disconnected. This is driven and by design (ananFrontEndAvailable() returns true with nothing connected), and it matches how the Flex-only pages already behave when disconnected. I'm noting it so the maintainer rules on it knowingly: a FLEX or HL2 owner opening Radio Setup offline now sees an ANAN-branded page.

What I tried to break

  • Built and ran anan_backend_test, anan_p2_protocol_test, anan_front_end_test, anan_discovery_test and anan_settings_test: 5/5 pass. I didn't build #6137's socket-owning anan_link_telemetry_test.

  • Mutations, each reverted (tree confirmed clean, then rebuilt; the probe build reports dirty:false, sha 11dda1683):

    • Delete the std::isfinite guard → anan_p2_protocol_test 2 FAILs (NaN, +inf), as 11dda168's message says.
    • Shift ADC0's filter into the high half of 1432–1435 → 3 FAILs.
    • Drop 255.255.255.255 from broadcastTargets() → anan_discovery_test 7 FAILs.
    • Set byte 1400 bit 0 as well → 2 FAILs.
    • Stop setDdc0FrequencyHz() storing hertz (the stale-filter case) → anan_front_end_test 5 FAILs.

    None of these tests asserts the implementation back to itself.

  • Drove the page (offscreen, isolated settings store, AETHER_AUTOMATION_NO_TX=1, demo simulator only):

    • Disconnected: "ANAN Front End" is listed under Radio, and all nine controls are present with their accessible names and the defaults the body claims (48 ksps, ADC0, dither/random on, bank Bypassed, both bypasses checked, speaker stream and amp mute off).
    • Interaction: setCurrentIndex 1 on ananFilterBoard → ananBypassAdc0/1 go "enabled": false. Clicking ananSpeakerAmpMute → the store holds Anan {"bandFilters":true,"speakerAmpMuted":true}.
    • Gate: connecting DEMO-0001 hides the page from the nav (the demo declares no adcFrontEnd), and disconnecting brings it back. The nav grab is byte-identical to the first disconnected grab.
    • Persistence: after killing and relaunching, the page reads back Follow the tuned frequency and mute checked, so the settings survive a process boundary.
  • Byte-for-byte default. With bandFiltersEnabled false, both filters stay nullopt and buildHighPriority() falls through to the pre-existing bypassAdc0Filters branch, and byte 1400 stays 0. The front-end test pins this.

  • CI freshness. Runs for 11dda168 were created 2026-10-04T14:49:42Z, and main is at 18:26:40Z. Green here means a merge about 3.5 h stale that includes #6137's commit, and none of the five ANAN targets is on the per-PR ctest -R gate.

Could not verify: anything on a live G2. That includes the bench claims, the four untested filter selections, and whether the directed sweep reaches a second-NIC radio. The demo has no P2 transport.

Recommendation

Needs maintainer decision. I found no code blocker. The verified protocol work and the mutation-proven tests make this a solid change at the code level. What gates it is D1–D5. Separately, it can't merge before #6137, which currently has a requested-changes review (the rxPackets loss denominator). Nits 1–4 are small and worth folding in with whatever the rulings require.

@ten9876

ten9876 commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Maintainer rulings on #6146 (following the review above)

  • Split the band-filter selector and the speaker-amp mute into their own PR. This PR should stay discovery plus the setup page.
  • Subnet discovery (sweeping every attached subnet, accepting replies from any sender) is accepted as-is.
  • The anan/frontend.set extension verb, and moving the seven connect-form controls to a Radio Setup page, need an RFC first (GOVERNANCE.md). Please open one covering the new verb and the page move. This PR waits on that, and it still merges after feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. #6137.

@tropo1234

Copy link
Copy Markdown
Contributor Author

Thanks for the review and the rulings. #6146 is now one commit on #6137's head b93217a1, holding subnet discovery and the setup page only.

Rulings

  1. Split. The band-filter selector, the speaker-amp mute and the anan/frontend.set verb have left this PR, together with their wire code, settings, connect params, page controls and anan_front_end_test. The verb is the live path for exactly those two controls, so it went with them. They open as their own PR once RFC [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209 is approved, as GOVERNANCE asks.
  2. Subnet discovery: unchanged.
  3. RFC: [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209 covers the page move and the verb. This PR waits on it and still merges after feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. #6137.

Nits

  1. AdcFrontEnd fields with no consumer: collapsed to a record with no fields, the second option you offered. ANAN declares it, the page checks has_value(), and the comment says a field arrives together with the consumer that leaves a control out.
  2. Stale supply-rail comments: RadioSetupDialog.h, the include note in RadioSetupDialog.cpp and ConnectionPanel.cpp now describe the current code. The ConnectionPanel.{h,cpp} comments no longer retell the move either.
  3. Amp-mute hint: travels with the split. RFC [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209 records the corrected wording, "A clean disconnect hands the radio back unmuted".
  4. Disabled bypass checkboxes: travels with the split. Without the filter-bank selector nothing disables them, so the state no longer exists here. The split PR carries the QCheckBox:disabled rule.
  5. The ANAN page listed for every family while disconnected: put to you in RFC [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209 as choice (a) or (b).

Also changed

  • The page intro. It said this radio "has no command" to change the rate, ADC, dither or randomization mid-session. That was wrong: the DDC-Specific packet can be re-sent on a live session, which is what setDdcRateLive() does. It now says the options take effect on the next connect, and that zoom can still change the rate mid-session.
  • The bypass checkboxes' texts are back to what is true without a selector: no band filter is ever selected, so leave them checked.
  • The supply-voltage row in the capabilities map is no longer touched here. That topic belongs to feat(anan): show the G2's supply voltage, read from the input that carries it. Principle VIII #6211, which publishes the G2's supply voltage from the input that actually carries it.
  • The design doc. docs/architecture/anan-p2-backend-design.md noted that the connect shape "shipped as a dedicated ANAN section in ConnectionPanel". That note now points to the page and [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209.
  • The Antares advisory concerned P2Client.cpp, which this PR no longer touches. For the record, the over-read it describes cannot happen: the status parse requires exactly 60 bytes, and the DDC parse requires the exact declared length.

Evidence

On b93217a1 plus this commit:

  • ctest: 656 tests, 654 passed, 1 skipped (the app_settings_safety environment skip), 1 failed (pgxl_panel_test, the known local failure).
  • Static checks: all 17 steps pass with --strict. The colour ratchet is +0, and the four GUI files' a11y findings are the base's nine, unchanged.
  • Six mutations, each killed by its own assert:
    • the global broadcast dropped
    • the null/non-IPv4 skip dropped
    • the loopback skip dropped
    • de-duplication dropped
    • a rate missing from the page's list
    • the record not declared
  • On the G2:
    • The page shows the seven controls in three groups, with no selector and no mute.
    • Connect by IP shows only the pointer line.
    • The radio is discovered and connects from Available radios.
    • The page stays listed while connected.

@tropo1234
tropo1234 force-pushed the anan-front-end branch 3 times, most recently from a52718e to 36ce5ce Compare October 8, 2026 12:59
@K5PTB

K5PTB commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Adversarial review at head 36ce5cec (one commit on 14ef3e78), built from a fresh clone of aethersdr/AetherSDR with the repo's /pr-review skill. I ran the PR's tests, tried mutations, ran a probe against the real AnanDiscovery::broadcastTargets() on a live host, and drove the page through the automation bridge (demo simulator, offscreen, isolated settings, AETHER_AUTOMATION_NO_TX=1). Anything not marked as run below is reasoned from source.

Issue fit

There is no linked issue. The PR is measured against its own body and the 10-04 maintainer rulings: subnet discovery was accepted as-is, and the page move was sent to an RFC. Both halves do what the body says, with one exception covered under D1 below: discovery reaches a second NIC only when that NIC's subnet is unique on the host. The tests pass, the static checks reproduce, and the page behaves as described when driven. The split from the rulings is done: the band-filter selector, amp mute and frontend.set are all gone from the diff.

Scope

File / group Change Claimed Verdict
anan/AnanDiscovery.{h,cpp} broadcastTargets(), per-subnet directed broadcast title + body In scope (accepted 10-04). See D1 and nit 1
gui/ConnectionPanel.{h,cpp}, gui/RadioSetupDialog.{h,cpp} 7 connect-form rows → "ANAN Front End" page title + body In scope, gated on RFC #6209 (see D2)
RadioCapabilities.h, AnanBackend.cpp, caps-map row AdcFrontEnd {} record, declared by ANAN body In scope. Record, not bool (#5262 M2). No-field shape answers the earlier nit
anan/AnanSettings.{h,cpp} validDdc0RatesKsps(); shape comment gains speakerAudioEnabled body In scope (EB3 workaround, stated)
models/RadioModel.cpp comment only body In scope
docs/architecture/* design-doc note, caps-map row body In scope
tests/*, tests.cmake anan_discovery_test (new), two extended body In scope. Socket-free: no bind, listen or fake peer

Everything in the diff is explained by the body. No CHANGELOG.md entry. No new settings key, verb or protocol surface.

Blockers

None in the code.

Needs maintainer decision

D1. src/core/backends/anan/AnanDiscovery.cpp:170-189: a single wildcard socket cannot reach a second NIC that shares a subnet with another interface. #6009, already in this PR's base, solved the same root cause the other way.

sweepNow() sends every target from one AnyIPv4 socket, and broadcastTargets() de-duplicates by broadcast address. Two interfaces on the same subnet therefore produce one datagram, and the kernel sends it out whichever interface its route prefers. That covers two link-local NICs (a radio cabled straight to a USB-Ethernet adapter with no DHCP, on a host that has another 169.254 interface) and a shack LAN that reuses the home LAN's default 192.168.1.0/24. Run against the PR's own function:

two link-local NICs targets: 255.255.255.255 169.254.255.255

The global broadcast also leaves by one interface, so in that topology nothing reaches the radio. Reasoned, not reproduced: I have no multi-NIC host.

This matters now because of what landed after the 10-04 ruling:

Related context, pre-existing and outside this diff: P2Client::start() binds AnyIPv4 (P2Client.cpp:113), and AnanBackend never reads RadioConnectRequest::localBindAddress, which #6009 added for this purpose. So a radio that is discovered still gets no interface pin for its session.

Options, your call:

  • (a) Take it as accepted: it fixes the bench case, a second NIC on its own subnet. Narrow the two comments and the body to "a second NIC on a subnet of its own".
  • (b) Adopt fix(hl2): discover on every local interface #6009's per-interface socket shape here, ideally as one shared helper so the two P2/P1 discovery classes stop diverging.

D2. RFC #6209 is still open, and its open question changes this PR's code.

On 10-04 you ruled that the page move needs an RFC first and that "This PR waits on that". GOVERNANCE.md:131 says: "Do not open a PR until the RFC issue is approved." #6209 is OPEN, labelled rfc only. The bot misclassified it as CI work, and the maintainer-review label requested on 10-07 is not there.

Its open question is whether the page is listed for every family while disconnected (a) or only for an ANAN profile (b). That is exactly RadioSetupDialog::ananFrontEndAvailable() (RadioSetupDialog.cpp:4681-4698), which implements (a). A ruling of (b) is a code change here, so this PR cannot be final before #6209 is.

The body discloses this. I am recording it as a merge gate, not a defect. Given that discovery was accepted on its own, you may also prefer to land D1's resolution separately rather than hold it behind the RFC. Your 10-04 ruling bundled them, so that is your call too.

Nits (non-blocking)

1. src/core/backends/anan/AnanDiscovery.cpp:69-91 and tests/anan_discovery_test.cpp:83-100: the point-to-point case is tested against a shape Qt never produces.

The test builds its tunnel entry with no broadcast address ("which is the real shape of a tun device"). But QNetworkInterface::allInterfaces() runs postProcess() (qtbase src/network/kernel/qnetworkinterface.cpp:23-47). That fills in any null IPv4 broadcast as ip | ~netmask. This host shows it live: lo0 has no broadcast flag, yet Qt reports bcast=127.255.255.255.

So a /32 tunnel (WireGuard, Tailscale, most utun) reaches broadcastTargets() with broadcast == ip, passes the isNull() guard, and the host's own address comes back as a target:

qt-shaped /32 tun targets: 255.255.255.255 100.101.102.103

Every 5 s sweep then sends a discovery datagram to this host's own :1024. That is mostly harmless, but the "skipped" branch is dead on real hosts. With the fixture set the way Qt sets it, the PR's own test fails:

FAIL: an interface with no broadcast address contributes no target

The fix below passes both the original fixture and the Qt-shaped one (both run):

        if (broadcast.isNull() || broadcast.protocol() != QAbstractSocket::IPv4Protocol)
            continue;
        // A /31 or /32 entry (a WireGuard or Tailscale tunnel, usually) has no
        // broadcast domain, but QNetworkInterface still reports ip | ~netmask
        // for it -- which for a /32 is this host's own address.
        if (entry.prefixLength() >= 31)
            continue;

In the test, replace the // No setBroadcast() line with:

        // What QNetworkInterface actually reports: postProcess() fills ip | ~netmask.
        tunnel.setBroadcast(QHostAddress(QLatin1String("10.8.0.6")));

2. src/gui/RadioSetupDialog.cpp:4750-4783: the two combos lost their accessible descriptions in the move.

The removed connect-form combos had setAccessibleDescription. Bridge dumpTree on the new page shows ananDdc0Rate and ananAdcSelect with an accessibleName and no accessibleDescription. All five checkboxes have one. docs/a11y.md §"Naming", item 2 says that for a QComboBox "the description should answer … what the choices mean", and that tooltips are "a poorer home". The body's "Nothing was lost in the move" holds for behaviour but not for this. Suggested addition, after each setAccessibleName:

    feRateCombo->setAccessibleDescription(QStringLiteral(
        "DDC0 sample rate in ksps, and the panadapter span a session starts at. Higher "
        "rates use more of the radio's Ethernet link. Takes effect on the next connect."));
    feAdcCombo->setAccessibleDescription(QStringLiteral(
        "Which receive chain feeds the receiver: ADC0, behind the switched ANT1/2/3 relay "
        "bank, or ADC1, wired directly to its own RX2 jack. Takes effect on the next connect."));

3. src/gui/ConnectionPanel.cpp:849-851: the pointer names five of the seven options. Dither and randomization are missing, so an operator looking for them on the connect form is not told where they went. Suggested text:

        QStringLiteral("Sample rate, ADC choice, dither, randomization, filter bypass and the "
                       "radio's own speaker are in Settings > Radio Setup > ANAN Front End. "
                       "They apply to the next connect."),

4. tests/anan_settings_test.cpp:87-89: the third check can only fail on the test's own earlier writes. The || ddc0RateKsps() == 48 half adds nothing, since 48 is in the list. AnanSettings::ddc0RateKsps() accepts any positive value (AnanSettings.cpp:49-53). The size-6 and endpoint checks carry the coverage, so this one could go.

What I tried to break

  • Build and tests. I built the clean head; the bridge reported sha 36ce5cec, dirty: false. anan_discovery_test, anan_settings_test and anan_backend_test pass, 3/3.
  • Mutation. Dropping the IPv4 half of the null/IPv4 guard survives. It is redundant with isNull() for every entry Qt produces, because IPv6 entries never get a broadcast. That is harmless and not a finding. The probe and fixture results for nit 1 and D1 are quoted above. Every mutant was reverted, its .o deleted, and the tree confirmed clean before the GUI build.
  • The page, driven against the demo (DEMO-0001).
    • Disconnected, "ANAN Front End" is listed with all seven controls at the claimed defaults.
    • Setting 384 ksps and dither off wrote Anan {"ddc0RateKsps":384,"dither":false}.
    • Connecting the demo removed the page from the nav, and the dialog moved to "Radio", so the hidden page is not left on screen. Disconnecting brought it back (nav grabbed both ways).
    • After kill and relaunch, the page read back 384 ksps with dither unchecked.
    • On Connect by IP, the pointer is visible for ANAN-G2 only (hidden for FlexRadio) and wraps without clipping.
  • "Nothing was lost in the move."
    • populateFamilyParams() reads all seven values from AnanSettings (RadioModel.cpp:729-745).
    • git grep finds no remaining reference to any removed object name or member.
    • The sample-rate tooltip ("the width of the panadapter span a session starts at") is still true after feat(anan): remember where the operator left the radio. Principle III. #6129: its commit records the ruling that the DDC0 rate always wins over a remembered span.
  • Canon checks.
  • CI freshness. Both runs for this head were created 2026-10-08T12:59Z. main is now d6f6a4f0 (2026-10-09T20:30Z), 32 commits later. git merge-tree against current main is clean. None of the three ANAN tests is on the per-PR ctest -R gate, so green CI did not run them.
  • Not tested.
    • Anything on a G2, Windows, or a multi-homed host, so D1 is reasoned from code.
    • The page while connected to an ANAN: the demo declares no adcFrontEnd.

Recommendation

Needs maintainer decision. I found no code blocker. The page move is clean and verified at runtime, and the capability shape follows canon. Two things gate it:

Nits 1 to 3 are small, and nit 1 comes with tested drop-in code.

@tropo1234

Copy link
Copy Markdown
Contributor Author

@K5PTB, thanks for the review, and for driving the page through the bridge.

Nits 1-4: all valid. I'll fold them into the same round as D1, so the branch is rebuilt once. On nit 1 I confirmed the cause in qtbase 6.12: postProcess() fills in ip | ~netmask for any IPv4 entry that has a netmask and no broadcast, and liveAddressEntries() does not drop point-to-point interfaces, so a /32 tunnel targets this host. Your prefixLength() >= 31 skip and the Qt-shaped fixture go in as given.

D1: you're right that the two comments overreach. Two interfaces on one subnet get one datagram, and it leaves by whichever interface the route prefers. I recommend (b), in the shape deskHPSDR uses: new_discovery.c opens one socket per up, running, non-loopback IPv4 interface, binds it to that interface's address and sends the discovery packet from it (to 255.255.255.255; the subnet-directed form is there but commented out). Here each socket would send the global broadcast and its own subnet's directed broadcast, and replies would still be merged by MAC. I'd keep this inside AnanDiscovery; a helper shared with the other discovery class would be a separate change.

@ten9876, two decisions are yours:

  1. D1: (a) narrow the comments and body to "a second NIC on a subnet of its own" and keep the single socket, or (b) the per-interface sockets above, in this PR. I'd do (b).
  2. RFC [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209: your 10-04 ruling holds this PR until the RFC is approved, and its open question (list the page for every family while disconnected, or only for an ANAN profile) decides ananFrontEndAvailable(). It still has only the rfc label. If you'd rather discovery not wait on the RFC, I can split it into its own PR.

Once you rule, D1, nits 1-4 and anything #6209 changes go in together.

@jensenpat

Copy link
Copy Markdown
Collaborator

@tropo1234 Heads up - this PR got delayed because it combined three sets of UX changes on connect, in radio setup, and subnet discovery. I'm going to merge this with single interface discovery, if you want to propose multi-interface discovery, I'd file that implementation in a separate PR.

@jensenpat

jensenpat commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

@K5PTB @tropo1234: the landing pass is now at signed head a8ae12c0ab1cbd20c7fe199b5d9273a1a80e71fe, rebased cleanly onto main 7864ef544.

  • D1: the operator explicitly chose to retain the current single-socket discovery. AnanDiscovery.{h,cpp}, the test comments, user guide and PR body now state the distinct-subnet scope and same-subnet NIC limitation. No per-interface socket redesign is included.
  • D2: RFC [RFC] ANAN-G2 front-end options: a Radio Setup page and the anan/frontend.set extension verb #6209 is approved, retaining visibility choice (a); the approval is recorded on that issue. The two new wire controls and frontend.set remain split out.
  • Nit 1: AnanDiscovery.cpp:84 excludes /31 and /32 entries. The /32 fixture now uses Qt's synthesized self-address broadcast, and a /31 case is also covered. Removing the guard produced both named failures; restoring it gives a passing anan_discovery_test.
  • Nits 2–3: both combos expose accessible descriptions in the bridge tree, and the ANAN-only connection hint names dither and randomization. The hint was checked under ANAN, Flex, HL2, Icom and RTL selections.
  • Nit 4: removed the redundant stored-rate assertion. The list size and endpoint checks remain.
  • User documentation: updated ANAN setup, troubleshooting, manual connection and Radio Setup pages, with refreshed Log Analyzer locations. npm ci + docs build pass; generated references are current; log rules pass 707 checks.

The final clean app reports sha=a8ae12c0a, dirty=false. The selected socket-free tests pass 3/3 (anan_discovery_test, anan_settings_test, anan_backend_test). The offscreen bridge verified all seven controls, all seven settings across a process restart, and page withdrawal on connecting DEMO-0001, followed by restoration on disconnect. Settings were isolated, TX was disabled, and proof instances were closed. Boundary findings match main, with no ratchet growth; accessibility remains main's nine warnings. No live-radio or multi-NIC hardware evidence was added.

The earlier Antares advisory is not reproducible from the current parser: unchanged P2Protocol.cpp:214 requires an exact 60-byte status packet, and :244–256 checks the header and exact declared DDC frame length before constructing the subspan. P2Client.cpp is outside this PR's diff.

No unresolved inline review threads or changes-requested reviews were present. Fresh PR CI is running on this exact head; approval and merge will follow its completion.

Generated with OpenAI Codex (GPT-6 Astra)

Send directed discovery broadcasts to distinct attached IPv4 subnets while
retaining the global broadcast. The single wildcard socket still follows
the preferred route when multiple NICs share a subnet. Exclude /31 and /32
point-to-point entries, including Qt-synthesized self-address broadcasts.

Move the seven existing ANAN connect-time options to Radio Setup's ANAN
Front End page, retaining the same AnanSettings values and next-connect
semantics. Keep the page available offline and capability-gated online,
with an ANAN-only pointer on the connection form and accessible descriptions
for both combos. Update the matching user guide pages and log references.

RFC aethersdr#6209 approves the setup page. The operator explicitly retained the
current discovery design. Band-filter selection, amplifier mute and the
frontend.set extension remain outside this change.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: jensenpat <patjensen@gmail.com>
@jensenpat

Copy link
Copy Markdown
Collaborator

Main advanced during the first CI cycle: 5813556e0 added the KPA500 integration, touching the shared Radio Setup/MainWindow/build files. #6146 is now cleanly rebased onto it at signed head 04400ba310d2aca416445ebbf1ca978cae427de4.

git range-diff reports a8ae12c0a = 04400ba31: this PR's patch is unchanged. The mutation evidence therefore applies to the same discovery source. Current-head validation was refreshed: macOS app build passes; the three focused ANAN CTests pass 3/3; base/head boundary findings remain identical; docs build, generated references and 707 Log Analyzer checks pass. The isolated TX-disabled bridge reports the new clean SHA and again verifies the seven controls, connection-family hint gating, demo connect/disconnect gating, and all seven preferences after process restart. Proof apps are closed.

Fresh CI on this head must pass before approval/merge; the earlier head's results are not being used for the new integration.

Generated with OpenAI Codex (GPT-6 Astra)

@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.

Approved at exact signed head 04400ba. RFC #6209 is approved with offline visibility choice (a); the operator explicitly retained single-socket discovery with its distinct-subnet scope. The four remaining review nits are fixed: Qt-shaped /31 and /32 tunnel filtering, accessible descriptions, the complete connection hint, and removal of the redundant rate assertion. Matching user docs and source references are updated.

Current-head evidence: macOS Qt 6.12 build passed; 100% tests passed, 0 tests failed out of 3 for anan_discovery_test, anan_settings_test and anan_backend_test. The prefix-guard mutation at a8ae12c caused both /31 and /32 assertions to fail; git range-diff proves that patch unchanged by this rebase, and restored code passes on this head. The isolated offscreen bridge reports sha=04400ba31, dirty=false; it verified seven controls and their descriptions, all seven preferences after process restart, the ANAN-only hint across five family selections, and withdrawal/restoration of the page across DEMO-0001 connect/disconnect. TX was disabled and proof instances were closed.

Base/head boundary findings are identical; capability and command-plane gates pass with no baseline growth. Docs build and 707 Log Analyzer checks pass. The manual final source pass found no remaining blocker; automated code-review was unavailable. No unresolved inline threads or blocking reviews remain. No live-radio or multi-NIC hardware claims are added.

The unchanged parser's exact-length guards refute the older advisory; P2Client.cpp is outside this diff. Windows, Docs and Static checks have passed on this head. Linux and macOS remain in progress; this approval does not claim they passed. GitHub's required checks and mergeability gates must pass before auto-merge can fire, under the operator's concurrent-merge workflow.

@jensenpat
jensenpat enabled auto-merge (squash) October 17, 2026 16:00
@jensenpat
jensenpat merged commit 94e3cd4 into aethersdr:main Oct 17, 2026
7 checks passed
@tropo1234
tropo1234 deleted the anan-front-end branch October 17, 2026 16:55
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.

4 participants