fix(daemon): a repeater that is not running is no longer reported as running (#1298) - #1309
Merged
Merged
Conversation
…running (#1298) The `EnableRepeater` thread `take()`s the `CrossBandRepeater` out of the runtime state, so when that thread exits the repeater is gone. Nothing observed it: `repeater_enabled` stayed true, `EnableRepeater` answered "already enabled" forever, and no command sequence could get back to a truthful state. The no-repeater arm was worse: it logged a warning, then set `repeater_enabled = true` and emitted `RepeaterChanged { enabled: true }` regardless — so a daemon with nothing to run reported a running repeater. Now: enabling with nothing to run returns a `CommandError` naming why; a thread that has exited is reaped at the top of the repeater commands; and the thread reports its own unexpected exit, on the error path only, since a clean stop is already reported by `DisableRepeater`. `apply_repeater_enable_disable_emits_state_changes` was pinning the defect — it ran with `repeater: None` and asserted that enabling SUCCEEDED. It now builds a real repeater so the success path is the one it tests. The issue's own premise was corrected in the process: it claimed the loop exits on the first non-decoding capture, making enabled-forever the normal state on a rig. #1300 changed that. The normal state on a rig is a live thread that never relays and never says so above DEBUG — which is why this lands before #1297, where "no relay" and "thread dead" would otherwise be the same observation. Implements: REQ-OBS-02 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
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
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.
Closes #1298. Lands before #1297 deliberately — see "Ordering".
What was wrong
The
EnableRepeaterthreadtake()s theCrossBandRepeaterout of the runtime state, so when that thread exits the repeater is gone. Nothing observed that:repeater_enabledstayedtrue, soEnableRepeateranswered "repeater already enabled" forever;DisableRepeater→EnableRepeaterhit the no-repeater arm, which logged awarn!and then setrepeater_enabled = trueand emittedRepeaterChanged { enabled: true }anyway.So a daemon with no repeater reported a running one, and no command sequence could get back to a truthful state.
What changed
CommandErrornaming why, instead of claiming success.repeater_enabledand emitting thefalseedge. At the top of the commands rather than from a periodic tick: the thread's own events notify clients promptly, and the reap only has to make the next command truthful — a tick poll would needruntime_stateon the tick path for no added signal.CommandErrorplus afalseedge, on the error path only. A clean stop is already reported byDisableRepeater, and emitting there too would put twofalseedges on one transition.An existing test was pinning the defect
apply_repeater_enable_disable_emits_state_changesran withRuntimeControlState::default()— which hasrepeater: None— and asserted that enabling succeeded. So "the daemon reports a running repeater when none exists" was a passing acceptance test, not an oversight. It now builds a realCrossBandRepeaterso the success path is the one it tests.The issue's own premise was stale, and the correction is mine
#1298 said the loop "exits on the first capture that does not decode, which on hardware is the first capture", making enabled-forever the normal state on a rig. #1300 changed that this morning: a non-decoding capture is now
Ok(None)and the loop continues. What kills the thread now is a PTT fault or a transmit error.So the normal state on a rig is not a dead thread reported as enabled — it is a live thread that never relays and never says so above DEBUG. Both defects survive the correction; the severity framing did not. The issue has been amended rather than left asserting something untrue.
Ordering
This is why it lands before #1297. Without it, "the repeater relays nothing because the band is quiet" and "the repeater thread died" are the same observation from outside the daemon — which would make an on-air attempt of #1297 undiagnosable.
Tests
enabling_a_repeater_that_does_not_exist_fails_instead_of_claiming_successa_repeater_thread_that_exited_is_reaped_rather_than_reported_enabled— uses a disabled repeater, whoserun_full_duplexreturnsOk(0)at once, standing in for any exit the daemon did not ask for.Sabotage-verified both ways: removing the reap calls fails the reap gate with "the dead thread was never reported"; reinstating the claim-success arm fails the enable gate with "the daemon reports a repeater as enabled while none exists".
GATE: PASS 67f6d2506ee14f83794a6db43a35754a578b37ea clean— 326 suites, 2502 passed, 0 failed.Consumer
apply_command_to_engine(lib.rs:2130) on the daemon's control-command task;EnableRepeater/DisableRepeaterare its only callers of this state.Prior art
#1260 put the repeater on
SharedPtt; #1300'sOk(None)change is what made this issue's premise stale. TheCommandError-instead-of-silent-success shape follows #1285's rule that an unusable backend must not look like a working one.Twins
The other spawned-thread state in the daemon is the PTT watchdog, which holds no ownership and cannot strand state this way.
repeater_stopis cleared alongsiderepeater_threadso a reaped session cannot leave a live stop flag behind.Filed alongside
#1308 — the repeater has no audio device configuration at all: both engines use the OS default input, and no config field anywhere could name a second card. #1297's accumulation fix cannot be exercised on a real two-rig station without it.
Implements: REQ-OBS-02
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6