Skip to content

Commit 04d269b

Browse files
on8stclaude
andauthored
fix(hl2): declare the modes this radio cannot transmit in — Principle II.
Hl2Backend declared an empty receiveOnlyModes list under a comment that the host TX chain was WDSP. It is not: Hl2TxDsp is a phasing SSB modulator whose mode is read only by isLowerSideband(), so AM/SAM/DSB/FM/NFM/WBFM/WFM/DRM would key as USB. This PR lists those modes (both spellings) so RadioModel::refuseKeyInReceiveOnlyMode() refuses MOX, CW keying, and TUNE. SSB and CW stay off the list. TUNE withdrawal is accepted as the existing capability contract (same as IC-705 WFM); per-activity granularity is left to a maintainer. Tests pin bit-identical IQ vs USB, the live capability list, and TUNE refusal above the seam. Squashed-from: #5680 Co-authored-by: on8st <258096273+on8st@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 510f43c commit 04d269b

5 files changed

Lines changed: 211 additions & 7 deletions

File tree

docs/architecture/radio-capabilities-map.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ RTL declaration check runs only when the RTL backend is built.
7777
| `tuningMinHz` / `tuningMaxHz` | — (0/0) | 0.1–38.4 MHz | — (0/0) | `MainWindow_Wiring.cpp`, `applyTuningRangeToOverlayMenu` | Refuses band buttons the receiver cannot reach. 0/0 means unconstrained |
7878
| `declaredBandRanges` | — (empty) | — (empty) | — (empty) | `SpectrumOverlayMenu::setDeclaredBands` | Optional canonical-name + native coverage list used for backend-authoritative band labels. Icom publishes the IC-9700's three discontinuous deck ranges; empty keeps canonical labels and existing Flex/HL2/Sim presentation |
7979
| `canTransmit` || `m_txAllowed` || `RadioModel::setTransmit`, MOX/TUNE key guards | **TX safety gate.** Fail-closed: false denies any keying intent |
80-
| `receiveOnlyModes` | — (empty) | — (empty) | — (empty) | `RadioModel::refuseKeyInReceiveOnlyMode` (MOX / TUNE / CW-key / `setTransmit`) | Modes the radio **demodulates but will not transmit in**, in the neutral vocabulary. Empty = transmits in everything it receives. Refusing here (not in the backend) is what rolls back `TransmitModel`'s optimistic MOX/TUNE state — a backend cannot reach `TransmitModel`, so a refusal made down there leaves the TX indicator lit and TUNE latched. Icom: `["WFM"]` on the IC-705, which receives 76–108 MHz broadcast and does not transmit there (#5040) |
80+
| `receiveOnlyModes` | — (empty) | AM · SAM · DSB · FM · NFM · WBFM · WFM · DRM | — (empty) | `RadioModel::refuseKeyInReceiveOnlyMode` (MOX / TUNE / CW-key / `setTransmit`) | Modes the radio **demodulates but will not transmit in**, in the neutral vocabulary. Empty = transmits in everything it receives. Refusing here (not in the backend) is what rolls back `TransmitModel`'s optimistic MOX/TUNE state — a backend cannot reach `TransmitModel`, so a refusal made down there leaves the TX indicator lit and TUNE latched. Icom: `["WFM"]` on the IC-705, which receives 76–108 MHz broadcast and does not transmit there (#5040). HL2: `Hl2TxDsp` is a phasing SSB modulator whose stored mode is read only by `isLowerSideband()`, so every mode above takes the USB branch and would go on the air as SSB announcing nothing — both spellings of each alias are listed because the guard compares the string the slice holds. **Note what a non-empty list costs:** `beginLocalTxActivity` runs this guard for EVERY `TxActivity`, so it withdraws TUNE in those modes as well as MOX and CW keying, even where the tune carrier itself would be clean |
8181
| `hostModulates` | — (❌) || — (❌) | `TciServer`, `MainWindow_Session` | Mic source collapses to PC; PC-audio lock. **Not the same question as `takesTxAudioOverSeam`** — see below |
8282
| `takesTxAudioOverSeam` |||| `MainWindow_Session` (capture, TX stream, PC-audio lock), `AudioEngine::setHostModulation`, `RadioModel::ensureDaxTxStream` | Whether transmit audio leaves through `submitTxAudio` rather than a DAX/VITA-49 stream. Icom: ✅ |
8383
| `hasRadioPttReadback` |||| `RadioModel::publishCommandedBackendTransmitEdge`, `Ax25HfPacketDecodeDialog::beginTransmitWhenReady` | The backend's `transmitChanged` / `keyingStateConfirmed` carry the **radio's own** PTT readback and `setKeying()` is intent only. True suppresses RadioModel's command-edge fallback, and makes AetherModem wait for `radioTransmittingChanged` / `radioTransmitConfirmed` (or an already-keyed radio) before releasing sample zero. Flex: ❌ because its interlock edge is decoded inside RadioModel, not through the seam. Icom: ✅ (decoded CI-V `1C 00`, #5311) |
@@ -538,7 +538,11 @@ predicates the readouts ask before printing. See [`HERMES.md`](../HERMES.md)
538538
(Principle I). That is derived-from-name truth being used to *seed*
539539
reported-by-backend truth; the two remain distinct concepts.
540540
- **Hl2Backend** reports `canTransmit` from its own TX gate (`m_txAllowed`) so a
541-
build with transmit disabled looks RX-only from above the seam.
541+
build with transmit disabled looks RX-only from above the seam. Its
542+
`receiveOnlyModes` is hand-listed rather than derived, and the evidence for the
543+
list is a test, not the list: `tests/hl2_txdsp_test.cpp` asserts each declared
544+
mode produces IQ **bit-identical** to USB, and `tests/hl2_family_transition_test.cpp`
545+
asserts the live backend still declares every one of them.
542546
- **IcomCivBackend** derives `receiveOnlyModes` from `modeListFor()` filtered by
543547
`modeIsReceiveOnly()` rather than listing it a third time, so a mode cannot be
544548
offered in the combo without a consistent transmit answer for it. It also

src/core/backends/hl2/Hl2Backend.cpp

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,11 +1681,71 @@ RadioCapabilities Hl2Backend::capabilities() const
16811681
// Reported from the gate, not hardcoded: the engine's TX guard keys off this,
16821682
// so a build with transmit disabled must look RX-only from above the seam.
16831683
c.canTransmit = m_txAllowed;
1684-
// The HL2 modulates on this host, so it transmits in whatever mode WDSP is
1685-
// told to build — there is no mode it receives and cannot send. The transmit
1686-
// gate (m_txAllowed) is the only thing that stops it, and that is
1687-
// canTransmit above.
1688-
c.receiveOnlyModes = {};
1684+
// THE MODES THIS RADIO DEMODULATES AND CANNOT MODULATE.
1685+
//
1686+
// The comment that stood here said the HL2 "transmits in whatever mode WDSP
1687+
// is told to build — there is no mode it receives and cannot send", and left
1688+
// the list empty on that basis. **The transmit chain is not WDSP.**
1689+
// Hl2TxDsp is a hand-written phasing SSB modulator: setMode() stores the
1690+
// mode and the only reader is isLowerSideband(), which returns true for Lsb,
1691+
// Cwl and Digl and false for everything else. So AM, SAM, DSB, FM, NFM, WBFM
1692+
// and DRM all take the upper-sideband branch and go on the air as SSB,
1693+
// announcing nothing.
1694+
//
1695+
// WHAT STAYS OFF THE LIST, deliberately:
1696+
//
1697+
// * USB / LSB / DIGU / DIGL are the SSB family and modulate correctly.
1698+
// * CW / CWU / CWL keys a carrier the GATEWARE shapes at the TX NCO
1699+
// (MetisClient::setCwKeyDown). That path never reaches Hl2TxDsp, so the
1700+
// sideband switch above does not apply to it and CW transmits correctly.
1701+
//
1702+
// These strings are the neutral vocabulary SliceModel carries, and both
1703+
// spellings of each mode appear because modeFromString() accepts both:
1704+
// refuseKeyInReceiveOnlyMode() compares what the slice holds, not what this
1705+
// backend would have mapped it to, so listing only one spelling would leave
1706+
// the other keying.
1707+
//
1708+
// THIS DECLARATION ALSO WITHDRAWS TUNE IN THESE MODES. Say so here rather
1709+
// than let an operator discover it.
1710+
//
1711+
// refuseKeyInReceiveOnlyMode() is not a MOX-and-CW guard.
1712+
// RadioModel::beginLocalTxActivity() runs it for EVERY TxActivity, ahead of
1713+
// the per-activity capability checks, so TxActivity::Tune is refused too —
1714+
// and that one is a real loss, not a theoretical one. setTune() below raises
1715+
// the carrier from the GATEWARE test-tone generator at zero offset
1716+
// (MetisClient::setTxTestTone), a path that never reaches Hl2TxDsp, exactly
1717+
// like the CW keyer exempted above. This radio could put a clean tune
1718+
// carrier on the air with the TX slice in AM or FM; after this list it will
1719+
// not, and the operator is told "Choose a transmit mode first" and has to
1720+
// move the slice to a mode that transmits. (TxActivity::Atu was already
1721+
// refused here for want of hasTuner, so the plain TUNE button is the only
1722+
// behaviour this changes.)
1723+
//
1724+
// ACCEPTED, deliberately, on two grounds:
1725+
//
1726+
// * It is what this capability already MEANS. The IC-705 declares WFM
1727+
// receive-only (#5040) and is refused on this same guard, with a second
1728+
// wire backstop in IcomCivBackend::refuseKeyingInReceiveOnlyMode() that
1729+
// its setTune() converges on through setKeying() — "shared by every path
1730+
// here that can start an emission", in its own words. HL2 is inheriting
1731+
// a settled contract, not inventing one.
1732+
// * receiveOnlyModes is ONE list of mode names with no per-activity
1733+
// granularity, so exempting tune is not expressible from a backend at
1734+
// all: it would mean changing RadioModel above the family seam, for
1735+
// every family at once. That is a maintainer's call.
1736+
//
1737+
// The CW/tune asymmetry is in the SHAPE of the list, not in the reasoning
1738+
// behind it: CW stays off because CW is a MODE this radio transmits
1739+
// correctly, and tune is an ACTIVITY, which a list of mode names has no
1740+
// vocabulary for.
1741+
//
1742+
// What the list itself reports is only what the modulator does today. When a
1743+
// mode genuinely transmits — the WDSP TXA chain carries all of these — its
1744+
// entry comes back off this list and the tune refusal lifts with it.
1745+
c.receiveOnlyModes = {QStringLiteral("AM"), QStringLiteral("SAM"),
1746+
QStringLiteral("DSB"), QStringLiteral("FM"),
1747+
QStringLiteral("NFM"), QStringLiteral("WBFM"),
1748+
QStringLiteral("WFM"), QStringLiteral("DRM")};
16891749
c.hostModulates = true;
16901750
// Same tap, same seam — see RadioCapabilities::takesTxAudioOverSeam.
16911751
c.takesTxAudioOverSeam = true; // PC runs the modulator; no on-radio mic jacks

tests/atu_seam_gate_test.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,37 @@ static void receiveOnlyModeBlocksStart()
135135
check(f.backend->atuStarts == 1, "transmit mode: start dispatches once");
136136
}
137137

138+
// The ATU cycle is not the only thing a receive-only mode withdraws.
139+
//
140+
// beginLocalTxActivity() runs refuseKeyInReceiveOnlyMode() for EVERY
141+
// TxActivity, ahead of the per-activity capability checks, so the plain TUNE
142+
// carrier goes with it — on every family, including one whose tune carrier is
143+
// raised in gateware and would have been perfectly clean (HL2's built-in test
144+
// tone at zero offset, PR #5680). Asserted rather than reasoned about: it is
145+
// the consequence an operator actually meets, and it needs its own fixture
146+
// because emitInterlockNotification() deduplicates an identical key for 5 s,
147+
// so a second refusal in the same fixture would be silent.
148+
//
149+
// The interlock key is what makes this discriminating: a TUNE stopped by the
150+
// PTT preflight or the CW-keyed tune admission raises pttBlocked and no
151+
// interlock at all, and would fail this rather than pass it.
152+
static void receiveOnlyModeBlocksTune()
153+
{
154+
Fixture f(QStringLiteral("icom"), /*canTransmit=*/true, {QStringLiteral("WFM")});
155+
f.setMode(QStringLiteral("WFM"));
156+
f.radio.transmitModel().startTune();
157+
check(!f.radio.transmitModel().isTuning(),
158+
"receive-only mode: TUNE does not latch");
159+
check(f.interlocks.contains(QStringLiteral("rx-only-mode:WFM")),
160+
"receive-only mode: the TUNE refusal names the mode");
161+
// Leaving the mode gives the carrier back.
162+
f.setMode(QStringLiteral("USB"));
163+
f.radio.transmitModel().startTune();
164+
check(f.radio.transmitModel().isTuning(),
165+
"transmit mode: TUNE latches again");
166+
f.radio.transmitModel().stopTune();
167+
}
168+
138169
static void panInhibitBlocksStart()
139170
{
140171
Fixture f(QStringLiteral("icom"), /*canTransmit=*/true);
@@ -178,6 +209,7 @@ int main(int argc, char** argv)
178209
permittedStartDispatchesOnce();
179210
receiveOnlyBackendBlocksStart();
180211
receiveOnlyModeBlocksStart();
212+
receiveOnlyModeBlocksTune();
181213
panInhibitBlocksStart();
182214
flexUsesTheSameTypedPath();
183215
std::printf("%d failure(s)\n", failures);

tests/hl2_family_transition_test.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,51 @@ int main(int argc, char** argv)
8888
check(model.panStream() == nullptr,
8989
"HL2 owns no PanadapterStream");
9090

91+
// ---- The modes the LIVE HL2 backend declares it will not transmit in ----
92+
//
93+
// hl2_txdsp_test proves the modulator half: AM/SAM/DSB/FM/WBFM/DRM each
94+
// produce IQ bit-identical to USB, so none of them is a distinct
95+
// modulation. What that test cannot see is the DECLARATION — delete a
96+
// string from Hl2Backend::capabilities()'s receiveOnlyModes and it still
97+
// passes. This is the live target that reads it, so the list and the
98+
// evidence cannot part company unnoticed.
99+
//
100+
// EIGHT strings, SIX enumerators: modeFromString() maps NFM onto Mode::Fm
101+
// and WFM onto Mode::Wbfm, and refuseKeyInReceiveOnlyMode() compares the
102+
// string the SLICE holds rather than the enumerator this backend would have
103+
// mapped it to — so dropping either alias leaves that spelling keying while
104+
// its twin is refused. That is the load-bearing claim, and nothing asserted
105+
// it before this block.
106+
{
107+
const RadioCapabilities caps = model.backendCapabilities();
108+
const QStringList declared = {
109+
QStringLiteral("AM"), QStringLiteral("SAM"),
110+
QStringLiteral("DSB"), QStringLiteral("FM"),
111+
QStringLiteral("NFM"), QStringLiteral("WBFM"),
112+
QStringLiteral("WFM"), QStringLiteral("DRM"),
113+
};
114+
for (const QString& m : declared) {
115+
check(modeIsReceiveOnly(caps, m),
116+
qPrintable(QStringLiteral("HL2 declares %1 receive-only").arg(m)));
117+
}
118+
// Exactly these. An ADDITION is a mode in which the operator silently
119+
// loses MOX, CW keying and TUNE, so it must not arrive without the
120+
// bit-identity evidence landing beside it.
121+
check(caps.receiveOnlyModes.size() == declared.size(),
122+
"HL2 declares exactly the modes hl2_txdsp_test carries evidence for");
123+
// The deliberate exclusions: SSB modulates correctly, and CW keys the
124+
// gateware NCO through MetisClient::setCwKeyDown without ever reaching
125+
// Hl2TxDsp. If one of these ever appears on the list it takes an
126+
// operator's transmit mode away.
127+
for (const QString& m : {QStringLiteral("USB"), QStringLiteral("LSB"),
128+
QStringLiteral("DIGU"), QStringLiteral("DIGL"),
129+
QStringLiteral("CW"), QStringLiteral("CWU"),
130+
QStringLiteral("CWL")}) {
131+
check(!modeIsReceiveOnly(caps, m),
132+
qPrintable(QStringLiteral("HL2 still transmits in %1").arg(m)));
133+
}
134+
}
135+
91136
// ── The normalized RX-audio bus ────────────────────────────────────────
92137
//
93138
// The CW decoder, the RTTY decoder and the QSO recorder's RX tap all ride

tests/hl2_txdsp_test.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,69 @@ int main(int argc, char** argv)
215215
}
216216
}
217217

218+
// ---- THE MODES Hl2Backend DECLARES RECEIVE-ONLY ARE BIT-IDENTICAL TO USB ----
219+
//
220+
// This is the evidence behind `Hl2Backend::capabilities`'s
221+
// `receiveOnlyModes` list, and it is deliberately stronger than asserting
222+
// that a list contains some strings. A list can drift from the modulator;
223+
// this cannot.
224+
//
225+
// Hl2TxDsp::setMode() stores the mode and the ONLY reader is
226+
// isLowerSideband(), which returns true for Lsb/Cwl/Digl and false for
227+
// everything else. So AM, SAM, DSB, FM, WBFM and DRM do not take some
228+
// degraded AM or FM path -- they take the USB path exactly, and what goes on
229+
// the air is single-sideband suppressed carrier while the mode indicator
230+
// says otherwise.
231+
//
232+
// If someone later teaches this chain a real AM or FM modulator, this block
233+
// FAILS, which is the point: the failure is the reminder to take that mode
234+
// back off the receive-only list.
235+
//
236+
// SIX enumerators here cover EIGHT declared strings: Hl2Backend's
237+
// modeFromString() maps NFM onto Mode::Fm and WFM onto Mode::Wbfm, so those
238+
// two spellings have no enumerator of their own to modulate. That the
239+
// DECLARATION still carries both — the guard compares the string the slice
240+
// holds, not the enumerator — is asserted in hl2_family_transition_test,
241+
// which reads capabilities() off a live backend. This file cannot see it.
242+
{
243+
const double usbBand[2] = {300.0, 2700.0};
244+
const auto reference = modulate(WdspChannel::Mode::Usb, kTone, 0.25,
245+
1.0, 1.0, nullptr, false, usbBand);
246+
check(!reference.empty(), "USB reference modulation produced IQ");
247+
248+
struct DeclaredReceiveOnly { const char* name; WdspChannel::Mode mode; };
249+
const DeclaredReceiveOnly declared[] = {
250+
{"AM", WdspChannel::Mode::Am},
251+
{"SAM", WdspChannel::Mode::Sam},
252+
{"DSB", WdspChannel::Mode::Dsb},
253+
{"FM", WdspChannel::Mode::Fm},
254+
{"WBFM", WdspChannel::Mode::Wbfm},
255+
{"DRM", WdspChannel::Mode::Drm},
256+
};
257+
258+
for (const DeclaredReceiveOnly& d : declared) {
259+
const auto iq = modulate(d.mode, kTone, 0.25, 1.0, 1.0,
260+
nullptr, false, usbBand);
261+
check(iq.size() == reference.size(),
262+
"declared receive-only mode produced the same sample count as USB");
263+
bool identical = (iq.size() == reference.size());
264+
std::size_t firstDiff = 0;
265+
for (std::size_t k = 0; identical && k < iq.size(); ++k) {
266+
if (iq[k] != reference[k]) { identical = false; firstDiff = k; }
267+
}
268+
std::fprintf(stderr,
269+
"%-4s vs USB: %s\n", d.name,
270+
identical ? "bit-identical (no distinct modulation)"
271+
: "DIFFERS -- a real modulator now exists");
272+
if (!identical) {
273+
std::fprintf(stderr, " first difference at sample %zu\n", firstDiff);
274+
}
275+
check(identical,
276+
"this mode is indistinguishable from USB, which is why "
277+
"Hl2Backend declares it receive-only");
278+
}
279+
}
280+
218281
// ---- audio outside the passband does not get transmitted ----
219282
{
220283
// 5 kHz is well above the 2700 Hz TX filter.

0 commit comments

Comments
 (0)