From b051974e4218e868556db0c0778b99ec18d8db82 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Mon, 7 Sep 2026 21:25:12 +0200 Subject: [PATCH 1/2] refactor: derive display levels and trace age from device capabilities Keep existing IQ profiles at -120 to 0 dBFS with a 500 ms trace-age limit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/app/builder/boot.rs | 70 +++++++++++++++-- src/app/input/core.rs | 103 ++++++++++++++++++++++++-- src/hardware/mod.rs | 5 +- src/hardware/native/hackrf/mod.rs | 4 + src/hardware/native/rtlsdr/mod.rs | 4 + src/hardware/soapy/caps.rs | 9 +++ src/hardware/traits.rs | 19 +++++ src/state/fixture.rs | 5 +- src/state/waterfall.rs | 2 + src/ui/panel.rs | 59 +++++++++++---- src/ui/panels/core/spectrum/axes.rs | 5 +- src/ui/panels/core/spectrum/labels.rs | 6 +- src/ui/panels/core/spectrum/mod.rs | 21 ++++++ src/ui/panels/core/waterfall/axes.rs | 45 +++++++++-- src/ui/panels/core/waterfall/cells.rs | 44 +++++++++-- src/ui/panels/core/waterfall/mod.rs | 32 ++++++-- 16 files changed, 379 insertions(+), 54 deletions(-) diff --git a/src/app/builder/boot.rs b/src/app/builder/boot.rs index ee86e23b..98e37518 100644 --- a/src/app/builder/boot.rs +++ b/src/app/builder/boot.rs @@ -361,8 +361,8 @@ pub(super) fn initial_metrics(cfg: &AppConfig, boot: Boot) -> SdrMetrics { observer, spectrum: SpectrumState { step_hz: 100_000, - y_min: -120.0, - y_max: 0.0, + y_min: caps.level_min_db, + y_max: caps.level_max_db, hold: None, cursor_freq: None, markers, @@ -372,10 +372,15 @@ pub(super) fn initial_metrics(cfg: &AppConfig, boot: Boot) -> SdrMetrics { // Clamped, not trusted: `save_config` writes the live buffer depth back, // so a config written before the floor existed carries a value too small // to fill a full-height waterfall. See [`WATERFALL_MIN_ROWS`]. - waterfall: WaterfallState::new( - cfg.display.waterfall_max_rows.max(WATERFALL_MIN_ROWS), - cfg.display.waterfall_palette, - ), + waterfall: { + let mut waterfall = WaterfallState::new( + cfg.display.waterfall_max_rows.max(WATERFALL_MIN_ROWS), + cfg.display.waterfall_palette, + ); + waterfall.db_min = caps.level_min_db; + waterfall.db_max = caps.level_max_db; + waterfall + }, system: SystemState { board_name: Arc::from(identity.board_name.as_str()), serial: Arc::from(identity.serial.as_str()), @@ -576,6 +581,59 @@ mod tests { } } + #[test] + fn iq_devices_keep_the_existing_startup_defaults() { + let cfg = AppConfig::default(); + for caps in [ + hardware::native::hackrf::caps(), + hardware::native::rtlsdr::observer_caps(), + ] { + let tuning = resolve_tuning(&cfg.radio, &caps); + let metrics = initial_metrics( + &cfg, + Boot::normal( + &cfg, + Arc::new(caps), + tuning, + &hardware::DeviceInfo::default(), + ), + ); + assert!(!metrics.radio.rx_enabled); + assert!(!metrics.radio.hw_streaming); + assert_eq!(metrics.spectrum.y_min, -120.0); + assert_eq!(metrics.spectrum.y_max, 0.0); + assert_eq!(metrics.waterfall.db_min, -120.0); + assert_eq!(metrics.waterfall.db_max, 0.0); + assert_eq!(metrics.caps.level_unit, hardware::LevelUnit::Dbfs); + assert_eq!(metrics.caps.level_unit.label(), "dBFS"); + assert_eq!(metrics.caps.trace_stale_ms, 500); + } + } + + #[test] + fn startup_uses_the_device_level_axis() { + let cfg = AppConfig::default(); + let mut caps = hardware::native::hackrf::caps(); + caps.level_min_db = -110.0; + caps.level_max_db = -10.0; + let tuning = resolve_tuning(&cfg.radio, &caps); + let metrics = initial_metrics( + &cfg, + Boot::normal( + &cfg, + Arc::new(caps), + tuning, + &hardware::DeviceInfo::default(), + ), + ); + assert!(!metrics.radio.rx_enabled); + assert!(!metrics.radio.hw_streaming); + assert_eq!(metrics.spectrum.y_min, -110.0); + assert_eq!(metrics.spectrum.y_max, -10.0); + assert_eq!(metrics.waterfall.db_min, -110.0); + assert_eq!(metrics.waterfall.db_max, -10.0); + } + /// A config that asks for less waterfall history than a full-height panel /// needs is raised, not honoured. /// diff --git a/src/app/input/core.rs b/src/app/input/core.rs index ea3b1b80..5bd9f8b0 100644 --- a/src/app/input/core.rs +++ b/src/app/input/core.rs @@ -113,14 +113,16 @@ pub(super) fn spectrum(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { let new_min = (m.spectrum.y_min + 10.0).min(m.spectrum.y_max - 20.0); m.spectrum.y_min = new_min; let ymax = m.spectrum.y_max; - m.push_log(format!("Zoom: {:.0}…{:.0} dBFS", new_min, ymax)); + let unit = m.caps.level_unit.label(); + m.push_log(format!("Zoom: {new_min:.0}\u{2026}{ymax:.0} {unit}")); } KeyCode::Down => { let mut m = metrics(state); - let new_min = (m.spectrum.y_min - 10.0).max(-120.0); + let new_min = (m.spectrum.y_min - 10.0).max(m.caps.level_min_db); m.spectrum.y_min = new_min; let ymax = m.spectrum.y_max; - m.push_log(format!("Zoom: {:.0}…{:.0} dBFS", new_min, ymax)); + let unit = m.caps.level_unit.label(); + m.push_log(format!("Zoom: {new_min:.0}\u{2026}{ymax:.0} {unit}")); } KeyCode::Char('j') => { let mut m = metrics(state); @@ -212,6 +214,7 @@ pub(super) fn spectrum(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { } KeyAction::Continue } + // ── Waterfall focus keys ────────────────────────────────────────────────────── pub(super) fn waterfall(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { @@ -219,15 +222,23 @@ pub(super) fn waterfall(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { match key.code { KeyCode::Up => { let mut m = metrics(state); - let new_min = (m.waterfall.db_min + 10.0).min(-20.0); + let new_min = (m.waterfall.db_min + 10.0).min(m.waterfall.db_max - 20.0); m.waterfall.db_min = new_min; - m.push_log(format!("Waterfall zoom: {:.0}…0 dBFS", new_min)); + let max = m.waterfall.db_max; + let unit = m.caps.level_unit.label(); + m.push_log(format!( + "Waterfall zoom: {new_min:.0}\u{2026}{max:.0} {unit}" + )); } KeyCode::Down => { let mut m = metrics(state); - let new_min = (m.waterfall.db_min - 10.0).max(-120.0); + let new_min = (m.waterfall.db_min - 10.0).max(m.caps.level_min_db); m.waterfall.db_min = new_min; - m.push_log(format!("Waterfall zoom: {:.0}…0 dBFS", new_min)); + let max = m.waterfall.db_max; + let unit = m.caps.level_unit.label(); + m.push_log(format!( + "Waterfall zoom: {new_min:.0}\u{2026}{max:.0} {unit}" + )); } KeyCode::Char('[') => { let mut m = metrics(state); @@ -305,6 +316,11 @@ pub(super) fn waterfall(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { #[cfg(test)] mod tests { use super::*; + use std::collections::HashMap; + use std::sync::{Arc, Mutex}; + + use crate::state::SdrMetrics; + use crate::ui::{LayoutEngine, PanelRegistry}; #[test] fn peak_jump_uses_the_captured_fft_frequency_after_retuning() { @@ -340,4 +356,77 @@ mod tests { 1_000_000 ); } + + #[test] + fn level_zoom_uses_device_bounds_and_preserves_iq_log_text() { + for (min, max) in [(-120.0, 0.0), (-110.0, -10.0)] { + for is_waterfall in [false, true] { + let mut m = SdrMetrics::fixture(); + Arc::make_mut(&mut m.caps).level_min_db = min; + Arc::make_mut(&mut m.caps).level_max_db = max; + m.spectrum.y_min = min; + m.spectrum.y_max = max; + m.waterfall.db_min = min; + m.waterfall.db_max = max; + let state = Arc::new(Mutex::new(m)); + let mut engine = LayoutEngine::new( + crate::config::LayoutConfig::default_config(), + PanelRegistry::new(), + ); + let mut show_footer = true; + let focus_keys = HashMap::new(); + let mut ctx = InputCtx { + state: &state, + device: None, + engine: &mut engine, + show_footer: &mut show_footer, + focus_keys: &focus_keys, + }; + let prefix = if is_waterfall { + "Waterfall zoom" + } else { + "Zoom" + }; + let mut press = |code| { + let key = KeyEvent::new(code, crossterm::event::KeyModifiers::NONE); + if is_waterfall { + waterfall(key, &mut ctx); + } else { + spectrum(key, &mut ctx); + } + }; + press(KeyCode::Up); + assert_eq!( + metrics(&state).ui.log.back().unwrap().text.as_ref(), + format!("{prefix}: {:.0}\u{2026}{max:.0} dBFS", min + 10.0) + ); + for _ in 0..20 { + press(KeyCode::Up); + } + { + let m = metrics(&state); + let floor = if is_waterfall { + m.waterfall.db_min + } else { + m.spectrum.y_min + }; + assert_eq!(floor, max - 20.0); + } + for _ in 0..20 { + press(KeyCode::Down); + } + let m = metrics(&state); + let floor = if is_waterfall { + m.waterfall.db_min + } else { + m.spectrum.y_min + }; + assert_eq!(floor, min); + assert_eq!( + m.ui.log.back().unwrap().text.as_ref(), + format!("{prefix}: {min:.0}\u{2026}{max:.0} dBFS") + ); + } + } + } } diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index 504e2523..5969df80 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -30,6 +30,7 @@ mod traits; pub use discovery::{list_all_devices, open_device, DeviceKind, DeviceListing}; pub use traits::{ - Boost, DeliveryModel, DeviceCapabilities, DeviceInfo, FeedHealth, GainModel, RxContext, - SampleFormat, SampleGeometry, SdrDevice, SoftwareStack, StageSpec, StreamBlock, + Boost, DeliveryModel, DeviceCapabilities, DeviceInfo, FeedHealth, GainModel, LevelUnit, + RxContext, SampleFormat, SampleGeometry, SdrDevice, SoftwareStack, StageSpec, StreamBlock, + IQ_TRACE_STALE_MS, }; diff --git a/src/hardware/native/hackrf/mod.rs b/src/hardware/native/hackrf/mod.rs index 8e59847f..5c4ee9f4 100644 --- a/src/hardware/native/hackrf/mod.rs +++ b/src/hardware/native/hackrf/mod.rs @@ -332,6 +332,10 @@ pub fn gain_model() -> GainModel { /// HackRF One capability descriptor - also used as the observer-mode default. pub fn caps() -> DeviceCapabilities { DeviceCapabilities { + level_unit: crate::hardware::LevelUnit::Dbfs, + level_min_db: -120.0, + level_max_db: 0.0, + trace_stale_ms: crate::hardware::IQ_TRACE_STALE_MS, freq_min_hz: 1_000_000, freq_max_hz: 6_000_000_000, sample_rate_min_hz: 2_000_000.0, diff --git a/src/hardware/native/rtlsdr/mod.rs b/src/hardware/native/rtlsdr/mod.rs index 347662d3..b53a7587 100644 --- a/src/hardware/native/rtlsdr/mod.rs +++ b/src/hardware/native/rtlsdr/mod.rs @@ -364,6 +364,10 @@ fn rtl_caps(tuner: c_int, gains_tenths: &[i32]) -> DeviceCapabilities { } DeviceCapabilities { + level_unit: crate::hardware::LevelUnit::Dbfs, + level_min_db: -120.0, + level_max_db: 0.0, + trace_stale_ms: crate::hardware::IQ_TRACE_STALE_MS, freq_min_hz, freq_max_hz, // RTL-SDR's usable upper band is 900_001..=3_200_000 Hz (the lower diff --git a/src/hardware/soapy/caps.rs b/src/hardware/soapy/caps.rs index d61e4345..d6d84400 100644 --- a/src/hardware/soapy/caps.rs +++ b/src/hardware/soapy/caps.rs @@ -163,6 +163,10 @@ pub fn capabilities(a: &DriverAnswers) -> Result { let boost = element_boost.or(a.has_gain_mode.then_some(Boost::GainMode)); let caps = DeviceCapabilities { + level_unit: crate::hardware::LevelUnit::Dbfs, + level_min_db: -120.0, + level_max_db: 0.0, + trace_stale_ms: crate::hardware::IQ_TRACE_STALE_MS, freq_min_hz: freq_min.max(0.0) as u64, freq_max_hz: freq_max.max(0.0) as u64, sample_rate_min_hz: rate_min, @@ -330,6 +334,11 @@ mod tests { #[test] fn a_soapy_hackrf_comes_out_the_way_the_probe_describes_it() { let c = capabilities(&soapy_hackrf()).unwrap().caps; + assert_eq!(c.level_unit, crate::hardware::LevelUnit::Dbfs); + assert_eq!(c.level_unit.label(), "dBFS"); + assert_eq!(c.level_min_db, -120.0); + assert_eq!(c.level_max_db, 0.0); + assert_eq!(c.trace_stale_ms, 500); assert_eq!(c.freq_min_hz, 0); assert_eq!(c.freq_max_hz, 7_250_000_000); assert_eq!(c.sample_rate_min_hz, 1e6); diff --git a/src/hardware/traits.rs b/src/hardware/traits.rs index bd0f04ea..5cb9a595 100644 --- a/src/hardware/traits.rs +++ b/src/hardware/traits.rs @@ -12,6 +12,21 @@ use std::sync::{Arc, Mutex}; use crate::state::SdrMetrics; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LevelUnit { + Dbfs, +} + +pub const IQ_TRACE_STALE_MS: u128 = 500; + +impl LevelUnit { + pub fn label(self) -> &'static str { + match self { + Self::Dbfs => "dBFS", + } + } +} + /// How raw USB bytes encode each I/Q component. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum SampleFormat { @@ -616,6 +631,10 @@ pub enum DeliveryModel { /// truth for every clamp, default, and UI capability check. Built once at open. #[derive(Clone, Debug)] pub struct DeviceCapabilities { + pub level_unit: LevelUnit, + pub level_min_db: f32, + pub level_max_db: f32, + pub trace_stale_ms: u128, pub freq_min_hz: u64, pub freq_max_hz: u64, pub sample_rate_min_hz: f64, diff --git a/src/state/fixture.rs b/src/state/fixture.rs index 677d5b59..c6e62c4b 100644 --- a/src/state/fixture.rs +++ b/src/state/fixture.rs @@ -160,12 +160,11 @@ impl SdrMetrics { self } - /// Age the newest FFT frame past [`crate::ui::panel::FFT_STALE_MS`], so the - /// staleness paths can be rendered without a test sleeping. + /// Age the newest FFT frame past the IQ trace limit pub(crate) fn with_stale_fft(mut self) -> Self { if let Some(fr) = self.waterfall.last_fft.as_mut() { fr.timestamp = Instant::now() - - std::time::Duration::from_millis(crate::ui::panel::FFT_STALE_MS as u64 + 50); + - std::time::Duration::from_millis(crate::hardware::IQ_TRACE_STALE_MS as u64 + 50); } self } diff --git a/src/state/waterfall.rs b/src/state/waterfall.rs index 3fbb1358..eb32805d 100644 --- a/src/state/waterfall.rs +++ b/src/state/waterfall.rs @@ -240,6 +240,7 @@ impl WaterfallBuffer { #[derive(Clone)] pub struct WaterfallState { pub db_min: f32, + pub db_max: f32, pub scroll_offset: usize, pub cursor_freq: Option, pub hz_zoom: u32, @@ -253,6 +254,7 @@ impl WaterfallState { pub fn new(max_rows: usize, palette: crate::palette::WaterfallPalette) -> Self { Self { db_min: -120.0, + db_max: 0.0, scroll_offset: 0, cursor_freq: None, hz_zoom: 1, diff --git a/src/ui/panel.rs b/src/ui/panel.rs index 2efce55b..2e697e9f 100644 --- a/src/ui/panel.rs +++ b/src/ui/panel.rs @@ -26,17 +26,13 @@ pub enum Staleness { /// Stale whenever the radio is not streaming. For anything read from /// hardware counters: timing, drops, gain staging, IQ balance. NotStreaming, - /// Stale when the newest FFT frame has aged past [`FFT_STALE_MS`], or there - /// is no frame yet. For anything derived from the spectrum. + /// Mark spectrum readings stale when the frame exceeds the device's trace-age limit + /// Missing frames are stale FftAge, /// Never stale. For panels that show configuration rather than measurement. Never, } -/// How old the newest FFT frame may get before an [`Staleness::FftAge`] panel -/// calls its readings stale. Shared with `widgets::micro_common::fft_stale`. -pub const FFT_STALE_MS: u128 = 500; - impl Staleness { /// Resolve the rule against a metrics snapshot. pub fn resolve(self, state: &SdrMetrics) -> bool { @@ -47,16 +43,19 @@ impl Staleness { .last_fft .as_ref() .map(|fr| fr.timestamp.elapsed().as_millis()), + state.caps.trace_stale_ms, ) } /// The rule itself, on plain inputs: `fft_age_ms` is `None` when no frame has /// arrived yet. Split out from [`resolve`](Self::resolve) so the decision can /// be tested without building a whole metrics snapshot. - fn decide(self, streaming: bool, fft_age_ms: Option) -> bool { + fn decide(self, streaming: bool, fft_age_ms: Option, trace_stale_ms: u128) -> bool { match self { Staleness::NotStreaming => !streaming, - Staleness::FftAge => fft_age_ms.map(|ms| ms > FFT_STALE_MS).unwrap_or(true), + Staleness::FftAge => fft_age_ms + .map(|milliseconds| milliseconds > trace_stale_ms) + .unwrap_or(true), Staleness::Never => false, } } @@ -373,21 +372,51 @@ mod tests { #[test] fn staleness_rules_are_independent_of_each_other() { + let stale_ms = crate::hardware::IQ_TRACE_STALE_MS; // A dead radio staleness NotStreaming, and nothing else. - assert!(Staleness::NotStreaming.decide(false, Some(0))); + assert!(Staleness::NotStreaming.decide(false, Some(0), stale_ms)); assert!( - !Staleness::FftAge.decide(false, Some(0)), + !Staleness::FftAge.decide(false, Some(0), stale_ms), "fresh frame, dead radio → live" ); - assert!(!Staleness::Never.decide(false, None), "Never means never"); + assert!( + !Staleness::Never.decide(false, None, stale_ms), + "Never means never" + ); // A streaming radio whose FFT has dried up staleness only FftAge. - assert!(!Staleness::NotStreaming.decide(true, None)); - assert!(Staleness::FftAge.decide(true, None), "no frame yet → stale"); - assert!(Staleness::FftAge.decide(true, Some(FFT_STALE_MS + 1))); + assert!(!Staleness::NotStreaming.decide(true, None, stale_ms)); assert!( - !Staleness::FftAge.decide(true, Some(FFT_STALE_MS)), + Staleness::FftAge.decide(true, None, stale_ms), + "no frame yet → stale" + ); + assert!(Staleness::FftAge.decide(true, Some(stale_ms + 1), stale_ms)); + assert!( + !Staleness::FftAge.decide(true, Some(stale_ms), stale_ms), "the threshold itself is live" ); } + + #[test] + fn trace_staleness_uses_the_device_limit() { + let mut state = SdrMetrics::fixture().streaming().with_carrier(0.0, 20.0); + std::sync::Arc::make_mut(&mut state.caps).trace_stale_ms = 60_000; + state.waterfall.last_fft.as_mut().unwrap().timestamp = + std::time::Instant::now() - std::time::Duration::from_secs(1); + assert!(!Staleness::FftAge.resolve(&state)); + + std::sync::Arc::make_mut(&mut state.caps).trace_stale_ms = 100; + assert!(Staleness::FftAge.resolve(&state)); + + state.waterfall.last_fft = None; + assert!(Staleness::FftAge.resolve(&state)); + } + + #[test] + fn trace_age_threshold_is_inclusive_for_each_device() { + for limit in [0, 100, 500, 60_000] { + assert!(!Staleness::FftAge.decide(false, Some(limit), limit)); + assert!(Staleness::FftAge.decide(true, Some(limit + 1), limit)); + } + } } diff --git a/src/ui/panels/core/spectrum/axes.rs b/src/ui/panels/core/spectrum/axes.rs index 5ba614d3..fdbe4605 100644 --- a/src/ui/panels/core/spectrum/axes.rs +++ b/src/ui/panels/core/spectrum/axes.rs @@ -49,12 +49,15 @@ pub(super) fn tuning( freq_hz: u64, step_hz: u64, cursor: Option<(f64, f32)>, + unit: &str, theme: &crate::Theme, ) { let step_str = fmt_spectrum_step(step_hz); let freq_str = format!(" {:.3} MHz ", freq_hz as f64 / 1_000_000.0); let readout = match cursor { - Some((mhz, pwr)) => format!(" cur: {mhz:.3} MHz {pwr:.1} dBFS step {step_str} J/K"), + Some((mhz, pwr)) => { + format!(" cur: {mhz:.3} MHz {pwr:.1} {unit} step {step_str} J/K") + } None => format!(" step {step_str} [/]"), }; diff --git a/src/ui/panels/core/spectrum/labels.rs b/src/ui/panels/core/spectrum/labels.rs index 07795d4a..a856bdca 100644 --- a/src/ui/panels/core/spectrum/labels.rs +++ b/src/ui/panels/core/spectrum/labels.rs @@ -285,7 +285,11 @@ pub(super) fn signal_annotations( // Noise-floor label - near the left edge, on the row the line actually sits. let nf_row = ((vert.frac_down_to(noise_floor) * (ch - 1) as f32) as u16).min(ch.saturating_sub(2)); - let nf_label = format!("noise floor {:.0} dBFS", noise_floor); + let nf_label = format!( + "noise floor {:.0} {}", + noise_floor, + state.caps.level_unit.label() + ); let nf_lw = nf_label.chars().count() as u16; if nf_lw < area.width { f.render_widget( diff --git a/src/ui/panels/core/spectrum/mod.rs b/src/ui/panels/core/spectrum/mod.rs index b491b8cc..b1ce8e8e 100644 --- a/src/ui/panels/core/spectrum/mod.rs +++ b/src/ui/panels/core/spectrum/mod.rs @@ -368,6 +368,7 @@ fn draw_instrument( state.radio.frequency, state.spectrum.step_hz, cursor, + state.caps.level_unit.label(), theme, ); } @@ -431,6 +432,26 @@ mod zoom_tests { SdrMetrics::fixture().streaming().with_carrier(0.0, 40.0) } + #[test] + fn level_axis_and_noise_floor_keep_the_iq_readout() { + let mut state = tuned(); + state.ui.active_preset = "lab_signal".into(); + for (min, max) in [(-120.0, 0.0), (-110.0, -10.0)] { + state.spectrum.y_min = min; + state.spectrum.y_max = max; + let rows = draw(SpectrumPanel, 100, 20, &state); + let gutter: String = rows + .iter() + .skip(1) + .take(18) + .flat_map(|row| row.chars().skip(1).take(6)) + .collect(); + assert!(gutter.contains(&format!("{min:.0}")), "{gutter}"); + assert!(gutter.contains(&format!("{max:.0}")), "{gutter}"); + assert!(rows.join("\n").contains("noise floor -100 dBFS")); + } + } + /// The frequency axis has to narrow with the zoom even when the spectrum is /// standalone. /// diff --git a/src/ui/panels/core/waterfall/axes.rs b/src/ui/panels/core/waterfall/axes.rs index 8ec9bac4..991a3771 100644 --- a/src/ui/panels/core/waterfall/axes.rs +++ b/src/ui/panels/core/waterfall/axes.rs @@ -23,7 +23,7 @@ use crate::palette::{magnitude_to_color_palette, ColorDepth, WaterfallPalette}; use crate::state::SdrMetrics; use crate::ui::panels::core::spectrum::fmt_spectrum_step; -use super::cells::{band_max, Columns, DB_MAX}; +use super::cells::{band_max, Columns}; /// Width of the dB gutter. Matches the spectrum's label column so the two plots /// start at the same x. @@ -35,6 +35,7 @@ pub(super) fn db_legend( f: &mut Frame, area: Rect, db_min: f32, + db_max: f32, palette: WaterfallPalette, theme: &crate::Theme, ) { @@ -46,9 +47,9 @@ pub(super) fn db_legend( let steps = (h * 2).max(2); let at = |t: f32| { magnitude_to_color_palette( - DB_MAX + (db_min - DB_MAX) * t, + db_max + (db_min - db_max) * t, db_min, - DB_MAX, + db_max, depth, theme, palette, @@ -60,9 +61,9 @@ pub(super) fn db_legend( let top = at((row * 2) as f32 / (steps - 1) as f32); let bot = at((row * 2 + 1) as f32 / (steps - 1) as f32); let label = match row { - 0 => format!("{:>+4} ", DB_MAX as i32), + 0 => format!("{:>+4} ", db_max as i32), r if r == h.saturating_sub(1) => format!("{:>4} ", db_min as i32), - r if r == h / 2 => format!("{:>4} ", ((DB_MAX + db_min) / 2.0) as i32), + r if r == h / 2 => format!("{:>4} ", ((db_max + db_min) / 2.0) as i32), _ => " ".to_string(), }; Line::from(vec![ @@ -89,7 +90,14 @@ pub(super) fn indicator( theme: &crate::Theme, ) { let text = match (state.waterfall.cursor_freq, cursor_col.zip(columns)) { - (Some(cf), Some((col, columns))) => cursor_readout(cf, col, rows, columns, skip_data), + (Some(cf), Some((col, columns))) => cursor_readout( + cf, + col, + rows, + columns, + skip_data, + state.caps.level_unit.label(), + ), // A cursor set outside the current zoom window: name it, but there is // nothing on screen to read a level from. (Some(cf), None) => format!(" cur: {:.3} MHz \u{2190} \u{2192} M", cf as f64 / 1e6), @@ -120,6 +128,7 @@ fn cursor_readout( rows: &VecDeque<(Instant, Arc>)>, columns: &Columns, skip_data: usize, + unit: &str, ) -> String { let mhz = freq_hz as f64 / 1e6; let Some((ts, row)) = rows.get(skip_data) else { @@ -129,10 +138,32 @@ fn cursor_readout( let db = band_max(row, lo, hi); if db.is_finite() { format!( - " cur: {mhz:.3} MHz {db:.1} dBFS {}s ago \u{2190} \u{2192} M", + " cur: {mhz:.3} MHz {db:.1} {unit} {}s ago \u{2190} \u{2192} M", ts.elapsed().as_secs() ) } else { format!(" cur: {mhz:.3} MHz \u{2190} \u{2192} M") } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cursor_readout_keeps_the_iq_unit_and_level() { + let rows = VecDeque::from([(Instant::now(), Arc::new(vec![-60.0]))]); + let window = crate::state::BinAxis::FftBins + .window(100_000_000, 1.0, 1, 1) + .unwrap(); + let text = cursor_readout( + 100_000_000, + 0, + &rows, + &Columns::new(window, 1, crate::state::BinAxis::FftBins), + 0, + crate::hardware::LevelUnit::Dbfs.label(), + ); + assert!(text.starts_with(" cur: 100.000 MHz -60.0 dBFS ")); + } +} diff --git a/src/ui/panels/core/waterfall/cells.rs b/src/ui/panels/core/waterfall/cells.rs index 02685a88..21187325 100644 --- a/src/ui/panels/core/waterfall/cells.rs +++ b/src/ui/panels/core/waterfall/cells.rs @@ -24,10 +24,6 @@ use ratatui::{ use crate::palette::{magnitude_to_color_palette, ColorDepth, WaterfallPalette}; use crate::state::{BinAxis, BinWindow}; -/// Top of the colour scale. The waterfall is always referenced to full scale; -/// only the floor (`db_min`) moves, under `↑`/`↓`. -pub(super) const DB_MAX: f32 = 0.0; - /// Max dB over the bin range `[start, end)` of one waterfall row, clamped to the /// row's own length. Rows are normally all the (fixed) FFT bin count, but reading /// each row against its own length means a row that ever differs - e.g. if the FFT @@ -89,12 +85,13 @@ pub(super) fn draw( cursor_col: Option, skip_data: usize, db_min: f32, + db_max: f32, palette: WaterfallPalette, theme: &crate::Theme, ) { let cols = area.width as usize; let depth = ColorDepth::detect(); - let color = |db: f32| magnitude_to_color_palette(db, db_min, DB_MAX, depth, theme, palette); + let color = |db: f32| magnitude_to_color_palette(db, db_min, db_max, depth, theme, palette); let floor = color(f32::NEG_INFINITY); let mut lines: Vec = Vec::with_capacity(area.height as usize); @@ -141,6 +138,43 @@ mod tests { .unwrap(); Columns::new(window, cols, axis) } + #[test] + fn cell_colors_use_the_configured_ceiling() { + let theme = crate::Theme::sdr(); + let palette = WaterfallPalette::default(); + let rows = VecDeque::from([(Instant::now(), Arc::new(vec![-10.0]))]); + for max in [0.0, -10.0] { + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(1, 1)).unwrap(); + terminal + .draw(|f| { + draw( + f, + f.size(), + &rows, + |row_bins| Some(columns(row_bins, 1, 1)), + None, + 0, + -110.0, + max, + palette, + &theme, + ) + }) + .unwrap(); + assert_eq!( + terminal.backend().buffer().get(0, 0).fg, + magnitude_to_color_palette( + -10.0, + -110.0, + max, + ColorDepth::detect(), + &theme, + palette + ) + ); + } + } #[test] fn band_max_reads_in_range() { diff --git a/src/ui/panels/core/waterfall/mod.rs b/src/ui/panels/core/waterfall/mod.rs index 7b600921..f4fa1ff6 100644 --- a/src/ui/panels/core/waterfall/mod.rs +++ b/src/ui/panels/core/waterfall/mod.rs @@ -208,13 +208,6 @@ fn contents( return; } - // The bonded status cap and the nameplate answer the same question, so they - // ask it once. This used to be a second copy of the rule against a local - // `STALE_MS = 500` sitting beside `panel::FFT_STALE_MS = 500`: the two agreed - // only by coincidence, and disagreed already on "no frame yet" (the copy said - // fresh, the plate said stale). Unreachable in practice - `contents` has - // returned by then if there are no rows - but two rules for one word is how - // the deck starts contradicting itself. let stale = Staleness::FftAge.resolve(state); // Clamp the reported scroll to what the buffer can actually give, so the // bonded status cap never promises history that is not there. The content @@ -292,6 +285,7 @@ fn contents( cursor_col, skip_data, wf.db_min, + wf.db_max, wf.palette, theme, ); @@ -309,6 +303,7 @@ fn contents( ..content }, wf.db_min, + wf.db_max, wf.palette, theme, ); @@ -394,6 +389,7 @@ mod tests { None, 0, wf.db_min, + wf.db_max, wf.palette, &theme, ) @@ -405,6 +401,28 @@ mod tests { assert_ne!(buffer.get(0, 0).bg, buffer.get(1, 0).bg); } + #[test] + fn the_legend_uses_both_level_bounds() { + let mut state = SdrMetrics::fixture().streaming().with_carrier(0.0, 40.0); + for (min, max, labels) in [ + (-120.0, 0.0, ["+0", "-60", "-120"]), + (-110.0, -10.0, ["-10", "-60", "-110"]), + ] { + state.waterfall.db_min = min; + state.waterfall.db_max = max; + let rows = crate::state::fixture::draw(WaterfallPanel, 100, 20, &state); + let gutter: String = rows + .iter() + .skip(1) + .take(18) + .flat_map(|row| row.chars().skip(1).take(DB_COL as usize)) + .collect(); + for label in labels { + assert!(gutter.contains(label), "{label} missing from {gutter}"); + } + } + } + #[test] fn the_ladders_walk_and_stop_at_their_ends() { assert_eq!(next_wf_stride(1), 2); From d17872b37b639c6f4379b19b09af6740bb8f45b3 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Tue, 8 Sep 2026 11:07:43 +0200 Subject: [PATCH 2/2] fix(display): enforce capability bounds and trace pacing Address musithang/sdrtop#13 review with shared range-safe zoom, explicit display validation, capability-based initialization, and budget-aware FFT pacing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/app/builder/boot.rs | 114 ++++++++++++++++++++++++++++++------ src/app/builder/mod.rs | 4 +- src/app/builder/registry.rs | 2 + src/app/input/core.rs | 74 ++++++++++++++++++++--- src/hardware/traits.rs | 25 ++++++++ src/signal/fft/publish.rs | 60 ++++++++++++++----- src/state/fixture.rs | 14 +++-- src/state/waterfall.rs | 12 +++- 8 files changed, 256 insertions(+), 49 deletions(-) diff --git a/src/app/builder/boot.rs b/src/app/builder/boot.rs index 98e37518..fde6db63 100644 --- a/src/app/builder/boot.rs +++ b/src/app/builder/boot.rs @@ -301,7 +301,7 @@ pub fn resolve_gains(radio: &RadioConfig, gm: &GainModel) -> (Vec, Vec SdrMetrics { +pub(super) fn initial_metrics(cfg: &AppConfig, boot: Boot) -> anyhow::Result { let Boot { caps, tuning, @@ -311,7 +311,9 @@ pub(super) fn initial_metrics(cfg: &AppConfig, boot: Boot) -> SdrMetrics { recall, } = boot; - SdrMetrics { + caps.validate_display()?; + + Ok(SdrMetrics { radio: RadioState { frequency: tuning.frequency_hz, config_sample_rate: tuning.sample_rate, @@ -372,15 +374,12 @@ pub(super) fn initial_metrics(cfg: &AppConfig, boot: Boot) -> SdrMetrics { // Clamped, not trusted: `save_config` writes the live buffer depth back, // so a config written before the floor existed carries a value too small // to fill a full-height waterfall. See [`WATERFALL_MIN_ROWS`]. - waterfall: { - let mut waterfall = WaterfallState::new( - cfg.display.waterfall_max_rows.max(WATERFALL_MIN_ROWS), - cfg.display.waterfall_palette, - ); - waterfall.db_min = caps.level_min_db; - waterfall.db_max = caps.level_max_db; - waterfall - }, + waterfall: WaterfallState::new( + cfg.display.waterfall_max_rows.max(WATERFALL_MIN_ROWS), + cfg.display.waterfall_palette, + caps.level_min_db, + caps.level_max_db, + ), system: SystemState { board_name: Arc::from(identity.board_name.as_str()), serial: Arc::from(identity.serial.as_str()), @@ -411,7 +410,7 @@ pub(super) fn initial_metrics(cfg: &AppConfig, boot: Boot) -> SdrMetrics { net: crate::state::NetState::default(), caps, acc: Accumulators::default(), - } + }) } #[cfg(test)] @@ -569,6 +568,7 @@ mod tests { ), ), ] { + let m = m.unwrap(); assert!(!m.radio.rx_enabled); assert!(!m.radio.hw_streaming); assert_eq!(m.radio.actual_sample_rate, 0); @@ -597,7 +597,8 @@ mod tests { tuning, &hardware::DeviceInfo::default(), ), - ); + ) + .unwrap(); assert!(!metrics.radio.rx_enabled); assert!(!metrics.radio.hw_streaming); assert_eq!(metrics.spectrum.y_min, -120.0); @@ -625,7 +626,8 @@ mod tests { tuning, &hardware::DeviceInfo::default(), ), - ); + ) + .unwrap(); assert!(!metrics.radio.rx_enabled); assert!(!metrics.radio.hw_streaming); assert_eq!(metrics.spectrum.y_min, -110.0); @@ -634,6 +636,78 @@ mod tests { assert_eq!(metrics.waterfall.db_max, -10.0); } + #[test] + fn startup_rejects_invalid_display_capabilities() { + let cfg = AppConfig::default(); + for (min, max, budget, error) in [ + (0.0, 0.0, 500, "Invalid display level bounds"), + (0.0, -10.0, 500, "Invalid display level bounds"), + (f32::NAN, 0.0, 500, "Invalid display level bounds"), + (-120.0, f32::NAN, 500, "Invalid display level bounds"), + (f32::NEG_INFINITY, 0.0, 500, "Invalid display level bounds"), + (-120.0, f32::INFINITY, 500, "Invalid display level bounds"), + (-f32::MAX, f32::MAX, 500, "Invalid display level bounds"), + (-120.0, 0.0, 0, "Invalid trace_stale_ms"), + ] { + let mut caps = hardware::native::hackrf::caps(); + caps.level_min_db = min; + caps.level_max_db = max; + caps.trace_stale_ms = budget; + let tuning = resolve_tuning(&cfg.radio, &caps); + let result = initial_metrics( + &cfg, + Boot::normal( + &cfg, + Arc::new(caps), + tuning, + &hardware::DeviceInfo::default(), + ), + ); + assert!(result + .err() + .expect("invalid caps must fail") + .to_string() + .contains(error)); + } + } + + #[test] + fn startup_accepts_valid_display_capabilities() { + let cfg = AppConfig::default(); + for (min, max, budget) in [(-10.0, 0.0, 1), (-120.0, 20.0, 5_000)] { + let mut caps = hardware::native::hackrf::caps(); + caps.level_min_db = min; + caps.level_max_db = max; + caps.trace_stale_ms = budget; + let tuning = resolve_tuning(&cfg.radio, &caps); + let m = initial_metrics( + &cfg, + Boot::normal( + &cfg, + Arc::new(caps), + tuning, + &hardware::DeviceInfo::default(), + ), + ) + .unwrap(); + assert_eq!((m.spectrum.y_min, m.spectrum.y_max), (min, max)); + assert_eq!((m.waterfall.db_min, m.waterfall.db_max), (min, max)); + assert_eq!(m.caps.trace_stale_ms, budget); + } + } + + #[test] + fn observer_startup_also_rejects_invalid_display_capabilities() { + let cfg = AppConfig::default(); + let mut boot = Boot::observer(&cfg, &sysinfo(), profile(hardware::DeviceKind::HackRf)); + Arc::make_mut(&mut boot.caps).trace_stale_ms = 0; + assert!(initial_metrics(&cfg, boot) + .err() + .expect("invalid observer caps must fail") + .to_string() + .contains("Invalid trace_stale_ms")); + } + /// A config that asks for less waterfall history than a full-height panel /// needs is raised, not honoured. /// @@ -647,7 +721,8 @@ mod tests { let m = initial_metrics( &cfg, Boot::observer(&cfg, &sysinfo(), profile(hardware::DeviceKind::HackRf)), - ); + ) + .unwrap(); assert_eq!( m.waterfall.buffer.max_rows, crate::state::WATERFALL_MIN_ROWS @@ -662,7 +737,8 @@ mod tests { let m = initial_metrics( &cfg, Boot::observer(&cfg, &sysinfo(), profile(hardware::DeviceKind::HackRf)), - ); + ) + .unwrap(); assert_eq!(m.waterfall.buffer.max_rows, 4_096); } @@ -685,14 +761,16 @@ mod tests { resolve_tuning(&cfg.radio, &hardware::native::hackrf::caps()), &hardware::DeviceInfo::default(), ), - ); + ) + .unwrap(); assert_eq!(live.spectrum.markers.len(), 1); assert_eq!(live.ui.recall[0], Some(100_000_000)); let obs = initial_metrics( &cfg, Boot::observer(&cfg, &sysinfo(), profile(hardware::DeviceKind::HackRf)), - ); + ) + .unwrap(); assert!(obs.spectrum.markers.is_empty()); assert!(obs.ui.recall.iter().all(|s| s.is_none())); assert!(obs.observer.active); diff --git a/src/app/builder/mod.rs b/src/app/builder/mod.rs index 69051de9..516f8b4c 100644 --- a/src/app/builder/mod.rs +++ b/src/app/builder/mod.rs @@ -89,7 +89,7 @@ impl App { let state = Arc::new(Mutex::new(initial_metrics( &cfg, Boot::normal(&cfg, Arc::clone(&caps), tuning, &info), - ))); + )?)); { let mut m = state.lock().unwrap_or_else(|e| e.into_inner()); @@ -204,7 +204,7 @@ impl App { let state = Arc::new(Mutex::new(initial_metrics( &cfg, Boot::observer(&cfg, &sysinfo, profile), - ))); + )?)); { let mut m = state.lock().unwrap_or_else(|e| e.into_inner()); diff --git a/src/app/builder/registry.rs b/src/app/builder/registry.rs index bfd83827..e5a39701 100644 --- a/src/app/builder/registry.rs +++ b/src/app/builder/registry.rs @@ -386,6 +386,8 @@ mod tests { m.waterfall = WaterfallState::new( WATERFALL_MIN_ROWS, crate::palette::WaterfallPalette::default(), + m.caps.level_min_db, + m.caps.level_max_db, ); for i in 0..WATERFALL_MIN_ROWS { let bins: Vec = (0..256) diff --git a/src/app/input/core.rs b/src/app/input/core.rs index 5bd9f8b0..ae596a60 100644 --- a/src/app/input/core.rs +++ b/src/app/input/core.rs @@ -32,6 +32,18 @@ fn strongest_bin_frequency(frame: &crate::state::FftFrame) -> Option { .filter(|frequency| *frequency >= 0.0) .map(|frequency| frequency.round() as u64) } +const LEVEL_ZOOM_STEP_DB: f32 = 10.0; +const LEVEL_MIN_WINDOW_DB: f32 = 20.0; +fn adjust_level_floor( + floor: f32, + ceiling: f32, + delta: f32, + caps: &crate::hardware::DeviceCapabilities, +) -> f32 { + let window = LEVEL_MIN_WINDOW_DB.min(caps.level_max_db - caps.level_min_db); + let highest_floor = (ceiling.min(caps.level_max_db) - window).max(caps.level_min_db); + (floor + delta).clamp(caps.level_min_db, highest_floor) +} // ── Spectrum focus keys ─────────────────────────────────────────────────────── @@ -110,7 +122,12 @@ pub(super) fn spectrum(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { } KeyCode::Up => { let mut m = metrics(state); - let new_min = (m.spectrum.y_min + 10.0).min(m.spectrum.y_max - 20.0); + let new_min = adjust_level_floor( + m.spectrum.y_min, + m.spectrum.y_max, + LEVEL_ZOOM_STEP_DB, + &m.caps, + ); m.spectrum.y_min = new_min; let ymax = m.spectrum.y_max; let unit = m.caps.level_unit.label(); @@ -118,7 +135,12 @@ pub(super) fn spectrum(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { } KeyCode::Down => { let mut m = metrics(state); - let new_min = (m.spectrum.y_min - 10.0).max(m.caps.level_min_db); + let new_min = adjust_level_floor( + m.spectrum.y_min, + m.spectrum.y_max, + -LEVEL_ZOOM_STEP_DB, + &m.caps, + ); m.spectrum.y_min = new_min; let ymax = m.spectrum.y_max; let unit = m.caps.level_unit.label(); @@ -222,7 +244,12 @@ pub(super) fn waterfall(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { match key.code { KeyCode::Up => { let mut m = metrics(state); - let new_min = (m.waterfall.db_min + 10.0).min(m.waterfall.db_max - 20.0); + let new_min = adjust_level_floor( + m.waterfall.db_min, + m.waterfall.db_max, + LEVEL_ZOOM_STEP_DB, + &m.caps, + ); m.waterfall.db_min = new_min; let max = m.waterfall.db_max; let unit = m.caps.level_unit.label(); @@ -232,7 +259,12 @@ pub(super) fn waterfall(key: KeyEvent, ctx: &mut InputCtx<'_>) -> KeyAction { } KeyCode::Down => { let mut m = metrics(state); - let new_min = (m.waterfall.db_min - 10.0).max(m.caps.level_min_db); + let new_min = adjust_level_floor( + m.waterfall.db_min, + m.waterfall.db_max, + -LEVEL_ZOOM_STEP_DB, + &m.caps, + ); m.waterfall.db_min = new_min; let max = m.waterfall.db_max; let unit = m.caps.level_unit.label(); @@ -357,9 +389,34 @@ mod tests { ); } + #[test] + fn either_zoom_direction_keeps_the_floor_within_device_limits() { + let mut caps = crate::hardware::native::hackrf::caps(); + for (min, max) in [(-120.0, 0.0), (-10.0, 0.0), (-10.5, -10.0)] { + caps.level_min_db = min; + caps.level_max_db = max; + for delta in [-LEVEL_ZOOM_STEP_DB, LEVEL_ZOOM_STEP_DB] { + for floor in [min - 100.0, min, max, max + 100.0] { + for ceiling in [max - 5.0, max, max + 100.0] { + let adjusted = adjust_level_floor(floor, ceiling, delta, &caps); + assert!((min..max).contains(&adjusted)); + assert!(adjusted <= max - (max - min).min(20.0)); + } + } + } + } + } + #[test] fn level_zoom_uses_device_bounds_and_preserves_iq_log_text() { - for (min, max) in [(-120.0, 0.0), (-110.0, -10.0)] { + for (min, max) in [ + (-120.0_f32, 0.0), + (-120.0, 20.0), + (-110.0, -10.0), + (-10.0, 0.0), + (-10.5, -10.0), + ] { + let highest_floor = max - (max - min).min(20.0_f32); for is_waterfall in [false, true] { let mut m = SdrMetrics::fixture(); Arc::make_mut(&mut m.caps).level_min_db = min; @@ -398,7 +455,10 @@ mod tests { press(KeyCode::Up); assert_eq!( metrics(&state).ui.log.back().unwrap().text.as_ref(), - format!("{prefix}: {:.0}\u{2026}{max:.0} dBFS", min + 10.0) + format!( + "{prefix}: {:.0}\u{2026}{max:.0} dBFS", + (min + 10.0).min(highest_floor) + ) ); for _ in 0..20 { press(KeyCode::Up); @@ -410,7 +470,7 @@ mod tests { } else { m.spectrum.y_min }; - assert_eq!(floor, max - 20.0); + assert_eq!(floor, highest_floor); } for _ in 0..20 { press(KeyCode::Down); diff --git a/src/hardware/traits.rs b/src/hardware/traits.rs index 5cb9a595..3cf65b54 100644 --- a/src/hardware/traits.rs +++ b/src/hardware/traits.rs @@ -631,9 +631,13 @@ pub enum DeliveryModel { /// truth for every clamp, default, and UI capability check. Built once at open. #[derive(Clone, Debug)] pub struct DeviceCapabilities { + /// Spectral levels and display bounds use this unit pub level_unit: LevelUnit, + /// The finite display floor must be below `level_max_db` pub level_min_db: f32, + /// The finite display ceiling must form a finite span with `level_min_db` pub level_max_db: f32, + /// A trace becomes stale after this positive millisecond budget pub trace_stale_ms: u128, pub freq_min_hz: u64, pub freq_max_hz: u64, @@ -669,6 +673,27 @@ pub struct DeviceCapabilities { pub delivery: DeliveryModel, } +impl DeviceCapabilities { + /// Reject invalid display bounds or a zero trace-age budget before initializing views + pub fn validate_display(&self) -> anyhow::Result<()> { + anyhow::ensure!( + self.level_min_db.is_finite() + && self.level_max_db.is_finite() + && self.level_min_db < self.level_max_db + && (self.level_max_db - self.level_min_db).is_finite(), + "Invalid display level bounds: {}..{} {}; expected finite ordered bounds with a finite span", + self.level_min_db, + self.level_max_db, + self.level_unit.label(), + ); + anyhow::ensure!( + self.trace_stale_ms > 0, + "Invalid trace_stale_ms: expected a positive millisecond budget", + ); + Ok(()) + } +} + /// The software layer between sdrtop and a radio that has no firmware of its /// own, for the header's firmware field. /// diff --git a/src/signal/fft/publish.rs b/src/signal/fft/publish.rs index a0b17b57..11ee63bc 100644 --- a/src/signal/fft/publish.rs +++ b/src/signal/fft/publish.rs @@ -13,7 +13,7 @@ //! comment, the way `tasks/rx/` makes its two lock blocks visible. use std::sync::{Arc, Mutex}; -use std::time::{Duration, Instant}; +use std::time::Instant; use crate::state::{FftFrame, SdrMetrics}; @@ -25,10 +25,6 @@ use super::analysis::Reading; /// lockstep. Signal metrics stay at full rate. const ROWS_PER_WATERFALL_LINE: u32 = 2; -/// Never let the drawn frame age past the panels' 500 ms STALE threshold, even at -/// large frames or row strides. -const SPECTRUM_STALE_GUARD: Duration = Duration::from_millis(400); - /// How much of a marker's channel the measured-bandwidth cut-offs bracket. const MARKER_BW_LOW: f32 = 0.005; const MARKER_BW_HIGH: f32 = 0.995; @@ -36,30 +32,32 @@ const MARKER_BW_HIGH: f32 = 0.995; /// The display-paced spectrum refresh's state, carried between frames. pub(super) struct Pacing { rows_since_spectrum: u32, - last_update: Instant, + last_update: Option, } impl Pacing { pub(super) fn new() -> Self { Self { rows_since_spectrum: 0, - last_update: Instant::now() - .checked_sub(SPECTRUM_STALE_GUARD) - .unwrap_or_else(Instant::now), + last_update: None, } } /// Whether to redraw the spectrum: there is none yet, a visible waterfall line /// has gone by, or it would otherwise age toward `[STALE]`. - fn due(&self, has_frame: bool) -> bool { + fn due(&self, has_frame: bool, trace_stale_ms: u128, now: Instant) -> bool { + // Reserve one fifth of the trace-age budget for display scheduling + let guard_ms = trace_stale_ms - trace_stale_ms.div_ceil(5); !has_frame || self.rows_since_spectrum >= ROWS_PER_WATERFALL_LINE - || self.last_update.elapsed() >= SPECTRUM_STALE_GUARD + || self + .last_update + .is_none_or(|last| now.duration_since(last).as_millis() >= guard_ms) } - fn mark(&mut self) { + fn mark(&mut self, now: Instant) { self.rows_since_spectrum = 0; - self.last_update = Instant::now(); + self.last_update = Some(now); } } @@ -118,8 +116,9 @@ pub(super) fn publish( pacing.rows_since_spectrum += 1; } - if pacing.due(m.waterfall.last_fft.is_some()) { - pacing.mark(); + let now = Instant::now(); + if pacing.due(m.waterfall.last_fft.is_some(), m.caps.trace_stale_ms, now) { + pacing.mark(now); refresh_spectrum(&mut m, &snap); } Some(alpha) @@ -256,6 +255,37 @@ fn refresh_spectrum(m: &mut SdrMetrics, snap: &Snapshot<'_>) { mod tests { use super::*; use crate::state::SpectrumMarker; + use std::time::Duration; + + #[test] + fn pacing_reserves_one_fifth_of_each_trace_age_budget() { + let now = Instant::now(); + let mut pacing = Pacing::new(); + for (budget, guard_ms) in [(500, 400), (100, 80), (5_000, 4_000), (2, 1)] { + pacing.mark(now); + assert!(!pacing.due(true, budget, now + Duration::from_millis(guard_ms - 1))); + assert!(pacing.due(true, budget, now + Duration::from_millis(guard_ms))); + } + assert!(pacing.due(true, 1, now)); + assert!(!pacing.due(true, u128::MAX, now + Duration::from_secs(1))); + } + + #[test] + fn pacing_still_refreshes_initial_frames_and_visible_waterfall_lines() { + let now = Instant::now(); + let mut pacing = Pacing::new(); + assert!(pacing.due(true, 500, now)); + pacing.mark(now); + assert!(pacing.due(false, 500, now)); + assert!(!pacing.due(true, 500, now)); + pacing.rows_since_spectrum = 1; + assert!(!pacing.due(true, 500, now)); + pacing.rows_since_spectrum = 2; + assert!(pacing.due(true, 500, now)); + pacing.mark(now); + assert_eq!(pacing.rows_since_spectrum, 0); + assert!(!pacing.due(true, 500, now)); + } fn marker(freq_hz: u64, channel_bw_hz: u64, measured_bw_hz: Option) -> SpectrumMarker { SpectrumMarker { diff --git a/src/state/fixture.rs b/src/state/fixture.rs index c6e62c4b..636c4a3e 100644 --- a/src/state/fixture.rs +++ b/src/state/fixture.rs @@ -35,6 +35,7 @@ impl SdrMetrics { /// also the state most likely to be got wrong - every "waiting for RX" and /// `[STALE]` path runs through it. pub(crate) fn fixture() -> Self { + let caps = Arc::new(crate::hardware::native::hackrf::caps()); SdrMetrics { radio: RadioState { frequency: 100_000_000, @@ -74,15 +75,20 @@ impl SdrMetrics { observer: ObserverState::default(), spectrum: SpectrumState { step_hz: 100_000, - y_min: -120.0, - y_max: 0.0, + y_min: caps.level_min_db, + y_max: caps.level_max_db, hold: None, cursor_freq: None, markers: vec![], pending_marker: None, style: SpectrumStyle::default(), }, - waterfall: WaterfallState::new(512, crate::palette::WaterfallPalette::default()), + waterfall: WaterfallState::new( + 512, + crate::palette::WaterfallPalette::default(), + caps.level_min_db, + caps.level_max_db, + ), system: SystemState { // The HackRF fixture reports its own firmware, so no stack row. stack: None, @@ -101,7 +107,7 @@ impl SdrMetrics { lab: LabState::default(), demod: DemodState::default(), net: crate::state::NetState::default(), - caps: Arc::new(crate::hardware::native::hackrf::caps()), + caps, acc: Accumulators::default(), } } diff --git a/src/state/waterfall.rs b/src/state/waterfall.rs index eb32805d..49c55a5e 100644 --- a/src/state/waterfall.rs +++ b/src/state/waterfall.rs @@ -251,10 +251,16 @@ pub struct WaterfallState { } impl WaterfallState { - pub fn new(max_rows: usize, palette: crate::palette::WaterfallPalette) -> Self { + /// Initialize the view with finite ordered bounds from validated device capabilities + pub fn new( + max_rows: usize, + palette: crate::palette::WaterfallPalette, + db_min: f32, + db_max: f32, + ) -> Self { Self { - db_min: -120.0, - db_max: 0.0, + db_min, + db_max, scroll_offset: 0, cursor_freq: None, hz_zoom: 1,