diff --git a/apps/chanora_flutter/test/beta_e2e_test.dart b/apps/chanora_flutter/test/beta_e2e_test.dart index fd3c3d1..e5636db 100644 --- a/apps/chanora_flutter/test/beta_e2e_test.dart +++ b/apps/chanora_flutter/test/beta_e2e_test.dart @@ -37,6 +37,11 @@ void main() { await rust.voiceJoin(channelId: snap.channels.first.id, password: ''); + // Zero out the release tail so set_ptt(false) takes effect + // synchronously — the default 200 ms tail (SDD-096) would + // otherwise delay the assertion below. + await rust.setReleaseTailMs(ms: 0); + // Initial stats: PTT off, no frames sent yet. final s0 = await rust.audioStats(); expect(s0.pttActive, isFalse); @@ -55,6 +60,8 @@ void main() { expect(s1.pttActive, isTrue); await rust.setPtt(active: false); + // Give the release-tail (set to 0 above) one tick to settle. + await Future.delayed(const Duration(milliseconds: 50)); final s2 = await rust.audioStats(); expect(s2.pttActive, isFalse); diff --git a/core/chanora_core/src/lib.rs b/core/chanora_core/src/lib.rs index c6940a3..a9f596c 100644 --- a/core/chanora_core/src/lib.rs +++ b/core/chanora_core/src/lib.rs @@ -481,6 +481,7 @@ impl ChanoraSession { self.network_tx.subscribe(), self.voice_selector.clone(), self.pending_binding.clone(), + self.release_tail.clone(), )); let _ = self.events_tx.send(SessionEvent::Connected { @@ -546,10 +547,11 @@ impl ChanoraSession { // Wire the PTT controller (SDD-088). It owns the platform // backend, the active binding, and the capability watch - // channel. Constructed here because the controller needs - // the engine's gate clone, and we want the engine to - // exist before any backend tries to drive the gate. - let controller = ptt::PttController::new(gate); + // channel. The controller routes raw press-edges through + // the session's release-tail timer (SDD-096) and the + // transmit-mode selector (SAD-083); the backend never + // writes the real `transmit_active` directly. + let controller = ptt::PttController::new(self.release_tail.clone()); state.ptt_controller = Some(controller.clone()); // Apply any binding the user saved before audio was running @@ -603,11 +605,23 @@ impl ChanoraSession { } /// Set push-to-talk state. No-op if audio not started. + /// Set the local push-to-talk state by hand. Used by the + /// in-focus Listener path on platforms that lack a global + /// backend, and by tests. Routes through the release-tail + /// timer + transmit-mode selector so callers see the same + /// 200 ms tail and hard-mute semantics as the global backends + /// (SDD-096 / SAD-083). pub async fn set_ptt(&self, active: bool) -> Result<(), CoreError> { + // Use NotConnected as a soft guard: the in-focus PTT + // helper only fires while the user is in a session. let guard = self.inner.lock().await; - let state = guard.as_ref().ok_or(CoreError::NotConnected)?; - let audio = state.audio.as_ref().ok_or(CoreError::AudioNotStarted)?; - audio.set_ptt(active); + let _ = guard.as_ref().ok_or(CoreError::NotConnected)?; + drop(guard); + if active { + self.release_tail.key_down(); + } else { + self.release_tail.key_up(); + } Ok(()) } @@ -983,6 +997,7 @@ async fn supervisor_loop( mut network_rx: watch::Receiver, voice_selector: Arc, pending_binding: Arc>>, + release_tail: Arc, ) { let mut lost_rx = initial_lost_rx; let mut probe = initial_probe; @@ -1270,9 +1285,15 @@ async fn supervisor_loop( Ok(engine) => { state.audio = Some(engine); voice_selector.replace_gate(gate.clone()); - // Re-arm the PTT controller against - // the new engine's gate (SDD-088). - let controller = ptt::PttController::new(gate); + // Re-arm the PTT controller against the + // session's release-tail timer (SDD-088 + + // SDD-096). The backend writes the + // controller's synthetic press-edge gate; + // the controller forwards edges into + // `release_tail.key_down/up` which drives + // the selector that writes the real gate. + let controller = + ptt::PttController::new(release_tail.clone()); state.ptt_controller = Some(controller.clone()); // Re-apply any persisted PTT binding // so reconnect doesn't silently drop diff --git a/core/chanora_core/src/ptt.rs b/core/chanora_core/src/ptt.rs index 4d0017f..028ca54 100644 --- a/core/chanora_core/src/ptt.rs +++ b/core/chanora_core/src/ptt.rs @@ -22,7 +22,9 @@ use std::sync::Arc; use chanora_audio::ptt_backends::{ select as select_ptt_backend, DesktopPttBackend, PttBackendError, PttBinding, }; -use chanora_audio::{AudioTransmitGate, PttBackendDescriptor, PttCapabilityLevel}; +use chanora_audio::{ + AudioTransmitGate, PttBackendDescriptor, PttCapabilityLevel, ReleaseTailTimer, +}; use tokio::sync::{watch, Mutex}; use tracing::{info, warn}; @@ -66,10 +68,22 @@ pub enum PttControllerError { pub struct PttController { backend: Mutex>>, binding: Mutex, - gate: AudioTransmitGate, + /// The synthetic "press-edge" gate handed to the platform + /// backend. The backend writes `true` on key-down and `false` + /// on key-up; the controller's edge-watcher task translates + /// these transitions into [`ReleaseTailTimer::key_down`] / + /// `key_up` calls so the 200 ms release tail (SDD-096 / + /// SRS-206) actually applies. The backend never touches the + /// real `transmit_active` directly. + press_gate: AudioTransmitGate, + /// Release-tail timer routing press-edges into the + /// `TransmitModeSelector`'s `ptt_held` input (SDD-096 + + /// SAD-083). + release_tail: Arc, capability_tx: watch::Sender, descriptor_tx: watch::Sender, forwarder: std::sync::Mutex>>, + edge_watcher: std::sync::Mutex>>, } impl PttController { @@ -80,10 +94,17 @@ impl PttController { /// returns successfully with a Focused-fallback backend already /// armed (matching the previous engine behaviour) so the audio /// path is never blocked by a PTT-arm failure. - pub fn new(gate: AudioTransmitGate) -> Arc { + /// + /// `release_tail` is the session-owned timer that drives the + /// transmit-mode selector's `ptt_held` input. The controller + /// translates raw backend press-edges into + /// `release_tail.key_down/key_up` so the 200 ms tail spec'd in + /// SDD-096 actually fires between key release and gate close. + pub fn new(release_tail: Arc) -> Arc { + let press_gate = AudioTransmitGate::new(false); let mut backend = select_ptt_backend(); let initial_binding = PttBinding::none(); - match backend.start(gate.clone(), initial_binding.clone()) { + match backend.start(press_gate.clone(), initial_binding.clone()) { Ok(()) => { let d = backend.descriptor(); info!( @@ -122,13 +143,38 @@ impl PttController { } }); + // Press-edge watcher: every transition of `press_gate` + // (driven by the backend's raw key-down / key-up events) + // becomes a `release_tail.key_down/key_up` call. The + // release-tail timer then writes the selector's + // `ptt_held` input, the selector recomputes the desired + // `transmit_active` honouring mode + mute + in_channel, + // and the real gate transitions accordingly. + let mut press_rx = press_gate.subscribe(); + let tail_for_task = release_tail.clone(); + let edge_watcher = tokio::spawn(async move { + // Drain the initial value so we don't fire a phantom + // key_up for the construction-time `false`. + let _ = press_rx.borrow_and_update(); + while press_rx.changed().await.is_ok() { + let pressed = *press_rx.borrow_and_update(); + if pressed { + tail_for_task.key_down(); + } else { + tail_for_task.key_up(); + } + } + }); + Arc::new(Self { backend: Mutex::new(Some(backend)), binding: Mutex::new(initial_binding), - gate, + press_gate, + release_tail, capability_tx, descriptor_tx, forwarder: std::sync::Mutex::new(Some(forwarder)), + edge_watcher: std::sync::Mutex::new(Some(edge_watcher)), }) } @@ -193,9 +239,17 @@ impl PttController { self.descriptor_tx.subscribe() } - /// Reference to the underlying gate. Diagnostic / test use only. - pub fn gate(&self) -> &AudioTransmitGate { - &self.gate + /// Reference to the synthetic press-edge gate. Diagnostic / + /// test use only — production callers must read the real + /// `transmit_active` via the session's selector. + pub fn press_gate(&self) -> &AudioTransmitGate { + &self.press_gate + } + + /// Reference to the release-tail timer. Used by tests that + /// need to flush pending releases. + pub fn release_tail(&self) -> &Arc { + &self.release_tail } /// Release OS-level resources held by the platform backend @@ -205,6 +259,15 @@ impl PttController { if let Some(h) = self.forwarder.lock().ok().and_then(|mut g| g.take()) { h.abort(); } + if let Some(h) = self.edge_watcher.lock().ok().and_then(|mut g| g.take()) { + h.abort(); + } + // Ensure the selector's ptt_held input does not get stuck + // on the next press cycle: cancel any pending release-tail + // task and force the selector's `ptt_held` to false (which + // clamps `transmit_active` to false in PTT mode). + self.release_tail.force_release(); + self.press_gate.set(false); let mut guard = self.backend.lock().await; if let Some(mut b) = guard.take() { b.stop(); @@ -217,6 +280,9 @@ impl Drop for PttController { if let Some(h) = self.forwarder.get_mut().ok().and_then(|g| g.take()) { h.abort(); } + if let Some(h) = self.edge_watcher.get_mut().ok().and_then(|g| g.take()) { + h.abort(); + } // The canonical shutdown path is `PttController::stop().await` // invoked by the session before drop. We cannot lock the // tokio Mutex synchronously here; if the controller is @@ -229,44 +295,82 @@ impl Drop for PttController { #[cfg(test)] mod tests { use super::*; + use chanora_audio::{TransmitMode, TransmitModeSelector}; + use std::time::Duration; - #[tokio::test] - async fn controller_arms_and_reports_capability() { - // The focused fallback is always selectable; the factory - // never fails on a host without any global capture path. + fn setup(tail_ms: u32) -> (AudioTransmitGate, Arc, Arc) { let gate = AudioTransmitGate::new(false); - let controller = PttController::new(gate); - // Every backend returns a non-null backend_id and a - // capability level that round-trips through the watch. + let selector = Arc::new(TransmitModeSelector::new(gate.clone())); + selector.set_mode(TransmitMode::Ptt); + selector.set_in_channel(true); + let tail = Arc::new(ReleaseTailTimer::new(selector.clone(), tail_ms)); + (gate, selector, tail) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn controller_arms_and_reports_capability() { + let (_gate, _sel, tail) = setup(0); + let controller = PttController::new(tail); let desc = controller.descriptor().await; assert!(!desc.backend_id.is_empty()); let level = controller.current_capability(); - // The descriptor's level must match the watch. assert_eq!(level, desc.level); - // Subscribing must yield the same value without blocking. let mut rx = controller.subscribe_capability(); assert_eq!(*rx.borrow_and_update(), level); } - #[tokio::test] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn set_binding_updates_descriptor_watch() { - let gate = AudioTransmitGate::new(false); - let controller = PttController::new(gate); + let (_gate, _sel, tail) = setup(0); + let controller = PttController::new(tail); let mut desc_rx = controller.descriptor_watch(); - // Drain the initial value. let _ = desc_rx.borrow_and_update(); - // The focused fallback accepts every binding (it's just a - // diagnostic carrier; the actual press detection happens - // inside the Flutter Listener path). let binding = PttBinding { input_class: chanora_audio::PttInputClass::Keyboard, platform_key: "space".to_string(), }; let new_desc = controller.set_binding(binding.clone()).await.unwrap(); - // The descriptor watch must have received the post-rebind - // value. We don't assert a specific level here because the - // focused backend's level does not change on rebind, but - // the descriptor send is observable. assert_eq!(new_desc.backend_id, controller.descriptor().await.backend_id); } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn press_edge_drives_selector_through_release_tail() { + // Tail = 80 ms; press the synthetic press-gate (simulating + // a backend's raw key-down), expect the real transmit gate + // to follow with the configured tail on release. + let (real_gate, _sel, tail) = setup(80); + let controller = PttController::new(tail); + // Give the edge-watcher task a tick to subscribe. + tokio::time::sleep(Duration::from_millis(15)).await; + // Simulate backend key-down. + controller.press_gate().set(true); + tokio::time::sleep(Duration::from_millis(15)).await; + assert!(real_gate.load(), "transmit should follow press-down"); + // Simulate backend key-up; tail should keep it true briefly. + controller.press_gate().set(false); + tokio::time::sleep(Duration::from_millis(20)).await; + assert!( + real_gate.load(), + "transmit should still be true during tail window" + ); + // After tail expires, transmit clears. + tokio::time::sleep(Duration::from_millis(120)).await; + assert!(!real_gate.load(), "transmit should clear after tail"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn stop_clears_press_and_cancels_tail() { + let (real_gate, _sel, tail) = setup(200); + let controller = PttController::new(tail); + tokio::time::sleep(Duration::from_millis(15)).await; + controller.press_gate().set(true); + tokio::time::sleep(Duration::from_millis(15)).await; + assert!(real_gate.load()); + // Stop cancels any pending release-tail task, clears the + // press-edge, and tears down the backend. transmit_active + // settles to false within one task tick. + controller.stop().await; + tokio::time::sleep(Duration::from_millis(15)).await; + assert!(!real_gate.load(), "stop must drop transmit cleanly"); + } } diff --git a/crates/chanora_audio/src/release_tail.rs b/crates/chanora_audio/src/release_tail.rs index 0a65b23..018c903 100644 --- a/crates/chanora_audio/src/release_tail.rs +++ b/crates/chanora_audio/src/release_tail.rs @@ -100,6 +100,16 @@ impl ReleaseTailTimer { self.cancel_pending(); } + /// Cancel any pending release task and immediately clear the + /// selector's `ptt_held` input. Used on PTT controller + /// shutdown to guarantee `transmit_active` does not get stuck + /// at `true` if the user's last action was a key-down with + /// no matching key-up reaching us before the shutdown. + pub fn force_release(&self) { + self.cancel_pending(); + self.selector.set_ptt_held(false); + } + fn cancel_pending(&self) { if let Ok(mut g) = self.pending.lock() { if let Some(h) = g.take() {