Skip to content

Fix IC-705 preamp cycling above 74.8 MHz - #6679

Merged
jensenpat merged 2 commits into
aethersdr:mainfrom
Ozy311:codex/fix-6497-icom-preamp
Oct 22, 2026
Merged

jensenpat merged 2 commits into
aethersdr:mainfrom
Ozy311:codex/fix-6497-icom-preamp

Conversation

@Ozy311

@Ozy311 Ozy311 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

On an IC-705 at 144 or 430 MHz, clicking the preamp button from P.AMP1 requested P.AMP2. The radio rejected that stage and readback restored P.AMP1, so another click repeated the invalid request instead of reaching OFF.

Fixes #6497.

The IC-705 model profile now limits its two-stage preamp ladder to frequencies below 74.800 MHz, matching the reporter's measured boundary. At and above that boundary, the existing control cycles OFF/P.AMP1. An unknown receive frequency also uses those shared positions until radio readback establishes the range.

Both polled and unsolicited CI-V frequency replies update the published choices when the effective ladder changes and request a preamp readback. Crossing a boundary never writes a preamp setting. The backend setter uses the same frequency-dependent bound, so a stale P.AMP2 request cannot send stage 2 in the upper range. New sessions discard the previous receive frequency. Other models, diagnostic wire-state reassertion, NAK confirmation behavior, and attenuator behavior are unchanged. The IC-705 user guide and generated log references are updated.

Validation:

  • macOS ARM64 / Qt 6.12: built baseline and fixed AetherSDR apps with QtKeychain disabled for the controlled proof build; six compiler jobs maximum.
  • Two socket-free focused tests pass (icom_control_profile_test, icom_backend_seam_test), including 26 added checks covering the exact boundary, unknown frequency, other models, both frequency reply forms, tune intent versus readback, model lifecycle, stage readback and emitted/queued CI-V writes.
  • Four removal mutations fail: boundary 4 assertions, ladder republish 4, setter bound 2, readback 1. Restored source passes both focused tests.
  • Broader icom_family_test was attempted and fails its existing display-state/Flex-command assertion after two pan display refusals. This test's unchanged WNB/display path is outside the preamp change; no successful baseline run is claimed. It uses TEST-NET socket paths and was not rerun as part of the socket-free validation.
  • Strict engine-boundary, command-plane, capability-record, test-registration and manifest checks pass. Generated references are current; log rules pass 707 checks across 45 rules.
  • Documentation: npm ci and npm run build pass.
  • RX-only hardware proof on an IC-705 at 446.15 MHz, using native control clicks through the agent automation bridge: baseline requested 16 02 02, received FA, and read back 16 02 01. The fixed control requested 16 02 00, received FB, and read back 16 02 00; another click restored stage 1 with confirmed readback. Frequency, FM/FIL1 and attenuator OFF were preserved.
  • An earlier fixed-app reconnect attempt had a CI-V stream stall despite active transport. That attempt was inconclusive; the successful proof used a fresh session. This patch does not claim to fix that separate stall.
  • Screenshots below are offscreen native-control captures, not GPU-renderer proof. Other bands and the exact 74.800 MHz boundary have offline test coverage and the reporter's measurements; they were not tested live in this run.

Hardware evidence (446.15 MHz):

Before: rejected stage 2, settled P.AMP1 After: confirmed OFF
Baseline P.AMP1 Fixed preamp OFF

Allowlisted preamp requests and radio replies. Evidence commit: f41a51cb53c4d7e27f1cbd14cf20ecd15d4880fe (signed).

Linux and Windows builds/runtime were not run. No transmit validation is claimed.

Base: 6bb29bb7c7b1988d7f3300e41fc596f62902c65b. Head: 65c3658e08f02d8a7762de090c07bfad2bd919ac (signed).

Generated with OpenAI Codex.

Review follow-up (67cfca6)

Range-change preamp readback now uses Operator priority so it precedes queued meter reads. S-meter publication waits for confirmed receive frequency on reconnect, preventing a transient HF reference on VHF; frequency-independent telemetry remains available. Removed the unrelated RadioModel line-number churn from rules.json entirely. The separate attenuator band-awareness follow-up is #6685.

Validation: 3/3 focused socket-free CTests (backend seam, control profile, meters), independent mutations that fail each new regression, native macOS app build, test registration, 707 Log Analyzer checks, and clean docs install/build passed. Independent source review found no blockers.

RX-only IC-705 check of this source and binary SHA256 54e40611536d265968212bb78fe27cba974b54c800aa71721ae2fa5c782e868d: confirmed 74.799 MHz/P.AMP2 and 74.800 MHz/P.AMP1 readbacks; final scheduler 2,819 replies, zero timeouts. Restored 446.150 MHz/FM/FIL1 and matching captured receive settings, then disconnected and stopped the owned app. The first displayed startup meter sample was not instrumented in this lab run; that guard is covered by the production-handler regression. No TX or merge.

@Ozy311
Ozy311 marked this pull request as ready for review October 22, 2026 07:57
@Ozy311
Ozy311 requested a review from a team as a code owner October 22, 2026 07:57

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

1. Issue fit

Yes. #6497's primary root cause — a band-unaware three-position ladder driving a modulo cycle into an absorbing state — is fixed at the right seam, and the boundary is the reporter's own measurement rather than an invented constant (k9mq: "Preamp1 and Preamp2 is active up until 74.79999mhz. At 74.800mhz on up, there is only one preamp"). src/core/backends/icom/IcomModels.cpp:475 uses frequencyHz >= limit, and tests/icom_control_profile_test.cpp:265-273 pins both sides of the exact edge (74'799'999 → 3, 74'800'000 → 2).

Mapping the issue's acceptance list to hunks:

#6497 expectation Where
VHF/UHF offers only OFF / P.AMP1 IcomModels.cpp:470-480 + IcomCivBackend.cpp:1564-1575
HF and 6 m keep the full cycle same, limit gate; icom_control_profile_test.cpp:265
The control follows the radio's actual state IcomCivBackend.cpp:1918-1926 readback
Never send 16 02 02 above the boundary IcomCivBackend.cpp:4358-4360 setter bound
On 0xFA, don't re-send the invalid value already correct pre-PR (triage established the repeats came from the GUI cycle, not a retry); removing the invalid step is the real fix
Check other Icom models verified below — no sibling defect for preamp

The one thread item not in the diff is the attenuator (nit 2). The issue body is preamp-only, so issue fit is unaffected.

Test layer is right: ladder contents and wire bytes are model-table/encoding concerns, so a socket-free CTest is the correct layer per AGENTS.md. No socket test added, modified, or removed — tests/icom_backend_seam_test.cpp has no QTcpSocket/QUdpSocket/bind/listen/connectToHost and uses an unstarted IcomSession as the transport stand-in. No fake radio peer was introduced, which is correct; I am not asking for one.

The added checks do discriminate. With preampLabelsFor ignoring frequency, icom_backend_seam_test.cpp:272 (info.last() == {"OFF","P.AMP1"}) and icom_control_profile_test.cpp:271 both fail — these are not assertions of the implementation back to itself.

2. Scope

File / group What it changes Claimed? Verdict
src/core/backends/icom/IcomModels.{h,cpp} ranged ladder + preampTwoStageUpperExclusiveHz on the IC-705 profile Yes In scope. New field is an internal profile member, not public/protocol surface.
src/core/backends/icom/IcomCivBackend.{h,cpp} publishPreampControls(), session reset, transition republish + readback, setter bound Yes In scope.
tests/icom_backend_seam_test.cpp new testIc705PreampFrequencyRanges, 3 access shims, deliver() address source Yes In scope; shim change is required, see below.
tests/icom_control_profile_test.cpp boundary / unknown-frequency / other-model checks Yes In scope.
docs/user/docs/networked-icom.md IC-705 preamp bullet Yes In scope and accurate — "Changing ranges does not change the radio's preamp setting" matches the code, which only reads.
docs/user/src/components/LogAnalyzer/rules.json 12 cited RadioModel.cpp line numbers Yes ("generated log references are updated") Explained by the body, but unrelated to the fix — see nit 1.

Nothing in the diff is unexplained, and no guard or confirmation was removed. One deleted comment names a symptom, so per §3 here it is:

// The HF preamp ladder is published even on VHF/UHF (00/01 only there): the radio refuses P.AMP2 and reports what it did, and republishing per band would rewrite the control under the operator.

What now answers it: the republish fires only when the effective ladder changes (std::ranges::equal guard at IcomCivBackend.cpp:1917), not on every band change, and it is immediately followed by an authoritative 16 02 read so the rewritten control is then told the truth. That is the answer the triage comment proposed, and it is implemented. Residual edge is nit 3.

3. Blockers

None.

4. Nits (all non-blocking)

1. docs/user/src/components/LogAnalyzer/rules.json:12,24,97,114,119,136,170,196,213,230,240,477 — unrelated generated churn that no gate needs, and three open PRs are regenerating it.

RadioModel.cpp is byte-identical between main (6bb29bb) and this head — I diffed it — so these 12 edits only repair line-number drift that already exists on main. And the checker treats stale lines as a NOTE, not a failure, by design:

  // The message still exists but moved: not a failure (edits elsewhere in a
  // busy file such as RadioModel.cpp would otherwise redden Static checks on
  // unrelated PRs). Report it, and let --update refresh the line number.

On main I get NOTE: 12 cited line number(s) moved followed by OK: 707 checks passed for 45 rules. — the same 707/45 this PR reports, with or without the hunk. Meanwhile #6673 ("this commit refreshes the line numbers that moved in RadioModel.cpp") and #6666 carry the identical regeneration, and #6676 deliberately declined it. Whichever lands first makes the others' hunk stale and forces a rebase, the same reason CHANGELOG.md is kept out of ordinary PRs. Suggest dropping the file from this PR and letting one dedicated commit own it.

2. src/core/backends/icom/IcomCivBackend.cpp:1569-1572 — the attenuator sibling the issue thread asked for in this change is still band-unaware, and its own comment still names the symptom. The diff rewrote the preamp half of that comment block and left:

    // ONE step, and naming it in dB is honest here where it was not for the
    // preamp: the guide gives this attenuator an actual figure. HF and 50 MHz
    // only — on higher bands the radio ignores the request and reports OFF.

attenStepsFor(*m_model) still takes no frequency, so on 2 m/70 cm the IC-705's ATT button remains a dead control. The triage comment called this "the same missing band awareness [that] should be fixed in the same change"; the PR body discloses the deferral ("attenuator behavior ... unchanged") and the reporter never raised it, so this is maintainer visibility rather than a defect in what shipped. Worth a follow-up issue so the IcomModelProfile ranged-table shape gets reused rather than reinvented.

3. src/core/backends/icom/IcomCivBackend.cpp:1921 — the authoritative readback is queued behind the meter poll, on the one radio whose log already shows control-priority items waiting seconds.

                    queueRead(read, semanticKey(read), IcomCivScheduler::Priority::Control);

Control is 4 in IcomCivScheduler.h:24-31, i.e. behind ActiveMeter (3) and Ptt (2), while the write-confirmation read at :4345 uses Operator (1). In the gap, SpectrumOverlayMenu::setPreampLabels (src/gui/SpectrumOverlayMenu.cpp:3301-3302) has already reset an out-of-range step to 0, so after tuning HF→VHF with P.AMP2 selected the button reads OFF while the radio is actually on P.AMP1 — and #6497's own log reports queueWaitMs of about 5–6 s for control priority items during load. Cosmetic and self-correcting (a click in that window sends 16 02 01, which converges), but Operator would match the sibling read and close it:

                    queueRead(read, semanticKey(read), IcomCivScheduler::Priority::Operator);

4. src/core/backends/icom/IcomCivBackend.cpp:1440 — clearing the frequency also resets the S-meter's S9 reference, now on reconnect as well as first connect.

    m_frequencyHz = 0; // A new session must establish its own receive frequency.

s9ReferenceFor(m_frequencyHz) at :2889 is unguarded, and usesVhfSReference is hz >= 30'000'000 (IcomMeters.h:67-69), so 0 selects the HF reference (−73 dBm) instead of VHF (−93). Meter polls are ActiveMeter (3) while the startup frequency read is Maintenance (5, :1067), so meters will generally answer first: a 2 m/70 cm session reads ~20 dB hot until the first 03/00 reply. This window already existed on every first connect (the member's construction default is 0); the line extends it to reconnects, where the stale-but-correct value used to carry over. No code block — the trade is a judgment call between guarding s9ReferenceFor's caller, raising the startup read's priority, and tracking ladder knowledge separately from the frequency. Worth a sentence in the body either way, since it is a side effect the preamp fix doesn't need.

5. What I tried to break

  • Sibling models on the shared ladder. Only two profiles use kHfPreampLabels — IC-705 (IcomModels.cpp:569) and IC-7300MK2 (:656); the IC-9700 has its own two-position kIc9700PreampLabels (:605), and 0x94/0x98/0x8E fall to kTunerOnlyProfile. The MK2 is HF/50 MHz hardware, so it cannot reach 74.8 MHz and needs no limit. No preamp sibling left broken.
  • The default argument widening the ladder elsewhere. preampLabelsFor(model, hz = 0) defaults to the narrow ladder, which could have silently truncated other callers. Every production call site passes m_frequencyHz (:1566, :1915, :1917, :4359); the only defaulted caller is tests/icom_family_test.cpp:874 on the IC-9700, whose limit == 0 makes the branch inert.
  • Truncation direction. labels.first(min(2, size)) keeps indices 0/1, and setPanPreamp sends the index straight as the wire value — so the truncation preserves the register meaning of every surviving position. Truncating the head instead would have silently remapped OFF; it doesn't.
  • Shrinking the ladder under a selected P.AMP2, and the diagnostic scrub. GUI resets the step (SpectrumOverlayMenu.cpp:3301); reassertPanPreampWireStep still clamps 0..2 unconditionally (:4370-4375), so if the scrub fires before the readback lands it can emit 16 02 02 at VHF — but with the setter now bounded, m_preampStep can only be 2 there if the radio itself reported 02, and the write's confirmation read corrects it either way. No new absorbing loop; I could not construct one.
  • Clearing m_frequencyHz against its other consumers. Of the ten readers, :1859, :3960, :3993 and :6049 already guard == 0; bandRatedPowerWatts(.,0) returns nullopt and the caller explicitly declines to borrow a rating. Only the S9 reference is unguarded — nit 4.
  • The test shim change (frame.from = b.m_model->civAddress → b.m_session->civAddress()), which touches every deliver() in the file. It is required, not incidental: onSessionConnected sets m_model = &unknownModel() (:1452), so the old expression would stamp the wrong source address on the reply that the new test delivers after reconnect, and the unexpected-responder guard at :1743 would drop it. It is also behaviour-neutral for the existing tests — prepare() sets m_session->setCivAddress(b.m_model->civAddress) (:36) and no test in the file ever makes the two differ.
  • "Tune intent alone cannot change the ladder" (icom_backend_seam_test.cpp:263). I checked this isn't vacuous: setSliceFrequency takes Hz (:3969-4005), so 144'300'000 is a real in-range request, and the IC-705 has no bandsFor table so it reaches sendUserCommand — which never writes m_frequencyHz. The assertion genuinely pins Principle II.
  • Includes and compile surface. std::ranges::equal / std::min need <algorithm>, present in IcomCivBackend.cpp, IcomModels.cpp and both tests; .first() needs <span>, present. Consistent with check-windows (the -DENABLE_ASR=OFF lane) and check-macos both green.
  • CodeGuard, verified not parroted. All 58 findings refute. The 26 CS-VLN-CMDI-BACKTICK hits are prose backticks inside // comments (the project's known lexical-comment class), CMDI-PIPE 5930-6060 and CMDI-AMP 5790 are C++ ||/&& in the scrub dispatch chain, CMDI-SEMI 1889 is a C++17 if-with-initializer in the kReadId case, and the rules.json hits (88/89/744/1003) are log-matching regex and prose at lines this PR does not touch. None intersect a changed hunk; nearest is 1968, below the frequency case ending at 1939. The frequency value from decodeFreq is used only as a comparison key against a constant — never as an index, length, or command text. Credit: security-audit (CodeGuard).

Honest limits. I ran headless: no build, no test execution, no GUI. Every finding above is reasoned from the code at 65c3658, not reproduced at runtime. The two things I did execute are the log-rule checker (707 checks / 45 rules, 12 stale-line NOTEs on main) and check_command_plane.py (125 raw Flex commands, baseline 125 — the PR's claim holds; the Icom backend is below the seam). I could not verify the 446.15 MHz hardware proof or the mutation runs; the preamp-proof.json and screenshots are consistent with the described bytes, but I take them on the author's word. I also cannot confirm the --update regeneration in nit 1 is byte-identical to what #6673 produces.

On "green": the frozen PR gate (ci.yml, #5405) runs ctest -R over only the ThumbDV four, cross_needle_meter_test, mac_nr_filter_test and asr_gpu_probe_test. Neither icom_control_profile_test nor icom_backend_seam_test ran in CI on this head — their pass is the author's local macOS run, and they first gate on full-suite.yml after merge to main. Separately, the icom_family_test failure the body discloses is pre-existing on main, not this PR: #6674's body records it failing at both 69a50077 and 6bb29bb7, and #6672 is the open fix. The body's disclosure of it is accurate and appropriately scoped.

6. Recommendation

Approve with nits. The fix sits at the correct seam, is bounded by the reporter's measured constant rather than an invented one, writes nothing on a range change, and strengthens rather than weakens Principle II — the backend now asks the radio instead of carrying a stale assumption. I tried the ladder-shrink race, the scrub reassert, the default-argument widening, the truncation direction, the sibling model profiles and the test's own discriminating power, and the change survived all of them.

Concrete next step: drop the rules.json hunk (nit 1) so this PR stops colliding with #6673/#6666, and consider the one-word priority change in nit 3. Nits 2 and 4 are follow-up issues, not rework here. Thanks for the precise boundary citation and for disclosing the icom_family_test state and the untested platforms rather than rounding the validation up.


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

@aethersdr-agent

Copy link
Copy Markdown
Contributor

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

  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in docs/user/src/components/LogAnalyzer/rules.json /tmp/aetherclaude/pr-6679/docs/user/src/components/LogAnalyzer/rules.json:89
  • [HIGH] CS-VLN-PATH-UNC — Vulnerability: CS-VLN-PATH-UNC in docs/user/src/components/LogAnalyzer/rules.json /tmp/aetherclaude/pr-6679/docs/user/src/components/LogAnalyzer/rules.json:744
  • [HIGH] CS-VLN-PATH-UNC — Vulnerability: CS-VLN-PATH-UNC in docs/user/src/components/LogAnalyzer/rules.json /tmp/aetherclaude/pr-6679/docs/user/src/components/LogAnalyzer/rules.json:1003
  • [HIGH] CS-PII-DL-1 — PII detected: drivers_license in docs/user/src/components/LogAnalyzer/rules.json /tmp/aetherclaude/pr-6679/docs/user/src/components/LogAnalyzer/rules.json:88
  • [CRITICAL] CS-VLN-CMDI-SEMI — Vulnerability: CS-VLN-CMDI-SEMI in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:1889
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5930
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5937
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5980
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:6006
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:6029
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:6038
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:6059
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:6060
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5790
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:39
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:72
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:82
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:254
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:333
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:441
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:512
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:618
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:717
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:1009
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:1968
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:2101
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3011
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3170
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3573
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3575
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3634
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3639
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3695
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:3813
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:4792
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:4828
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5261
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5428
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5449
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/icom/IcomCivBackend.cpp /tmp/aetherclaude/pr-6679/src/core/backends/icom/IcomCivBackend.cpp:5509

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: $15.9681 · model: claude-opus-5

Ozy311 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review nits in signed commit 67cfca6:

Three focused tests, two discriminating mutation checks, macOS app/docs builds and independent source review passed. RX-only IC-705 validation confirmed P.AMP2 at 74.799 MHz and P.AMP1 at 74.800 MHz, with 2,819 replies and zero timeouts. Captured receive settings were restored and the owned app stopped. The cold-start meter guard is regression-tested; this lab run did not instrument the first displayed meter sample. Full validation details are in the updated PR body. No merge or auto-merge.

@jensenpat jensenpat self-assigned this Oct 22, 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 exact head 67cfca650a2960e8e9b86078205687b4d30db036. No blockers or outstanding nits; approve.

Issue fit and scope

#6497 documents the rejected P.AMP2 loop on VHF/UHF, and the reporter confirms the 74.800 MHz boundary. The patch uses radio frequency readback, bounds operator requests, and preserves the existing confirmation and diagnostic paths.

Files Change Claimed Verdict
IcomModels.h/.cpp Model-owned frequency limit and effective ladder Yes In scope
IcomCivBackend.h/.cpp Republish on confirmed range change; bound setter; reset frequency per session; defer S-meter until frequency known Yes In scope; meter guard addresses the reset's calibration consequence
icom_control_profile_test.cpp, icom_backend_seam_test.cpp Socket-free profile, wire-intent, scheduler and lifecycle regressions Yes In scope
networked-icom.md Current control and reconnect behavior Yes In scope

No GUI redesign, persisted radio state, dependency, default timer, TX permission, workflow or CHANGELOG change. No new above-seam coupling or ratchet exception. The attenuator follow-up is explicitly scoped separately as #6685.

What I tried to break

  • Exact boundary and unknown frequency, tune intent versus radio readback, polled versus unsolicited replies, repeated changes within one range, and return to HF. The focused tests pass.
  • Stale stage-2 requests and shrinking the published ladder: the setter bounds the request using the same profile, while a range change only publishes choices and requests a read; it sends no preamp write. Existing radio readback remains authoritative.
  • Reconnect before frequency arrives: the changed handler withholds S-meter publication, keeps frequency-independent telemetry, and adopts the confirmed HF/VHF reference. Scheduler coverage verifies preamp confirmation before a queued meter after the in-flight frequency transaction.
  • Sibling models and the diagnostic scrub: unrestricted profiles retain their ladders; the separate wire-state reassertion still preserves the radio-reported value. The test shim's source-address change is needed after identity reset and retains existing fixture behavior.
  • Mutation: temporarily disabled the profile limit, lowered range-read priority, and disabled the unknown-frequency meter guard together. icom_control_profile_test and icom_backend_seam_test failed, including boundary/clamp and startup-meter assertions; icom_meters_test stayed green. Restored byte-identical head source, rebuilt and reran: 3/3 pass. This combined mutation is not claimed as independent isolation of each guard.

Validation and limits

Local macOS arm64 / Qt 6.12 test-target build and headless CTest: 3/3 passed (icom_control_profile_test, icom_backend_seam_test, icom_meters_test). Minimal test configuration disables ASR, RADE, D-STAR, DFNR and QtKeychain; no local app/package build is claimed.

Strict engine-boundary, touchpoint manifest, capability and command-plane checks pass and their complete findings match merge base 6bb29bb7 exactly. Test registration, generated references, whitespace and 707 Log Analyzer checks pass. Required hosted Linux/macOS/Windows/static checks and hosted Docs build pass on this head; focused Icom tests are local evidence rather than part of the frozen PR CI test selection. All contributor commits have valid signatures. No unresolved threads or blocking reviews.

The automated code-review skill is unavailable; this was a manual adversarial pass. Demo cannot exercise Icom CI-V/profile behavior, so no bridge or live-radio run was performed in this review. The author's RX-only IC-705 captures and boundary measurements are supporting evidence, not independently repeated hardware proof. No TX, other-model hardware, native rendering, full-suite or post-merge result is claimed.

@jensenpat
jensenpat merged commit b5e0c57 into aethersdr:main Oct 22, 2026
11 checks passed
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.

IC-705: Preamp button cycles through P.AMP2 on VHF/UHF, radio NAKs it, and the control gets stuck on P.AMP1

2 participants