fix(srtla_send): apply the --config toml file

README.md

@@ -133,7 +133,7 @@ srtla_send [OPTIONS] SRT_LISTEN_PORT SRTLA_HOST SRTLA_PORT BIND_IPS_FILE
 - `--stall-min-in-flight <N>`: In-flight backlog (packets) at or above which a link becomes a stall candidate (default 32)
 - `--stall-ack-stale-ms <MS>`: Delivery-proof staleness window in milliseconds after which a stall candidate is deselected (default 3000)
 - `--no-rehome`: Disable whole-bond re-home (on by default). When every uplink has been down for longer than the all-links-failed window *and* the receiver hostname no longer resolves to the address the bond is pinned to, the sender moves every uplink together to the newly-resolved address and re-registers from scratch over the ordinary REG1/REG2/REG3 flow. It is deliberately conservative: a bond with any live uplink is never touched, a merely reordered DNS answer is not a move, a failed lookup is not a move, and at most one migration is attempted per minute. Pass this to keep the old behaviour of staying on the cached address until the process is restarted.
-- `--config <PATH>`: Path to a TOML config file (reloaded on SIGHUP)
+- `--config <PATH>`: Path to a TOML config file, read once at startup. Each key is the long flag name with underscores (for example `stall_ack_stale_ms = 2000`), and a flag given on the command line wins over the file. The supported keys are `mode`, `no_quality`, `no_stall_deselect`, `stall_min_in_flight`, `stall_ack_stale_ms` and `conn_timeout_ms`. An unknown key or a file that fails to parse stops startup.
 - `--control-socket <PATH>`: Unix domain socket path for remote control (e.g., `/tmp/srtla.sock`)
 - `--priority-bind <ADDR:PORT>`: UDP sidecar address for encoder keyframe priority hints
 - `--metrics-bind <ADDR:PORT>`: Expose a Prometheus scrape endpoint at `/metrics`

src/main.rs

@@ -5,8 +5,9 @@
 // two, and keeps the library's embedder-facing surface (the Android and Apple
 // binders no CLI ever constructs) from reading as dead code here.
 use anyhow::{Context, Result};
-use clap::Parser;
 use clap::builder::{PossibleValuesParser, TypedValueParser};
+use clap::parser::ValueSource;
+use clap::{ArgMatches, CommandFactory, FromArgMatches, Parser};
 use srtla_core::mode::SchedulingMode;
 use srtla_send::{
     config, control_socket, metrics, net, priority_listener, sender, stats, subscriptions,
@@ -53,7 +54,9 @@ struct Cli {
     #[arg(long = "control-socket")]
     control_socket: Option<String>,
 
-    /// Path to TOML config file (reloaded on SIGHUP)
+    /// Path to a TOML config file, read once at startup. Keys match the long
+    /// flag names with underscores (`conn_timeout_ms`); a flag given on the
+    /// command line wins over the file
     #[arg(long = "config")]
     config_file: Option<String>,
 
@@ -128,6 +131,34 @@ struct Cli {
     metrics_bind: Option<std::net::SocketAddr>,
 }
 
+impl Cli {
+    /// Replace each option the user did not type on the command line with its
+    /// value from the config file. The precedence is command line, then file,
+    /// then the clap default.
+    fn apply_config_file(&mut self, matches: &ArgMatches, file: toml_config::TomlConfig) {
+        let typed = |id: &str| matches.value_source(id) == Some(ValueSource::CommandLine);
+        // The derive names each arg after its field, so one identifier serves
+        // as both the arg id and the field to fill.
+        macro_rules! fill {
+            ($($field:ident),+ $(,)?) => {$(
+                if let Some(value) = file.$field {
+                    if !typed(stringify!($field)) {
+                        self.$field = value;
+                    }
+                }
+            )+};
+        }
+        fill!(
+            mode,
+            no_quality,
+            no_stall_deselect,
+            stall_min_in_flight,
+            stall_ack_stale_ms,
+            conn_timeout_ms,
+        );
+    }
+}
+
 /// Warn when a sidecar is bound to a non-loopback address. These endpoints
 /// are unauthenticated same-device IPC (encoder front-end and local scrapers),
 /// so a routable bind exposes an open control / scrape surface. We warn rather
@@ -150,23 +181,24 @@ async fn main() -> Result<()> {
         .with_target(false)
         .init();
 
-    let args = Cli::parse();
+    let matches = Cli::command().get_matches();
+    let mut args = Cli::from_arg_matches(&matches).unwrap_or_else(|e| e.exit());
     if args.print_version {
         println!("{}", version::version_line());
         return Ok(());
     }
 
+    if let Some(path) = args.config_file.clone() {
+        let file = toml_config::TomlConfig::load(std::path::Path::new(&path))?;
+        tracing::info!("loaded config from {path}");
+        args.apply_config_file(&matches, file);
+    }
+
     let local_srt_port = args.local_srt_port.expect("required");
     let receiver_host = args.receiver_host.as_deref().expect("required");
     let receiver_port = args.receiver_port.expect("required");
     let ips_file = args.ips_file.as_deref().expect("required");
 
-    // Load TOML config (if specified), then apply CLI overrides
-    if let Some(ref path) = args.config_file {
-        let toml_cfg = toml_config::TomlConfig::load_or_default(std::path::Path::new(path));
-        tracing::debug!("TOML config loaded: {:?}", toml_cfg);
-    }
-
     let config = config::DynamicConfig::from_cli(
         args.mode,
         args.no_quality,
@@ -244,3 +276,58 @@ async fn main() -> Result<()> {
     .await
     .context("srtla_send failed")
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn parse_with_file(flags: &[&str], file: &str) -> Cli {
+        let argv = ["srtla_send", "5000", "rec.example", "5001", "ips.txt"]
+            .into_iter()
+            .chain(flags.iter().copied());
+        let matches = Cli::command().try_get_matches_from(argv).unwrap();
+        let mut cli = Cli::from_arg_matches(&matches).unwrap();
+        cli.apply_config_file(&matches, toml::from_str(file).unwrap());
+        cli
+    }
+
+    #[test]
+    fn file_overrides_clap_defaults() {
+        let cli = parse_with_file(
+            &[],
+            r#"
+                mode = "classic"
+                no_quality = true
+                no_stall_deselect = true
+                stall_min_in_flight = 64
+                stall_ack_stale_ms = 1500
+                conn_timeout_ms = 8000
+            "#,
+        );
+        assert_eq!(cli.mode, SchedulingMode::Classic);
+        assert!(cli.no_quality);
+        assert!(cli.no_stall_deselect);
+        assert_eq!(cli.stall_min_in_flight, 64);
+        assert_eq!(cli.stall_ack_stale_ms, 1500);
+        assert_eq!(cli.conn_timeout_ms, 8000);
+    }
+
+    #[test]
+    fn typed_flag_overrides_file_even_at_default_value() {
+        let default = config::CONN_TIMEOUT_MS.to_string();
+        let cli = parse_with_file(
+            &["--conn-timeout-ms", &default, "--no-quality"],
+            "conn_timeout_ms = 8000\nno_quality = false",
+        );
+        assert_eq!(cli.conn_timeout_ms, config::CONN_TIMEOUT_MS);
+        assert!(cli.no_quality);
+    }
+
+    #[test]
+    fn absent_file_keys_keep_cli_values() {
+        let cli = parse_with_file(&["--stall-ack-stale-ms", "2000"], "");
+        assert_eq!(cli.mode, SchedulingMode::Enhanced);
+        assert_eq!(cli.stall_ack_stale_ms, 2000);
+        assert_eq!(cli.conn_timeout_ms, config::CONN_TIMEOUT_MS);
+    }
+}

src/toml_config.rs

@@ -1,94 +1,44 @@
 //! Optional TOML file configuration for srtla_send.
 //!
-//! Loaded at startup via `--config <path>` and reloaded on SIGHUP.
-//! All fields use `#[serde(default)]` so a partial config file is valid.
+//! Loaded once at startup via `--config <path>`. Each key mirrors the CLI flag
+//! of the same name, and a flag typed on the command line wins over the file.
+//! Unknown keys fail the load, so a misspelled or retired key stops startup
+//! instead of silently doing nothing.
 
 use std::path::Path;
 
-use serde::Deserialize;
-use tracing::{info, warn};
+use anyhow::{Context, Result};
+use serde::{Deserialize, Deserializer};
+use srtla_core::mode::SchedulingMode;
 
-/// Top-level TOML configuration.
-#[derive(Debug, Clone, Deserialize)]
-#[serde(default)]
+/// Values read from the config file. `None` means the key was absent.
+#[derive(Debug, Default, Deserialize)]
+#[serde(default, deny_unknown_fields)]
 pub struct TomlConfig {
-    /// Scheduling mode: classic, enhanced.
-    pub mode: String,
-    /// Disable quality scoring.
-    pub no_quality: bool,
-    /// Disable the stalled-link deselect guard (on by default).
-    pub no_stall_deselect: bool,
-    /// In-flight backlog at or above which a link becomes a stall candidate.
-    pub stall_min_in_flight: i32,
-    /// Delivery-proof staleness window (ms) before a stall candidate is deselected.
-    pub stall_ack_stale_ms: u64,
-    /// Per-link liveness timeout (ms); silence past this re-registers the link.
-    pub conn_timeout_ms: u64,
-
-    // --- Congestion control ---
-    /// RTT velocity threshold (ms/sample) above which window recovery is halved.
-    pub rtt_velocity_gate: f64,
-
-    // --- Link lifecycle ---
-    /// RTT probes required during warming phase before going Live.
-    pub warming_rtt_probes: u32,
-    /// Maximum time (ms) in warming phase before auto-promoting.
-    pub warming_timeout_ms: u64,
-    /// Quality threshold below which a Live link becomes Degraded.
-    pub degraded_quality_threshold: f64,
-    /// NAK burst count threshold for degradation.
-    pub degraded_nak_burst_threshold: i32,
-    /// Cooldown duration (ms) before re-entering Live from Degraded.
-    pub cooldown_duration_ms: u64,
-
-    // --- Selection ---
-    /// Minimum time (ms) between connection switches.
-    pub min_switch_interval_ms: u64,
-    /// Switch hysteresis threshold (1.10 = 10% better required).
-    pub switch_hysteresis: f64,
+    #[serde(deserialize_with = "deserialize_mode")]
+    pub mode: Option<SchedulingMode>,
+    pub no_quality: Option<bool>,
+    pub no_stall_deselect: Option<bool>,
+    pub stall_min_in_flight: Option<i32>,
+    pub stall_ack_stale_ms: Option<u64>,
+    pub conn_timeout_ms: Option<u64>,
 }
 
-impl Default for TomlConfig {
-    fn default() -> Self {
-        Self {
-            mode: "enhanced".to_string(),
-            no_quality: false,
-            no_stall_deselect: false,
-            stall_min_in_flight: crate::config::STALL_MIN_IN_FLIGHT_PACKETS,
-            stall_ack_stale_ms: crate::config::STALL_ACK_STALE_MS,
-            conn_timeout_ms: crate::config::CONN_TIMEOUT_MS,
-            rtt_velocity_gate: 2.0,
-            warming_rtt_probes: 2,
-            warming_timeout_ms: 5_000,
-            degraded_quality_threshold: 0.5,
-            degraded_nak_burst_threshold: 5,
-            cooldown_duration_ms: 5_000,
-            min_switch_interval_ms: 15,
-            switch_hysteresis: 1.10,
-        }
-    }
+// `SchedulingMode` is a pure core type with no serde dependency, so the file
+// goes through the same `FromStr` the CLI uses.
+fn deserialize_mode<'de, D: Deserializer<'de>>(d: D) -> Result<Option<SchedulingMode>, D::Error> {
+    String::deserialize(d)?
+        .parse()
+        .map(Some)
+        .map_err(serde::de::Error::custom)
 }
 
 impl TomlConfig {
-    /// Load config from a TOML file.
-    pub fn load(path: &Path) -> Result<Self, String> {
-        let content =
-            std::fs::read_to_string(path).map_err(|e| format!("failed to read {path:?}: {e}"))?;
-        toml::from_str(&content).map_err(|e| format!("failed to parse {path:?}: {e}"))
-    }
-
-    /// Load config, logging errors and falling back to defaults.
-    pub fn load_or_default(path: &Path) -> Self {
-        match Self::load(path) {
-            Ok(cfg) => {
-                info!("loaded config from {}", path.display());
-                cfg
-            }
-            Err(e) => {
-                warn!("config load failed: {e}, using defaults");
-                Self::default()
-            }
-        }
+    pub fn load(path: &Path) -> Result<Self> {
+        let content = std::fs::read_to_string(path)
+            .with_context(|| format!("failed to read config file {}", path.display()))?;
+        toml::from_str(&content)
+            .with_context(|| format!("failed to parse config file {}", path.display()))
     }
 }
 
@@ -97,43 +47,41 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_defaults() {
-        let cfg = TomlConfig::default();
-        assert_eq!(cfg.mode, "enhanced");
-        assert!(!cfg.no_quality);
-        assert!((cfg.rtt_velocity_gate - 2.0).abs() < f64::EPSILON);
+    fn absent_keys_are_none() {
+        let cfg: TomlConfig = toml::from_str("mode = \"classic\"").unwrap();
+        assert_eq!(cfg.mode, Some(SchedulingMode::Classic));
+        assert_eq!(cfg.no_quality, None);
+        assert_eq!(cfg.conn_timeout_ms, None);
     }
 
     #[test]
-    fn test_partial_toml() {
+    fn full_toml() {
         let toml_str = r#"
-            mode = "classic"
-            rtt_velocity_gate = 3.5
+            mode = "enhanced"
+            no_quality = true
+            no_stall_deselect = true
+            stall_min_in_flight = 64
+            stall_ack_stale_ms = 1500
+            conn_timeout_ms = 8000
         "#;
         let cfg: TomlConfig = toml::from_str(toml_str).unwrap();
-        assert_eq!(cfg.mode, "classic");
-        assert!((cfg.rtt_velocity_gate - 3.5).abs() < f64::EPSILON);
-        // Defaults for unspecified fields
-        assert!(!cfg.no_quality);
+        assert_eq!(cfg.mode, Some(SchedulingMode::Enhanced));
+        assert_eq!(cfg.no_quality, Some(true));
+        assert_eq!(cfg.no_stall_deselect, Some(true));
+        assert_eq!(cfg.stall_min_in_flight, Some(64));
+        assert_eq!(cfg.stall_ack_stale_ms, Some(1500));
+        assert_eq!(cfg.conn_timeout_ms, Some(8000));
     }
 
     #[test]
-    fn test_full_toml() {
-        let toml_str = r#"
-            mode = "classic"
-            no_quality = true
-            rtt_velocity_gate = 1.0
-            warming_rtt_probes = 3
-            warming_timeout_ms = 10000
-            degraded_quality_threshold = 0.3
-            degraded_nak_burst_threshold = 10
-            cooldown_duration_ms = 8000
-            min_switch_interval_ms = 30
-            switch_hysteresis = 1.20
-        "#;
-        let cfg: TomlConfig = toml::from_str(toml_str).unwrap();
-        assert_eq!(cfg.mode, "classic");
-        assert!(cfg.no_quality);
-        assert_eq!(cfg.warming_rtt_probes, 3);
+    fn unknown_key_is_rejected() {
+        let err = toml::from_str::<TomlConfig>("switch_hysteresis = 1.2").unwrap_err();
+        assert!(err.to_string().contains("switch_hysteresis"), "{err}");
+    }
+
+    #[test]
+    fn invalid_mode_is_rejected() {
+        let err = toml::from_str::<TomlConfig>("mode = \"fastest\"").unwrap_err();
+        assert!(err.to_string().contains("fastest"), "{err}");
     }
 }