From 22457959db7a96744f05acb46765c373b497b310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Wed, 19 Aug 2026 18:58:39 +0000 Subject: [PATCH 1/4] Make biquad output clamps atomic settings --- src/iir/config.rs | 184 +++++++++++++++++++++++++++++++++------------- 1 file changed, 134 insertions(+), 50 deletions(-) diff --git a/src/iir/config.rs b/src/iir/config.rs index 91c782c..4e2c3fa 100644 --- a/src/iir/config.rs +++ b/src/iir/config.rs @@ -4,7 +4,7 @@ use core::any::Any; use miniconf::Tree; use num_traits::{AsPrimitive, Float, FloatConst}; -use serde::{Serialize, de::DeserializeOwned}; +use serde::{Deserialize, Deserializer, Serialize, de::DeserializeOwned}; use crate::{ Build, @@ -15,6 +15,79 @@ use crate::{ }, }; +/// Optional output bounds. Missing bounds are unbounded. +#[derive(Clone, Copy, Debug, PartialEq, Serialize)] +pub struct ClampConfig { + /// Lower output bound. + pub min: Option, + /// Upper output bound. + pub max: Option, +} + +impl ClampConfig { + /// Create a bounded output range. + pub const fn bounded(min: T, max: T) -> Self { + Self { + min: Some(min), + max: Some(max), + } + } +} + +impl Default for ClampConfig { + fn default() -> Self { + Self { + min: None, + max: None, + } + } +} + +impl ClampConfig { + fn validate(&self) -> Result<(), Error> { + if self.min.is_some_and(|value| !value.is_finite()) + || self.max.is_some_and(|value| !value.is_finite()) + { + return Err(Error::NonFinite("output_clamp")); + } + if self.min.zip(self.max).is_some_and(|(min, max)| min > max) { + return Err(Error::InvertedRange("output_clamp")); + } + Ok(()) + } + + fn limits(&self) -> (T, T) { + ( + self.min.unwrap_or_else(T::neg_infinity), + self.max.unwrap_or_else(T::infinity), + ) + } +} + +impl<'de, T> Deserialize<'de> for ClampConfig +where + T: Deserialize<'de> + Float, +{ + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + struct Repr { + min: Option, + max: Option, + } + + let repr = Repr::deserialize(deserializer)?; + let clamp = Self { + min: repr.min, + max: repr.max, + }; + clamp.validate().map_err(serde::de::Error::custom)?; + Ok(clamp) + } +} + /// Floating point BA coefficients before quantization. #[derive(Debug, Clone, Tree)] #[tree(meta(doc, typename))] @@ -24,10 +97,9 @@ pub struct BaConfig { pub ba: [[T; 3]; 2], /// Summing junction offset. pub offset: T, - /// Output lower limit. - pub min: T, - /// Output upper limit. - pub max: T, + /// Output clamp. + #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned + Float", any="T: Any"))] + pub clamp: ClampConfig, } impl Default for BaConfig { @@ -35,8 +107,7 @@ impl Default for BaConfig { Self { ba: [[T::zero(); 3], [T::one(), T::zero(), T::zero()]], offset: T::zero(), - min: T::neg_infinity(), - max: T::infinity(), + clamp: ClampConfig::default(), } } } @@ -61,10 +132,9 @@ pub struct FilterConfig { pub shape: Shape, /// Summing junction offset. pub offset: T, - /// Lower output limit. - pub min: T, - /// Upper output limit. - pub max: T, + /// Output clamp. + #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned + Float", any="T: Any"))] + pub clamp: ClampConfig, } impl Default for FilterConfig { @@ -76,8 +146,7 @@ impl Default for FilterConfig { shelf_db: T::zero(), shape: Shape::default(), offset: T::zero(), - min: T::neg_infinity(), - max: T::infinity(), + clamp: ClampConfig::default(), } } } @@ -143,14 +212,11 @@ pub struct PidConfig { /// /// Units: input. pub setpoint: T, - /// Output lower limit. + /// Output clamp. /// /// Units: output. - pub min: T, - /// Output upper limit. - /// - /// Units: output. - pub max: T, + #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned + Float", any="T: Any"))] + pub clamp: ClampConfig, } impl Default for PidConfig { @@ -160,8 +226,7 @@ impl Default for PidConfig { gain: GainsConfig::default(), limit: GainsConfig::splat(T::infinity()), setpoint: T::zero(), - min: T::neg_infinity(), - max: T::infinity(), + clamp: ClampConfig::default(), } } } @@ -180,15 +245,31 @@ mod tests { #[test] fn biquad_config_try_build_rejects_invalid_ba_config() { let ba = BaConfig { - min: 1.0, - max: 0.0, + clamp: ClampConfig::bounded(1.0, 0.0), ..Default::default() }; let config = BiquadConfig::::Ba(ba); assert_eq!( config.try_build(&Units::default()), - Err(Error::InvertedRange("output_limits")) + Err(Error::InvertedRange("output_clamp")) + ); + } + + #[test] + fn clamp_validation_checks_the_complete_range() { + assert!(ClampConfig::bounded(-2.0, 2.0).validate().is_ok()); + assert_eq!( + ClampConfig::bounded(3.0, 2.0).validate(), + Err(Error::InvertedRange("output_clamp")) + ); + assert_eq!( + ClampConfig { + min: Some(f32::INFINITY), + max: None + } + .validate(), + Err(Error::NonFinite("output_clamp")) ); } @@ -205,15 +286,16 @@ mod tests { } } -impl From<&PidConfig> for Pid { +impl From<&PidConfig> for Pid { fn from(config: &PidConfig) -> Self { + let (min, max) = config.clamp.limits(); Self { order: config.order, gain: Gains::from(&config.gain), limit: Gains::from(&config.limit), setpoint: config.setpoint, - min: config.min, - max: config.max, + min, + max, } } } @@ -306,24 +388,17 @@ where } } -fn check_offset_limits( - name: &'static str, - offset: T, - min: T, - max: T, -) -> Result<(), Error> { +fn check_offset(offset: T) -> Result<(), Error> { if !offset.is_finite() { return Err(Error::NonFinite("offset")); } - if min.is_nan() || max.is_nan() { - return Err(Error::NonFinite(name)); - } - if min > max { - return Err(Error::InvertedRange(name)); - } Ok(()) } +fn check_clamp(clamp: &ClampConfig) -> Result<(), Error> { + clamp.validate() +} + fn check_units(units: &Units, check_t: bool) -> Result<(), Error> { for (name, value) in [("x", units.x), ("y", units.y)] { if !value.is_finite() { @@ -359,17 +434,19 @@ where let yx = units.x * yu; match self { Self::Ba(ba) => { + let (min, max) = ba.clamp.limits(); let mut bba = ba.ba; bba[0] = bba[0].map(|b| b * yx); let mut b: BiquadClamp = bba.into(); b.u = (ba.offset * yu).as_(); - b.min = (ba.min * yu).as_(); - b.max = (ba.max * yu).as_(); + b.min = (min * yu).as_(); + b.max = (max * yu).as_(); b } Self::Raw(raw) => raw.clone(), Self::Pid(pid) => Pid::from(pid).build(units), Self::Filter(filter) => { + let (min, max) = filter.clamp.limits(); let mut f = Filter::default(); f.gain_db(filter.gain_db); f.critical_frequency(filter.frequency * units.t); @@ -379,8 +456,8 @@ where ba[0] = ba[0].map(|b| b * yx); let mut b: BiquadClamp = ba.into(); b.u = (filter.offset * yu).as_(); - b.min = (filter.min * yu).as_(); - b.max = (filter.max * yu).as_(); + b.min = (min * yu).as_(); + b.max = (max * yu).as_(); b } } @@ -391,7 +468,9 @@ where match self { Self::Ba(ba) => { check_units(units, false)?; - check_offset_limits("output_limits", ba.offset, ba.min, ba.max)?; + check_offset(ba.offset)?; + check_clamp(&ba.clamp)?; + let (min, max) = ba.clamp.limits(); let yu = units.y.recip(); let yx = units.x * yu; for row in ba.ba { @@ -405,15 +484,20 @@ where bba[0] = bba[0].map(|b| b * yx); let mut b: BiquadClamp = bba.into(); b.u = (ba.offset * yu).as_(); - b.min = (ba.min * yu).as_(); - b.max = (ba.max * yu).as_(); + b.min = (min * yu).as_(); + b.max = (max * yu).as_(); Ok(b) } Self::Raw(raw) => Ok(raw.clone()), - Self::Pid(pid) => Pid::from(pid).try_build(units), + Self::Pid(pid) => { + check_clamp(&pid.clamp)?; + Pid::from(pid).try_build(units) + } Self::Filter(filter) => { check_units(units, true)?; - check_offset_limits("output_limits", filter.offset, filter.min, filter.max)?; + check_offset(filter.offset)?; + check_clamp(&filter.clamp)?; + let (min, max) = filter.clamp.limits(); let yu = units.y.recip(); let yx = units.x * yu; let mut f = Filter::default(); @@ -425,8 +509,8 @@ where ba[0] = ba[0].map(|b| b * yx); let mut b: BiquadClamp = ba.into(); b.u = (filter.offset * yu).as_(); - b.min = (filter.min * yu).as_(); - b.max = (filter.max * yu).as_(); + b.min = (min * yu).as_(); + b.max = (max * yu).as_(); Ok(b) } } From 9e4b77d1a72d00923e81112a488d3a5bca4173ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Wed, 19 Aug 2026 19:33:42 +0000 Subject: [PATCH 2/4] Use atomic interval biquad clamps --- Cargo.toml | 1 + src/iir/config.rs | 167 +++++++++++++++++++++++++--------------------- 2 files changed, 92 insertions(+), 76 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4b9223d..0837123 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ pyo3 = { version = "0.29.2", features = ["extension-module"], optional = true } numpy = { version = "0.29", optional = true } [dev-dependencies] +miniconf = { version = "0.21.0", default-features = false, features = ["json-core"] } quickcheck = "1.0.3" quickcheck_macros = "1.0.0" rand = "0.10.2" diff --git a/src/iir/config.rs b/src/iir/config.rs index 4e2c3fa..9620b97 100644 --- a/src/iir/config.rs +++ b/src/iir/config.rs @@ -2,9 +2,9 @@ use core::any::Any; -use miniconf::Tree; +use miniconf::{Tree, leaf}; use num_traits::{AsPrimitive, Float, FloatConst}; -use serde::{Deserialize, Deserializer, Serialize, de::DeserializeOwned}; +use serde::{Deserialize, Deserializer, Serialize, Serializer, de::DeserializeOwned}; use crate::{ Build, @@ -15,53 +15,62 @@ use crate::{ }, }; -/// Optional output bounds. Missing bounds are unbounded. -#[derive(Clone, Copy, Debug, PartialEq, Serialize)] -pub struct ClampConfig { - /// Lower output bound. - pub min: Option, - /// Upper output bound. - pub max: Option, +/// Output bounds `[min, max]`. +/// +/// Serde maps an unbounded endpoint between `null` and the corresponding infinity. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct ClampConfig([T; 2]); + +impl Serialize for ClampConfig +where + T: Serialize + Float, +{ + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + [ + (self.0[0] != T::neg_infinity()).then_some(self.0[0]), + (self.0[1] != T::infinity()).then_some(self.0[1]), + ] + .serialize(serializer) + } } impl ClampConfig { - /// Create a bounded output range. - pub const fn bounded(min: T, max: T) -> Self { - Self { - min: Some(min), - max: Some(max), - } + /// Create an output range from lower and upper bounds. + pub const fn new(min: T, max: T) -> Self { + Self([min, max]) } } -impl Default for ClampConfig { +impl ClampConfig { + /// Return `[min, max]`. + pub const fn bounds(&self) -> [T; 2] { + self.0 + } +} + +impl Default for ClampConfig { fn default() -> Self { - Self { - min: None, - max: None, - } + Self([T::neg_infinity(), T::infinity()]) } } impl ClampConfig { fn validate(&self) -> Result<(), Error> { - if self.min.is_some_and(|value| !value.is_finite()) - || self.max.is_some_and(|value| !value.is_finite()) + if self.0[0].is_nan() + || self.0[1].is_nan() + || self.0[0] == T::infinity() + || self.0[1] == T::neg_infinity() { return Err(Error::NonFinite("output_clamp")); } - if self.min.zip(self.max).is_some_and(|(min, max)| min > max) { + if self.0[0] > self.0[1] { return Err(Error::InvertedRange("output_clamp")); } Ok(()) } - - fn limits(&self) -> (T, T) { - ( - self.min.unwrap_or_else(T::neg_infinity), - self.max.unwrap_or_else(T::infinity), - ) - } } impl<'de, T> Deserialize<'de> for ClampConfig @@ -72,17 +81,11 @@ where where D: Deserializer<'de>, { - #[derive(Deserialize)] - struct Repr { - min: Option, - max: Option, - } - - let repr = Repr::deserialize(deserializer)?; - let clamp = Self { - min: repr.min, - max: repr.max, - }; + let bounds = <[Option; 2]>::deserialize(deserializer)?; + let clamp = Self([ + bounds[0].unwrap_or_else(T::neg_infinity), + bounds[1].unwrap_or_else(T::infinity), + ]); clamp.validate().map_err(serde::de::Error::custom)?; Ok(clamp) } @@ -93,12 +96,12 @@ where #[tree(meta(doc, typename))] pub struct BaConfig { /// Coefficient array: [[b0, b1, b2], [a0, a1, a2]] - #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned", any="T: Any"))] + #[tree(with=leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned", any="T: Any"))] pub ba: [[T; 3]; 2], /// Summing junction offset. pub offset: T, /// Output clamp. - #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned + Float", any="T: Any"))] + #[tree(with=leaf, bounds(serialize="T: Serialize + Float", deserialize="T: DeserializeOwned + Float", any="T: Any"))] pub clamp: ClampConfig, } @@ -117,7 +120,7 @@ impl Default for BaConfig { #[tree(meta(doc, typename))] pub struct FilterConfig { /// Filter style. - #[tree(with=miniconf::leaf)] + #[tree(with=leaf)] pub typ: Type, /// Relative critical frequency in units of the sample rate. pub frequency: T, @@ -128,12 +131,12 @@ pub struct FilterConfig { /// Used for peaking, low shelf, and high shelf filters. pub shelf_db: T, /// Q, bandwidth, or slope. - #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned", any="T: Any"))] + #[tree(with=leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned", any="T: Any"))] pub shape: Shape, /// Summing junction offset. pub offset: T, /// Output clamp. - #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned + Float", any="T: Any"))] + #[tree(with=leaf, bounds(serialize="T: Serialize + Float", deserialize="T: DeserializeOwned + Float", any="T: Any"))] pub clamp: ClampConfig, } @@ -191,7 +194,7 @@ impl From<&GainsConfig> for Gains { #[tree(meta(doc, typename))] pub struct PidConfig { /// Feedback term order. - #[tree(with=miniconf::leaf)] + #[tree(with=leaf)] pub order: Order, /// Gain. /// @@ -215,7 +218,7 @@ pub struct PidConfig { /// Output clamp. /// /// Units: output. - #[tree(with=miniconf::leaf, bounds(serialize="T: Serialize", deserialize="T: DeserializeOwned + Float", any="T: Any"))] + #[tree(with=leaf, bounds(serialize="T: Serialize + Float", deserialize="T: DeserializeOwned + Float", any="T: Any"))] pub clamp: ClampConfig, } @@ -234,6 +237,7 @@ impl Default for PidConfig { #[cfg(test)] mod tests { use super::*; + use miniconf::json_core; #[test] fn biquad_config_tag_conversion() { @@ -245,7 +249,7 @@ mod tests { #[test] fn biquad_config_try_build_rejects_invalid_ba_config() { let ba = BaConfig { - clamp: ClampConfig::bounded(1.0, 0.0), + clamp: ClampConfig::new(1.0, 0.0), ..Default::default() }; let config = BiquadConfig::::Ba(ba); @@ -257,20 +261,35 @@ mod tests { } #[test] - fn clamp_validation_checks_the_complete_range() { - assert!(ClampConfig::bounded(-2.0, 2.0).validate().is_ok()); - assert_eq!( - ClampConfig::bounded(3.0, 2.0).validate(), - Err(Error::InvertedRange("output_clamp")) - ); - assert_eq!( - ClampConfig { - min: Some(f32::INFINITY), - max: None - } - .validate(), - Err(Error::NonFinite("output_clamp")) - ); + fn clamp_miniconf_leaf_requires_a_complete_valid_range() { + let mut config = BaConfig:: { + clamp: ClampConfig::new(-1.0, 1.0), + ..Default::default() + }; + + json_core::set(&mut config, "/clamp", br#"[-2.0,2.0]"#).unwrap(); + assert_eq!(config.clamp, ClampConfig::new(-2.0, 2.0)); + + for value in [ + br#"[-3.0]"#.as_slice(), + br#"[-3.0,3.0,4.0]"#, + br#"[3.0,-3.0]"#, + br#"{"min":-3.0,"max":3.0}"#, + ] { + assert!(json_core::set(&mut config, "/clamp", value).is_err()); + assert_eq!(config.clamp, ClampConfig::new(-2.0, 2.0)); + } + + json_core::set(&mut config, "/clamp", br#"[null,2.0]"#).unwrap(); + assert_eq!(config.clamp, ClampConfig::new(f32::NEG_INFINITY, 2.0)); + + let mut data = [0; 64]; + let len = json_core::get(&config, "/clamp", &mut data).unwrap(); + assert_eq!(&data[..len], br#"[null,2.0]"#); + let mut copy = BaConfig::::default(); + json_core::set(&mut copy, "/clamp", &data[..len]).unwrap(); + assert_eq!(copy.clamp, config.clamp); + assert!(json_core::set(&mut copy, "/clamp/0", b"-1.0").is_err()); } #[test] @@ -288,7 +307,7 @@ mod tests { impl From<&PidConfig> for Pid { fn from(config: &PidConfig) -> Self { - let (min, max) = config.clamp.limits(); + let [min, max] = config.clamp.bounds(); Self { order: config.order, gain: Gains::from(&config.gain), @@ -327,7 +346,7 @@ where Ba(BaConfig), /// Raw, unscaled, possibly fixed-point machine-unit coefficients. Raw( - #[tree(with=miniconf::leaf, bounds( + #[tree(with=leaf, bounds( serialize="C: Serialize, Y: Serialize", deserialize="C: DeserializeOwned, Y: DeserializeOwned", any="C: Any, Y: Any"))] @@ -395,10 +414,6 @@ fn check_offset(offset: T) -> Result<(), Error> { Ok(()) } -fn check_clamp(clamp: &ClampConfig) -> Result<(), Error> { - clamp.validate() -} - fn check_units(units: &Units, check_t: bool) -> Result<(), Error> { for (name, value) in [("x", units.x), ("y", units.y)] { if !value.is_finite() { @@ -434,7 +449,7 @@ where let yx = units.x * yu; match self { Self::Ba(ba) => { - let (min, max) = ba.clamp.limits(); + let [min, max] = ba.clamp.bounds(); let mut bba = ba.ba; bba[0] = bba[0].map(|b| b * yx); let mut b: BiquadClamp = bba.into(); @@ -446,7 +461,7 @@ where Self::Raw(raw) => raw.clone(), Self::Pid(pid) => Pid::from(pid).build(units), Self::Filter(filter) => { - let (min, max) = filter.clamp.limits(); + let [min, max] = filter.clamp.bounds(); let mut f = Filter::default(); f.gain_db(filter.gain_db); f.critical_frequency(filter.frequency * units.t); @@ -469,8 +484,8 @@ where Self::Ba(ba) => { check_units(units, false)?; check_offset(ba.offset)?; - check_clamp(&ba.clamp)?; - let (min, max) = ba.clamp.limits(); + ba.clamp.validate()?; + let [min, max] = ba.clamp.bounds(); let yu = units.y.recip(); let yx = units.x * yu; for row in ba.ba { @@ -490,14 +505,14 @@ where } Self::Raw(raw) => Ok(raw.clone()), Self::Pid(pid) => { - check_clamp(&pid.clamp)?; + pid.clamp.validate()?; Pid::from(pid).try_build(units) } Self::Filter(filter) => { check_units(units, true)?; check_offset(filter.offset)?; - check_clamp(&filter.clamp)?; - let (min, max) = filter.clamp.limits(); + filter.clamp.validate()?; + let [min, max] = filter.clamp.bounds(); let yu = units.y.recip(); let yx = units.x * yu; let mut f = Filter::default(); From 6fe314d2f0dfda81345e44bb6df2f01aaa4fa49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Wed, 19 Aug 2026 22:27:59 +0000 Subject: [PATCH 3/4] Clarify PID bound validation --- src/iir/config.rs | 33 +++++++++++++++++++++++++++------ src/iir/pid.rs | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/iir/config.rs b/src/iir/config.rs index 9620b97..0f13a08 100644 --- a/src/iir/config.rs +++ b/src/iir/config.rs @@ -59,14 +59,13 @@ impl Default for ClampConfig { impl ClampConfig { fn validate(&self) -> Result<(), Error> { - if self.0[0].is_nan() - || self.0[1].is_nan() - || self.0[0] == T::infinity() - || self.0[1] == T::neg_infinity() + let [min, max] = self.0; + if !(min.is_finite() || min == T::neg_infinity()) + || !(max.is_finite() || max == T::infinity()) { return Err(Error::NonFinite("output_clamp")); } - if self.0[0] > self.0[1] { + if min > max { return Err(Error::InvertedRange("output_clamp")); } Ok(()) @@ -210,6 +209,7 @@ pub struct PidConfig { /// * P gain limit is ignored /// * Limits outside the range `order..order + 3` are ignored /// * P gain sign determines sign of all gain limits + /// * `null` selects an unbounded limit pub limit: GainsConfig, /// Setpoint. /// @@ -303,15 +303,36 @@ mod tests { assert!(config.try_build(&units).is_ok()); } + + #[test] + fn pid_null_gain_limit_is_unbounded() { + let mut pid = PidConfig::::default(); + json_core::set(&mut pid, "/limit/i", b"null").unwrap(); + let runtime = Pid::from(&pid); + + let mut data = [0; 8]; + let len = json_core::get(&pid, "/limit/i", &mut data).unwrap(); + assert_eq!(&data[..len], b"null"); + assert_eq!( + runtime.limit.value[crate::iir::pid::Action::I as usize], + f32::INFINITY + ); + let config: BiquadConfig = BiquadConfig::Pid(pid); + assert!(config.try_build(&Units::default()).is_ok()); + } } impl From<&PidConfig> for Pid { fn from(config: &PidConfig) -> Self { let [min, max] = config.clamp.bounds(); + let mut limit = Gains::from(&config.limit); + limit.value = limit + .value + .map(|value| if value.is_nan() { T::infinity() } else { value }); Self { order: config.order, gain: Gains::from(&config.gain), - limit: Gains::from(&config.limit), + limit, setpoint: config.setpoint, min, max, diff --git a/src/iir/pid.rs b/src/iir/pid.rs index 9c2e377..63df6a7 100644 --- a/src/iir/pid.rs +++ b/src/iir/pid.rs @@ -201,12 +201,11 @@ impl Builder { if *period <= T::zero() { return Err(Error::NonPositive("period")); } - for (name, values) in [("gain", &self.gain), ("limit", &self.limit)] { - for value in values { - if !value.is_finite() && !value.is_infinite() { - return Err(Error::NonFinite(name)); - } - } + if self.gain.iter().any(|value| !value.is_finite()) { + return Err(Error::NonFinite("gain")); + } + if self.limit.iter().any(|value| value.is_nan()) { + return Err(Error::NonFinite("limit")); } for action in [Action::I2, Action::I, Action::D, Action::D2] { let gain = self.gain[action as usize]; @@ -546,13 +545,7 @@ where let p = self.gain.value[Action::P as usize]; let mut biquad: BiquadClamp = Builder { gain: self.gain.value.map(|g| yx * g.copysign(p)), - limit: self.limit.value.map(|mut l| { - // infinite gain limit is meaningful but json can only do null/nan - if l.is_nan() { - l = T::infinity() - } - yx * l.copysign(p) - }), + limit: self.limit.value.map(|l| yx * l.copysign(p)), order: self.order, } .build(&units.t) @@ -617,4 +610,27 @@ mod test { ); } } + + #[test] + fn validation_distinguishes_gains_and_limits() { + let period = 1.0; + assert_eq!( + pid::Builder::default() + .gain(pid::Action::P, f32::INFINITY) + .validate(&period), + Err(Error::NonFinite("gain")) + ); + assert!( + pid::Builder::default() + .limit(pid::Action::I, f32::INFINITY) + .validate(&period) + .is_ok() + ); + assert_eq!( + pid::Builder::default() + .limit(pid::Action::I, f32::NAN) + .validate(&period), + Err(Error::NonFinite("limit")) + ); + } } From 8ed4aab218afbf053398629af519cda86f358d7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 20 Aug 2026 10:07:49 +0200 Subject: [PATCH 4/4] changelog: ClampConfig --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b051939..2ca6799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ See also: ## [UNRELEASED](https://github.com/quartiq/idsp/compare/v0.22.1...HEAD) - DATE +### Changed + +* `iir::{FilterConfig,BaConfig,PidConfig}` take `ClampConfig([T; 2])` to make + clamp changes atomic + ## [0.22.1](https://github.com/quartiq/idsp/compare/v0.22.0...v0.22.1) - 2026-08-04 ### Added