[fix](trx-config): validate every rig, not just the legacy flat one

ServerConfig::validate() checked the flat [rig]/[audio]/[behavior] fields and
gave [[rigs]] entries only an id/audio-port uniqueness pass, and
validate_sdr() returned early unless the *flat* access type was "sdr".  A
multi-rig SDR station therefore got no Nyquist, stream_opus, duplicate-decoder
or tx_enabled checking at all, and a rig entry with frame_duration_ms = 7 or a
missing baud rate started and failed at runtime.

Move the per-rig rules into validate_rig_instance() and validate_sdr_instance()
and run them over resolved_rigs(), which already synthesises the flat layout as
a single entry.  Both layouts now go through the same code, and multi-rig
messages name the rig they came from.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SyX26FCpMQxiBoC7r5K1A7
Signed-off-by: Stan Grams <sjg@haxx.space>
This commit is contained in:
sjg
2026-08-06 20:57:59 +02:00
co-authored by Claude Opus 5
parent 335922fecc
commit d42ca4f030
+374 -213
View File
@@ -471,113 +471,44 @@ impl Default for SdrChannelConfig {
} }
impl ServerConfig { impl ServerConfig {
/// Validate the whole configuration.
///
/// Everything that belongs to a rig is checked through `resolved_rigs()`,
/// so the legacy flat `[rig]` / `[audio]` / … layout and `[[rigs]]` entries
/// are held to exactly the same rules.
pub fn validate(&self) -> Result<(), String> { pub fn validate(&self) -> Result<(), String> {
validate_log_level(self.general.log_level.as_deref())?; validate_log_level(self.general.log_level.as_deref())?;
validate_coordinates(self.general.latitude, self.general.longitude)?; validate_coordinates(self.general.latitude, self.general.longitude)?;
if self.rig.initial_freq_hz == 0 {
return Err("[rig].initial_freq_hz must be > 0".to_string());
}
validate_access(&self.rig.access)?;
if self.behavior.poll_interval_ms == 0 {
return Err("[behavior].poll_interval_ms must be > 0".to_string());
}
if self.behavior.poll_interval_tx_ms == 0 {
return Err("[behavior].poll_interval_tx_ms must be > 0".to_string());
}
if self.behavior.max_retries == 0 {
return Err("[behavior].max_retries must be > 0".to_string());
}
if self.behavior.retry_base_delay_ms == 0 {
return Err("[behavior].retry_base_delay_ms must be > 0".to_string());
}
validate_tokens("[listen.auth].tokens", &self.listen.auth.tokens)?; validate_tokens("[listen.auth].tokens", &self.listen.auth.tokens)?;
if self.listen.enabled && self.listen.port == 0 { if self.listen.enabled && self.listen.port == 0 {
return Err("[listen].port must be > 0 when listener is enabled".to_string()); return Err("[listen].port must be > 0 when listener is enabled".to_string());
} }
if self.audio.enabled { self.validate_rig_uniqueness()?;
if self.audio.port == 0 {
return Err("[audio].port must be > 0 when audio is enabled".to_string()); let multi = !self.rigs.is_empty();
} for rig in &self.resolved_rigs() {
if !self.audio.rx_enabled && !self.audio.tx_enabled { validate_rig_instance(&rig_prefix(multi, rig), rig, &self.general)?;
return Err(
"[audio] enabled but both rx_enabled and tx_enabled are false".to_string(),
);
}
if self.audio.sample_rate < 8_000 || self.audio.sample_rate > 192_000 {
return Err("[audio].sample_rate must be in range 8000..=192000".to_string());
}
if !(1..=2).contains(&self.audio.channels) {
return Err("[audio].channels must be 1 or 2".to_string());
}
match self.audio.frame_duration_ms {
3 | 5 | 10 | 20 | 40 | 60 => {}
_ => {
return Err(
"[audio].frame_duration_ms must be one of: 3, 5, 10, 20, 40, 60"
.to_string(),
)
}
}
if self.audio.bitrate_bps == 0 {
return Err("[audio].bitrate_bps must be > 0".to_string());
}
} }
if self.pskreporter.enabled { Ok(())
if self.pskreporter.host.trim().is_empty() {
return Err("[pskreporter].host must not be empty".to_string());
}
if self.pskreporter.port == 0 {
return Err("[pskreporter].port must be > 0".to_string());
}
if self.pskreporter.receiver_locator.is_none()
&& (self.general.latitude.is_none() || self.general.longitude.is_none())
{
return Err(
"[pskreporter] enabled requires either [pskreporter].receiver_locator \
or [general].latitude and [general].longitude"
.to_string(),
);
}
} }
if self.aprsfi.enabled { /// Check that enabled `[[rigs]]` entries do not collide with one another.
if self.aprsfi.host.trim().is_empty() { fn validate_rig_uniqueness(&self) -> Result<(), String> {
return Err("[aprsfi].host must not be empty".to_string()); if self.rigs.is_empty() {
} return Ok(());
if self.aprsfi.port == 0 {
return Err("[aprsfi].port must be > 0".to_string());
}
} }
if let Some(max_gain) = self.sdr.gain.max_value { let mut seen_ids: std::collections::HashSet<&str> = std::collections::HashSet::new();
if !max_gain.is_finite() {
return Err("[sdr.gain].max_value must be finite".to_string());
}
if max_gain < 0.0 {
return Err("[sdr.gain].max_value must be >= 0".to_string());
}
}
validate_sdr_squelch_config("[sdr.squelch]", &self.sdr.squelch)?;
validate_sdr_nb_config("[sdr.noise_blanker]", &self.sdr.noise_blanker)?;
// Multi-rig uniqueness checks.
if !self.rigs.is_empty() {
let mut seen_ids: std::collections::HashSet<String> = std::collections::HashSet::new();
let mut seen_ports: std::collections::HashSet<u16> = std::collections::HashSet::new(); let mut seen_ports: std::collections::HashSet<u16> = std::collections::HashSet::new();
let mut enabled_count = 0usize; let mut enabled_count = 0usize;
for rig in &self.rigs {
if !rig.enable { for rig in self.rigs.iter().filter(|r| r.enable) {
continue;
}
enabled_count += 1; enabled_count += 1;
// Check for explicit duplicate IDs (empty IDs are auto-generated later). // Empty ids are auto-generated later, so only explicit ones collide.
if !rig.id.trim().is_empty() && !seen_ids.insert(rig.id.clone()) { if !rig.id.trim().is_empty() && !seen_ids.insert(rig.id.as_str()) {
return Err(format!("[[rigs]] duplicate rig id: \"{}\"", rig.id)); return Err(format!("[[rigs]] duplicate rig id: \"{}\"", rig.id));
} }
if rig.audio.enabled && !seen_ports.insert(rig.audio.port) { if rig.audio.enabled && !seen_ports.insert(rig.audio.port) {
@@ -586,123 +517,25 @@ impl ServerConfig {
rig.audio.port, rig.id rig.audio.port, rig.id
)); ));
} }
if let Some(max_gain) = rig.sdr.gain.max_value {
if !max_gain.is_finite() {
return Err(format!(
"[[rigs]] [sdr.gain].max_value must be finite (rig id: \"{}\")",
rig.id
));
}
if max_gain < 0.0 {
return Err(format!(
"[[rigs]] [sdr.gain].max_value must be >= 0 (rig id: \"{}\")",
rig.id
));
}
}
validate_sdr_squelch_config(
&format!("[[rigs]] [sdr.squelch] (rig id: \"{}\")", rig.id),
&rig.sdr.squelch,
)?;
validate_sdr_nb_config(
&format!("[[rigs]] [sdr.noise_blanker] (rig id: \"{}\")", rig.id),
&rig.sdr.noise_blanker,
)?;
} }
if enabled_count == 0 { if enabled_count == 0 {
return Err( return Err(
"[[rigs]] has no enabled entries; set at least one [[rigs]].enable = true" "[[rigs]] has no enabled entries; set at least one [[rigs]].enable = true"
.to_string(), .to_string(),
); );
} }
}
if self.decode_logs.enabled {
if self.decode_logs.dir.trim().is_empty() {
return Err("[decode_logs].dir must not be empty when enabled".to_string());
}
if self.decode_logs.aprs_file.trim().is_empty()
|| self.decode_logs.cw_file.trim().is_empty()
|| self.decode_logs.ft8_file.trim().is_empty()
|| self.decode_logs.wspr_file.trim().is_empty()
{
return Err("[decode_logs] file names must not be empty when enabled".to_string());
}
}
Ok(()) Ok(())
} }
/// Validate SDR-specific config rules (see SDR.md §11). /// Validate SDR-specific config rules for every rig (see SDR.md §11).
/// Returns a Vec of error strings; empty means valid. /// Returns a Vec of error strings; empty means valid.
pub fn validate_sdr(&self) -> Vec<String> { pub fn validate_sdr(&self) -> Vec<String> {
let mut errors = Vec::new(); let multi = !self.rigs.is_empty();
self.resolved_rigs()
// Only validate if access type is "sdr" .iter()
let is_sdr = self.rig.access.access_type.as_deref() == Some("sdr"); .flat_map(|rig| validate_sdr_instance(&rig_prefix(multi, rig), rig))
if !is_sdr { .collect()
return errors;
}
// args must be non-empty
if self
.rig
.access
.args
.as_deref()
.map(str::is_empty)
.unwrap_or(true)
{
errors.push("[rig.access] args must be non-empty for type = \"sdr\"".into());
}
// sample_rate must be non-zero
if self.sdr.sample_rate == 0 {
errors.push("[sdr] sample_rate must be > 0".into());
}
// Every channel's IF must fit within the captured bandwidth
let half_rate = self.sdr.sample_rate as i64 / 2;
for ch in &self.sdr.channels {
let channel_if = self.sdr.center_offset_hz + ch.offset_hz;
if channel_if.abs() >= half_rate {
errors.push(format!(
"[sdr.channels] id=\"{}\" IF frequency {} Hz exceeds Nyquist limit ±{} Hz",
ch.id, channel_if, half_rate
));
}
}
// At most one channel may have stream_opus = true
let opus_count = self.sdr.channels.iter().filter(|c| c.stream_opus).count();
if opus_count > 1 {
errors.push(format!(
"[sdr.channels] at most one channel may have stream_opus = true (found {})",
opus_count
));
}
// tx_enabled must be false with SDR backend
if self.audio.tx_enabled {
errors.push("[audio] tx_enabled must be false when using the soapysdr backend".into());
}
// Decoder names must not appear in more than one channel
let mut seen: std::collections::HashMap<String, String> = std::collections::HashMap::new();
for ch in &self.sdr.channels {
for dec in &ch.decoders {
if let Some(prev_id) = seen.get(dec) {
errors.push(format!(
"[sdr.channels] decoder \"{}\" appears in both \"{}\" and \"{}\"",
dec, prev_id, ch.id
));
} else {
seen.insert(dec.clone(), ch.id.clone());
}
}
}
errors
} }
/// Load configuration from a specific file path. /// Load configuration from a specific file path.
@@ -817,7 +650,205 @@ fn validate_coordinates(latitude: Option<f64>, longitude: Option<f64>) -> Result
} }
} }
fn validate_access(access: &AccessConfig) -> Result<(), String> { /// Message prefix naming the rig a problem belongs to.
///
/// Empty for the legacy flat layout, so its messages read exactly as before.
fn rig_prefix(multi: bool, rig: &RigInstanceConfig) -> String {
if multi {
format!("[[rigs]] \"{}\": ", rig.id)
} else {
String::new()
}
}
/// Validate everything that belongs to a single rig.
///
/// Called once per entry of `resolved_rigs()`, which is what makes the flat
/// layout and `[[rigs]]` entries obey the same rules.
fn validate_rig_instance(
prefix: &str,
rig: &RigInstanceConfig,
general: &GeneralConfig,
) -> Result<(), String> {
if rig.rig.initial_freq_hz == 0 {
return Err(format!("{prefix}[rig].initial_freq_hz must be > 0"));
}
validate_access(prefix, &rig.rig.access)?;
if rig.behavior.poll_interval_ms == 0 {
return Err(format!("{prefix}[behavior].poll_interval_ms must be > 0"));
}
if rig.behavior.poll_interval_tx_ms == 0 {
return Err(format!("{prefix}[behavior].poll_interval_tx_ms must be > 0"));
}
if rig.behavior.max_retries == 0 {
return Err(format!("{prefix}[behavior].max_retries must be > 0"));
}
if rig.behavior.retry_base_delay_ms == 0 {
return Err(format!("{prefix}[behavior].retry_base_delay_ms must be > 0"));
}
if rig.audio.enabled {
if rig.audio.port == 0 {
return Err(format!("{prefix}[audio].port must be > 0 when audio is enabled"));
}
if !rig.audio.rx_enabled && !rig.audio.tx_enabled {
return Err(format!(
"{prefix}[audio] enabled but both rx_enabled and tx_enabled are false"
));
}
if rig.audio.sample_rate < 8_000 || rig.audio.sample_rate > 192_000 {
return Err(format!(
"{prefix}[audio].sample_rate must be in range 8000..=192000"
));
}
if !(1..=2).contains(&rig.audio.channels) {
return Err(format!("{prefix}[audio].channels must be 1 or 2"));
}
match rig.audio.frame_duration_ms {
3 | 5 | 10 | 20 | 40 | 60 => {}
_ => {
return Err(format!(
"{prefix}[audio].frame_duration_ms must be one of: 3, 5, 10, 20, 40, 60"
))
}
}
if rig.audio.bitrate_bps == 0 {
return Err(format!("{prefix}[audio].bitrate_bps must be > 0"));
}
}
if rig.pskreporter.enabled {
if rig.pskreporter.host.trim().is_empty() {
return Err(format!("{prefix}[pskreporter].host must not be empty"));
}
if rig.pskreporter.port == 0 {
return Err(format!("{prefix}[pskreporter].port must be > 0"));
}
if rig.pskreporter.receiver_locator.is_none()
&& (general.latitude.is_none() || general.longitude.is_none())
{
return Err(format!(
"{prefix}[pskreporter] enabled requires either [pskreporter].receiver_locator \
or [general].latitude and [general].longitude"
));
}
}
if rig.aprsfi.enabled {
if rig.aprsfi.host.trim().is_empty() {
return Err(format!("{prefix}[aprsfi].host must not be empty"));
}
if rig.aprsfi.port == 0 {
return Err(format!("{prefix}[aprsfi].port must be > 0"));
}
}
if let Some(max_gain) = rig.sdr.gain.max_value {
if !max_gain.is_finite() {
return Err(format!("{prefix}[sdr.gain].max_value must be finite"));
}
if max_gain < 0.0 {
return Err(format!("{prefix}[sdr.gain].max_value must be >= 0"));
}
}
validate_sdr_squelch_config(&format!("{prefix}[sdr.squelch]"), &rig.sdr.squelch)?;
validate_sdr_nb_config(&format!("{prefix}[sdr.noise_blanker]"), &rig.sdr.noise_blanker)?;
if rig.decode_logs.enabled {
if rig.decode_logs.dir.trim().is_empty() {
return Err(format!(
"{prefix}[decode_logs].dir must not be empty when enabled"
));
}
if rig.decode_logs.aprs_file.trim().is_empty()
|| rig.decode_logs.cw_file.trim().is_empty()
|| rig.decode_logs.ft8_file.trim().is_empty()
|| rig.decode_logs.wspr_file.trim().is_empty()
{
return Err(format!(
"{prefix}[decode_logs] file names must not be empty when enabled"
));
}
}
Ok(())
}
/// Validate the SDR pipeline of a single rig (see SDR.md §11).
///
/// Returns every problem found rather than stopping at the first, so a
/// misconfigured channel list can be fixed in one pass.
fn validate_sdr_instance(prefix: &str, rig: &RigInstanceConfig) -> Vec<String> {
let mut errors = Vec::new();
if rig.rig.access.access_type.as_deref() != Some("sdr") {
return errors;
}
if rig
.rig
.access
.args
.as_deref()
.map(str::is_empty)
.unwrap_or(true)
{
errors.push(format!(
"{prefix}[rig.access] args must be non-empty for type = \"sdr\""
));
}
if rig.sdr.sample_rate == 0 {
errors.push(format!("{prefix}[sdr] sample_rate must be > 0"));
}
// Every channel's IF must fit within the captured bandwidth.
let half_rate = rig.sdr.sample_rate as i64 / 2;
for ch in &rig.sdr.channels {
let channel_if = rig.sdr.center_offset_hz + ch.offset_hz;
if channel_if.abs() >= half_rate {
errors.push(format!(
"{prefix}[sdr.channels] id=\"{}\" IF frequency {} Hz exceeds Nyquist limit ±{} Hz",
ch.id, channel_if, half_rate
));
}
}
let opus_count = rig.sdr.channels.iter().filter(|c| c.stream_opus).count();
if opus_count > 1 {
errors.push(format!(
"{prefix}[sdr.channels] at most one channel may have stream_opus = true (found {})",
opus_count
));
}
if rig.audio.tx_enabled {
errors.push(format!(
"{prefix}[audio] tx_enabled must be false when using the soapysdr backend"
));
}
// A decoder may only be fed by one channel.
let mut seen: std::collections::HashMap<&str, &str> = std::collections::HashMap::new();
for ch in &rig.sdr.channels {
for dec in &ch.decoders {
if let Some(prev_id) = seen.get(dec.as_str()) {
errors.push(format!(
"{prefix}[sdr.channels] decoder \"{}\" appears in both \"{}\" and \"{}\"",
dec, prev_id, ch.id
));
} else {
seen.insert(dec.as_str(), ch.id.as_str());
}
}
}
errors
}
fn validate_access(prefix: &str, access: &AccessConfig) -> Result<(), String> {
let serial_fields_set = access.port.is_some() || access.baud.is_some(); let serial_fields_set = access.port.is_some() || access.baud.is_some();
let tcp_fields_set = access.host.is_some() || access.tcp_port.is_some(); let tcp_fields_set = access.host.is_some() || access.tcp_port.is_some();
@@ -828,38 +859,34 @@ fn validate_access(access: &AccessConfig) -> Result<(), String> {
match access.access_type.as_deref().unwrap_or("serial") { match access.access_type.as_deref().unwrap_or("serial") {
"serial" => { "serial" => {
if access.port.as_deref().unwrap_or("").trim().is_empty() { if access.port.as_deref().unwrap_or("").trim().is_empty() {
return Err( return Err(format!(
"[rig.access].port must be set for serial access ([rig.access].type='serial')" "{prefix}[rig.access].port must be set for serial access ([rig.access].type='serial')"
.to_string(), ));
);
} }
if access.baud.unwrap_or(0) == 0 { if access.baud.unwrap_or(0) == 0 {
return Err( return Err(format!(
"[rig.access].baud must be > 0 for serial access ([rig.access].type='serial')" "{prefix}[rig.access].baud must be > 0 for serial access ([rig.access].type='serial')"
.to_string(), ));
);
} }
} }
"tcp" => { "tcp" => {
if access.host.as_deref().unwrap_or("").trim().is_empty() { if access.host.as_deref().unwrap_or("").trim().is_empty() {
return Err( return Err(format!(
"[rig.access].host must be set for tcp access ([rig.access].type='tcp')" "{prefix}[rig.access].host must be set for tcp access ([rig.access].type='tcp')"
.to_string(), ));
);
} }
if access.tcp_port.unwrap_or(0) == 0 { if access.tcp_port.unwrap_or(0) == 0 {
return Err( return Err(format!(
"[rig.access].tcp_port must be > 0 for tcp access ([rig.access].type='tcp')" "{prefix}[rig.access].tcp_port must be > 0 for tcp access ([rig.access].type='tcp')"
.to_string(), ));
);
} }
} }
"sdr" => { "sdr" => {
// SDR-specific validation is handled by validate_sdr() // SDR-specific validation is handled by validate_sdr_instance().
} }
other => { other => {
return Err(format!( return Err(format!(
"[rig.access].type '{}' is invalid (expected 'serial', 'tcp', or 'sdr')", "{prefix}[rig.access].type '{}' is invalid (expected 'serial', 'tcp', or 'sdr')",
other other
)) ))
} }
@@ -1499,6 +1526,140 @@ port = 4531
); );
} }
// --- Per-rig validation: [[rigs]] entries obey the same rules as the
// legacy flat layout, which they previously escaped entirely. ---
#[test]
fn test_validate_rejects_bad_audio_in_rig_entry() {
let toml_str = r#"
[[rigs]]
id = "hf"
[rigs.rig]
model = "ft817"
[rigs.rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
[rigs.audio]
port = 4531
frame_duration_ms = 7
"#;
let cfg: ServerConfig = toml::from_str(toml_str).unwrap();
let err = cfg
.validate()
.expect_err("expected the rig entry's audio config to be validated");
assert!(
err.contains("frame_duration_ms") && err.contains("hf"),
"unexpected error: {err}"
);
}
#[test]
fn test_validate_rejects_incomplete_access_in_rig_entry() {
let toml_str = r#"
[[rigs]]
id = "hf"
[rigs.rig]
model = "ft817"
[rigs.rig.access]
type = "serial"
port = "/dev/ttyUSB0"
[rigs.audio]
port = 4531
"#;
let cfg: ServerConfig = toml::from_str(toml_str).unwrap();
let err = cfg
.validate()
.expect_err("expected the rig entry's access config to be validated");
assert!(err.contains("baud"), "unexpected error: {err}");
}
#[test]
fn test_validate_sdr_checks_rig_entries() {
// sample_rate 1_000_000 => Nyquist 500_000; the channel sits beyond it,
// two channels claim the Opus stream, and TX is on with an SDR backend.
let toml_str = r#"
[[rigs]]
id = "sdr"
[rigs.rig]
model = "soapysdr"
[rigs.rig.access]
type = "sdr"
args = "driver=rtlsdr"
[rigs.audio]
port = 4532
tx_enabled = true
[rigs.sdr]
sample_rate = 1000000
center_offset_hz = 0
[[rigs.sdr.channels]]
id = "ch_high"
offset_hz = 600000
stream_opus = true
[[rigs.sdr.channels]]
id = "ch_two"
offset_hz = 10000
stream_opus = true
"#;
let cfg: ServerConfig = toml::from_str(toml_str).unwrap();
let errors = cfg.validate_sdr();
assert!(
errors.iter().any(|e| e.contains("Nyquist")),
"expected a Nyquist error, got: {errors:?}"
);
assert!(
errors.iter().any(|e| e.contains("stream_opus")),
"expected a stream_opus error, got: {errors:?}"
);
assert!(
errors.iter().any(|e| e.contains("tx_enabled")),
"expected a tx_enabled error, got: {errors:?}"
);
assert!(
errors.iter().all(|e| e.contains("sdr")),
"every error should name the rig it belongs to: {errors:?}"
);
}
#[test]
fn test_validate_sdr_ignores_non_sdr_rig_entries() {
let toml_str = r#"
[[rigs]]
id = "hf"
[rigs.rig]
model = "ft817"
[rigs.rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
[rigs.audio]
port = 4531
"#;
let cfg: ServerConfig = toml::from_str(toml_str).unwrap();
assert!(cfg.validate_sdr().is_empty());
}
#[test]
fn test_validate_rejects_bad_squelch_in_rig_entry() {
let toml_str = r#"
[[rigs]]
id = "sdr"
[rigs.rig]
model = "soapysdr"
[rigs.rig.access]
type = "sdr"
args = "driver=rtlsdr"
[rigs.audio]
port = 4532
tx_enabled = false
[rigs.sdr.squelch]
threshold_db = 10.0
"#;
let cfg: ServerConfig = toml::from_str(toml_str).unwrap();
let err = cfg.validate().expect_err("expected a squelch error");
assert!(err.contains("threshold_db"), "unexpected error: {err}");
}
#[test] #[test]
fn test_validate_accepts_multi_rig_unique_ids_and_ports() { fn test_validate_accepts_multi_rig_unique_ids_and_ports() {
let toml_str = r#" let toml_str = r#"