fix(icom): IC-9700 support — a GUI crash, a dBm runaway, and a self-inflicted reconnect loop

[nigelfenton]

Tested #4784 against a real IC-9700 the evening it merged. It connects, and RX works — but there were four faults in the way, one of which is a crash that is live on main right now and is not Icom-specific.

@jensenpat — this is the PR you asked for. Everything here is verified on the radio; the one thing that is not verified is transmit, and I have said so explicitly at the bottom rather than let it look tested.

Three commits: two were written while #4784 was open (they were meant as review fixes and are now fixes to shipped code), and one is tonight's work.

---

1. Null PanadapterStream — crashes the GUI thread on connect

⚠️ This one is live on main and affects HL2 and Kiwi too, not just Icom.

RadioModel's panRangeChanged handler dereferenced m_panStream bare:

if (pan->setRange(minDbm, maxDbm)) {
    m_panStream->setDbmRange(...);   // null on any self-decoding backend
}

m_panStream is assigned only on the Flex and Sim paths. Any backend that decodes its own scope leaves it null, so the first dBm range the backend reports kills the GUI thread. Four minidumps, all 0xC0000005 reading 0x30, all reaching PanadapterStream::setDbmRange from this lambda.

This is the same defect radiomodel_dax_null_test already fixed four times — its own header says "the same crash already fixed once at the startDax() entry, still reachable by four paths behind it." This is the fifth path, and unlike the DAX ones it needs no user action: it fires by itself on connect.

radiomodel_pan_range_null_test reproduces it with no hardware. Guard removed, it exits 0xC0000005.

2. dBm auto-floor runaway — the loop waits for an echo that never comes

The noise-floor auto-adjust slides the reference level, requests the new range, and waits for the radio to confirm before moving again. An Icom has no command plane for a display range, so the request is dropped, the confirmation never arrives, the unchanged floor reads as "not there yet", and it steps again. Linear 24 dB/s, span locked at 90 dB:

-154/-64   dropped ("no command plane for this backend")
-178/-88   dropped
-202/-112  REJECTED  <- first "implausible dBm range"
   ... 71 rejections later ...
-1882/-1792

The implausible dBm range warnings are the symptom. The run was already 48 dB adrift before the first one, so raising that floor would have hidden it, not fixed it.

Fix: new RadioCapabilities::radioOwnsDbmScale, defaulting true so every existing backend is untouched.

Worth a close look in review: gating the outbound command paths alone is not sufficient, and I had it wrong that way first. m_refLevel is local widget state, so the traffic stops while the ratchet keeps running — the sendCommand lines vanished from the log and the rejections carried on. The gate has to be on applyNoiseFloorAutoAdjust itself.

Deliberately not setNoiseFloorEnable(false) — that is the operator's own toggle, and forcing it would fight the overlay menu and persist to the next radio.

3. Configuration advice on a fatal channel — a self-inflicted reconnect loop

The MOD Input check emitted connectionError, which every consumer treats as fatal (RadioModel starts its reconnect timer on it unconditionally). The session came up healthy and the warning tore it down 4 ms later:

21:19:09.353  CI-V stream live
21:19:09.357  connection error: "The radio is not listening to network audio..."
21:19:09.357  connection error — reconnecting in 5s

Then again every 5 s, forever. From the operator's side that is a radio that will not stay connected, plus a message about a menu setting, with no way to tell that the message is the disconnect.

Added IRadioBackend::configurationWarning for advice that does not end the session. Status bar, 15 s rather than the usual 4 — it names a four-level menu path you have to walk on the radio's front panel while reading it.

The original intent is kept: this genuinely does cost the operator every transmission and should be visible. Just not on a channel that is fatal.

4. The MOD Input warning is wrong on every non-705 networked Icom

It demands MOD Input = WLAN (0x03). The IC-9700 is LAN-only — there is no WLAN option in its menu. Set correctly on the front panel, it reports 0x01, and the warning fired on every session.

kModels already carried the discriminator:

Model	Wi-Fi
IC-705 (0xA4)	yes
IC-9700, IC-7300, IC-7610, IC-7300MK2, IC-905	no

Exactly one model has Wi-Fi — the IC-705, whose 0xA4 is also the default CI-V address, so almost certainly the radio this was written and tested against. On every other networked Icom it asked for a modulation source the radio does not have, and no front-panel action could satisfy it.

Now gated on hasWifi, and the health readout no longer appends — NOT WLAN to a correctly configured radio. Nothing is lost: the warning was wrong here, not merely noisy.

Left alone deliberately — needs your call. The 1A 05 item numbers (118/119) and the value enum are hardcoded from one model's CI-V Reference Guide and sent to every Icom, but each model numbers its own SET menu. So on an IC-9700 either 118/119 is not MOD Input at all, or 0x01 is its network source. Both need that model's guide to settle, and I was not willing to guess at a TX-path setting. If you know the 9700 mapping I will wire it up per model.

Also included (written during #4784, now fixes to merged code)

• CI-V open retry. One buildSerialOpen was not reliably enough: the radio accepted the open and sent nothing. Retries at 100 ms up to 50 attempts, stops on the first real payload, and warns with a specific cause if it never starts. Took the CI-V stream from dead (0 frames in 45 s) to live at 76 ms.
• CI-V address field in the connection panel. The address is user-changeable and other Icoms share this transport, so it cannot be assumed. Accepts A2, 0xA2, A2h; unparseable input is ignored rather than clamped.
• IC-9700 scope geometry recorded as measured (475 points / 618 frames), deliberately keeping verified: false — measured on hardware is not the same as confirmed against that model's own CI-V guide.

---

Verified on hardware — IC-9700, 2026-08-05

	Before	After
implausible dBm rejections	71	0
connection errors	every 5 s	0
reconnects	continuous loop	0
CI-V bring-ups	one per reconnect	1
crash on connect	yes	no

Each fix was falsified by breaking it deliberately and watching the test catch it, rather than by a green re-run. I re-ran the crash falsification after rebasing onto main, because a rebase can silently drop a hunk and a green test alone would not tell the difference.

Tests

• radiomodel_pan_range_null_test — reproduces the crash with no hardware
• dbm_range_plausibility_test — pins the ratchet arithmetic and the defaults-true gate
• icom_family_test — extended with the capability and the hasWifi discriminator

Not verified

Transmit. Whether TX audio actually reaches the modulator depends on the 118/119 mapping above, which is unresolved on this model. RX, scope and stability are solid; I have not keyed the radio.

Unrelated but visible in the logs: TgxlConnection/PgxlConnection retry a refused socket every ~9 s on a non-Flex radio. Harmless, pre-existing, untouched here — happy to look at it separately if you want.

[aethersdr-agent]

Thanks for this one — the IC-9700 write-ups are excellent, especially catching that the mod-input warning was itself the disconnect. The CI failure is unrelated to any of that; it's a one-line style call. 👍

What failed

Only Static checks → Hardcoded-colour ratchet (run 31063733326). Every other step in that job passed, and build / check-macos / check-windows were still in progress at the time — nothing there had failed.

=== ratchet (vs PR base) ===
  OK   unique_colours       614  (base 614, +0)
  OK   total_references    2749  (base 2749, +0)
  OVER setstylesheet       1111  (base 1110, +1)

Reproduced locally at ac9a800e against origin/main, same numbers. No new colours — you added one new setStyleSheet() call site, and that counter is a separate ratchet.

The line

src/gui/ConnectionPanel.cpp:743, in the new CI-V address field:

m_manualIcomCivEdit->setStyleSheet(lineEditStyle);

The two edits it sits directly beneath route through ThemeManager instead:

ConnectionPanel.cpp:709  ThemeManager::instance().applyStyleSheet(m_manualIcomUserEdit, lineEditStyle);
ConnectionPanel.cpp:720  ThemeManager::instance().applyStyleSheet(m_manualIcomPassEdit, lineEditStyle);

The fix

ThemeManager::instance().applyStyleSheet(m_manualIcomCivEdit, lineEditStyle);

That's the whole thing — same lineEditStyle string, and it matches the username/password fields two lines up, so the new field will also re-style on a live theme change rather than keeping its connect-time appearance. Ratchet goes back to 1110 and the step passes.

Worth knowing for next time: the ratchet is a delta vs. your PR's base, not a fixed ceiling (static-checks.yml:132-151), so it only ever complains about what your own diff adds. And the setstylesheet counter tracks call sites rather than colours — the tool's own advice is explicit that migrating a colour to a token won't clear it; you have to go through an existing themed path, which applyStyleSheet is.

You can reproduce it locally without waiting on CI:

python3 tools/audit_colours.py --src src --compare-src <base>/src --summary-only --strict

No reviewer or Copilot comments on the PR yet, so nothing else outstanding. Once this is pushed the remaining three jobs should get a clean run.

---

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

[aethersdr-agent]

Thanks — this is a strong, well-evidenced PR, and the m_panStream null guard is a genuine live-on-main crash fix that I'd want in regardless of the Icom work. The radioOwnsDbmScale capability defaulting true is the right shape, and your note that gating only the outbound command paths is insufficient (because m_refLevel is local widget state) is correct — the gate in applyNoiseFloorAutoAdjust is where it has to be. Splitting configurationWarning off connectionError is also clearly right; a fatal channel carrying menu advice was always going to loop.

One real defect found in the new code, plus a few smaller things. Nothing here changes my view that the crash fix and the reconnect-loop fix should land.

Would like fixed before merge

• The new panRangeChanged emit adds referenceDb where toDbm() subtracts it, so the axis moves the wrong way when the scope reference level is non-zero — and the range is never re-published when it changes.

Polish

• The hasWifi gate also silences the MOD Input check on an unidentified radio, because kUnknown.hasWifi == false.
• An unparseable CI-V address is discarded with no feedback, and there's no obvious way back to "auto" once an override is stored.
• Orphaned comment left behind in IcomSettings.cpp when the constant moved to the header.

Non-blocking notes

• dbm_range_plausibility_test asserts against its own copy of dbmRangeLooksPlausible(), so it can't catch a change to the real constants.
• CodeGuard's five CG-PATH-001 hits on MainWindow.cpp (7977, 8089, 8208–8210) are false positives for this PR: your only change to that file is at ~6556, and those lines are untouched pre-existing code.

On the 1A 05 118/119 mapping you left open — agreed that guessing at a TX-path setting on someone else's model is the wrong call, and flagging transmit as unverified rather than letting it look tested is the right way to ship this.

---

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

[aethersdr-agent]

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

• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-4786/src/gui/MainWindow.cpp:7977
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-4786/src/gui/MainWindow.cpp:8089
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-4786/src/gui/MainWindow.cpp:8208
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-4786/src/gui/MainWindow.cpp:8209
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-4786/src/gui/MainWindow.cpp:8210

_{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: $7.0606 · model: claude-opus-5}

[aethersdr-agent]

Antares Detector — candidate vulnerable file(s)

• src/core/HidEncoderManager.cpp

HidEncoderManager reads HID reports into a fixed-size buffer using m_parser->reportSize() without validating available data, enabling a heap/buffer overflow and crash.

_{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: $7.0606 · model: claude-opus-5}

[nigelfenton]

Thanks — the sign defect is real and I've fixed it. Good catch; it was invisible at the default reference of 0, which is exactly why it survived a live session on the radio.

The sign. toDbm() maps a sample to floorDbm + (v/max)*spanDb - referenceDb, and my emit added referenceDb. So the trace and the axis it is drawn against moved in opposite directions, wrong by twice the reference. Both now go through one publishScopeDbmRange() derived from the same ScopeCalibration toDbm() decodes with, so they cannot drift apart again — and it is called on every scope.reference change, not just at connect, which was the stale-axis half of the same finding.

Pinned in icom_scope_test against toDbm's own output at a non-zero reference rather than against a constant, since the relationship is the thing that broke.

Polish, all taken:

• The hasWifi gate silencing the check on an unidentified radio is correct, and there is a second reason worth recording: kUnknown is also hasTransmit=false, so it is a radio this client will not let key — no modulation path to warn about.
• An unparseable CI-V address now refuses the connect and says why. Silently ignoring it reproduced the exact symptom the field exists to cure, since a wrong CI-V address fails silently at the radio.
• Clearing the field restores the default, so an override can be taken back. Previously blank read as "auto" in the placeholder while the old value was still in force.
• Orphaned comment in IcomSettings.cpp removed.

On the duplicated predicate — you are right, and I have said so plainly in the test header rather than leaving it implied: that file cannot catch a change to the real constants. It pins the arithmetic and the shape of the runaway; the behavioural half is pinned in icom_family_test against the real capability. Worth exporting the predicate if a third caller ever needs it.

One correction to my own work while fixing this: I briefly added a null guard on m_model believing it was null before the radio identifies. It is not — the constructor seeds it with unknownModel(). I removed the guard rather than leave a defensive check justified by a wrong premise. The early return is on kUnknown.hasScope == false, which icom_family_test now pins.

Also worth flagging separately: connection_panel_size_test fails on current main at this screen size (height=700 vbarMax=14), independent of this PR — I checked by confirming my diff adds no net row and that CI's Windows job passed before I touched any layout. Not mine to fix here, but someone should know.

[nigelfenton]

Dispositioning the two automated scanner comments — both are out of scope for this PR, but they are not equally empty.

Cisco CodeGuard (5 × CG-PATH-001, MainWindow.cpp 7977 / 8089 / 8208 / 8209 / 8210) — false positives, and not in this diff.

This PR touches MainWindow.cpp in exactly one hunk, at ~6556 (applyCapabilitiesToUi, adding the radioOwnsDbmScale propagation). Every flagged line is 1400+ lines away from anything I changed. Reading them on the tree: 7977 is the UI-scale restart path (QProcess::startDetached(QCoreApplication::applicationFilePath(), ...) plus the macOS open -n bundle walk), and 8089/8208-8210 are applet-panel dock/float state. The "path" in the restart case is applicationFilePath() — our own binary, not attacker-influenced input — and the applet lines are settings keys, not filesystem paths. No operator-controlled string reaches a file open in any of the five.

So: not introduced here, not modified here, and not a defect on inspection.

Antares (HidEncoderManager.cpp) — out of scope, but it has a real kernel and I would rather not wave it off.

The file is not in this PR at all (last touched by #3401 / #3542 / #4062). The finding is also imprecise about the mechanism — it says "without validating available data", but hid_read's return res is checked (<0 disconnect, ==0 break) and the parse is bounded by res, so the reported overflow-on-short-read does not exist.

What is actually there, one step across:

uint8_t m_buf[64]{};                                    // HidEncoderManager.h:187
int res = hid_read(m_device, m_buf, m_parser->reportSize());   // .cpp:591

The length argument is m_parser->reportSize(), and nothing clamps it to sizeof(m_buf). Today every parser in the tree reports ≤64 so this is latent rather than live, but it is a descriptor-derived value bounding a write into a fixed 64-byte buffer, and the safety of that is a property of the parsers rather than of this call. A std::min(m_parser->reportSize(), sizeof(m_buf)) would make it locally correct.

Unrelated to the IC-9700 work, so I do not want to fold it into this PR's diff. Happy to open it separately — say the word and I will, or leave it if you would rather it ride with whoever next touches the HID path.

Neither finding blocks this PR.

[nigelfenton]

Opened the HID clamp separately as #4819 so it stays out of this diff — std::min(reportSize(), sizeof(m_buf)) plus a test that fails when the clamp is removed. Nothing here changes.

[nigelfenton]

Correction to my note above: I've closed #4819 (the clamp) and replaced it with #4820, a comment-only note at the reportSize() declaration.

The clamp was speculative hardening — no parser exceeds 64, so nothing overflows today, and the failure needs a future parser to return a larger value. What's actually missing is that the virtual's declaration never said it bounds a write into a fixed 64-byte buffer. #4820 says it, and records that TMate2 sits at exactly 64 with no headroom.

Still nothing changing in this PR.

[jensenpat]

Reviewed and verified. The null m_panStream guard is a real fix to a crash live on main that affects every self-decoding backend, not just Icom. radioOwnsDbmScale defaults true so no existing backend changes behavior, and gating applyNoiseFloorAutoAdjust itself rather than the outbound commands is the correct placement — you're right that gating the commands alone leaves the ratchet running on local widget state.

configurationWarning as a separate non-fatal channel is the right call; advice should never reach a consumer that treats it as a session teardown.

On the 1A 05 118/119 mapping: agreed, leave it. Per-model SET menu numbering means guessing at a TX-path setting, and silencing on hasWifi loses nothing that was correct. I'll pull the IC-9700 guide and wire it per model separately.

Transmit being explicitly marked unverified is appreciated — that's the right way to hand it over.

Noting the connection_panel_size_test failure on main separately; not yours.