Feature/rigctld sstv mode - #62
Merged
Merged
Conversation
…ta-mode option
Two independent bugs in the Band Plan → Direct Serial tune path.
## Bug 1: FT-450D frequency changes were silently rejected
Root cause: Yaesu's `FA` (set-frequency) CAT command uses a
model-dependent digit width. FT-450/450D wants 8 digits, no leading
zero ("FA14230000;"); FT-991A and the other "modern CAT" rigs
(FT-891, FT-710, FTDX10, FTDX101, FT-950) want 9, zero-padded
("FA014230000;"). YaesuRig.set_freq() hardcoded the 9-digit format,
so every FA command sent to an FT-450D was malformed and the radio
rejected it outright with "?;".
That rejection was completely invisible, for two compounding
reasons:
- Yaesu/Kenwood *set* commands are fire-and-forget by design (the
radio sends no response at all to a legitimate set, so
_write_command() never reads one back) — there was no way to
observe a "?;" reply to the set itself.
- _RigPollWorker.tune() wrapped set_freq()/get_mode()/set_mode() in
one `except Exception: _log.warning(...)` with nothing reaching
the GUI, and the transient "Tuning to X (mode)..." status message
wasn't corrected on failure — so a rejected tune looked exactly
like a successful one.
Meanwhile MD (mode), a single-digit command, isn't sensitive to a
digit-width mismatch, so mode changes kept working — which is
exactly the asymmetry reported: freq never moved, mode did.
Fix:
- serial_rig.py: YaesuRig now detects the FA digit width from a live
get_freq() response (already lenient about either length) and
caches it per connection instead of hardcoding 9. set_freq() probes
once via get_freq() if the width isn't known yet, so a tune right
after Connect can't race the 1 Hz poll loop for this detection.
- main_window.py: _RigPollWorker.tune() now reads back get_freq()
after every set_freq() and raises/reports a mismatch (readback of 0
is treated as "this backend doesn't report frequency", not a
mismatch — e.g. SerialPttRig). A new tune_failed signal carries the
reason to the GUI thread, which now replaces the status-bar message
instead of only logging it — so any future rejection (this radio or
another) is visible instead of silent, whatever the root cause.
## Bug 2: Band Plan always forced plain USB/LSB, never a data mode
SSTV_BAND_PLAN hardcodes rig_mode="USB"/"LSB"/"FM", and that literal
was passed straight to Rig.set_mode() whenever tune() decided a mode
change was needed. Every backend's own mode map only knows how to
turn that literal into *plain* USB/LSB — even YaesuRig, which already
supports "DATA-U"/"DATA-L" if only asked for them. There was no
setting anywhere for which CAT mode Band Plan tuning should actually
request.
Fix: a WSJT-X-style "SSTV mode" policy (Settings -> Radio -> Direct
Serial): None / Voice (default, today's behavior) / Data-Pkt. "Data"
resolves through a small per-protocol table in band_plan.py
(resolve_tune_mode / DATA_MODE_BY_PROTOCOL) — currently populated for
Yaesu CAT only (DATA-U/DATA-L), since Icom's data mode is a separate
CI-V sub-command (0x1A 0x06) and Kenwood/Elecraft's is model-specific
(e.g. K3's DT command) — neither verified against real hardware, so
both intentionally fall back to Voice with a logged warning rather
than guess a wrong CAT string.
Docs: README's Direct Serial / Band Plan sections and
docs/hamlib-integration-notes.md (the Hamlib-direct research doc's
"known gotchas" list) updated to describe both the digit-width quirk
and the new SSTV-mode setting. CHANGELOG and version intentionally
left untouched.
Tests: TestRigPollWorkerTune (main_window), TestYaesuFreqDigitWidth
(serial_rig), TestResolveTuneMode (band_plan) — 348 tests green
across tests/radio, tests/config, tests/ui/test_main_window.py, and
tests/ui/test_settings_dialog.py.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e poisoning, missing log Follow-up to the FT-450D / Band Plan fix (0c4e7b9), addressing bucknova's review on bucknova#47: bucknova#47 (comment) ## 1. Readback false-fails on async-cached backends (TCI, FlexRadio) — must fix _RigPollWorker.tune() is the Band Plan path for every backend (the Band Plan button is gated only on "connected", not connection mode). The set_freq() -> get_freq() -> raise-on-mismatch sequence added in the original fix assumed get_freq() is a synchronous radio query. It isn't, on two of our four backends: - TciRig.get_freq() returns a locally-cached _last_freq that only updates when the server's async push arrives on the receive thread; set_freq() is a fire-and-forget send(). The readback ran essentially immediately after, so it read the pre-tune value almost every time. - FlexRig.get_freq() reads self._freq_hz, updated from an async slice- status push; set_freq()'s _command() blocks for the reply, but not for that separate push. Worse, the mismatch raise sat before the `if mode:` block, so a false frequency failure also skipped the mode change — a functional regression on a previously-working path. Fix: set_freq(), the mode-change block, and frequency verification each catch their own exception independently now and collect into an error list, so a frequency false-positive (or a real failure) can never block the mode change. Frequency verification is a small settle-and-retry loop (_verify_freq_settled): on mismatch, sleep ~150 ms and re-read, up to 2 extra attempts (~300 ms total budget, nothing added on the success path), with a 10 Hz tolerance instead of exact equality to tolerate step- quantizing rigs. Chosen over a synchronous-backend capability flag (the reviewer's other suggested option) because it needs no per-backend classification a future backend could forget to set correctly. ## 2. _freq_digits cache could be poisoned by a garbled response — small fix YaesuRig.get_freq() cached the detected FA digit width before int() confirmed the response actually parsed. A noise-garbled but "FA"- prefixed, correctly-terminated response could poison the cached width for the rest of the connection. Fixed: the width is cached only after a successful parse. ## 3. resolve_tune_mode() silently fell back without the promised log — small fix The original PR description said an unsupported protocol under the "data" policy falls back to Voice "with a logged warning" — the code didn't actually log anything, so a user picking Data/Pkt on e.g. Icom got plain USB with zero indication why. Added a _log.warning in the fallback branch (only for USB/LSB families — FM has no data-mode concept at all, so passing it through isn't a missing mapping and doesn't warn). Tests: TestRigPollWorkerTune gets a new async-catch-up case (get_freq() side_effect returning stale values before the real one) and the existing mismatch test now asserts the mode change still ran; TestYaesuFreqDigitWidth gets two garbled-response cases; TestResolveTuneMode gets caplog-based assertions that the fallback warns (and that FM, known mappings, and voice/none policies stay silent). 348 tests green across tests/radio, tests/config, tests/ui/test_main_window.py, and tests/ui/test_settings_dialog.py. Two points from the review are explicitly out of scope for this PR per the reviewer's own note: extending the mode policy to rigctld users, and using topic branches instead of the fork's main for future PRs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Settings → Radio → rigctld gains the same "SSTV mode" dropdown (Don't change mode / Voice / Data-Pkt) the Direct Serial connection already had. On a Band Plan tune, "Data-Pkt" resolves to Hamlib's universal PKTUSB / PKTLSB instead of plain USB / LSB, so the rig drops the mic path and speech processor. A backend without a data mode rejects M PKTUSB and the existing tune-failed path surfaces it. Reuses resolve_tune_mode() and the shared rig_tune_mode_policy field via a new RIGCTLD_PROTOCOL key in DATA_MODE_BY_PROTOCOL. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… family The Band Plan tune skipped set_mode whenever the current mode's sideband family matched the target's — which preserves an operator's own data mode on a "voice" tune, but also meant a "Data/Pkt" tune never moved a station off plain USB into PKTUSB (same family). _on_tune_requested now flags the request "exact" when the policy actually resolved a data-mode variant; tune() then compares the full mode string and switches unless the rig is already on exactly that mode. Voice tunes keep the family-match behaviour. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rigctld: honour an "SSTV mode" (Voice / Data-Pkt) setting on Band Plan tuning
Why
Settings → Radio → Direct Serial already has an "SSTV mode" dropdown
(
Don't change mode/Voice (USB/LSB)/Data/Pkt): on a Band Plan tune itresolves the plan entry's plain
USB/LSBinto the rig's data-mode variant sothe mic path and speech processor are bypassed — what SSTV needs.
The rigctld connection had no equivalent — a Band Plan tune always sent plain
USB/LSB/FM. Hamlib, unlike a single vendor's CAT, has universalPKTUSB/PKTLSBtokens accepted byM <mode>on every backend with a datamode, so for rigctld this works for any rig.
What changed
band_plan.py— newRIGCTLD_PROTOCOLkey inDATA_MODE_BY_PROTOCOLmapping
USB→PKTUSB,LSB→PKTLSB.resolve_tune_mode()logic is unchanged.main_window.py—_on_tune_requestednow runsresolve_tune_mode()forRIGCTLDconnections too (previouslySERIALonly).settings_dialog.py— the rigctld group gets its own "SSTV mode" combo,sharing the existing
rig_tune_mode_policyconfig field;result_config()reads whichever combo matches the active connection mode.
set_modewhenever thecurrent mode's sideband family matched the target's (a guard that preserves
an operator's self-selected data mode on a Voice tune). Because
PKTUSBandUSBare the same family, a "Data/Pkt" tune never moved a station off plainUSB.
_on_tune_requestednow flags the request exact when the policyactually produced a data variant;
_RigPollWorker.tune(..., mode_is_exact=True)then compares the full mode string and switches unless the rig is already on
exactly that mode. Voice tunes keep the family-match behaviour.
_request_tunesignal gains a 4th
boolarg.A rig/backend without a data mode rejects
M PKTUSBwith a non-zeroRPRT,already surfaced as a "tune failed" status message — no new error handling.
Tests
test_band_plan.py—RIGCTLD_PROTOCOLresolution (PKTUSB/PKTLSB), FMpass-through, voice/none, no spurious warning.
test_settings_dialog.py— rigctld "SSTV mode" combo seeded from config, savedwhen connection mode is rigctld, and not overriding the serial combo otherwise.
test_main_window.py—_on_tune_requestedemits the resolved mode + exactflag for data/voice/none;
tune()switches USB→PKTUSB whenmode_is_exactandskips when already on target.
pytestgreen (209 in the touched suites); no newrufffindings.Manual verification
rigctld -m 1(Hamlib dummy): with SSTV mode = Data/Pkt a Band Plan presssends
+M PKTUSB 0andget_modereads backPKTUSB; Voice sends+M USB 0; Don't change mode sends noM.Docs
docs/rigctld-protocol.md(new "Band Plan tuningand the SSTV mode setting" section).