[jensenpat]

Fixes #6172.

What was wrong

On Icom, AetherSDR's squelch could get stuck off.

There is no squelch enable on these radios: the 14 03 threshold is the control. IcomCivBackend therefore derived squelchOn = level > 0 from every readback, and the confirmation read of our own "on, level 0" write published Off. SliceModel confirms receive controls from the radio, so from then on:

• every setSquelch(receiveSquelchOn(), level) was sent as Off and wrote 0000 again. That covers TCI sql_level/sql_enable, the SQL shortcut and controller bindings;
• once the remembered manual level was 0, SQL → Manual wrote 0000 and its readback returned the slice to Off. Squelch could not be turned on from AetherSDR again.

This affects the IC-7300MK2, IC-705 and IC-9700, which share the write, the decode and the confirmation read. The MK2 also re-reads 14 03 every few seconds, so it re-asserted Off even without a write.

Live on v26.10.1, Manual at 75 when a TCI client connected:

TCI rx: "sql_enable:0,true;"   TCI rx: "sql_level:0,0;"
TX -> ... 14 03 00 00 fd       RX <- 14 03 00 00
TCI tx→all: sql_enable:0,false;            # our own write read back as Off
TCI rx: "sql_level:0,20;" ... "sql_level:0,100;"
TX -> ... 14 03 00 00 fd   (all six)       # every later level written as 0

The fix

IcomCivBackend remembers that its last 14 03 write was on at threshold 0 (m_squelchOnAtZero).

• That write's 0 readback, and the MK2's periodic 0 poll, stay on.
• A radio-side non-zero value clears the flag, so a later front-panel return to 0 still reads as off.
• An explicit Off write reads as off, and disconnect forgets the flag.
• The controls scrub re-asserts on-at-0 as on.

Proof (live IC-7300MK2, proof build)

1. The trapped state is no longer a trap. Starting from SQL Off with the remembered manual level at 0, clicking SQL → Manual wrote 0000. The radio echoed 0000 and squelch stayed on through three periodic polls. Before this fix it bounced back to Off.

2. The TCI sweep that used to stick at 0 (sql_enable:true, then sql_level 0 → 100) now writes every level as itself: 0000, 0051, 0102, 0153, 0204, 0255. No sql_enable:false is broadcast back. RX audio over TCI shows the radio honouring each write: open at -17 dBFS through 0102, digital silence (-200 dBFS) from 0153 up.

Tests (headless)

icom_control_profile_test runs a new case per model (IC-7300MK2, IC-705, IC-9700):

Readback	Expected
connect-time 0	off
0 after an on-at-0 write	on
periodic 0 poll after an on-at-0 write	on
non-zero threshold	on, at that level
radio-side 0 after a non-zero threshold	off
0 after an explicit Off write	off

Run against the pre-fix backend, the two on-at-0 checks fail on all three models. All 27 Icom and squelch-related ctest targets pass.

Not in this PR: the SQL line / Auto SQL scale on Icom

Icom still publishes legacyDbmSquelchScale() (Flex's −160 + level dBm, #6127). I measured the MK2's SSB S-meter squelch through this build: 14 03 stepped while the TCI RX audio was watched for the gate, on 14.2968 USB band noise.

Front end	S-meter	Gate (14 03 raw)
preamp off	−106.5 dBm (raw ≈ 46)	≈ 113
P.AMP1	−86.5 dBm (raw ≈ 90)	≈ 146
P.AMP2	−77 dBm (raw ≈ 111)	≈ 166
20 dB ATT	pinned at S0	≈ 85

That fits roughly gate_raw ≈ 0.82 · smeter_raw + 75, or about −168 + 1.4 · level dBm below S9. The current line is therefore around 15–20 dB below where the radio gates.

A correct line also needs the Icom pan's nominal dBm axis reconciled with the S-meter: the pan floor read −123 while the S-meter read −87 on the same noise. That is a calibration change for its own PR.

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

Yes. #6172's mechanism is exactly the asymmetry between the write (on ? percentToLevelRaw(level) : 0) and the decode (squelchOn = pct > 0), and this PR closes it at the only seam where it can be closed — 14 03 00 00 is the same frame for "off" and "on at threshold 0", so the enable intent has to be carried somewhere. The issue's four acceptance cases all hold in the code as written:

event	code path	result
echo of our own (on, 0) write	IcomCivBackend.cpp:2136-2139, flag set at :4644	on ✅
MK2 periodic 14 03 poll of 0	same, flag untouched by pct == 0	on ✅
front-panel non-zero, then back to 0	:2136 clears on the non-zero	off ✅
explicit Off write	:4644 on && raw == 0 is false	off ✅

It also picks up the two things the triage comment on #6172 flagged beyond the report: scrubDrive (:5650) no longer re-infers on/off from m_squelchPercent > 0, and the manual-memory-at-0 state is left as an honest draggable state rather than clamped away.

The implementation differs from the triage's proposed sketch (clear-on-non-zero-readback instead of comparing the previous m_squelchPercent inside the zero branch). I walked both: they agree on all four cases above and on the mixed sequences I could construct, and the flag version has one fewer ordering dependency on when m_squelchPercent is overwritten. No objection.

Scope

File	What it changes	Claimed by the body?	Verdict
src/core/backends/icom/IcomCivBackend.cpp	flag set on write (:4644), consumed in the level::kSquelch decode (:2136-2139), used by scrubDrive (:5650), reset in onSessionDisconnected (:1572)	yes, all four bullets	in scope
src/core/backends/icom/IcomCivBackend.h	one bool member + why-comment (:521-523)	yes	in scope
tests/icom_control_profile_test.cpp	deliverSquelch helper + six assertions × 3 models	yes	in scope

Everything in the diff is explained by the issue. Specifically checked and clean:

• No CHANGELOG.md entry — correct.
• The PR body carries an SSB S-meter calibration table, and no code touches it: squelchLevelScale = legacyDbmSquelchScale() at IcomCivBackend.cpp:513 is untouched, so the measurement really is deferred as the body says.
• No new public/protocol/settings/capability surface. SliceDelta::squelchOn already existed; m_squelchOnAtZero is a private member.
• No - lines beyond the two comment/expression lines being replaced; no guard or early return removed.
• Sibling backends (Hl2Backend, RtlSdrBackend) have real squelch enables and need nothing — this defect is Icom-specific.
• Socket tests: none added, modified or removed. icom_control_profile_test is socket-free (direct onCivFrame/seam calls) and already registered at tests/tests.cmake:1134-1137, including in the list at :7611.

CI on 1a9aa771: 5/5 green (build, check-macos, check-windows, Static checks, Sanitizer option configures).

Blockers

None.

Nits (all non-blocking)

• src/core/backends/icom/IcomCivBackend.cpp:1572 — "disconnect forgets the flag" is only true for a radio-side drop. onSessionDisconnected() is reached only from the session's disconnected() signal (connected at :881), and IcomSession::stop() (IcomSession.cpp:125-170) sets m_connected = false without emitting it — only fail() (:177-188) does. So an operator-initiated disconnectRadio() (:940-948, m_session->stop(); m_session.reset();) carries m_squelchOnAtZero into the next session, where the connect-time 14 03 read of a genuine 0 publishes squelchOn = true. Consequence is cosmetic rather than a trap — threshold 0 is wide open either way and the control still writes — and the sibling mirrors reset in the same block (m_micGainReported, m_pcAudioEnabled) leak identically, so this is pre-existing shape, not a regression. Either narrow the body's claim or add it to disconnectRadio's own reset block:

      m_controlsValueKnown.clear();
      m_squelchOnAtZero = false;

• src/core/backends/icom/IcomControls.cpp:172-173 — the ControlSpec note still reads "NO separate enable exists on this radio — the threshold IS the control, and squelch is off at zero." That is now only true absent an on-at-0 write, and this string is surfaced by civ controls. The triage comment on #6172 asked for the reword in the same change. Same drift in the retained comment at IcomCivBackend.cpp:4638-4642 ("squelch is 'off' when it sits at zero") and in docs/architecture/aetherd-icom-civ-backend-design.md:1292 ("No enable exists: the threshold IS the control and off is zero").
• tests/icom_control_profile_test.cpp:433-463 — three cheap cases missing, all socket-free and constructible with the existing fixture: (a) the reset — on-at-0 write, onSessionDisconnected, re-prepare, deliver 0 → must read off (this is the one that would have caught the nit above, at least for the signal path); (b) the scrubDrive branch at :5650 has no coverage; (c) nothing asserts the outbound frame, so the issue's headline symptom ("every later level written as 0") is not pinned — IcomCivBackendTestAccess::lastOutboundCiv already exists and prepareSession gives a live m_session, so setSliceSquelch(0, true, 20) could assert 14 03 00 51 directly.
• The flag is not cleared if the radio answers the 14 03 write with NG/FA. Only frequency writes handle NG (:1811-1815), so this introduces no new gap — noting it only because the flag is optimistic by construction.

What I tried to break

• Stale 14 03 readback clearing the flag. The attack: an MK2 periodic poll (:6715-6716) is in flight or queued when the operator writes on-at-0, its old non-zero value arrives after the flag is set, clears it, and then the write's own confirmation read of 0 publishes Off — the original trap, intermittently. It does not survive the scheduler: sendUserCommand → queueWrite(..., supersedes=true) bumps the civ.20.3 generation (IcomCivScheduler.cpp:74-77), observe() returns Stale, and onCivFrame returns at :1804 before the decode. A queued-not-yet-dispatched older read is erased by the confirmation read's own enqueue (IcomCivScheduler.cpp:111-113, it->generation < currentGeneration). The comment at :3886-3888 claims exactly this and it checks out.
• An off-by-one making a non-zero level look like on-at-0. percentToLevelRaw is a ceiling (CivCodec.cpp:294-298), so pct 1 → raw 3; on && raw == 0 is exactly on && level <= 0. No percentage can accidentally arm the flag.
• An unguarded caller setting the flag with nothing on the wire. sendUserCommand returns early on !m_session || !m_connected (:3871) while the flag is set before it. Grepped the whole tree: outside the backend, setSliceSquelch exists only as the IRadioBackend.h:673 virtual and the HL2/RTL overrides — the only Icom routes in are requestSliceSquelch (:4354-4360, guarded on m_connected && m_session && m_model) and scrubDrive, which bails at :5638 because m_controlsValueKnown is cleared on teardown (:982). The disconnected-write attack only lands through the already-torn-down path in the nit above.
• Downstream consumers newly seeing a state that was previously unreachable. squelchOn == true && level == 0 now happens, which flips SmartCatProtocol.cpp:1397 (BY → busy) and feeds RigctlProtocol.cpp:2151 (DCD). Both already encode "squelch at 0 means always open" deliberately in their own comments, so this is the semantics they were written for, not a regression.
• The test proving nothing. The two on-at-0 assertions (:449-453) read on == true where the pre-fix decode emits false, and the optional-based capture fails on a missing delta rather than passing vacuously — so they do fail against the unfixed backend, as the body claims. The three negatives (radio-side 0 after non-zero, connect-time 0, explicit Off) each exercise a distinct branch.
• Coverage boundary I could not close and do not think should be: on the IC-705 and IC-9700 nothing polls 14 03, so a front-panel 0 → N → 0 between our writes is invisible and the UI stays "on at 0". That is the pre-existing no-poll limitation, not something this PR worsens, and chasing it would mean polling those profiles. The live proof in the body (the TCI sweep writing 0000…0255, RX audio gating from 0153 up) is the right evidence layer for that half and is not reproducible headlessly.

On Principle II: the enable bit here is synthetic, but the PR keeps the threshold radio-authoritative — every readback still adopts pct into m_squelchPercent and d.squelchLevel — and carries client intent across only the one frame the wire genuinely cannot disambiguate. I read that as within II rather than against it.

All of the above is reasoned from the head checkout; I have no build or runtime here, so nothing is reproduced empirically.

Recommendation

Approve with nits. The fix is at the right seam, the case table is complete, the scheduler already defends the stale-readback race I expected to find, and the scope is exactly three files with the calibration work correctly deferred. The disconnect-path nit and the stale "off at zero" notes are worth a follow-up commit on this branch but none of them block — and the maintainer may want this reviewed alongside #6171, which the triage identified as the same defect class on COMP/PROC. Nice report-to-fix pair, @jensenpat; the pre-fix-failure claim for the new assertions is the part that made this quick to check.

---

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

[jensenpat]

Addressed in afda8e9:

• Disconnect path: disconnectRadio() now resets m_squelchOnAtZero. Confirmed that IcomSession::stop() emits no disconnected().
• Stale notes: the squelch ControlSpec note, the setSliceSquelch comment and the backend-design row now state the on-at-0 rule.
• Tests (a) and (b): two new per-model cases, scrub keeps on-at-0 on, and a connect-time 0 after an operator disconnect reads off. Each fails with its fix removed.

Not changed:

• (c), the outbound-frame pin. setSliceSquelch(0, true, 20) sent 0051 before the fix as well; the trap was on=false arriving from the model. The published-state assertions pin that, and they fail on pre-fix code. A frame check here would not discriminate.
• NG/FA handling. Pre-existing, as you noted.

31 Icom/squelch/P-CW ctest targets pass headless.

🤖 Addressed by Claude Code

[ten9876]

The fix addresses #6172 at the shared Icom write/readback seam: an explicit on-at-zero remains on through confirmation and polling, while a subsequent nonzero radio value, explicit Off, and disconnect clear the remembered intent. No blocker found at head afda8e96a06a3bb246b99622b16e0623fbe3f00b.

File/group	Change claimed in body	Scope verdict
IcomCivBackend.cpp/.h	Remember on-at-zero; clear on radio changes/disconnect	Direct fix
IcomControls.cpp	Declare the matching squelch semantics	Direct fix
icom_control_profile_test.cpp	Inject confirmation/poll/reset cases for all three models	Regression coverage
Backend design document	Explain the zero-threshold behavior	In scope

Everything in the diff is explained by the issue; neither SQL axis calibration nor Auto SQL calibration is bundled.

Blockers: None.

Non-blocking coverage boundary: src/core/backends/icom/IcomCivBackend.cpp:4639 implements a shared protocol path, but the author's live IC-7300MK2 evidence was not independently repeated and IC-705/IC-9700 firmware convergence remains unverified. Demo cannot exercise an Icom threshold confirmation; I used production-handler injection rather than a synthetic firmware peer.

What I tried to break: Built and ran icom_control_profile_test on the exact head (1/1 passed). Replacing the production decode with its pre-fix squelchOn = pct > 0 behavior made nine assertions fail: confirmation, periodic poll, and controls scrub at zero across the three profiles. Restored the production source and rebuilt successfully. Inspected the explicit-Off, nonzero-to-zero and both disconnect/reset paths; the previous review's disconnect concern is fixed in this head.

The head/base engine-boundary, command-plane, capability, test-registration, frozen CI gate, settings and timeout audits passed without per-file growth; touchpoint manifests are current. No new socket-owning test was found. Required hosted checks passed, but their run was created before current main, so they do not establish integration with today's main. The separate code-review skill was unavailable; existing bot leads were checked manually.

Recommendation: Approve. This is a COMMENT review; approval remains the maintainer's decision.

Final CI recheck: required checks still pass and the reviewed head is unchanged. Current main is 14ef3e78841009d73ddf2e7de22b9a5a83d5ee0a (2026-10-08 08:23:34 UTC), newer than this head's earliest pull-request run creation. With non-strict branch protection, the green checks do not prove current-main integration.

[ten9876]

Landing pass — 61036a926

Automated review (medium) raised 8 findings against afda8e96. Each is resolved below with its proof.

#	Finding	Disposition
1	TCI sql_enable:true;sql_level:20; burst: cmdSqlLevel read receiveSquelchOn() before the queued enable ran, so it wrote Off	Fixed (maintainer call to fold here). TciProtocol holds each half of the pair until its queued setSquelch runs (m_pendingSquelch, queueSquelch). Proved by reroute_dead_controls_test → TCI: sql_enable true then sql_level 20 in one burst requests on at 20, the reverse order (level 35 then enable false keeps level 35), and a lone sql_level after the burst uses the slice's own enable. With the pending lookup reverted, the first three go red.
2	On/off and the flag clear used the rounded %, so raw 1..2 read Off	Fixed. The kSquelch decode uses *raw > 0. Proved by a radio-side 0 after a raw 1 threshold reads as off and scrubbing a raw 2 squelch re-asserts it as on, ×3 models. Red with pct restored.
3	An Off readback published level 0, which replaced the remembered manual level, so the next SQL → Manual came back on at 0	Fixed (maintainer call to fold here). A 0 that reads as Off publishes squelchOn=false only. Proved by an Off readback publishes no level, so the remembered 40 survives and a connect-time 0 … publishes no level, ×3. Red with the unconditional squelchLevel restored.
4	The flag was set even when sendUserCommand dropped the write (session present, not connected)	Fixed. setSliceSquelch returns before recording anything outside a connected session. Proved by an on-at-0 write dropped while disconnected leaves a 0 readback off, ×3. Red with the guard removed.
5	IC-705 / IC-9700 don't poll 14 03, so the flag never clears	Refuted. On those models every 14 03 read is either the connect-time read, after the flag is reset at both disconnect sites, or the confirmation of our own write. Each write re-derives the flag in setSliceSquelch, so no read can observe a stale flag.
6	Altitude: model "no squelch enable" as a capability	Declined. The shape of the fix was accepted when the PR was approved for landing. This pass doesn't re-open it.
7	The scrub duplicated the on/off rule	Fixed. The scrub re-asserts the published m_squelchOn instead of re-deriving it.
8	No raw 1..2 or burst coverage	Fixed by the tests in 1, 2 and 4.

Gates on head: check_engine_boundary --strict, gen_touchpoint_manifest --check, check_capability_records --strict, check_command_plane --strict, check_test_registration and check_qsettings are all rc=0. The only finding in a touched file is the pre-existing, baselined EB3 DaxIqModel.h include. Locally, a full build plus ctest -R "icom|tci|squelch|sql|reroute" gives 36/36 passing.

[ten9876]

Approved at 61036a9. Every review finding is resolved with proof (dispositions: #6175 (comment)). Each new test goes red with its fix reverted; the related ctest set passes locally 36/36; required CI is green on this head; the static gates are rc=0.