fix(ptt,ui): Continuous mode no longer self-disables after 30 s; rename PTT label to Mic
Issue 1: in Continuous transmit mode the talk indicator turned
gray-out / mic disabled after ~30 s and could only be revived by
toggling mic mute. Root cause: SAD-079 MissedKeyUpWatchdog
subscribed to AudioTransmitGate.transmit_active and force-cleared
it after 30 s of true. In PTT mode this is correct (stuck key =
bug). In Continuous mode transmit_active is *supposed* to stay
true indefinitely; the watchdog assumption doesn't hold.
Fix: the watchdog now subscribes to a new ptt_held watch on the
TransmitModeSelector (the raw key-state input, not the resolved
gate). In Continuous mode ptt_held is never set true, so the
watchdog never fires. In PTT mode it still fires on a stuck
key-down as before. The session owns the watchdog (was on the
engine) so it survives engine restarts; it's spawned lazily on the
first start_audio.
MissedKeyUpWatchdog gains spawn_on_signal(rx, on_timeout, timeout)
alongside the existing spawn(gate, timeout) — old shape preserved
for backwards compat. run_watchdog generalised to take any
watch::Receiver<bool> + Box<dyn Fn() + Send + Sync>.
Two new tests:
- watchdog_on_signal_does_not_fire_when_ptt_held_stays_false
(the Continuous-mode regression test)
- watchdog_on_signal_fires_when_signal_stays_true
(the stuck-key case still fires)
Issue 2: the Voice Bar stats line said 'PTT on/off' even when the
user was in Continuous mode where no PTT key is involved. Renamed
to 'Mic on/off' (mode-neutral) and l10n-ised the on/off literal:
- en: 'Mic on' / 'Mic off'
- zh: '麦克风 开启' / '麦克风 关闭'
cargo test --workspace --lib: 80 passed / 0 failed / 1 ignored
(was 78, +2 watchdog tests).
flutter analyze: clean (6 pre-existing Radio.groupValue infos).
This commit is contained in:
@@ -113,6 +113,17 @@ pub struct AudioEngine {
|
||||
/// observes the gate directly. The platform input backend is
|
||||
/// owned by `chanora_core::ptt::PttController` (SDD-088), not
|
||||
/// by the engine.
|
||||
///
|
||||
/// In the post-rc.7 architecture this field is unused: the
|
||||
/// missed-key-up watchdog now lives on the session and
|
||||
/// subscribes to `TransmitModeSelector::subscribe_ptt_held`
|
||||
/// rather than the gate. Watching the gate caused the watchdog
|
||||
/// to fire in Continuous mode (where the gate is intentionally
|
||||
/// pinned to `true`) which clearing surfaced as the bug
|
||||
/// "Continuous transmission disabled after some time". The
|
||||
/// field stays here as `None` for now to preserve the existing
|
||||
/// engine-stop teardown flow; a follow-up commit can remove it
|
||||
/// entirely.
|
||||
ptt_watchdog: Option<crate::ptt::MissedKeyUpWatchdog>,
|
||||
}
|
||||
|
||||
@@ -368,13 +379,12 @@ impl AudioEngine {
|
||||
// responsible only for the cpal streams and the
|
||||
// missed-key-up watchdog (SDD-092).
|
||||
|
||||
// Spawn the missed-key-up watchdog. The task aborts on
|
||||
// Drop of `MissedKeyUpWatchdog`, so the engine's `stop`
|
||||
// / Drop chain releases it without explicit cleanup.
|
||||
let ptt_watchdog = crate::ptt::MissedKeyUpWatchdog::spawn(
|
||||
transmit_gate.clone(),
|
||||
crate::ptt::MissedKeyUpWatchdog::DEFAULT_TIMEOUT,
|
||||
);
|
||||
// The engine no longer spawns a watchdog against the
|
||||
// gate (see comment on the `ptt_watchdog` field for the
|
||||
// rationale). The session spawns the watchdog against the
|
||||
// selector's `ptt_held` signal instead, so it never fires
|
||||
// in Continuous mode.
|
||||
let ptt_watchdog: Option<crate::ptt::MissedKeyUpWatchdog> = None;
|
||||
|
||||
Ok(Self {
|
||||
transmit_gate,
|
||||
@@ -386,7 +396,7 @@ impl AudioEngine {
|
||||
_output_stream: Mutex::new(Some(output_stream)),
|
||||
shutdown_tx: Some(shutdown_tx),
|
||||
capture_active,
|
||||
ptt_watchdog: Some(ptt_watchdog),
|
||||
ptt_watchdog,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
+121
-21
@@ -207,13 +207,51 @@ impl MissedKeyUpWatchdog {
|
||||
/// Default timeout per DEC-028 — 30 seconds.
|
||||
pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Spawn the watchdog. The returned handle aborts the task on
|
||||
/// Drop. The watchdog needs a tokio runtime context; the audio
|
||||
/// engine spawns it inside `start_audio` where the bridge's
|
||||
/// runtime is available.
|
||||
/// Spawn the watchdog against an [`AudioTransmitGate`].
|
||||
///
|
||||
/// Provided for backwards compatibility with the original
|
||||
/// SAD-079 wiring (which spec'd the watchdog as subscribing to
|
||||
/// `transmit_active`). In the current implementation this is
|
||||
/// only correct for PTT mode — in Continuous mode the gate is
|
||||
/// supposed to stay `true` indefinitely. Prefer
|
||||
/// [`Self::spawn_on_ptt_held`] which subscribes to the
|
||||
/// raw key-down signal instead and is mode-agnostic.
|
||||
pub fn spawn(gate: AudioTransmitGate, timeout: Duration) -> Self {
|
||||
let rx = gate.subscribe();
|
||||
let on_timeout: Box<dyn Fn() + Send + Sync> = Box::new(move || gate.set(false));
|
||||
Self::spawn_inner(rx, on_timeout, timeout)
|
||||
}
|
||||
|
||||
/// Spawn the watchdog against an arbitrary `bool` watch
|
||||
/// channel — typically the `ptt_held` signal exposed by
|
||||
/// [`crate::TransmitModeSelector::subscribe_ptt_held`]. The
|
||||
/// `on_timeout` callback is invoked when the signal has been
|
||||
/// `true` for longer than `timeout` without transitioning back
|
||||
/// to `false`; it should clear whatever upstream produced the
|
||||
/// stuck-true state (e.g. `selector.set_ptt_held(false)`).
|
||||
///
|
||||
/// This variant is the correct shape for the new selector +
|
||||
/// transmit-mode architecture: it watches the PTT-key signal,
|
||||
/// not the resolved gate, so it never fires in Continuous mode
|
||||
/// where the gate is intentionally held `true`.
|
||||
pub fn spawn_on_signal<F>(
|
||||
signal: tokio::sync::watch::Receiver<bool>,
|
||||
on_timeout: F,
|
||||
timeout: Duration,
|
||||
) -> Self
|
||||
where
|
||||
F: Fn() + Send + Sync + 'static,
|
||||
{
|
||||
Self::spawn_inner(signal, Box::new(on_timeout), timeout)
|
||||
}
|
||||
|
||||
fn spawn_inner(
|
||||
rx: tokio::sync::watch::Receiver<bool>,
|
||||
on_timeout: Box<dyn Fn() + Send + Sync>,
|
||||
timeout: Duration,
|
||||
) -> Self {
|
||||
let handle = tokio::spawn(async move {
|
||||
run_watchdog(gate, timeout).await;
|
||||
run_watchdog(rx, on_timeout, timeout).await;
|
||||
});
|
||||
Self {
|
||||
handle: Some(handle),
|
||||
@@ -229,11 +267,14 @@ impl Drop for MissedKeyUpWatchdog {
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_watchdog(gate: AudioTransmitGate, timeout: Duration) {
|
||||
let mut rx = gate.subscribe();
|
||||
// Track whether we are currently in a `transmit_active = true`
|
||||
async fn run_watchdog(
|
||||
mut rx: tokio::sync::watch::Receiver<bool>,
|
||||
on_timeout: Box<dyn Fn() + Send + Sync>,
|
||||
timeout: Duration,
|
||||
) {
|
||||
// Track whether we are currently in a `signal = true`
|
||||
// window. We don't need the actual timestamp; we just need a
|
||||
// bounded wait that wakes on either the gate going false or
|
||||
// bounded wait that wakes on either the signal going false or
|
||||
// the timeout elapsing.
|
||||
info!(
|
||||
target: "chanora_audio",
|
||||
@@ -243,34 +284,34 @@ async fn run_watchdog(gate: AudioTransmitGate, timeout: Duration) {
|
||||
loop {
|
||||
// Wait for a transition.
|
||||
if rx.changed().await.is_err() {
|
||||
// Gate dropped; exit.
|
||||
// Source dropped; exit.
|
||||
return;
|
||||
}
|
||||
let is_active = *rx.borrow_and_update();
|
||||
if !is_active {
|
||||
// Either the user released, or someone else cleared
|
||||
// the gate. Either way the watchdog has nothing to
|
||||
// the signal. Either way the watchdog has nothing to
|
||||
// do until the next press.
|
||||
continue;
|
||||
}
|
||||
// The gate just went true. Wait for either a release
|
||||
// The signal just went true. Wait for either a release
|
||||
// transition or for the timeout to elapse.
|
||||
let release_or_timeout = tokio::select! {
|
||||
r = rx.changed() => r.map(|_| *rx.borrow_and_update()),
|
||||
_ = tokio::time::sleep(timeout) => {
|
||||
// Timeout — self-clear and emit the privacy-safe
|
||||
// diagnostic record. The field set here is
|
||||
// deliberately limited to the values the gen2
|
||||
// sanitizer permits (DEC-027): the active
|
||||
// capability descriptor is owned by the controller
|
||||
// and is not in scope here, so we emit only the
|
||||
// watchdog-relevant fact.
|
||||
// Timeout — invoke the registered callback and
|
||||
// emit the privacy-safe diagnostic record. The
|
||||
// field set here is deliberately limited to the
|
||||
// values the gen2 sanitizer permits (DEC-027): the
|
||||
// active capability descriptor is owned by the
|
||||
// controller and is not in scope here, so we emit
|
||||
// only the watchdog-relevant fact.
|
||||
warn!(
|
||||
target: "chanora_audio",
|
||||
timeout_secs = timeout.as_secs(),
|
||||
"missed-key-up watchdog fired; clearing transmit_active"
|
||||
"missed-key-up watchdog fired; clearing ptt_held"
|
||||
);
|
||||
gate.set(false);
|
||||
on_timeout();
|
||||
continue;
|
||||
}
|
||||
};
|
||||
@@ -382,4 +423,63 @@ mod tests {
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
assert!(!g.load(), "watchdog should fire on the second press as well");
|
||||
}
|
||||
|
||||
/// Continuous-mode regression: when the watchdog subscribes to
|
||||
/// the selector's `ptt_held` signal instead of the resolved
|
||||
/// gate, holding the gate at `true` for longer than the
|
||||
/// timeout (the natural Continuous-mode state) must NOT fire
|
||||
/// the watchdog. This is the bug the user reported as
|
||||
/// "Continuous transmission disabled after some time".
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn watchdog_on_signal_does_not_fire_when_ptt_held_stays_false() {
|
||||
use tokio::sync::watch;
|
||||
// ptt_held watch starts at false and never goes true; the
|
||||
// gate (simulating Continuous mode) is held at true the
|
||||
// whole test.
|
||||
let g = AudioTransmitGate::new(true);
|
||||
let (tx, rx) = watch::channel(false);
|
||||
let cleared = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||
let cleared_for_cb = cleared.clone();
|
||||
let _wd = MissedKeyUpWatchdog::spawn_on_signal(
|
||||
rx,
|
||||
move || cleared_for_cb.store(true, std::sync::atomic::Ordering::SeqCst),
|
||||
Duration::from_millis(80),
|
||||
);
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
// Simulate Continuous mode: gate stays true; ptt_held
|
||||
// stays false the whole window.
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
assert!(
|
||||
g.load(),
|
||||
"Continuous-mode gate is pinned true; watchdog must not touch it"
|
||||
);
|
||||
assert!(
|
||||
!cleared.load(std::sync::atomic::Ordering::SeqCst),
|
||||
"watchdog callback must not fire while ptt_held stays false"
|
||||
);
|
||||
drop(tx);
|
||||
}
|
||||
|
||||
/// Spawn-on-signal still fires when the signal itself is stuck
|
||||
/// at true past the timeout. This is the actual stuck-PTT case
|
||||
/// (e.g. OS suppressed the key-up while the app was minimised).
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn watchdog_on_signal_fires_when_signal_stays_true() {
|
||||
use tokio::sync::watch;
|
||||
let (tx, rx) = watch::channel(false);
|
||||
let cleared = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||
let cleared_for_cb = cleared.clone();
|
||||
let _wd = MissedKeyUpWatchdog::spawn_on_signal(
|
||||
rx,
|
||||
move || cleared_for_cb.store(true, std::sync::atomic::Ordering::SeqCst),
|
||||
Duration::from_millis(80),
|
||||
);
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
let _ = tx.send(true);
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
assert!(
|
||||
cleared.load(std::sync::atomic::Ordering::SeqCst),
|
||||
"watchdog callback must fire when signal stays true past the timeout"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,8 @@
|
||||
|
||||
use std::sync::atomic::{AtomicBool, AtomicU8, Ordering};
|
||||
|
||||
use tokio::sync::watch;
|
||||
|
||||
use crate::ptt::AudioTransmitGate;
|
||||
use crate::transmit_mode::TransmitMode;
|
||||
|
||||
@@ -32,6 +34,14 @@ pub struct TransmitModeSelector {
|
||||
in_channel: AtomicBool,
|
||||
hard_mute: AtomicBool,
|
||||
ptt_held: AtomicBool,
|
||||
/// Watch channel mirroring `ptt_held` transitions. The
|
||||
/// missed-key-up watchdog (SAD-079) subscribes here rather
|
||||
/// than to the gate, so it only fires when an actual PTT key
|
||||
/// press has been "stuck" for the timeout. In Continuous
|
||||
/// mode `ptt_held` is never written, so the watchdog never
|
||||
/// fires — that is the desired behaviour (Continuous is
|
||||
/// supposed to keep transmitting indefinitely).
|
||||
ptt_held_tx: watch::Sender<bool>,
|
||||
}
|
||||
|
||||
impl TransmitModeSelector {
|
||||
@@ -41,12 +51,14 @@ impl TransmitModeSelector {
|
||||
/// the first mutating call (which then writes the resolved
|
||||
/// value).
|
||||
pub fn new(gate: AudioTransmitGate) -> Self {
|
||||
let (ptt_held_tx, _rx) = watch::channel(false);
|
||||
Self {
|
||||
gate: std::sync::RwLock::new(gate),
|
||||
mode: AtomicU8::new(TransmitMode::default().as_u8()),
|
||||
in_channel: AtomicBool::new(false),
|
||||
hard_mute: AtomicBool::new(false),
|
||||
ptt_held: AtomicBool::new(false),
|
||||
ptt_held_tx,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,6 +112,10 @@ impl TransmitModeSelector {
|
||||
/// [`crate::ReleaseTailTimer`]).
|
||||
pub fn set_ptt_held(&self, v: bool) {
|
||||
self.ptt_held.store(v, Ordering::Relaxed);
|
||||
// Best-effort publish for the missed-key-up watchdog;
|
||||
// watch::Sender::send returns Err only when there are no
|
||||
// receivers, which is fine.
|
||||
let _ = self.ptt_held_tx.send(v);
|
||||
self.recompute();
|
||||
}
|
||||
|
||||
@@ -108,6 +124,15 @@ impl TransmitModeSelector {
|
||||
self.ptt_held.load(Ordering::Relaxed)
|
||||
}
|
||||
|
||||
/// Subscribe to `ptt_held` transitions. Used by the
|
||||
/// missed-key-up watchdog (SAD-079) so it fires on the actual
|
||||
/// PTT-key-down lifetime, not on the resolved `transmit_active`
|
||||
/// (which is supposed to stay `true` indefinitely in
|
||||
/// Continuous mode).
|
||||
pub fn subscribe_ptt_held(&self) -> watch::Receiver<bool> {
|
||||
self.ptt_held_tx.subscribe()
|
||||
}
|
||||
|
||||
/// Shared snapshot of the underlying gate. Provided for the
|
||||
/// audio engine's hot read path. Returns a clone so callers
|
||||
/// don't hold the internal lock.
|
||||
|
||||
Reference in New Issue
Block a user