From 3754f22137fc82e90572d4781e73065802326391 Mon Sep 17 00:00:00 2001 From: Raffael Rott Date: Sun, 12 Apr 2026 16:20:24 +0200 Subject: [PATCH 01/19] Added basic config --- src/config/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config/mod.rs b/src/config/mod.rs index 3c04e19..f47e448 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -20,3 +20,4 @@ pub fn load_config(path: &str) -> Result { .try_deserialize() .with_context(|| format!("Failed to deserialize config from {}", path)) } + From 227e4f7ad0daa1fc6deee66de873686649d6b506 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 4 May 2026 03:47:03 +0200 Subject: [PATCH 02/19] add mapping.toml support Supports specifying: - a unique name for fields - a slope + offset conversion for a "mapped value" - a range-based conversion for a "logical value", that also includes a color code for the frontend The NodeManager now supports reading raw, mapped, and logical values by mapping name, requesting mapped fields from nodes, and writing mapped parameter values back as raw CAN values. --- Cargo.lock | 9 + Cargo.toml | 1 + src/config/mod.rs | 1 + src/lib.rs | 4 +- src/nodes/mapping.rs | 725 ++++++++++++++++++++++++++++++++++++ src/nodes/mod.rs | 1 + src/nodes/node_manager.rs | 349 ++++++++++++++++- tests/emulator.rs | 6 +- tests/mapping/example1.toml | 41 ++ 9 files changed, 1131 insertions(+), 6 deletions(-) create mode 100644 src/nodes/mapping.rs create mode 100644 tests/mapping/example1.toml diff --git a/Cargo.lock b/Cargo.lock index af7b163..fb619bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -686,6 +686,7 @@ dependencies = [ "serde_json", "socketcan", "testcontainers", + "toml 1.1.2+spec-1.1.0", ] [[package]] @@ -2547,10 +2548,12 @@ version = "1.1.2+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81f3d15e84cbcd896376e6730314d59fb5a87f31e4b038454184435cd57defee" dependencies = [ + "indexmap 2.14.0", "serde_core", "serde_spanned", "toml_datetime 1.1.1+spec-1.1.0", "toml_parser", + "toml_writer", "winnow 1.0.1", ] @@ -2581,6 +2584,12 @@ dependencies = [ "winnow 1.0.1", ] +[[package]] +name = "toml_writer" +version = "1.1.1+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" + [[package]] name = "tonic" version = "0.14.5" diff --git a/Cargo.toml b/Cargo.toml index f024f09..f6b371a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ serde = { version = "1.0.228", features = ["derive"] } # Test-only helper binary dependencies (enabled via feature) caps = { version = "0.5", optional = true } libc = { version = "0.2", optional = true } +toml = "1.1.2" [features] # Enables the `ferroflow-vcan` helper binary used by integration tests. diff --git a/src/config/mod.rs b/src/config/mod.rs index f47e448..06a48e7 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -9,6 +9,7 @@ pub struct Config { pub can_bus_interfaces: Vec, pub heartbeat_period: u64, pub database_url: String, + pub mapping_path: String, } pub fn load_config(path: &str) -> Result { diff --git a/src/lib.rs b/src/lib.rs index f75ccd3..3593e12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,7 +13,9 @@ pub mod socket; pub fn run_with_config(config: Config) -> anyhow::Result<()> { let event_dispatcher = events::EventDispatcher::new(); - let node_manager = nodes::NodeManager::new(&event_dispatcher); + let mapping = nodes::mapping::NodeMapping::load_mapping_from_file(&config.mapping_path)?; + + let node_manager = nodes::NodeManager::new(&event_dispatcher, mapping); run_with_dependencies(&event_dispatcher, &node_manager, config) } diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs new file mode 100644 index 0000000..b335b8b --- /dev/null +++ b/src/nodes/mapping.rs @@ -0,0 +1,725 @@ +use anyhow::{Context, bail, ensure}; +use liquidcan::payloads::{CanDataType, CanDataValue}; +use serde::Deserialize; +use std::collections::HashSet; +use toml::Value; + +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(default)] +pub struct NodeMapping { + pub id: String, + pub description: String, + pub mapping: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct MappingEntry { + pub name: String, + pub raw: RawField, + #[serde(rename = "type")] + pub field_type: FieldType, + #[serde(default)] + pub value: ValueParams, + #[serde(default)] + pub logical: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct RawField { + pub node: String, + pub field: String, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct ValueParams { + pub slope: f64, + pub offset: f64, + #[serde(default)] + pub unit: String, +} + +impl Default for ValueParams { + fn default() -> Self { + Self { + slope: 1.0, + offset: 0.0, + unit: "".to_string(), + } + } +} + +#[derive(Debug, Clone, Deserialize)] +pub struct LogicalRule { + pub range: LogicalRangeConfig, + pub value: Value, + #[serde(default)] + pub color: Option, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct LogicalRangeConfig { + /// Inclusive lower bound by default. If omitted, the range is unbounded below. + #[serde(default)] + pub min: Option, + /// Exclusive upper bound by default. If omitted, the range is unbounded above. + #[serde(default)] + pub max: Option, + #[serde(default = "default_min_inclusive")] + pub min_inclusive: bool, + #[serde(default)] + pub max_inclusive: bool, +} + +fn default_min_inclusive() -> bool { + true +} + +#[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum FieldType { + Telemetry, + Parameter, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct LogicalValue { + pub value: Value, + pub color: Option, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct MappedValue { + pub value: f64, + pub unit: String, +} + +impl NodeMapping { + pub fn load_mapping_from_file(path: &str) -> anyhow::Result { + if path.is_empty() { + return Ok(Self::default()); + } + + let toml_str = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read mapping config file at {}", path))?; + Self::parse_mapping(&toml_str) + } + + pub fn parse_mapping(toml_str: &str) -> anyhow::Result { + let config = toml::from_str::(toml_str) + .map_err(|err| anyhow::anyhow!("Failed to parse mapping config: {}", err))?; + + config.validate().with_context(|| { + format!( + "Mapping validation failed for config with id: {}", + config.id + ) + })?; + + Ok(config) + } + + pub fn validate(&self) -> anyhow::Result<()> { + let mut names = HashSet::new(); + let mut raw_fields = HashSet::new(); + for mapping in &self.mapping { + // check name is not empty and unique + ensure!( + !mapping.name.trim().is_empty(), + "mapping name must not be empty" + ); + if !names.insert(mapping.name.as_str()) { + anyhow::bail!("Duplicate mapping name: {}", mapping.name); + } + + // check raw.node and raw.field are not empty and unique in combination + let raw_id = (mapping.raw.node.as_str(), mapping.raw.field.as_str()); + ensure!( + !raw_id.0.trim().is_empty(), + "mapping {} has empty raw.node", + mapping.name + ); + ensure!( + !raw_id.1.trim().is_empty(), + "mapping {} has empty raw.field", + mapping.name + ); + if !raw_fields.insert(raw_id) { + anyhow::bail!( + "Duplicate raw field mapping for node '{}' field '{}'", + mapping.raw.node, + mapping.raw.field + ); + } + mapping.validate()?; + } + + Ok(()) + } + + pub fn get_mapping_for_name(&self, name: &str) -> Option<&MappingEntry> { + self.mapping.iter().find(|m| m.name == name) + } + + pub fn get_mapping_for_raw( + &self, + node: &str, + field: &str, + field_type: FieldType, + ) -> Option<&MappingEntry> { + self.mapping.iter().find(|mapping| { + mapping.raw.node == node + && mapping.raw.field == field + && mapping.field_type == field_type + }) + } +} + +impl MappingEntry { + fn validate(&self) -> anyhow::Result<()> { + ensure!( + !self.raw.node.trim().is_empty(), + "mapping {} must specify raw.node", + self.name + ); + ensure!( + !self.raw.field.trim().is_empty(), + "mapping {} must specify raw.field", + self.name + ); + + ensure!( + self.value.slope.is_finite(), + "mapping {} has a non-finite slope", + self.name + ); + ensure!( + self.value.slope != 0.0, + "mapping {} has a slope of zero, which is not allowed", + self.name + ); + ensure!( + self.value.offset.is_finite(), + "mapping {} has a non-finite offset", + self.name + ); + + self.validate_logical_rules()?; + + Ok(()) + } + + /// Validates that logical rules form an unambiguous partition of all mapped values. + /// + /// Empty logical rules are allowed. Once any logical rule is present, the ranges must be + /// non-empty, non-overlapping, and exhaustive over `(-inf, inf)` so every mapped value has + /// exactly one logical value. + fn validate_logical_rules(&self) -> anyhow::Result<()> { + if self.logical.is_empty() { + return Ok(()); + } + + let mut covered_ranges = Vec::new(); + + for (index, rule) in self.logical.iter().enumerate() { + let range = rule.range.to_logical_range().with_context(|| { + format!( + "Logical rule {} for mapping {} has an invalid range", + index + 1, + self.name + ) + })?; + + if !range.is_non_empty() { + bail!( + "Logical rule {} for mapping {} has an empty range {}", + index + 1, + self.name, + range.describe() + ); + } + + for (covered_index, covered_range) in covered_ranges.iter().enumerate() { + if let Some(overlap) = range.intersection(covered_range) { + bail!( + "Logical rule {} for mapping {} overlaps with rule {} in {}; overlapping ranges are ambiguous", + index + 1, + self.name, + covered_index + 1, + overlap.describe() + ); + } + } + + covered_ranges.push(range); + } + + let uncovered_ranges = covered_ranges.iter().fold( + vec![LogicalRange::all()], + |remaining_uncovered_ranges, covered_range| { + remaining_uncovered_ranges + .into_iter() + .flat_map(|range| range.difference(covered_range)) + .collect::>() + }, + ); + + if let Some(uncovered_range) = uncovered_ranges.first() { + bail!( + "Logical rules for mapping {} are not exhaustive; values in {} are not matched", + self.name, + uncovered_range.describe() + ); + } + + Ok(()) + } + + /// Applies the linear mapping `mapped = raw * slope + offset`. + pub fn mapped_value(&self, raw_value: &CanDataValue) -> anyhow::Result { + let numeric_raw_value = can_data_value_to_f64(raw_value)?; + + Ok(MappedValue { + unit: self.value.unit.clone(), + value: numeric_raw_value * self.value.slope + self.value.offset, + }) + } + + /// Inverts the linear mapping and converts the result to the concrete CAN data type. + pub fn raw_value_from_mapped( + &self, + mapped_value: f64, + data_type: CanDataType, + ) -> anyhow::Result { + ensure!( + mapped_value.is_finite(), + "mapped value for {} must be finite", + self.name + ); + + ensure!( + self.value.slope != 0.0, + "cannot invert mapping {} because slope is zero", + self.name + ); + + can_data_value_from_f64( + (mapped_value - self.value.offset) / self.value.slope, + data_type, + ) + } + + pub fn logical_value(&self, mapped_value: f64) -> Option { + self.logical + .iter() + .find(|rule| rule.matches(mapped_value)) + .map(|rule| LogicalValue { + value: rule.value.clone(), + color: rule.color.clone(), + }) + } +} + +impl LogicalRule { + fn matches(&self, mapped_value: f64) -> bool { + self.range + .to_logical_range() + .is_ok_and(|range| range.contains(mapped_value)) + } +} + +impl LogicalRangeConfig { + /// Converts the TOML range representation into the internal interval type + fn to_logical_range(&self) -> anyhow::Result { + if let Some(min) = self.min { + ensure!(min.is_finite(), "range min must be finite"); + } + if let Some(max) = self.max { + ensure!(max.is_finite(), "range max must be finite"); + } + + Ok(LogicalRange::new( + RangeBound { + value: self.min, + inclusive: self.min_inclusive, + }, + RangeBound { + value: self.max, + inclusive: self.max_inclusive, + }, + )) + } +} + +#[derive(Debug, Clone, Copy, PartialEq)] +struct LogicalRange { + lower: RangeBound, + upper: RangeBound, +} + +impl LogicalRange { + /// The complete domain + fn all() -> Self { + Self::new( + RangeBound::negative_infinity(), + RangeBound::positive_infinity(), + ) + } + + fn new(lower: RangeBound, upper: RangeBound) -> Self { + Self { lower, upper } + } + + fn contains(&self, value: f64) -> bool { + let above_lower = match self.lower.value { + Some(lower) if self.lower.inclusive => value >= lower, + Some(lower) => value > lower, + None => true, + }; + let below_upper = match self.upper.value { + Some(upper) if self.upper.inclusive => value <= upper, + Some(upper) => value < upper, + None => true, + }; + + above_lower && below_upper + } + + fn intersection(&self, other: &Self) -> Option { + let intersection = Self::new( + RangeBound::max_lower(self.lower, other.lower), + RangeBound::min_upper(self.upper, other.upper), + ); + + intersection.is_non_empty().then_some(intersection) + } + + fn difference(&self, other: &Self) -> Vec { + let Some(intersection) = self.intersection(other) else { + return vec![*self]; + }; + + let mut remaining = Vec::new(); + + if intersection.lower.value.is_some() { + let left = Self::new( + self.lower, + RangeBound { + value: intersection.lower.value, + inclusive: !intersection.lower.inclusive, + }, + ); + if left.is_non_empty() { + remaining.push(left); + } + } + + if intersection.upper.value.is_some() { + let right = Self::new( + RangeBound { + value: intersection.upper.value, + inclusive: !intersection.upper.inclusive, + }, + self.upper, + ); + if right.is_non_empty() { + remaining.push(right); + } + } + + remaining + } + + fn is_non_empty(&self) -> bool { + match (self.lower.value, self.upper.value) { + (Some(lower), Some(upper)) if lower > upper => false, + (Some(lower), Some(upper)) if lower == upper => { + self.lower.inclusive && self.upper.inclusive + } + _ => true, + } + } + + fn describe(&self) -> String { + let lower = match self.lower.value { + Some(value) if self.lower.inclusive => format!("[{value}"), + Some(value) => format!("({value}"), + None => "(-inf".to_string(), + }; + let upper = match self.upper.value { + Some(value) if self.upper.inclusive => format!("{value}]"), + Some(value) => format!("{value})"), + None => "inf)".to_string(), + }; + + format!("{lower}, {upper}") + } +} + +#[derive(Debug, Clone, Copy, PartialEq)] +struct RangeBound { + value: Option, + inclusive: bool, +} + +impl RangeBound { + fn negative_infinity() -> Self { + Self { + value: None, + inclusive: false, + } + } + + fn positive_infinity() -> Self { + Self { + value: None, + inclusive: false, + } + } + + fn finite(value: f64, inclusive: bool) -> Self { + Self { + value: Some(value), + inclusive, + } + } + + fn max_lower(left: Self, right: Self) -> Self { + match (left.value, right.value) { + (None, _) => right, + (_, None) => left, + (Some(left_value), Some(right_value)) if left_value > right_value => left, + (Some(left_value), Some(right_value)) if left_value < right_value => right, + (Some(value), Some(_)) => Self::finite(value, left.inclusive && right.inclusive), + } + } + + fn min_upper(left: Self, right: Self) -> Self { + match (left.value, right.value) { + (None, _) => right, + (_, None) => left, + (Some(left_value), Some(right_value)) if left_value < right_value => left, + (Some(left_value), Some(right_value)) if left_value > right_value => right, + (Some(value), Some(_)) => Self::finite(value, left.inclusive && right.inclusive), + } + } +} + +fn can_data_value_to_f64(value: &CanDataValue) -> anyhow::Result { + match value { + CanDataValue::Float32(value) => Ok(*value as f64), + CanDataValue::Int32(value) => Ok(*value as f64), + CanDataValue::Int16(value) => Ok(*value as f64), + CanDataValue::Int8(value) => Ok(*value as f64), + CanDataValue::UInt32(value) => Ok(*value as f64), + CanDataValue::UInt16(value) => Ok(*value as f64), + CanDataValue::UInt8(value) => Ok(*value as f64), + CanDataValue::Boolean(value) => Ok(if *value { 1.0 } else { 0.0 }), + CanDataValue::Raw(_) => bail!("raw CAN data must be decoded before applying a mapping"), + } +} + +/// Converts a mapped numeric value back into a typed CAN payload value. +fn can_data_value_from_f64(value: f64, data_type: CanDataType) -> anyhow::Result { + ensure!(value.is_finite(), "raw value must be finite"); + + match data_type { + CanDataType::Float32 => Ok(CanDataValue::Float32(value as f32)), + CanDataType::Int32 => Ok(CanDataValue::Int32(checked_integer::(value)?)), + CanDataType::Int16 => Ok(CanDataValue::Int16(checked_integer::(value)?)), + CanDataType::Int8 => Ok(CanDataValue::Int8(checked_integer::(value)?)), + CanDataType::UInt32 => Ok(CanDataValue::UInt32(checked_integer::(value)?)), + CanDataType::UInt16 => Ok(CanDataValue::UInt16(checked_integer::(value)?)), + CanDataType::UInt8 => Ok(CanDataValue::UInt8(checked_integer::(value)?)), + CanDataType::Boolean => { + if (value - 0.0).abs() < f64::EPSILON { + Ok(CanDataValue::Boolean(false)) + } else if (value - 1.0).abs() < f64::EPSILON { + Ok(CanDataValue::Boolean(true)) + } else { + bail!("boolean raw values must map back to 0 or 1, got {value}") + } + } + } +} + +/// Checks that a floating-point inverse-mapped value can be represented as an integer CAN type. +fn checked_integer(value: f64) -> anyhow::Result +where + T: TryFrom, + >::Error: std::fmt::Debug, +{ + let rounded = value.round(); + + T::try_from(rounded as i128).map_err(|_| anyhow::anyhow!("raw value {rounded} is out of range")) +} + +#[cfg(test)] +mod tests { + use liquidcan::payloads::{CanDataType, CanDataValue}; + use toml::Value; + + use super::{LogicalValue, NodeMapping}; + + #[test] + fn parses_and_applies_mapping_schema() { + let mapping = NodeMapping::parse_mapping( + r##" +id = "example" + +[[mapping]] +name = "tank_pressure" +type = "telemetry" +raw = { node = "ECU", field = "pressure_adc" } +value = { slope = 0.5, offset = 1.0, unit = "bar" } + +[[mapping.logical]] +range = { min = 100 } +value = "High" +color = "#ff0000" + +[[mapping.logical]] +range = { max = 100 } +value = "Normal" +"##, + ) + .expect("mapping should parse"); + + let entry = mapping + .get_mapping_for_name("tank_pressure") + .expect("entry should exist"); + + let mapped = entry + .mapped_value(&CanDataValue::UInt16(198)) + .expect("raw value should map"); + assert_eq!(mapped.value, 100.0); + assert_eq!(mapped.unit, "bar"); + + assert_eq!( + entry.logical_value(mapped.value), + Some(LogicalValue { + value: Value::String("High".to_string()), + color: Some("#ff0000".to_string()), + }) + ); + } + + #[test] + fn rejects_duplicate_mapping_names() { + let error = NodeMapping::parse_mapping( + r#" +[[mapping]] +name = "duplicate" +raw = { node = "node1", field = "field1" } +type = "telemetry" +value = { slope = 1.0, offset = 0.0 } + +[[mapping]] +name = "duplicate" +raw = { node = "node1", field = "field2" } +type = "telemetry" +value = { slope = 1.0, offset = 0.0 } +"#, + ) + .expect_err("duplicate names should fail validation"); + + assert!(format!("{error:#}").contains("Duplicate mapping name")); + } + + #[test] + fn converts_mapped_value_back_to_raw_parameter_type() { + let mapping = NodeMapping::parse_mapping( + r#" +[[mapping]] +name = "valve_opening" +type = "parameter" +raw = { node = "ECU", field = "valve_raw" } +value = { slope = 0.5, offset = 10.0, unit = "%" } +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("valve_opening").unwrap(); + let raw = entry + .raw_value_from_mapped(60.0, CanDataType::UInt8) + .expect("mapped value should invert to raw"); + + assert_eq!(raw, CanDataValue::UInt8(100)); + } + + #[test] + fn checked_in_example_mapping_is_valid() { + NodeMapping::load_mapping_from_file("tests/mapping/example1.toml") + .expect("example mapping should be valid"); + } + + #[test] + fn rejects_non_exhaustive_logical_rules() { + let error = NodeMapping::parse_mapping( + r#" +[[mapping]] +name = "temperature" +type = "telemetry" +raw = { node = "ECU", field = "temperature" } + +[[mapping.logical]] +range = { max = 10 } +value = "Cold" + +[[mapping.logical]] +range = { min = 10, min_inclusive = false } +value = "Hot" +"#, + ) + .expect_err("rules should miss exactly 10"); + + assert!(format!("{error:#}").contains("not exhaustive")); + } + + #[test] + fn rejects_overlapping_logical_rules() { + let error = NodeMapping::parse_mapping( + r#" +[[mapping]] +name = "temperature" +type = "telemetry" +raw = { node = "ECU", field = "temperature" } + +[[mapping.logical]] +range = { max = 100 } +value = "Low" + +[[mapping.logical]] +range = { max = 50 } +value = "Very low" + +[[mapping.logical]] +range = { min = 100 } +value = "High" +"#, + ) + .expect_err("second rule should overlap with the first"); + + assert!(format!("{error:#}").contains("overlaps")); + } + + #[test] + fn accepts_adjacent_ranges() { + NodeMapping::parse_mapping( + r#" +[[mapping]] +name = "temperature" +type = "telemetry" +raw = { node = "ECU", field = "temperature" } + +[[mapping.logical]] +range = { max = 10 } +value = "Cold" + +[[mapping.logical]] +range = { min = 10 } +value = "Hot" +"#, + ) + .expect("adjacent ranges should cover the threshold exactly once"); + } +} diff --git a/src/nodes/mod.rs b/src/nodes/mod.rs index b4d9876..2657f56 100644 --- a/src/nodes/mod.rs +++ b/src/nodes/mod.rs @@ -1,6 +1,7 @@ //! Contains code for managing the CAN nodes that are connected to FerroFlow, their fields and data types. mod can_node; +pub mod mapping; mod node_manager; pub use node_manager::NodeManager; diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index 9b95b9c..03b0452 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -7,16 +7,19 @@ use dashmap::DashMap; use liquidcan::{ CanMessage, CanMessageId, payloads::{ - CanDataValue, FieldGetResPayload, FieldRegistrationPayload, HeartbeatPayload, - NodeInfoResPayload, TelemetryGroupDefinitionPayload, TelemetryGroupUpdatePayload, + CanDataType, CanDataValue, FieldGetReqPayload, FieldGetResPayload, + FieldRegistrationPayload, HeartbeatPayload, NodeInfoResPayload, ParameterSetReqPayload, + TelemetryGroupDefinitionPayload, TelemetryGroupUpdatePayload, }, }; +use crate::nodes::mapping::{self, LogicalValue, MappedValue, MappingEntry, NodeMapping}; use crate::{db::FieldLog, events}; use super::can_node::{CanNode, FieldInfo, RegistrationInfo, TelemetryGroupDefinition}; pub struct NodeManager<'a> { + mapping: NodeMapping, can_nodes: DashMap, // Nodes that did not yet receive all their field registrations. @@ -25,8 +28,9 @@ pub struct NodeManager<'a> { } impl<'a> NodeManager<'a> { - pub fn new(event_dispatcher: &'a events::EventDispatcher) -> Self { + pub fn new(event_dispatcher: &'a events::EventDispatcher, mapping: NodeMapping) -> Self { Self { + mapping, can_nodes: DashMap::new(), registering_nodes: Mutex::new(HashMap::new()), event_dispatcher, @@ -370,4 +374,343 @@ impl<'a> NodeManager<'a> { CanDataValue::Raw(items) => serde_json::json!(items), } } + + /// Returns the latest cached raw CAN value for a mapped field name. + /// + /// This does not send a CAN request. Call `request_value` first if a fresh value is needed. + /// + /// Use this `try_` variant to distinguish missing values from invalid mappings or fields + /// that have not registered yet. + pub fn try_get_raw_value(&self, mapped_name: &str) -> Result> { + let mapping_entry = self + .mapping + .get_mapping_for_name(mapped_name) + .with_context(|| format!("no mapping exists for {mapped_name}"))?; + let target = self + .resolve_mapping_target(mapping_entry) + .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + + Ok(self.can_nodes.get(&target.node_id).and_then(|node| { + node.values + .get(&target.field_id) + .map(|value| value.1.clone()) + })) + } + + /// Convenience wrapper around `try_get_raw_value` that treats errors as missing values. + pub fn get_raw_value(&self, mapped_name: &str) -> Option { + self.try_get_raw_value(mapped_name).ok().flatten() + } + + /// Returns the latest cached value after applying the mapping's slope/offset conversion. + /// + /// `Ok(None)` means the mapping and raw field exist, but no value has been received yet. + pub fn try_get_mapped_value(&self, mapped_name: &str) -> Result> { + let Some(raw_value) = self.try_get_raw_value(mapped_name)? else { + return Ok(None); + }; + + let mapping_entry = self + .mapping + .get_mapping_for_name(mapped_name) + .with_context(|| format!("no mapping exists for {mapped_name}"))?; + + Ok(Some(mapping_entry.mapped_value(&raw_value)?)) + } + + /// Convenience wrapper around `try_get_mapped_value` that treats errors as missing values. + pub fn get_mapped_value(&self, mapped_name: &str) -> Option { + self.try_get_mapped_value(mapped_name).ok().flatten() + } + + /// Returns the logical value associated with the current mapped value. + /// + /// Logical values are derived from the configured range table. If the mapping has no logical + /// rules, this returns `Ok(None)` even when a mapped numeric value is available. + pub fn try_get_logical_value(&self, mapped_name: &str) -> Result> { + let Some(mapped_value) = self.try_get_mapped_value(mapped_name)? else { + return Ok(None); + }; + + let mapping_entry = self + .mapping + .get_mapping_for_name(mapped_name) + .with_context(|| format!("no mapping exists for {mapped_name}"))?; + + Ok(mapping_entry.logical_value(mapped_value.value)) + } + + /// Convenience wrapper around `try_get_logical_value` that treats errors as missing values. + pub fn get_logical_value(&self, mapped_name: &str) -> Option { + self.try_get_logical_value(mapped_name).ok().flatten() + } + + /// Sends a `FieldGetReq` for the raw field behind a mapped name. + /// + /// The response is processed asynchronously by the normal CAN message handler and updates the + /// cached value read by `get_raw_value`, `get_mapped_value`, and `get_logical_value`. + pub fn request_value(&self, mapped_name: &str) -> Result<()> { + let mapping_entry = self + .mapping + .get_mapping_for_name(mapped_name) + .with_context(|| format!("no mapping exists for {mapped_name}"))?; + let target = self + .resolve_mapping_target(mapping_entry) + .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + + self.event_dispatcher + .dispatch(events::Event::SendCanMessage { + receiver_node_id: target.node_id, + message: CanMessage::FieldGetReq { + payload: FieldGetReqPayload { + field_id: target.field_id, + }, + }, + }); + + Ok(()) + } + + /// Writes a mapped value to a mapped parameter field. + /// + /// The value is converted back to the raw CAN type using the inverse of the configured linear + /// mapping, then sent as a `ParameterSetReq`. + pub fn set_mapped_value(&self, mapped_name: &str, mapped_value: f64) -> Result<()> { + let mapping_entry = self + .mapping + .get_mapping_for_name(mapped_name) + .with_context(|| format!("no mapping exists for {mapped_name}"))?; + let target = self + .resolve_mapping_target(mapping_entry) + .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + + if mapping_entry.field_type != mapping::FieldType::Parameter { + bail!("mapped field {mapped_name} is not writable because it is not a parameter"); + } + + let raw_value = mapping_entry.raw_value_from_mapped(mapped_value, target.data_type)?; + self.set_raw_value(mapped_name, raw_value) + } + + /// Writes a raw CAN value to a mapped parameter field. + pub fn set_raw_value(&self, mapped_name: &str, raw_value: CanDataValue) -> Result<()> { + let mapping_entry = self + .mapping + .get_mapping_for_name(mapped_name) + .with_context(|| format!("no mapping exists for {mapped_name}"))?; + let target = self + .resolve_mapping_target(mapping_entry) + .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + + if mapping_entry.field_type != mapping::FieldType::Parameter { + bail!("mapped field {mapped_name} is not writable because it is not a parameter"); + } + + self.event_dispatcher + .dispatch(events::Event::SendCanMessage { + receiver_node_id: target.node_id, + message: CanMessage::ParameterSetReq { + payload: ParameterSetReqPayload { + parameter_id: target.field_id, + value: raw_value, + }, + }, + }); + + Ok(()) + } + + /// Resolves a mapping entry to the currently registered node id, field id, and field type. + /// + /// Mappings are written against stable device/field names, but LiquidCAN requests need numeric + /// ids learned during node registration. + fn resolve_mapping_target( + &self, + mapping_entry: &MappingEntry, + ) -> Option { + self.can_nodes.iter().find_map(|node| { + if node.registration_info.device_name != mapping_entry.raw.node { + return None; + } + + let fields = match mapping_entry.field_type { + mapping::FieldType::Telemetry => &node.telemetry_fields, + mapping::FieldType::Parameter => &node.parameter_fields, + }; + + fields + .iter() + .find(|(_, field)| field.name == mapping_entry.raw.field) + .map(|(field_id, field)| ResolvedMappingTarget { + node_id: *node.key(), + field_id: *field_id, + data_type: field.data_type, + }) + }) + } +} + +struct ResolvedMappingTarget { + node_id: u8, + field_id: u8, + data_type: CanDataType, +} + +#[cfg(test)] +mod tests { + use std::{sync::mpsc, time::Duration}; + + use chrono::Utc; + use liquidcan::payloads::{CanDataType, CanDataValue}; + use toml::Value; + + use crate::events::{Event, EventDispatcher, EventKind}; + + use super::*; + + #[test] + fn reads_raw_mapped_and_logical_values_by_mapping_name() { + let dispatcher = EventDispatcher::new(); + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + assert_eq!( + manager.get_raw_value("tank_pressure"), + Some(CanDataValue::UInt16(198)) + ); + + let mapped = manager + .get_mapped_value("tank_pressure") + .expect("mapped value should be available"); + assert_eq!(mapped.value, 100.0); + assert_eq!(mapped.unit, "bar"); + + let logical = manager + .get_logical_value("tank_pressure") + .expect("logical value should be available"); + assert_eq!(logical.value, Value::String("High".to_string())); + assert_eq!(logical.color, Some("#ff0000".to_string())); + } + + #[test] + fn writes_mapped_parameter_values_as_raw_can_values() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + manager + .set_mapped_value("valve_opening", 60.0) + .expect("mapped parameter should be writable"); + + let event = rx + .recv_timeout(Duration::from_millis(200)) + .expect("send event should be dispatched"); + + match event { + Event::SendCanMessage { + receiver_node_id, + message: + CanMessage::ParameterSetReq { + payload: + ParameterSetReqPayload { + parameter_id, + value, + }, + }, + } => { + assert_eq!(receiver_node_id, 5); + assert_eq!(parameter_id, 20); + assert_eq!(value, CanDataValue::UInt8(100)); + } + other => panic!("unexpected event: {other:?}"), + } + } + + #[test] + fn requests_field_get_for_mapped_values() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + manager + .request_value("tank_pressure") + .expect("mapped field should be requestable"); + + let event = rx + .recv_timeout(Duration::from_millis(200)) + .expect("send event should be dispatched"); + + match event { + Event::SendCanMessage { + receiver_node_id, + message: CanMessage::FieldGetReq { payload }, + } => { + assert_eq!(receiver_node_id, 5); + assert_eq!(payload.field_id, 10); + } + other => panic!("unexpected event: {other:?}"), + } + } + + fn test_mapping() -> NodeMapping { + NodeMapping::parse_mapping( + r##" +[[mapping]] +name = "tank_pressure" +type = "telemetry" +raw = { node = "ECU", field = "pressure_adc" } +value = { slope = 0.5, offset = 1.0, unit = "bar" } + +[[mapping.logical]] +range = { min = 100 } +value = "High" +color = "#ff0000" + +[[mapping.logical]] +range = { max = 100 } +value = "Normal" + +[[mapping]] +name = "valve_opening" +type = "parameter" +raw = { node = "ECU", field = "valve_raw" } +value = { slope = 0.5, offset = 10.0, unit = "%" } +"##, + ) + .expect("mapping should parse") + } + + fn insert_test_node(manager: &NodeManager<'_>) { + let mut node = CanNode::new(RegistrationInfo { + telemetry_count: 1, + parameter_count: 1, + firmware_hash: 0, + protocol_hash: 0, + device_name: "ECU".to_string(), + }); + node.telemetry_fields.insert( + 10, + FieldInfo { + data_type: CanDataType::UInt16, + name: "pressure_adc".to_string(), + }, + ); + node.parameter_fields.insert( + 20, + FieldInfo { + data_type: CanDataType::UInt8, + name: "valve_raw".to_string(), + }, + ); + node.values + .insert(10, (Utc::now(), CanDataValue::UInt16(198))); + + manager.can_nodes.insert(5, node); + } } diff --git a/tests/emulator.rs b/tests/emulator.rs index 334829b..7b247f1 100644 --- a/tests/emulator.rs +++ b/tests/emulator.rs @@ -3,6 +3,7 @@ mod common; use crate::common::ShutdownGuard; use chrono::{DateTime, Utc}; use ferro_flow::config::Config; +use ferro_flow::nodes::mapping::NodeMapping; use ferro_flow::{events, nodes, run_with_dependencies}; use liquidcan::payloads::CanDataType; use std::{io::Write, time::Instant}; @@ -17,7 +18,7 @@ fn test_node_registration() { let emulator_config = ecuemulator_test_config_toml(&vcan_iface); let event_dispatcher = events::EventDispatcher::new(); - let node_manager = nodes::NodeManager::new(&event_dispatcher); + let node_manager = nodes::NodeManager::new(&event_dispatcher, NodeMapping::default()); let config = build_test_config(&vcan_iface); std::thread::scope(|s| { @@ -109,7 +110,7 @@ fn test_telemetry_group_updates() { let emulator_config = ecuemulator_test_config_toml(&vcan_iface); let event_dispatcher = events::EventDispatcher::new(); - let node_manager = nodes::NodeManager::new(&event_dispatcher); + let node_manager = nodes::NodeManager::new(&event_dispatcher, NodeMapping::default()); let config = build_test_config(&vcan_iface); println!("Starting application with test config: {:?}", config); @@ -185,6 +186,7 @@ fn build_test_config(can_iface: &str) -> Config { can_bus_interfaces: vec![can_iface.to_string()], heartbeat_period: 1, database_url: "".to_string(), + mapping_path: "".to_string(), } } diff --git a/tests/mapping/example1.toml b/tests/mapping/example1.toml new file mode 100644 index 0000000..49b7c77 --- /dev/null +++ b/tests/mapping/example1.toml @@ -0,0 +1,41 @@ +id = "example1" +description = "This is an example mapping file." + +[[mapping]] +raw.node = "node1" +raw.field = "field1" +name = "Example Mapping 1" +type = "telemetry" +value.unit = "mAh" +value.slope = 0.5 +value.offset = 1.0 + +[[mapping.logical]] +range = { min = 100 } +value = "High" +color = "#ff0000" + +[[mapping.logical]] +range = { min = 50, max = 100 } +value = "Normal" + +[[mapping.logical]] +range = { max = 50 } +value = "Low" + + +# alternatively + +[[mapping]] +name = "Example Mapping 2" +type = "parameter" + +raw = { node = "node1", field = "field2" } + +value = { slope = 0.5, offset = 1.0, unit = "mAh" } + +logical = [ + { range = { min = 100 }, value = "High", color = "#ff0000" }, + { range = { min = 50, max = 100 }, value = "Normal" }, + { range = { max = 50 }, value = "Low" }, +] From 1410b7cb3a2ae2855f7e332b62a172ec612be122 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 7 May 2026 17:29:57 +0200 Subject: [PATCH 03/19] implement suggestions - load mappings from a directory - change mapping to be grouped by node - simplify some code --- README.md | 37 +- src/config/mod.rs | 1 - src/lib.rs | 2 +- src/nodes/mapping.rs | 597 ++++++++++++++++++-------------- src/nodes/node_manager.rs | 224 ++++++------ tests/emulator.rs | 6 +- tests/mapping/example1.toml | 19 +- tests/mapping/split/engine.toml | 10 + tests/mapping/split/fuel.toml | 5 + 9 files changed, 520 insertions(+), 381 deletions(-) create mode 100644 tests/mapping/split/engine.toml create mode 100644 tests/mapping/split/fuel.toml diff --git a/README.md b/README.md index 144bc8d..e8e2f19 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ # Ferroflow -Ferroflow is the new control software for all Liquid Rocketry projects at the TU Wien Space Team. + +Ferroflow is the new control software for all Liquid Rocketry projects at the TU Wien Space Team. It interfaces with our custom Engine Control Units ECUs, through our custom [LiquidCAN protocol](https://github.com/SpaceTeam/LiquidCAN/). On the other end, it provides a high-level API for our [ECUI](https://github.com/SpaceTeam/web_ecui_houbolt), which is the user interface for our ECUs. @@ -10,33 +11,58 @@ On the other end, it provides a high-level API for our [ECUI](https://github.com Some integration tests talk to the ECUemulator over SocketCAN. For that you use a virtual CAN interface. ### Test helper: `ferroflow-vcan` + For test environments, this repo provides a small helper binary that can be granted `CAP_NET_ADMIN` once via `setcap`. Integration tests will automatically use it (if it’s available on `PATH`) to create/delete `vcan` interfaces without sudo. Build the helper (feature-gated; not part of normal builds): + ```bash cargo build --release --features test-vcan --bin ferroflow-vcan ``` + Put it on PATH (recommended for tests): + ```bash install -m 0755 ./target/release/ferroflow-vcan ~/.local/bin/ferroflow-vcan sudo setcap cap_net_admin+ep ~/.local/bin/ferroflow-vcan ``` Manual usage: + ```bash ferroflow-vcan up vcan0 ferroflow-vcan down vcan0 ``` - ## Development +### Mapping Configuration + +`mapping_path` in `config.yml` points to a directory containing `.toml` files, which are loaded in sorted order and validated together. + +Mappings are grouped by node name: + +```toml +[[mapping.FuelECU]] +name = "fuel_level" +type = "telemetry" +raw_field = "level_adc" +value = { slope = 0.5, offset = 1.0, unit = "mAh" } + +logical = [ + { range = { min = 100 }, value = "High", color = "#ff0000" }, + { range = { min = 50, max = 100 }, value = "Normal" }, + { range = { max = 50 }, value = "Low" }, +] +``` + ### Running CI Checks The repository includes a CI script (`ci-rust.sh`) that runs all quality checks on the Rust implementation. This script is used both locally and in GitHub Actions **Run all checks:** + ```bash ./ci-rust.sh # or explicitly @@ -44,17 +70,20 @@ The repository includes a CI script (`ci-rust.sh`) that runs all quality checks ``` **Run individual checks:** + ```bash ./ci-rust.sh build # Build the project ./ci-rust.sh test # Run tests ./ci-rust.sh fmt # Check code formatting ./ci-rust.sh clippy # Run clippy linter ``` + You can fix formatting or linter issues by adding the -fix suffix to the command. e.g: `./ci-rust.sh clippy-fix` ### Running `fmt` and `clippy` as a pre-commit hook A pre-commit hook script is available in `.githooks`, which executes the CI script with `fmt` and `clippy` only and without the `fix` option. To setup the hook, configure git to use the `.githooks` directory and make the `pre-commit` file executable. + ```bash git config core.hooksPath .githooks chmod u+x .githooks/pre-commit @@ -66,6 +95,7 @@ chmod u+x .githooks/pre-commit We use TimescaleDB, which is an extension of PostgreSQL optimized for time-series data. You can install it by following the instructions on the [TimescaleDB installation page](https://docs.timescale.com/install/latest/). Using docker is recommended for local development (if you already have another instance of postgres running, use e.g. `-p 5433:5432` instead of `-p 5432:5432`): + ```bash docker run -d --name timescaledb -p 5432:5432 -e POSTGRES_PASSWORD=yourpassword timescale/timescaledb:latest-pg18 ``` @@ -76,6 +106,7 @@ The project uses Diesel for database interactions. Diesel CLI is recommended for **Running Diesel CLI** Here's some common commands: + ```bash export DATABASE_URL=postgres://postgres:yourpassword@localhost:5432/ferroflow # Set the database URL diesel setup # Set up the database @@ -97,4 +128,4 @@ Database tests use `testcontainers` to start a temporary TimescaleDB/PostgreSQL There are two examples in the repository: - a unit test in `src/db/mod.rs` -- an integration test in `tests/db_logging.rs` \ No newline at end of file +- an integration test in `tests/db_logging.rs` diff --git a/src/config/mod.rs b/src/config/mod.rs index 06a48e7..a9b2e5f 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -21,4 +21,3 @@ pub fn load_config(path: &str) -> Result { .try_deserialize() .with_context(|| format!("Failed to deserialize config from {}", path)) } - diff --git a/src/lib.rs b/src/lib.rs index 3593e12..9d2dc8d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,7 +13,7 @@ pub mod socket; pub fn run_with_config(config: Config) -> anyhow::Result<()> { let event_dispatcher = events::EventDispatcher::new(); - let mapping = nodes::mapping::NodeMapping::load_mapping_from_file(&config.mapping_path)?; + let mapping = nodes::mapping::Mapping::load_mapping_from_path(&config.mapping_path)?; let node_manager = nodes::NodeManager::new(&event_dispatcher, mapping); diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index b335b8b..dc55425 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -1,35 +1,31 @@ use anyhow::{Context, bail, ensure}; use liquidcan::payloads::{CanDataType, CanDataValue}; use serde::Deserialize; -use std::collections::HashSet; +use std::{ + collections::{BTreeMap, HashSet}, + fs, + path::Path, +}; use toml::Value; #[derive(Debug, Clone, Default, Deserialize)] #[serde(default)] -pub struct NodeMapping { - pub id: String, - pub description: String, - pub mapping: Vec, +pub struct Mapping { + pub mapping: BTreeMap>, } #[derive(Debug, Clone, Deserialize)] pub struct MappingEntry { pub name: String, - pub raw: RawField, #[serde(rename = "type")] pub field_type: FieldType, + pub raw_field: String, #[serde(default)] pub value: ValueParams, #[serde(default)] pub logical: Vec, } -#[derive(Debug, Clone, Deserialize)] -pub struct RawField { - pub node: String, - pub field: String, -} - #[derive(Debug, Clone, Deserialize)] pub struct ValueParams { pub slope: f64, @@ -50,26 +46,34 @@ impl Default for ValueParams { #[derive(Debug, Clone, Deserialize)] pub struct LogicalRule { - pub range: LogicalRangeConfig, + pub range: LogicalRange, pub value: Value, #[serde(default)] pub color: Option, } #[derive(Debug, Clone, Deserialize)] -pub struct LogicalRangeConfig { +pub struct LogicalRange { /// Inclusive lower bound by default. If omitted, the range is unbounded below. - #[serde(default)] - pub min: Option, + #[serde(default = "default_unbounded_min")] + pub min: f64, /// Exclusive upper bound by default. If omitted, the range is unbounded above. - #[serde(default)] - pub max: Option, + #[serde(default = "default_unbounded_max")] + pub max: f64, #[serde(default = "default_min_inclusive")] pub min_inclusive: bool, #[serde(default)] pub max_inclusive: bool, } +fn default_unbounded_min() -> f64 { + f64::NEG_INFINITY +} + +fn default_unbounded_max() -> f64 { + f64::INFINITY +} + fn default_min_inclusive() -> bool { true } @@ -93,71 +97,129 @@ pub struct MappedValue { pub unit: String, } -impl NodeMapping { +pub struct MappingLookupResult<'a> { + pub node_name: &'a str, + pub mapping_entry: &'a MappingEntry, +} + +impl Mapping { pub fn load_mapping_from_file(path: &str) -> anyhow::Result { if path.is_empty() { return Ok(Self::default()); } - let toml_str = std::fs::read_to_string(path) - .with_context(|| format!("Failed to read mapping config file at {}", path))?; - Self::parse_mapping(&toml_str) + Self::load_mapping_file(Path::new(path)) + } + + pub fn load_mapping_from_path(path: &str) -> anyhow::Result { + if path.is_empty() { + return Ok(Self::default()); + } + + Self::load_mapping_directory(Path::new(path)) } pub fn parse_mapping(toml_str: &str) -> anyhow::Result { - let config = toml::from_str::(toml_str) + let config = toml::from_str::(toml_str) .map_err(|err| anyhow::anyhow!("Failed to parse mapping config: {}", err))?; - config.validate().with_context(|| { - format!( - "Mapping validation failed for config with id: {}", - config.id - ) - })?; + config.validate()?; Ok(config) } + fn load_mapping_file(path: &Path) -> anyhow::Result { + let toml_str = fs::read_to_string(path) + .with_context(|| format!("Failed to read mapping config file at {}", path.display()))?; + + Self::parse_mapping(&toml_str) + .with_context(|| format!("Failed to load mapping config from {}", path.display())) + } + + fn load_mapping_directory(path: &Path) -> anyhow::Result { + let mut entries = fs::read_dir(path) + .with_context(|| format!("Failed to read mapping directory {}", path.display()))? + .map(|entry| entry.map(|entry| entry.path())) + .collect::>>() + .with_context(|| format!("Failed to list mapping directory {}", path.display()))?; + + entries.retain(|entry| { + entry.is_file() + && entry + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("toml")) + }); + entries.sort(); + + ensure!( + !entries.is_empty(), + "mapping directory {} contains no TOML files", + path.display() + ); + + let mut combined = Self::default(); + for entry in entries { + let mapping = Self::load_mapping_file(&entry).with_context(|| { + format!("Failed to load mapping config from {}", entry.display()) + })?; + + for (node, fields) in mapping.mapping { + combined + .mapping + .entry(node) + .or_default() + .extend(fields.into_iter()); + } + } + + combined.validate().with_context(|| { + format!("Mapping validation failed for directory {}", path.display()) + })?; + + Ok(combined) + } + pub fn validate(&self) -> anyhow::Result<()> { let mut names = HashSet::new(); let mut raw_fields = HashSet::new(); - for mapping in &self.mapping { - // check name is not empty and unique + for (node, fields) in &self.mapping { ensure!( - !mapping.name.trim().is_empty(), - "mapping name must not be empty" + !node.trim().is_empty(), + "mapping contains an entry with an empty node name" ); - if !names.insert(mapping.name.as_str()) { - anyhow::bail!("Duplicate mapping name: {}", mapping.name); - } + for field in fields { + field.validate().with_context(|| { + format!("mapping for node {} field {} is invalid", node, field.name) + })?; + + let raw_id = (node.as_str(), field.raw_field.as_str()); + if !raw_fields.insert(raw_id) { + anyhow::bail!( + "Duplicate raw field mapping for node '{}' field '{}'", + node, + field.raw_field + ); + } - // check raw.node and raw.field are not empty and unique in combination - let raw_id = (mapping.raw.node.as_str(), mapping.raw.field.as_str()); - ensure!( - !raw_id.0.trim().is_empty(), - "mapping {} has empty raw.node", - mapping.name - ); - ensure!( - !raw_id.1.trim().is_empty(), - "mapping {} has empty raw.field", - mapping.name - ); - if !raw_fields.insert(raw_id) { - anyhow::bail!( - "Duplicate raw field mapping for node '{}' field '{}'", - mapping.raw.node, - mapping.raw.field - ); + if !names.insert(field.name.as_str()) { + anyhow::bail!("Duplicate mapping name '{}'", field.name); + } } - mapping.validate()?; } Ok(()) } - pub fn get_mapping_for_name(&self, name: &str) -> Option<&MappingEntry> { - self.mapping.iter().find(|m| m.name == name) + pub fn get_mapping_for_name(&self, name: &str) -> Option> { + self.mapping.iter().find_map(|(node, fields)| { + fields + .iter() + .find(|field| field.name == name) + .map(|field| MappingLookupResult { + node_name: node.as_str(), + mapping_entry: field, + }) + }) } pub fn get_mapping_for_raw( @@ -165,28 +227,33 @@ impl NodeMapping { node: &str, field: &str, field_type: FieldType, - ) -> Option<&MappingEntry> { - self.mapping.iter().find(|mapping| { - mapping.raw.node == node - && mapping.raw.field == field - && mapping.field_type == field_type - }) + ) -> Option> { + self.mapping + .get_key_value(node) + .and_then(|(node, mapping_entries)| { + mapping_entries + .iter() + .find(|mapping| mapping.raw_field == field && mapping.field_type == field_type) + .map(|mapping| MappingLookupResult { + node_name: node, + mapping_entry: mapping, + }) + }) } } impl MappingEntry { fn validate(&self) -> anyhow::Result<()> { ensure!( - !self.raw.node.trim().is_empty(), - "mapping {} must specify raw.node", - self.name + !self.name.trim().is_empty(), + "mapping name must be non-empty", ); + ensure!( - !self.raw.field.trim().is_empty(), - "mapping {} must specify raw.field", + !self.raw_field.trim().is_empty(), + "mapping {} has an empty raw_field", self.name ); - ensure!( self.value.slope.is_finite(), "mapping {} has a non-finite slope", @@ -221,25 +288,17 @@ impl MappingEntry { let mut covered_ranges = Vec::new(); for (index, rule) in self.logical.iter().enumerate() { - let range = rule.range.to_logical_range().with_context(|| { - format!( - "Logical rule {} for mapping {} has an invalid range", - index + 1, - self.name - ) - })?; - - if !range.is_non_empty() { + if !rule.range.is_non_empty() { bail!( "Logical rule {} for mapping {} has an empty range {}", index + 1, self.name, - range.describe() + rule.range.describe() ); } for (covered_index, covered_range) in covered_ranges.iter().enumerate() { - if let Some(overlap) = range.intersection(covered_range) { + if let Some(overlap) = rule.range.intersection(covered_range) { bail!( "Logical rule {} for mapping {} overlaps with rule {} in {}; overlapping ranges are ambiguous", index + 1, @@ -250,7 +309,7 @@ impl MappingEntry { } } - covered_ranges.push(range); + covered_ranges.push(rule.range.clone()); } let uncovered_ranges = covered_ranges.iter().fold( @@ -321,186 +380,117 @@ impl MappingEntry { impl LogicalRule { fn matches(&self, mapped_value: f64) -> bool { - self.range - .to_logical_range() - .is_ok_and(|range| range.contains(mapped_value)) + self.range.contains(mapped_value) } } -impl LogicalRangeConfig { - /// Converts the TOML range representation into the internal interval type - fn to_logical_range(&self) -> anyhow::Result { - if let Some(min) = self.min { - ensure!(min.is_finite(), "range min must be finite"); - } - if let Some(max) = self.max { - ensure!(max.is_finite(), "range max must be finite"); - } - - Ok(LogicalRange::new( - RangeBound { - value: self.min, - inclusive: self.min_inclusive, - }, - RangeBound { - value: self.max, - inclusive: self.max_inclusive, - }, - )) - } -} - -#[derive(Debug, Clone, Copy, PartialEq)] -struct LogicalRange { - lower: RangeBound, - upper: RangeBound, -} - impl LogicalRange { /// The complete domain fn all() -> Self { - Self::new( - RangeBound::negative_infinity(), - RangeBound::positive_infinity(), - ) - } - - fn new(lower: RangeBound, upper: RangeBound) -> Self { - Self { lower, upper } + Self { + min: f64::NEG_INFINITY, + max: f64::INFINITY, + min_inclusive: false, + max_inclusive: false, + } } fn contains(&self, value: f64) -> bool { - let above_lower = match self.lower.value { - Some(lower) if self.lower.inclusive => value >= lower, - Some(lower) => value > lower, - None => true, + let above_lower = if self.min_inclusive { + value >= self.min + } else { + value > self.min }; - let below_upper = match self.upper.value { - Some(upper) if self.upper.inclusive => value <= upper, - Some(upper) => value < upper, - None => true, + let below_upper = if self.max_inclusive { + value <= self.max + } else { + value < self.max }; - above_lower && below_upper } fn intersection(&self, other: &Self) -> Option { - let intersection = Self::new( - RangeBound::max_lower(self.lower, other.lower), - RangeBound::min_upper(self.upper, other.upper), - ); + let max_cmp = self.max.partial_cmp(&other.max).unwrap(); + let min_cmp = self.min.partial_cmp(&other.min).unwrap(); - intersection.is_non_empty().then_some(intersection) + let max = self.max.min(other.max); + let min = self.min.max(other.min); + + let min_inclusive = match min_cmp { + std::cmp::Ordering::Less => other.min_inclusive, + std::cmp::Ordering::Greater => self.min_inclusive, + std::cmp::Ordering::Equal => self.min_inclusive && other.min_inclusive, + }; + + let max_inclusive = match max_cmp { + std::cmp::Ordering::Less => self.max_inclusive, + std::cmp::Ordering::Greater => other.max_inclusive, + std::cmp::Ordering::Equal => self.max_inclusive && other.max_inclusive, + }; + + let intersection = Self { + min, + max, + min_inclusive, + max_inclusive, + }; + if intersection.is_non_empty() { + Some(intersection) + } else { + None + } } fn difference(&self, other: &Self) -> Vec { let Some(intersection) = self.intersection(other) else { - return vec![*self]; + return vec![self.clone()]; }; let mut remaining = Vec::new(); - if intersection.lower.value.is_some() { - let left = Self::new( - self.lower, - RangeBound { - value: intersection.lower.value, - inclusive: !intersection.lower.inclusive, - }, - ); - if left.is_non_empty() { - remaining.push(left); - } + let left = Self { + min: self.min, + max: intersection.min, + min_inclusive: self.min_inclusive, + max_inclusive: !intersection.min_inclusive, + }; + if left.is_non_empty() { + remaining.push(left); } - if intersection.upper.value.is_some() { - let right = Self::new( - RangeBound { - value: intersection.upper.value, - inclusive: !intersection.upper.inclusive, - }, - self.upper, - ); - if right.is_non_empty() { - remaining.push(right); - } + let right = Self { + min: intersection.max, + max: self.max, + min_inclusive: !intersection.max_inclusive, + max_inclusive: self.max_inclusive, + }; + if right.is_non_empty() { + remaining.push(right); } remaining } fn is_non_empty(&self) -> bool { - match (self.lower.value, self.upper.value) { - (Some(lower), Some(upper)) if lower > upper => false, - (Some(lower), Some(upper)) if lower == upper => { - self.lower.inclusive && self.upper.inclusive - } - _ => true, + if self.max > self.min { + return true; } - } - fn describe(&self) -> String { - let lower = match self.lower.value { - Some(value) if self.lower.inclusive => format!("[{value}"), - Some(value) => format!("({value}"), - None => "(-inf".to_string(), - }; - let upper = match self.upper.value { - Some(value) if self.upper.inclusive => format!("{value}]"), - Some(value) => format!("{value})"), - None => "inf)".to_string(), - }; - - format!("{lower}, {upper}") - } -} - -#[derive(Debug, Clone, Copy, PartialEq)] -struct RangeBound { - value: Option, - inclusive: bool, -} - -impl RangeBound { - fn negative_infinity() -> Self { - Self { - value: None, - inclusive: false, - } - } - - fn positive_infinity() -> Self { - Self { - value: None, - inclusive: false, - } - } - - fn finite(value: f64, inclusive: bool) -> Self { - Self { - value: Some(value), - inclusive, + if self.max == self.min { + return self.min_inclusive && self.max_inclusive; } - } - fn max_lower(left: Self, right: Self) -> Self { - match (left.value, right.value) { - (None, _) => right, - (_, None) => left, - (Some(left_value), Some(right_value)) if left_value > right_value => left, - (Some(left_value), Some(right_value)) if left_value < right_value => right, - (Some(value), Some(_)) => Self::finite(value, left.inclusive && right.inclusive), - } + false } - fn min_upper(left: Self, right: Self) -> Self { - match (left.value, right.value) { - (None, _) => right, - (_, None) => left, - (Some(left_value), Some(right_value)) if left_value < right_value => left, - (Some(left_value), Some(right_value)) if left_value > right_value => right, - (Some(value), Some(_)) => Self::finite(value, left.inclusive && right.inclusive), - } + fn describe(&self) -> String { + format!( + "{}{}, {}{}", + if self.min_inclusive { "[" } else { "(" }, + self.min, + self.max, + if self.max_inclusive { "]" } else { ")" } + ) } } @@ -549,53 +539,60 @@ where >::Error: std::fmt::Debug, { let rounded = value.round(); + ensure!( + (value - rounded).abs() <= 1e-9, + "raw value {value} is not an integer" + ); T::try_from(rounded as i128).map_err(|_| anyhow::anyhow!("raw value {rounded} is out of range")) } #[cfg(test)] mod tests { + use std::{fs, path::PathBuf}; + use liquidcan::payloads::{CanDataType, CanDataValue}; use toml::Value; - use super::{LogicalValue, NodeMapping}; + use super::{LogicalValue, Mapping}; #[test] fn parses_and_applies_mapping_schema() { - let mapping = NodeMapping::parse_mapping( + let mapping = Mapping::parse_mapping( r##" id = "example" -[[mapping]] +[[mapping.ECU]] name = "tank_pressure" type = "telemetry" -raw = { node = "ECU", field = "pressure_adc" } +raw_field = "pressure_adc" value = { slope = 0.5, offset = 1.0, unit = "bar" } -[[mapping.logical]] +[[mapping.ECU.logical]] range = { min = 100 } value = "High" color = "#ff0000" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { max = 100 } value = "Normal" "##, ) .expect("mapping should parse"); - let entry = mapping + let lookup = mapping .get_mapping_for_name("tank_pressure") .expect("entry should exist"); - let mapped = entry + let mapped = lookup + .mapping_entry .mapped_value(&CanDataValue::UInt16(198)) .expect("raw value should map"); assert_eq!(mapped.value, 100.0); assert_eq!(mapped.unit, "bar"); assert_eq!( - entry.logical_value(mapped.value), + lookup.mapping_entry.logical_value(mapped.value), Some(LogicalValue { value: Value::String("High".to_string()), color: Some("#ff0000".to_string()), @@ -605,17 +602,17 @@ value = "Normal" #[test] fn rejects_duplicate_mapping_names() { - let error = NodeMapping::parse_mapping( + let error = Mapping::parse_mapping( r#" -[[mapping]] +[[mapping.node1]] name = "duplicate" -raw = { node = "node1", field = "field1" } +raw_field = "field1" type = "telemetry" value = { slope = 1.0, offset = 0.0 } -[[mapping]] +[[mapping.node1]] name = "duplicate" -raw = { node = "node1", field = "field2" } +raw_field = "field2" type = "telemetry" value = { slope = 1.0, offset = 0.0 } "#, @@ -627,45 +624,119 @@ value = { slope = 1.0, offset = 0.0 } #[test] fn converts_mapped_value_back_to_raw_parameter_type() { - let mapping = NodeMapping::parse_mapping( + let mapping = Mapping::parse_mapping( r#" -[[mapping]] +[[mapping.ECU]] name = "valve_opening" type = "parameter" -raw = { node = "ECU", field = "valve_raw" } +raw_field = "valve_raw" value = { slope = 0.5, offset = 10.0, unit = "%" } "#, ) .expect("mapping should parse"); - let entry = mapping.get_mapping_for_name("valve_opening").unwrap(); - let raw = entry + let lookup = mapping.get_mapping_for_name("valve_opening").unwrap(); + let raw = lookup + .mapping_entry .raw_value_from_mapped(60.0, CanDataType::UInt8) .expect("mapped value should invert to raw"); assert_eq!(raw, CanDataValue::UInt8(100)); } + #[test] + fn rejects_fractional_raw_values_for_integer_parameters() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "valve_opening" +type = "parameter" +raw_field = "valve_raw" +value = { slope = 1.0, offset = 0.0 } +"#, + ) + .expect("mapping should parse"); + + let lookup = mapping.get_mapping_for_name("valve_opening").unwrap(); + let error = lookup + .mapping_entry + .raw_value_from_mapped(10.2, CanDataType::UInt8) + .expect_err("fractional integer raw values should fail"); + + assert!(format!("{error:#}").contains("is not an integer")); + } + + #[test] + fn rejects_duplicate_raw_fields_across_mapping_files() { + let dir = temp_mapping_dir("duplicate_raw"); + fs::write( + dir.join("a.toml"), + r#" +[[mapping.ECU]] +name = "first" +type = "telemetry" +raw_field = "pressure" +"#, + ) + .unwrap(); + fs::write( + dir.join("b.toml"), + r#" +[[mapping.ECU]] +name = "second" +type = "telemetry" +raw_field = "pressure" +"#, + ) + .unwrap(); + + let error = Mapping::load_mapping_from_path(dir.to_str().unwrap()) + .expect_err("duplicate raw fields across files should fail"); + + assert!(format!("{error:#}").contains("Duplicate raw field")); + let _ = fs::remove_dir_all(dir); + } + + #[test] + fn loads_mapping_directory() { + let mapping = Mapping::load_mapping_from_path("tests/mapping/split") + .expect("split mapping directory should be valid"); + + assert!(mapping.get_mapping_for_name("fuel_level").is_some()); + assert!(mapping.get_mapping_for_name("throttle_state").is_some()); + } + + #[test] + fn rejects_empty_mapping_directory() { + let dir = temp_mapping_dir("empty"); + + let error = Mapping::load_mapping_from_path(dir.to_str().unwrap()) + .expect_err("empty mapping directories should fail"); + + assert!(format!("{error:#}").contains("contains no TOML files")); + let _ = fs::remove_dir_all(dir); + } + #[test] fn checked_in_example_mapping_is_valid() { - NodeMapping::load_mapping_from_file("tests/mapping/example1.toml") + Mapping::load_mapping_from_file("tests/mapping/example1.toml") .expect("example mapping should be valid"); } #[test] fn rejects_non_exhaustive_logical_rules() { - let error = NodeMapping::parse_mapping( + let error = Mapping::parse_mapping( r#" -[[mapping]] +[[mapping.ECU]] name = "temperature" type = "telemetry" -raw = { node = "ECU", field = "temperature" } +raw_field = "temperature" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { max = 10 } value = "Cold" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { min = 10, min_inclusive = false } value = "Hot" "#, @@ -677,22 +748,22 @@ value = "Hot" #[test] fn rejects_overlapping_logical_rules() { - let error = NodeMapping::parse_mapping( + let error = Mapping::parse_mapping( r#" -[[mapping]] +[[mapping.ECU]] name = "temperature" type = "telemetry" -raw = { node = "ECU", field = "temperature" } +raw_field = "temperature" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { max = 100 } value = "Low" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { max = 50 } value = "Very low" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { min = 100 } value = "High" "#, @@ -704,22 +775,30 @@ value = "High" #[test] fn accepts_adjacent_ranges() { - NodeMapping::parse_mapping( + Mapping::parse_mapping( r#" -[[mapping]] +[[mapping.ECU]] name = "temperature" type = "telemetry" -raw = { node = "ECU", field = "temperature" } +raw_field = "temperature" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { max = 10 } value = "Cold" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { min = 10 } value = "Hot" "#, ) .expect("adjacent ranges should cover the threshold exactly once"); } + + fn temp_mapping_dir(name: &str) -> PathBuf { + let path = + std::env::temp_dir().join(format!("ferro_flow_mapping_{name}_{}", std::process::id())); + let _ = fs::remove_dir_all(&path); + fs::create_dir_all(&path).unwrap(); + path + } } diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index 03b0452..639485c 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -13,13 +13,13 @@ use liquidcan::{ }, }; -use crate::nodes::mapping::{self, LogicalValue, MappedValue, MappingEntry, NodeMapping}; +use crate::nodes::mapping::{self, LogicalValue, MappedValue, Mapping, MappingLookupResult}; use crate::{db::FieldLog, events}; use super::can_node::{CanNode, FieldInfo, RegistrationInfo, TelemetryGroupDefinition}; pub struct NodeManager<'a> { - mapping: NodeMapping, + mapping: Mapping, can_nodes: DashMap, // Nodes that did not yet receive all their field registrations. @@ -28,7 +28,7 @@ pub struct NodeManager<'a> { } impl<'a> NodeManager<'a> { - pub fn new(event_dispatcher: &'a events::EventDispatcher, mapping: NodeMapping) -> Self { + pub fn new(event_dispatcher: &'a events::EventDispatcher, mapping: Mapping) -> Self { Self { mapping, can_nodes: DashMap::new(), @@ -215,22 +215,24 @@ impl<'a> NodeManager<'a> { ) })?; - let field_infos = field_ids.iter().map(|id| { - node.telemetry_fields - .get(id) - .with_context(|| { - format!( - "received telemetry group update for node {} and group {} but field {} is not defined", - node_id, group_id, id - ) + let field_infos = field_ids + .iter() + .map(|id| { + node.telemetry_fields.get(id).with_context(|| { + format!( + "received telemetry group update for node {} and group {} but field {} is not defined", + node_id, group_id, id + ) + }) }) - }).collect::>>()?; + .collect::>>()?; + + let raw_values = group_update + .values + .unpack(field_infos.iter().map(|info| info.data_type)) + .collect::>(); - for (&id, value) in field_ids.iter().zip( - group_update - .values - .unpack(field_infos.iter().map(|info| info.data_type)), - ) { + for ((&id, field_info), value) in field_ids.iter().zip(field_infos).zip(raw_values) { let value = value.with_context(|| { format!( "failed to unpack value for node {} group {} field {}", @@ -239,8 +241,6 @@ impl<'a> NodeManager<'a> { })?; node.values.insert(id, (timestamp, value.clone())); - let field_info = node.telemetry_fields.get(&id).unwrap(); - let telemetry_log = FieldLog { timestamp, node_id: node_id as i16, @@ -382,19 +382,9 @@ impl<'a> NodeManager<'a> { /// Use this `try_` variant to distinguish missing values from invalid mappings or fields /// that have not registered yet. pub fn try_get_raw_value(&self, mapped_name: &str) -> Result> { - let mapping_entry = self - .mapping - .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}"))?; - let target = self - .resolve_mapping_target(mapping_entry) - .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + let (_, target) = self.resolve_mapping_by_name(mapped_name)?; - Ok(self.can_nodes.get(&target.node_id).and_then(|node| { - node.values - .get(&target.field_id) - .map(|value| value.1.clone()) - })) + Ok(self.latest_raw_value(&target)) } /// Convenience wrapper around `try_get_raw_value` that treats errors as missing values. @@ -406,16 +396,12 @@ impl<'a> NodeManager<'a> { /// /// `Ok(None)` means the mapping and raw field exist, but no value has been received yet. pub fn try_get_mapped_value(&self, mapped_name: &str) -> Result> { - let Some(raw_value) = self.try_get_raw_value(mapped_name)? else { + let (mapping, target) = self.resolve_mapping_by_name(mapped_name)?; + let Some(raw_value) = self.latest_raw_value(&target) else { return Ok(None); }; - let mapping_entry = self - .mapping - .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}"))?; - - Ok(Some(mapping_entry.mapped_value(&raw_value)?)) + Ok(Some(mapping.mapping_entry.mapped_value(&raw_value)?)) } /// Convenience wrapper around `try_get_mapped_value` that treats errors as missing values. @@ -432,12 +418,11 @@ impl<'a> NodeManager<'a> { return Ok(None); }; - let mapping_entry = self - .mapping - .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}"))?; + let mapping_lookup = self.lookup_mapping(mapped_name)?; - Ok(mapping_entry.logical_value(mapped_value.value)) + Ok(mapping_lookup + .mapping_entry + .logical_value(mapped_value.value)) } /// Convenience wrapper around `try_get_logical_value` that treats errors as missing values. @@ -450,13 +435,7 @@ impl<'a> NodeManager<'a> { /// The response is processed asynchronously by the normal CAN message handler and updates the /// cached value read by `get_raw_value`, `get_mapped_value`, and `get_logical_value`. pub fn request_value(&self, mapped_name: &str) -> Result<()> { - let mapping_entry = self - .mapping - .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}"))?; - let target = self - .resolve_mapping_target(mapping_entry) - .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + let (_, target) = self.resolve_mapping_by_name(mapped_name)?; self.event_dispatcher .dispatch(events::Event::SendCanMessage { @@ -476,36 +455,60 @@ impl<'a> NodeManager<'a> { /// The value is converted back to the raw CAN type using the inverse of the configured linear /// mapping, then sent as a `ParameterSetReq`. pub fn set_mapped_value(&self, mapped_name: &str, mapped_value: f64) -> Result<()> { - let mapping_entry = self - .mapping - .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}"))?; - let target = self - .resolve_mapping_target(mapping_entry) - .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + let (mapping_lookup, target) = self.resolve_mapping_by_name(mapped_name)?; - if mapping_entry.field_type != mapping::FieldType::Parameter { + if mapping_lookup.mapping_entry.field_type != mapping::FieldType::Parameter { bail!("mapped field {mapped_name} is not writable because it is not a parameter"); } - let raw_value = mapping_entry.raw_value_from_mapped(mapped_value, target.data_type)?; - self.set_raw_value(mapped_name, raw_value) + let raw_value = mapping_lookup + .mapping_entry + .raw_value_from_mapped(mapped_value, target.data_type)?; + self.dispatch_parameter_set(target, raw_value); + + Ok(()) } /// Writes a raw CAN value to a mapped parameter field. pub fn set_raw_value(&self, mapped_name: &str, raw_value: CanDataValue) -> Result<()> { - let mapping_entry = self - .mapping + let (mapping_lookup, target) = self.resolve_mapping_by_name(mapped_name)?; + + if mapping_lookup.mapping_entry.field_type != mapping::FieldType::Parameter { + bail!("mapped field {mapped_name} is not writable because it is not a parameter"); + } + + self.dispatch_parameter_set(target, raw_value); + + Ok(()) + } + + fn lookup_mapping(&self, mapped_name: &str) -> Result> { + self.mapping .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}"))?; + .with_context(|| format!("no mapping exists for {mapped_name}")) + } + + fn resolve_mapping_by_name( + &self, + mapped_name: &str, + ) -> Result<(MappingLookupResult<'_>, ResolvedMappingTarget)> { + let mapping_lookup = self.lookup_mapping(mapped_name)?; let target = self - .resolve_mapping_target(mapping_entry) + .resolve_mapping_target(&mapping_lookup) .with_context(|| format!("mapped field {mapped_name} is not registered"))?; - if mapping_entry.field_type != mapping::FieldType::Parameter { - bail!("mapped field {mapped_name} is not writable because it is not a parameter"); - } + Ok((mapping_lookup, target)) + } + + fn latest_raw_value(&self, target: &ResolvedMappingTarget) -> Option { + self.can_nodes.get(&target.node_id).and_then(|node| { + node.values + .get(&target.field_id) + .map(|value| value.1.clone()) + }) + } + fn dispatch_parameter_set(&self, target: ResolvedMappingTarget, raw_value: CanDataValue) { self.event_dispatcher .dispatch(events::Event::SendCanMessage { receiver_node_id: target.node_id, @@ -516,8 +519,6 @@ impl<'a> NodeManager<'a> { }, }, }); - - Ok(()) } /// Resolves a mapping entry to the currently registered node id, field id, and field type. @@ -526,21 +527,21 @@ impl<'a> NodeManager<'a> { /// ids learned during node registration. fn resolve_mapping_target( &self, - mapping_entry: &MappingEntry, + mapping_lookup_result: &MappingLookupResult, ) -> Option { self.can_nodes.iter().find_map(|node| { - if node.registration_info.device_name != mapping_entry.raw.node { + if node.registration_info.device_name != mapping_lookup_result.node_name { return None; } - let fields = match mapping_entry.field_type { + let fields = match mapping_lookup_result.mapping_entry.field_type { mapping::FieldType::Telemetry => &node.telemetry_fields, mapping::FieldType::Parameter => &node.parameter_fields, }; fields .iter() - .find(|(_, field)| field.name == mapping_entry.raw.field) + .find(|(_, field)| field.name == mapping_lookup_result.mapping_entry.raw_field) .map(|(field_id, field)| ResolvedMappingTarget { node_id: *node.key(), field_id: *field_id, @@ -605,28 +606,26 @@ mod tests { .set_mapped_value("valve_opening", 60.0) .expect("mapped parameter should be writable"); - let event = rx - .recv_timeout(Duration::from_millis(200)) - .expect("send event should be dispatched"); + assert_eq!( + receive_parameter_set(&rx), + (5, 20, CanDataValue::UInt8(100)) + ); + } - match event { - Event::SendCanMessage { - receiver_node_id, - message: - CanMessage::ParameterSetReq { - payload: - ParameterSetReqPayload { - parameter_id, - value, - }, - }, - } => { - assert_eq!(receiver_node_id, 5); - assert_eq!(parameter_id, 20); - assert_eq!(value, CanDataValue::UInt8(100)); - } - other => panic!("unexpected event: {other:?}"), - } + #[test] + fn writes_raw_parameter_values() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + manager + .set_raw_value("valve_opening", CanDataValue::UInt8(42)) + .expect("raw parameter should be writable"); + + assert_eq!(receive_parameter_set(&rx), (5, 20, CanDataValue::UInt8(42))); } #[test] @@ -658,34 +657,55 @@ mod tests { } } - fn test_mapping() -> NodeMapping { - NodeMapping::parse_mapping( + fn test_mapping() -> Mapping { + Mapping::parse_mapping( r##" -[[mapping]] +[[mapping.ECU]] name = "tank_pressure" type = "telemetry" -raw = { node = "ECU", field = "pressure_adc" } +raw_field = "pressure_adc" value = { slope = 0.5, offset = 1.0, unit = "bar" } -[[mapping.logical]] +[[mapping.ECU.logical]] range = { min = 100 } value = "High" color = "#ff0000" -[[mapping.logical]] +[[mapping.ECU.logical]] range = { max = 100 } value = "Normal" -[[mapping]] +[[mapping.ECU]] name = "valve_opening" type = "parameter" -raw = { node = "ECU", field = "valve_raw" } +raw_field = "valve_raw" value = { slope = 0.5, offset = 10.0, unit = "%" } "##, ) .expect("mapping should parse") } + fn receive_parameter_set(rx: &mpsc::Receiver) -> (u8, u8, CanDataValue) { + let event = rx + .recv_timeout(Duration::from_millis(200)) + .expect("send event should be dispatched"); + + match event { + Event::SendCanMessage { + receiver_node_id, + message: + CanMessage::ParameterSetReq { + payload: + ParameterSetReqPayload { + parameter_id, + value, + }, + }, + } => (receiver_node_id, parameter_id, value), + other => panic!("unexpected event: {other:?}"), + } + } + fn insert_test_node(manager: &NodeManager<'_>) { let mut node = CanNode::new(RegistrationInfo { telemetry_count: 1, diff --git a/tests/emulator.rs b/tests/emulator.rs index 7b247f1..0623760 100644 --- a/tests/emulator.rs +++ b/tests/emulator.rs @@ -3,7 +3,7 @@ mod common; use crate::common::ShutdownGuard; use chrono::{DateTime, Utc}; use ferro_flow::config::Config; -use ferro_flow::nodes::mapping::NodeMapping; +use ferro_flow::nodes::mapping::Mapping; use ferro_flow::{events, nodes, run_with_dependencies}; use liquidcan::payloads::CanDataType; use std::{io::Write, time::Instant}; @@ -18,7 +18,7 @@ fn test_node_registration() { let emulator_config = ecuemulator_test_config_toml(&vcan_iface); let event_dispatcher = events::EventDispatcher::new(); - let node_manager = nodes::NodeManager::new(&event_dispatcher, NodeMapping::default()); + let node_manager = nodes::NodeManager::new(&event_dispatcher, Mapping::default()); let config = build_test_config(&vcan_iface); std::thread::scope(|s| { @@ -110,7 +110,7 @@ fn test_telemetry_group_updates() { let emulator_config = ecuemulator_test_config_toml(&vcan_iface); let event_dispatcher = events::EventDispatcher::new(); - let node_manager = nodes::NodeManager::new(&event_dispatcher, NodeMapping::default()); + let node_manager = nodes::NodeManager::new(&event_dispatcher, Mapping::default()); let config = build_test_config(&vcan_iface); println!("Starting application with test config: {:?}", config); diff --git a/tests/mapping/example1.toml b/tests/mapping/example1.toml index 49b7c77..e044bfa 100644 --- a/tests/mapping/example1.toml +++ b/tests/mapping/example1.toml @@ -1,36 +1,31 @@ -id = "example1" -description = "This is an example mapping file." - -[[mapping]] -raw.node = "node1" -raw.field = "field1" +[[mapping.node1]] name = "Example Mapping 1" type = "telemetry" +raw_field = "field1" value.unit = "mAh" value.slope = 0.5 value.offset = 1.0 -[[mapping.logical]] +[[mapping.node1.logical]] range = { min = 100 } value = "High" color = "#ff0000" -[[mapping.logical]] +[[mapping.node1.logical]] range = { min = 50, max = 100 } value = "Normal" -[[mapping.logical]] +[[mapping.node1.logical]] range = { max = 50 } value = "Low" # alternatively -[[mapping]] +[[mapping.node1]] name = "Example Mapping 2" type = "parameter" - -raw = { node = "node1", field = "field2" } +raw_field = "field2" value = { slope = 0.5, offset = 1.0, unit = "mAh" } diff --git a/tests/mapping/split/engine.toml b/tests/mapping/split/engine.toml new file mode 100644 index 0000000..6d80406 --- /dev/null +++ b/tests/mapping/split/engine.toml @@ -0,0 +1,10 @@ +[[mapping.EngineECU]] +name = "throttle_state" +type = "parameter" +raw_field = "throttle_raw" +value = { slope = 0.25, offset = 0.0, unit = "%" } +logical = [ + { range = { min = 100 }, value = "High", color = "#ff0000" }, + { range = { min = 50, max = 100 }, value = "Normal" }, + { range = { max = 50 }, value = "Low" }, +] diff --git a/tests/mapping/split/fuel.toml b/tests/mapping/split/fuel.toml new file mode 100644 index 0000000..eb72155 --- /dev/null +++ b/tests/mapping/split/fuel.toml @@ -0,0 +1,5 @@ +[[mapping.FuelECU]] +name = "fuel_level" +type = "telemetry" +raw_field = "level_adc" +value = { slope = 0.5, offset = 1.0, unit = "mAh" } From 7445262c8579bdf058c8c426db256e560e0c52ef Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 7 May 2026 20:28:52 +0200 Subject: [PATCH 04/19] remove id from mapping --- src/nodes/mapping.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index dc55425..b826613 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -560,8 +560,6 @@ mod tests { fn parses_and_applies_mapping_schema() { let mapping = Mapping::parse_mapping( r##" -id = "example" - [[mapping.ECU]] name = "tank_pressure" type = "telemetry" From 808aff365828d14cd52c96c5d681733c55f4d473 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 7 May 2026 20:30:17 +0200 Subject: [PATCH 05/19] add schema --- README.md | 2 + schemas/mapping.schema.json | 121 ++++++++++++++++++++++++++++++++++++ taplo.toml | 5 ++ 3 files changed, 128 insertions(+) create mode 100644 schemas/mapping.schema.json create mode 100644 taplo.toml diff --git a/README.md b/README.md index e8e2f19..94d86fe 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,8 @@ logical = [ ] ``` +The repository includes [schemas/mapping.schema.json](schemas/mapping.schema.json) and [taplo.toml](taplo.toml) so Taplo-compatible editors, including VS Code with Even Better TOML, can validate mapping files before the application loads them. The schema is associated with `mapping.toml` files and TOML files under `mapping/` or `mappings/` directories. + ### Running CI Checks The repository includes a CI script (`ci-rust.sh`) that runs all quality checks on the Rust implementation. This script is used both locally and in GitHub Actions diff --git a/schemas/mapping.schema.json b/schemas/mapping.schema.json new file mode 100644 index 0000000..533d48a --- /dev/null +++ b/schemas/mapping.schema.json @@ -0,0 +1,121 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://spaceteam.at/ferroflow/schemas/mapping.schema.json", + "title": "FerroFlow Mapping", + "description": "TOML schema for FerroFlow node mapping files.", + "type": "object", + "required": ["mapping"], + "additionalProperties": false, + "properties": { + "mapping": { + "type": "object", + "description": "Mappings grouped by LiquidCAN node/device name.", + "minProperties": 1, + "additionalProperties": false, + "patternProperties": { + ".+": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/mappingEntry" + } + } + } + } + }, + "$defs": { + "mappingEntry": { + "type": "object", + "additionalProperties": false, + "required": ["name", "type", "raw_field"], + "properties": { + "name": { + "type": "string", + "minLength": 1, + "description": "Unique application-facing mapping name." + }, + "type": { + "type": "string", + "enum": ["telemetry", "parameter"], + "description": "Whether the raw field is telemetry or a writable parameter." + }, + "raw_field": { + "type": "string", + "minLength": 1, + "description": "Raw LiquidCAN field name on the enclosing node." + }, + "value": { + "$ref": "#/$defs/valueParams" + }, + "logical": { + "type": "array", + "description": "Logical labels for mapped numeric ranges. Runtime validation requires these ranges to be non-overlapping and exhaustive when present.", + "items": { + "$ref": "#/$defs/logicalRule" + } + } + } + }, + "valueParams": { + "type": "object", + "additionalProperties": false, + "required": ["slope", "offset"], + "properties": { + "slope": { + "type": "number", + "not": { "const": 0 }, + "default": 1.0, + "description": "Linear conversion slope: mapped = raw * slope + offset." + }, + "offset": { + "type": "number", + "default": 0.0, + "description": "Linear conversion offset: mapped = raw * slope + offset." + }, + "unit": { + "type": "string", + "default": "" + } + } + }, + "logicalRule": { + "type": "object", + "additionalProperties": false, + "required": ["range", "value"], + "properties": { + "range": { + "$ref": "#/$defs/logicalRange" + }, + "value": { + "description": "Logical value returned when the mapped numeric value is inside the range." + }, + "color": { + "type": "string", + "description": "Optional display color, commonly a hex color such as #ff0000." + } + } + }, + "logicalRange": { + "type": "object", + "additionalProperties": false, + "properties": { + "min": { + "type": "number", + "description": "Lower bound. Omit for an unbounded lower range." + }, + "max": { + "type": "number", + "description": "Upper bound. Omit for an unbounded upper range." + }, + "min_inclusive": { + "type": "boolean", + "default": true + }, + "max_inclusive": { + "type": "boolean", + "default": false + } + } + } + } +} diff --git a/taplo.toml b/taplo.toml new file mode 100644 index 0000000..cc11e9b --- /dev/null +++ b/taplo.toml @@ -0,0 +1,5 @@ +[[rule]] +include = ["**/mapping.toml", "**/mapping/**/*.toml", "**/mappings/**/*.toml"] + +[rule.schema] +path = "./schemas/mapping.schema.json" From 2d86fd276cfd7bcf6cfdd63eea5808f2ff1c0b36 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 9 May 2026 01:17:38 +0200 Subject: [PATCH 06/19] remove exhaustiveness check for logical mappings --- schemas/mapping.schema.json | 2 +- src/nodes/mapping.rs | 85 +------------------------------------ 2 files changed, 2 insertions(+), 85 deletions(-) diff --git a/schemas/mapping.schema.json b/schemas/mapping.schema.json index 533d48a..acf0137 100644 --- a/schemas/mapping.schema.json +++ b/schemas/mapping.schema.json @@ -49,7 +49,7 @@ }, "logical": { "type": "array", - "description": "Logical labels for mapped numeric ranges. Runtime validation requires these ranges to be non-overlapping and exhaustive when present.", + "description": "Logical labels for mapped numeric ranges. Runtime validation requires these ranges to be non-overlapping.", "items": { "$ref": "#/$defs/logicalRule" } diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index b826613..eb66556 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -277,9 +277,7 @@ impl MappingEntry { /// Validates that logical rules form an unambiguous partition of all mapped values. /// - /// Empty logical rules are allowed. Once any logical rule is present, the ranges must be - /// non-empty, non-overlapping, and exhaustive over `(-inf, inf)` so every mapped value has - /// exactly one logical value. + /// Empty logical rules are allowed. Ranges must be non-empty and non-overlapping. fn validate_logical_rules(&self) -> anyhow::Result<()> { if self.logical.is_empty() { return Ok(()); @@ -312,24 +310,6 @@ impl MappingEntry { covered_ranges.push(rule.range.clone()); } - let uncovered_ranges = covered_ranges.iter().fold( - vec![LogicalRange::all()], - |remaining_uncovered_ranges, covered_range| { - remaining_uncovered_ranges - .into_iter() - .flat_map(|range| range.difference(covered_range)) - .collect::>() - }, - ); - - if let Some(uncovered_range) = uncovered_ranges.first() { - bail!( - "Logical rules for mapping {} are not exhaustive; values in {} are not matched", - self.name, - uncovered_range.describe() - ); - } - Ok(()) } @@ -385,16 +365,6 @@ impl LogicalRule { } impl LogicalRange { - /// The complete domain - fn all() -> Self { - Self { - min: f64::NEG_INFINITY, - max: f64::INFINITY, - min_inclusive: false, - max_inclusive: false, - } - } - fn contains(&self, value: f64) -> bool { let above_lower = if self.min_inclusive { value >= self.min @@ -441,36 +411,6 @@ impl LogicalRange { } } - fn difference(&self, other: &Self) -> Vec { - let Some(intersection) = self.intersection(other) else { - return vec![self.clone()]; - }; - - let mut remaining = Vec::new(); - - let left = Self { - min: self.min, - max: intersection.min, - min_inclusive: self.min_inclusive, - max_inclusive: !intersection.min_inclusive, - }; - if left.is_non_empty() { - remaining.push(left); - } - - let right = Self { - min: intersection.max, - max: self.max, - min_inclusive: !intersection.max_inclusive, - max_inclusive: self.max_inclusive, - }; - if right.is_non_empty() { - remaining.push(right); - } - - remaining - } - fn is_non_empty(&self) -> bool { if self.max > self.min { return true; @@ -721,29 +661,6 @@ raw_field = "pressure" .expect("example mapping should be valid"); } - #[test] - fn rejects_non_exhaustive_logical_rules() { - let error = Mapping::parse_mapping( - r#" -[[mapping.ECU]] -name = "temperature" -type = "telemetry" -raw_field = "temperature" - -[[mapping.ECU.logical]] -range = { max = 10 } -value = "Cold" - -[[mapping.ECU.logical]] -range = { min = 10, min_inclusive = false } -value = "Hot" -"#, - ) - .expect_err("rules should miss exactly 10"); - - assert!(format!("{error:#}").contains("not exhaustive")); - } - #[test] fn rejects_overlapping_logical_rules() { let error = Mapping::parse_mapping( From eb52e2ac2386c4b42129ecbf137e9e50197cb248 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 28 May 2026 21:15:59 +0200 Subject: [PATCH 07/19] remove color from mapping --- README.md | 2 +- schemas/mapping.schema.json | 4 ---- src/nodes/mapping.rs | 6 ------ src/nodes/node_manager.rs | 2 -- tests/mapping/example1.toml | 3 +-- tests/mapping/split/engine.toml | 2 +- 6 files changed, 3 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 94d86fe..39f0842 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ raw_field = "level_adc" value = { slope = 0.5, offset = 1.0, unit = "mAh" } logical = [ - { range = { min = 100 }, value = "High", color = "#ff0000" }, + { range = { min = 100 }, value = "High" }, { range = { min = 50, max = 100 }, value = "Normal" }, { range = { max = 50 }, value = "Low" }, ] diff --git a/schemas/mapping.schema.json b/schemas/mapping.schema.json index acf0137..c0abe5c 100644 --- a/schemas/mapping.schema.json +++ b/schemas/mapping.schema.json @@ -88,10 +88,6 @@ }, "value": { "description": "Logical value returned when the mapped numeric value is inside the range." - }, - "color": { - "type": "string", - "description": "Optional display color, commonly a hex color such as #ff0000." } } }, diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index eb66556..7cd7096 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -48,8 +48,6 @@ impl Default for ValueParams { pub struct LogicalRule { pub range: LogicalRange, pub value: Value, - #[serde(default)] - pub color: Option, } #[derive(Debug, Clone, Deserialize)] @@ -88,7 +86,6 @@ pub enum FieldType { #[derive(Debug, Clone, PartialEq)] pub struct LogicalValue { pub value: Value, - pub color: Option, } #[derive(Debug, Clone, PartialEq)] @@ -353,7 +350,6 @@ impl MappingEntry { .find(|rule| rule.matches(mapped_value)) .map(|rule| LogicalValue { value: rule.value.clone(), - color: rule.color.clone(), }) } } @@ -509,7 +505,6 @@ value = { slope = 0.5, offset = 1.0, unit = "bar" } [[mapping.ECU.logical]] range = { min = 100 } value = "High" -color = "#ff0000" [[mapping.ECU.logical]] range = { max = 100 } @@ -533,7 +528,6 @@ value = "Normal" lookup.mapping_entry.logical_value(mapped.value), Some(LogicalValue { value: Value::String("High".to_string()), - color: Some("#ff0000".to_string()), }) ); } diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index 639485c..c8c8589 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -590,7 +590,6 @@ mod tests { .get_logical_value("tank_pressure") .expect("logical value should be available"); assert_eq!(logical.value, Value::String("High".to_string())); - assert_eq!(logical.color, Some("#ff0000".to_string())); } #[test] @@ -669,7 +668,6 @@ value = { slope = 0.5, offset = 1.0, unit = "bar" } [[mapping.ECU.logical]] range = { min = 100 } value = "High" -color = "#ff0000" [[mapping.ECU.logical]] range = { max = 100 } diff --git a/tests/mapping/example1.toml b/tests/mapping/example1.toml index e044bfa..9848d5b 100644 --- a/tests/mapping/example1.toml +++ b/tests/mapping/example1.toml @@ -9,7 +9,6 @@ value.offset = 1.0 [[mapping.node1.logical]] range = { min = 100 } value = "High" -color = "#ff0000" [[mapping.node1.logical]] range = { min = 50, max = 100 } @@ -30,7 +29,7 @@ raw_field = "field2" value = { slope = 0.5, offset = 1.0, unit = "mAh" } logical = [ - { range = { min = 100 }, value = "High", color = "#ff0000" }, + { range = { min = 100 }, value = "High" }, { range = { min = 50, max = 100 }, value = "Normal" }, { range = { max = 50 }, value = "Low" }, ] diff --git a/tests/mapping/split/engine.toml b/tests/mapping/split/engine.toml index 6d80406..ace9a7a 100644 --- a/tests/mapping/split/engine.toml +++ b/tests/mapping/split/engine.toml @@ -4,7 +4,7 @@ type = "parameter" raw_field = "throttle_raw" value = { slope = 0.25, offset = 0.0, unit = "%" } logical = [ - { range = { min = 100 }, value = "High", color = "#ff0000" }, + { range = { min = 100 }, value = "High" }, { range = { min = 50, max = 100 }, value = "Normal" }, { range = { max = 50 }, value = "Low" }, ] From a48c6cbb67a8f6f0326b336f01e081e087476be3 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 29 May 2026 00:12:40 +0200 Subject: [PATCH 08/19] fix clippy --- src/nodes/mapping.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index 7cd7096..9a05704 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -161,11 +161,7 @@ impl Mapping { })?; for (node, fields) in mapping.mapping { - combined - .mapping - .entry(node) - .or_default() - .extend(fields.into_iter()); + combined.mapping.entry(node).or_default().extend(fields); } } From ed16f30a0231811be957a636a7b01d97c92a1743 Mon Sep 17 00:00:00 2001 From: Raffael Rott Date: Mon, 1 Jun 2026 15:33:42 +0200 Subject: [PATCH 09/19] Added tests --- src/nodes/node_manager.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index c8c8589..47e8cf7 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -470,6 +470,8 @@ impl<'a> NodeManager<'a> { } /// Writes a raw CAN value to a mapped parameter field. + /// + /// The value is sent as a `ParameterSetReq`. pub fn set_raw_value(&self, mapped_name: &str, raw_value: CanDataValue) -> Result<()> { let (mapping_lookup, target) = self.resolve_mapping_by_name(mapped_name)?; @@ -590,6 +592,30 @@ mod tests { .get_logical_value("tank_pressure") .expect("logical value should be available"); assert_eq!(logical.value, Value::String("High".to_string())); + + let non_existant_mapped = manager.try_get_mapped_value("non_existent"); + assert!( + non_existant_mapped + .is_err_and(|e| { e.to_string() == "no mapping exists for non_existent" }) + ); + + let non_existant_logical = manager.try_get_logical_value("non_existent"); + assert!( + non_existant_logical + .is_err_and(|e| { e.to_string() == "no mapping exists for non_existent" }) + ); + + let non_existant_raw = manager.try_get_raw_value("non_existent"); + assert!( + non_existant_raw + .is_err_and(|e| { e.to_string() == "no mapping exists for non_existent" }) + ); + + let non_registered_mapped = manager.try_get_mapped_value("tank_temp"); + assert!( + non_registered_mapped + .is_err_and(|e| { e.to_string() == "mapped field tank_temp is not registered" }) + ); } #[test] @@ -678,6 +704,12 @@ name = "valve_opening" type = "parameter" raw_field = "valve_raw" value = { slope = 0.5, offset = 10.0, unit = "%" } + +[[mapping.ECU]] +name = "tank_temp" +type = "telemetry" +raw_field = "temp_adc" +value = { slope = 0.5, offset = 10.0, unit = "%" } "##, ) .expect("mapping should parse") From 68612a168b536c0cfb71b20b2c4c90c7eaa1db5e Mon Sep 17 00:00:00 2001 From: Raffael Rott Date: Mon, 1 Jun 2026 17:38:53 +0200 Subject: [PATCH 10/19] Added tests: - mapping - node_manager --- src/nodes/mapping.rs | 494 ++++++++++++++++++++++++++++++++++++++ src/nodes/node_manager.rs | 285 ++++++++++++++++++++++ 2 files changed, 779 insertions(+) diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index 9a05704..f2ed425 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -483,6 +483,7 @@ where mod tests { use std::{fs, path::PathBuf}; + use chrono::Utc; use liquidcan::payloads::{CanDataType, CanDataValue}; use toml::Value; @@ -528,6 +529,499 @@ value = "Normal" ); } + #[test] + fn rejects_empty_node_name() { + let error = Mapping::parse_mapping( + r#" +[mapping] +" " = [{ name = "x", type = "telemetry", raw_field = "f" }] +"#, + ) + .expect_err("empty node name should fail validation"); + + assert!(format!("{error:#}").contains("empty node name")); + } + + #[test] + fn rejects_empty_mapping_name() { + let error = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "" +type = "telemetry" +raw_field = "field" +"#, + ) + .expect_err("empty mapping name should fail validation"); + + assert!(format!("{error:#}").contains("mapping name must be non-empty")); + } + + #[test] + fn rejects_empty_raw_field() { + let error = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "" +"#, + ) + .expect_err("empty raw_field should fail validation"); + + assert!(format!("{error:#}").contains("has an empty raw_field")); + } + + #[test] + fn rejects_zero_slope() { + let error = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" +value = { slope = 0.0, offset = 0.0 } +"#, + ) + .expect_err("zero slope should fail validation"); + + assert!(format!("{error:#}").contains("slope of zero")); + } + + #[test] + fn rejects_non_finite_slope() { + let mapping = Mapping { + mapping: [( + "ECU".to_string(), + vec![super::MappingEntry { + name: "x".to_string(), + field_type: super::FieldType::Telemetry, + raw_field: "f".to_string(), + value: super::ValueParams { + slope: f64::INFINITY, + offset: 0.0, + unit: "".to_string(), + }, + logical: vec![], + }], + )] + .into_iter() + .collect(), + }; + + let error = mapping + .validate() + .expect_err("non-finite slope should fail validation"); + assert!(format!("{error:#}").contains("non-finite slope")); + } + + #[test] + fn rejects_non_finite_offset() { + let mapping = Mapping { + mapping: [( + "ECU".to_string(), + vec![super::MappingEntry { + name: "x".to_string(), + field_type: super::FieldType::Telemetry, + raw_field: "f".to_string(), + value: super::ValueParams { + slope: 1.0, + offset: f64::NAN, + unit: "".to_string(), + }, + logical: vec![], + }], + )] + .into_iter() + .collect(), + }; + + let error = mapping + .validate() + .expect_err("non-finite offset should fail validation"); + assert!(format!("{error:#}").contains("non-finite offset")); + } + + #[test] + fn rejects_duplicate_mapping_name_across_nodes() { + let error = Mapping::parse_mapping( + r#" +[[mapping.NodeA]] +name = "dup" +type = "telemetry" +raw_field = "a" + +[[mapping.NodeB]] +name = "dup" +type = "telemetry" +raw_field = "b" +"#, + ) + .expect_err("duplicate mapping names across nodes should fail validation"); + + assert!(format!("{error:#}").contains("Duplicate mapping name")); + } + + #[test] + fn looks_up_mapping_by_raw_field_and_type() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "tank_pressure" +type = "telemetry" +raw_field = "pressure_adc" + +[[mapping.ECU]] +name = "valve_opening" +type = "parameter" +raw_field = "valve_raw" +"#, + ) + .expect("mapping should parse"); + + let telemetry = mapping + .get_mapping_for_raw("ECU", "pressure_adc", super::FieldType::Telemetry) + .expect("telemetry mapping should exist"); + assert_eq!(telemetry.mapping_entry.name, "tank_pressure"); + + let parameter = mapping + .get_mapping_for_raw("ECU", "valve_raw", super::FieldType::Parameter) + .expect("parameter mapping should exist"); + assert_eq!(parameter.mapping_entry.name, "valve_opening"); + + assert!( + mapping + .get_mapping_for_raw("ECU", "pressure_adc", super::FieldType::Parameter) + .is_none() + ); + } + + #[test] + fn logical_value_returns_none_when_value_in_gap() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" + +[[mapping.ECU.logical]] +range = { min = 0, max = 10 } +value = "Low" + +[[mapping.ECU.logical]] +range = { min = 20, max = 30 } +value = "High" +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + assert_eq!(entry.mapping_entry.logical_value(15.0), None); + } + + #[test] + fn logical_range_inclusive_exclusive_boundaries() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" + +[[mapping.ECU.logical]] +range = { max = 10 } +value = "Low" + +[[mapping.ECU.logical]] +range = { min = 10 } +value = "High" +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + assert_eq!( + entry.mapping_entry.logical_value(10.0), + Some(LogicalValue { + value: Value::String("High".to_string()) + }) + ); + } + + #[test] + fn rejects_overlap_due_to_inclusive_boundary() { + let error = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" + +[[mapping.ECU.logical]] +range = { max = 10, max_inclusive = true } +value = "Low" + +[[mapping.ECU.logical]] +range = { min = 10, min_inclusive = true } +value = "High" +"#, + ) + .expect_err("inclusive boundary overlap should fail validation"); + + assert!(format!("{error:#}").contains("overlaps")); + } + + #[test] + fn accepts_single_point_range_when_both_inclusive() { + Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" + +[[mapping.ECU.logical]] +range = { min = 10, max = 10, min_inclusive = true, max_inclusive = true } +value = "Ten" +"#, + ) + .expect("single-point inclusive range should be valid"); + } + + #[test] + fn rejects_single_point_range_when_any_exclusive() { + let error = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" + +[[mapping.ECU.logical]] +range = { min = 10, max = 10, min_inclusive = true, max_inclusive = false } +value = "Ten" +"#, + ) + .expect_err("single-point exclusive range should be rejected"); + + assert!(format!("{error:#}").contains("empty range")); + } + + #[test] + fn mapped_value_converts_all_can_types_to_f64() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" +value = { slope = 1.0, offset = 0.0 } +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::Int8(-5)) + .unwrap(); + assert!((mapped.value - (-5.0)).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::Int16(-300)) + .unwrap(); + assert!((mapped.value - (-300.0)).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::Int32(-100_000)) + .unwrap(); + assert!((mapped.value - (-100_000.0)).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::UInt8(250)) + .unwrap(); + assert!((mapped.value - 250.0).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::UInt16(50_000)) + .unwrap(); + assert!((mapped.value - 50_000.0).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::UInt32(1_000_000)) + .unwrap(); + assert!((mapped.value - 1_000_000.0).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::Float32(1.5)) + .unwrap(); + assert!((mapped.value - 1.5).abs() < 1e-6); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::Boolean(true)) + .unwrap(); + assert!((mapped.value - 1.0).abs() < 1e-12); + + let mapped = entry + .mapping_entry + .mapped_value(&CanDataValue::Boolean(false)) + .unwrap(); + assert!((mapped.value - 0.0).abs() < 1e-12); + } + + #[test] + fn mapped_value_rejects_raw() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "telemetry" +raw_field = "f" +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + let error = entry + .mapping_entry + .mapped_value(&CanDataValue::Raw(vec![1, 2, 3])) + .expect_err("raw values should be rejected"); + + assert!( + format!("{error:#}").contains("raw CAN data must be decoded before applying a mapping") + ); + } + + #[test] + fn raw_value_from_mapped_rejects_out_of_range_integers() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "parameter" +raw_field = "f" +value = { slope = 1.0, offset = 0.0 } +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + let error = entry + .mapping_entry + .raw_value_from_mapped(9999.0, CanDataType::UInt8) + .expect_err("out of range values should fail"); + assert!(format!("{error:#}").contains("out of range")); + } + + #[test] + fn raw_value_from_mapped_boolean_accepts_only_0_or_1() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "parameter" +raw_field = "f" +value = { slope = 1.0, offset = 0.0 } +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + assert_eq!( + entry + .mapping_entry + .raw_value_from_mapped(0.0, CanDataType::Boolean) + .unwrap(), + CanDataValue::Boolean(false) + ); + assert_eq!( + entry + .mapping_entry + .raw_value_from_mapped(1.0, CanDataType::Boolean) + .unwrap(), + CanDataValue::Boolean(true) + ); + + let error = entry + .mapping_entry + .raw_value_from_mapped(0.5, CanDataType::Boolean) + .expect_err("non 0/1 boolean values should fail"); + assert!(format!("{error:#}").contains("must map back to 0 or 1")); + } + + #[test] + fn raw_value_from_mapped_rejects_nan_or_inf() { + let mapping = Mapping::parse_mapping( + r#" +[[mapping.ECU]] +name = "x" +type = "parameter" +raw_field = "f" +value = { slope = 1.0, offset = 0.0 } +"#, + ) + .expect("mapping should parse"); + + let entry = mapping.get_mapping_for_name("x").unwrap(); + let error = entry + .mapping_entry + .raw_value_from_mapped(f64::NAN, CanDataType::UInt8) + .expect_err("NaN should fail"); + assert!(format!("{error:#}").contains("must be finite")); + + let error = entry + .mapping_entry + .raw_value_from_mapped(f64::INFINITY, CanDataType::UInt8) + .expect_err("Inf should fail"); + assert!(format!("{error:#}").contains("must be finite")); + } + + #[test] + fn load_mapping_from_file_empty_path_returns_default() { + let mapping = Mapping::load_mapping_from_file("").unwrap(); + assert!(mapping.mapping.is_empty()); + } + + #[test] + fn load_mapping_from_path_empty_path_returns_default() { + let mapping = Mapping::load_mapping_from_path("").unwrap(); + assert!(mapping.mapping.is_empty()); + } + + #[test] + fn load_mapping_from_file_missing_file_errors_with_context() { + let path = std::env::temp_dir().join(format!( + "ferro_flow_mapping_missing_{}_{}.toml", + std::process::id(), + Utc::now().timestamp_nanos_opt().unwrap_or(0) + )); + + let error = Mapping::load_mapping_from_file(path.to_str().unwrap()) + .expect_err("missing mapping file should error"); + assert!(format!("{error:#}").contains("Failed to read mapping config file")); + } + + #[test] + fn load_mapping_from_path_missing_dir_errors_with_context() { + let path = std::env::temp_dir().join(format!( + "ferro_flow_mapping_missing_dir_{}_{}", + std::process::id(), + Utc::now().timestamp_nanos_opt().unwrap_or(0) + )); + + let error = Mapping::load_mapping_from_path(path.to_str().unwrap()) + .expect_err("missing mapping directory should error"); + assert!(format!("{error:#}").contains("Failed to read mapping directory")); + } + #[test] fn rejects_duplicate_mapping_names() { let error = Mapping::parse_mapping( diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index 47e8cf7..d627ffc 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -618,6 +618,36 @@ mod tests { ); } + #[test] + fn try_get_mapped_value_returns_ok_none_when_no_value_cached() { + let dispatcher = EventDispatcher::new(); + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + assert_eq!(manager.try_get_raw_value("valve_opening").unwrap(), None); + assert_eq!(manager.try_get_mapped_value("valve_opening").unwrap(), None); + assert_eq!( + manager.try_get_logical_value("valve_opening").unwrap(), + None + ); + } + + #[test] + fn get_returns_none_on_missing_mapping_or_unregistered() { + let dispatcher = EventDispatcher::new(); + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + assert_eq!(manager.get_raw_value("non_existent"), None); + assert_eq!(manager.get_mapped_value("non_existent"), None); + assert_eq!(manager.get_logical_value("non_existent"), None); + + // mapping exists but is not registered on the inserted test node + assert_eq!(manager.get_raw_value("tank_temp"), None); + assert_eq!(manager.get_mapped_value("tank_temp"), None); + assert_eq!(manager.get_logical_value("tank_temp"), None); + } + #[test] fn writes_mapped_parameter_values_as_raw_can_values() { let dispatcher = EventDispatcher::new(); @@ -682,6 +712,227 @@ mod tests { } } + #[test] + fn set_value_rejects_telemetry_values() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + let err = manager + .set_mapped_value("tank_pressure", 10.0) + .expect_err("telemetry mappings should not be writable"); + assert!(err.to_string().contains("is not writable")); + + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + + let err = manager + .set_raw_value("tank_pressure", CanDataValue::UInt16(1)) + .expect_err("telemetry mappings should not be writable"); + assert!(err.to_string().contains("is not writable")); + + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + } + + #[test] + fn set_mapped_value_rejects_nan() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + assert!(manager.set_mapped_value("valve_opening", f64::NAN).is_err()); + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + } + + #[test] + fn set_mapped_value_rejects_out_of_range() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + let err = manager + .set_mapped_value("valve_opening", 1000.0) + .expect_err("out of range values should fail"); + assert!(err.to_string().contains("out of range")); + + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + } + + #[test] + fn set_mapped_value_rejects_fractional_for_integer_type() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + let err = manager + .set_mapped_value("valve_opening", 10.1) + .expect_err("fractional inverse-mapped raw values should fail"); + assert!(err.to_string().contains("is not an integer")); + + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + } + + #[test] + fn request_value_errors_when_mapping_missing() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + let err = manager + .request_value("non_existent") + .expect_err("missing mappings should error"); + assert_eq!(err.to_string(), "no mapping exists for non_existent"); + + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + } + + #[test] + fn request_value_errors_when_unregistered() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + let err = manager + .request_value("tank_temp") + .expect_err("unregistered mappings should error"); + assert_eq!(err.to_string(), "mapped field tank_temp is not registered"); + + assert!(matches!( + rx.recv_timeout(Duration::from_millis(50)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + } + + #[test] + fn telemetry_group_update_with_multiple_fields_pairs_values_correctly() { + use liquidcan::payloads::PackedCanDataValues; + + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe( + tx, + vec![EventKind::NodeFieldUpdated], + "test-node-field-updated", + ); + + let manager = NodeManager::new(&dispatcher, Mapping::default()); + insert_two_field_test_node(&manager); + + let values = PackedCanDataValues::<62>::try_from(&[ + CanDataValue::UInt16(0x1234), + CanDataValue::UInt32(0x89ABCDEF), + ] as &[CanDataValue]) + .expect("values should pack"); + + let payload = TelemetryGroupUpdatePayload { + group_id: 1, + values, + }; + let msg_id = CanMessageId::new() + .with_sender_id(5) + .with_receiver_id(liquidcan::NODE_ID_SERVER); + + manager + .handle_telemetry_group_update(msg_id, payload) + .expect("update should succeed"); + + let node = manager.can_nodes.get(&5).unwrap(); + assert_eq!( + node.values.get(&10).unwrap().value().1, + CanDataValue::UInt16(0x1234) + ); + assert_eq!( + node.values.get(&11).unwrap().value().1, + CanDataValue::UInt32(0x89ABCDEF) + ); + + // Two update events with correct field ids and names. + let evt1 = rx.recv_timeout(Duration::from_millis(200)).unwrap(); + let evt2 = rx.recv_timeout(Duration::from_millis(200)).unwrap(); + + let mut logs = vec![]; + for evt in [evt1, evt2] { + match evt { + Event::NodeFieldUpdated(log) => logs.push(log), + other => panic!("unexpected event: {other:?}"), + } + } + logs.sort_by_key(|l| l.field_id); + + assert_eq!(logs[0].node_id, 5); + assert_eq!(logs[0].field_id, 10); + assert_eq!(logs[0].field_name, "a"); + assert_eq!(logs[0].field_value, serde_json::json!(0x1234u16)); + + assert_eq!(logs[1].node_id, 5); + assert_eq!(logs[1].field_id, 11); + assert_eq!(logs[1].field_name, "b"); + assert_eq!(logs[1].field_value, serde_json::json!(0x89ABCDEFu32)); + } + + #[test] + fn telemetry_group_update_unpacked_value_error_mentions_node_group_field() { + use liquidcan::payloads::PackedCanDataValues; + + let dispatcher = EventDispatcher::new(); + let manager = NodeManager::new(&dispatcher, Mapping::default()); + insert_two_field_test_node(&manager); + + // Only pack one value, but the group expects two (UInt16 + UInt32). + let values = + PackedCanDataValues::<62>::try_from(&[CanDataValue::UInt16(0x1234)] as &[CanDataValue]) + .expect("values should pack"); + + let payload = TelemetryGroupUpdatePayload { + group_id: 1, + values, + }; + let msg_id = CanMessageId::new() + .with_sender_id(5) + .with_receiver_id(liquidcan::NODE_ID_SERVER); + + let err = manager + .handle_telemetry_group_update(msg_id, payload) + .expect_err("unpack should fail"); + + assert!(format!("{err:#}").contains("failed to unpack value for node 5 group 1 field 11")); + } + fn test_mapping() -> Mapping { Mapping::parse_mapping( r##" @@ -763,4 +1014,38 @@ value = { slope = 0.5, offset = 10.0, unit = "%" } manager.can_nodes.insert(5, node); } + + fn insert_two_field_test_node(manager: &NodeManager<'_>) { + let mut node = CanNode::new(RegistrationInfo { + telemetry_count: 2, + parameter_count: 0, + firmware_hash: 0, + protocol_hash: 0, + device_name: "ECU".to_string(), + }); + + node.telemetry_fields.insert( + 10, + FieldInfo { + data_type: CanDataType::UInt16, + name: "a".to_string(), + }, + ); + node.telemetry_fields.insert( + 11, + FieldInfo { + data_type: CanDataType::UInt32, + name: "b".to_string(), + }, + ); + + node.telemetry_groups.insert( + 1, + TelemetryGroupDefinition { + fields: vec![10, 11], + }, + ); + + manager.can_nodes.insert(5, node); + } } From e8ad3ccd444550ce4c79605dd86d4912605ffc87 Mon Sep 17 00:00:00 2001 From: Raffael Rott Date: Mon, 1 Jun 2026 17:42:29 +0200 Subject: [PATCH 11/19] Removed test which tests wrong behavior --- src/nodes/node_manager.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index d627ffc..1a8b01c 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -798,26 +798,6 @@ mod tests { )); } - #[test] - fn request_value_errors_when_mapping_missing() { - let dispatcher = EventDispatcher::new(); - let (tx, rx) = mpsc::channel(); - dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); - - let manager = NodeManager::new(&dispatcher, test_mapping()); - insert_test_node(&manager); - - let err = manager - .request_value("non_existent") - .expect_err("missing mappings should error"); - assert_eq!(err.to_string(), "no mapping exists for non_existent"); - - assert!(matches!( - rx.recv_timeout(Duration::from_millis(50)), - Err(mpsc::RecvTimeoutError::Timeout) - )); - } - #[test] fn request_value_errors_when_unregistered() { let dispatcher = EventDispatcher::new(); From acac147010e5b85c242b5ed7ad6ebb41b436da36 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 6 Jun 2026 03:48:54 +0200 Subject: [PATCH 12/19] treat names containing ':' as raw names of the format : --- src/nodes/mapping.rs | 24 ++---- src/nodes/node_manager.rs | 134 +++++++++--------------------- src/nodes/node_manager/command.rs | 123 +++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 108 deletions(-) create mode 100644 src/nodes/node_manager/command.rs diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index f2ed425..0e75486 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -215,18 +215,13 @@ impl Mapping { }) } - pub fn get_mapping_for_raw( - &self, - node: &str, - field: &str, - field_type: FieldType, - ) -> Option> { + pub fn get_mapping_for_raw(&self, node: &str, field: &str) -> Option> { self.mapping .get_key_value(node) .and_then(|(node, mapping_entries)| { mapping_entries .iter() - .find(|mapping| mapping.raw_field == field && mapping.field_type == field_type) + .find(|mapping| mapping.raw_field == field) .map(|mapping| MappingLookupResult { node_name: node, mapping_entry: mapping, @@ -241,6 +236,11 @@ impl MappingEntry { !self.name.trim().is_empty(), "mapping name must be non-empty", ); + ensure!( + !self.name.contains(':'), + "mapping name '{}' cannot contain colons, which are reserved characters to differentiate raw names from mapped names", + self.name + ); ensure!( !self.raw_field.trim().is_empty(), @@ -680,20 +680,14 @@ raw_field = "valve_raw" .expect("mapping should parse"); let telemetry = mapping - .get_mapping_for_raw("ECU", "pressure_adc", super::FieldType::Telemetry) + .get_mapping_for_raw("ECU", "pressure_adc") .expect("telemetry mapping should exist"); assert_eq!(telemetry.mapping_entry.name, "tank_pressure"); let parameter = mapping - .get_mapping_for_raw("ECU", "valve_raw", super::FieldType::Parameter) + .get_mapping_for_raw("ECU", "valve_raw") .expect("parameter mapping should exist"); assert_eq!(parameter.mapping_entry.name, "valve_opening"); - - assert!( - mapping - .get_mapping_for_raw("ECU", "pressure_adc", super::FieldType::Parameter) - .is_none() - ); } #[test] diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index 1a8b01c..df6d337 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -1,3 +1,5 @@ +pub mod command; + use std::{collections::HashMap, sync::Mutex}; use anyhow::anyhow; @@ -7,9 +9,8 @@ use dashmap::DashMap; use liquidcan::{ CanMessage, CanMessageId, payloads::{ - CanDataType, CanDataValue, FieldGetReqPayload, FieldGetResPayload, - FieldRegistrationPayload, HeartbeatPayload, NodeInfoResPayload, ParameterSetReqPayload, - TelemetryGroupDefinitionPayload, TelemetryGroupUpdatePayload, + CanDataType, CanDataValue, FieldGetResPayload, FieldRegistrationPayload, HeartbeatPayload, + NodeInfoResPayload, TelemetryGroupDefinitionPayload, TelemetryGroupUpdatePayload, }, }; @@ -18,6 +19,11 @@ use crate::{db::FieldLog, events}; use super::can_node::{CanNode, FieldInfo, RegistrationInfo, TelemetryGroupDefinition}; +/// Manages the CAN nodes connected to FerroFlow. +/// +/// By convention, methods taking a `field_name` argument expect either +/// - a name as defined in the mapping file +/// - a raw name in the format `node_name:raw_field_name` pub struct NodeManager<'a> { mapping: Mapping, can_nodes: DashMap, @@ -375,28 +381,28 @@ impl<'a> NodeManager<'a> { } } - /// Returns the latest cached raw CAN value for a mapped field name. + /// Returns the latest cached raw CAN value. /// /// This does not send a CAN request. Call `request_value` first if a fresh value is needed. /// /// Use this `try_` variant to distinguish missing values from invalid mappings or fields /// that have not registered yet. - pub fn try_get_raw_value(&self, mapped_name: &str) -> Result> { - let (_, target) = self.resolve_mapping_by_name(mapped_name)?; + pub fn try_get_raw_value(&self, field_name: &str) -> Result> { + let (_, target) = self.resolve_mapping_by_name(field_name)?; Ok(self.latest_raw_value(&target)) } /// Convenience wrapper around `try_get_raw_value` that treats errors as missing values. - pub fn get_raw_value(&self, mapped_name: &str) -> Option { - self.try_get_raw_value(mapped_name).ok().flatten() + pub fn get_raw_value(&self, field_name: &str) -> Option { + self.try_get_raw_value(field_name).ok().flatten() } /// Returns the latest cached value after applying the mapping's slope/offset conversion. /// /// `Ok(None)` means the mapping and raw field exist, but no value has been received yet. - pub fn try_get_mapped_value(&self, mapped_name: &str) -> Result> { - let (mapping, target) = self.resolve_mapping_by_name(mapped_name)?; + pub fn try_get_mapped_value(&self, field_name: &str) -> Result> { + let (mapping, target) = self.resolve_mapping_by_name(field_name)?; let Some(raw_value) = self.latest_raw_value(&target) else { return Ok(None); }; @@ -405,20 +411,20 @@ impl<'a> NodeManager<'a> { } /// Convenience wrapper around `try_get_mapped_value` that treats errors as missing values. - pub fn get_mapped_value(&self, mapped_name: &str) -> Option { - self.try_get_mapped_value(mapped_name).ok().flatten() + pub fn get_mapped_value(&self, field_name: &str) -> Option { + self.try_get_mapped_value(field_name).ok().flatten() } /// Returns the logical value associated with the current mapped value. /// /// Logical values are derived from the configured range table. If the mapping has no logical /// rules, this returns `Ok(None)` even when a mapped numeric value is available. - pub fn try_get_logical_value(&self, mapped_name: &str) -> Result> { - let Some(mapped_value) = self.try_get_mapped_value(mapped_name)? else { + pub fn try_get_logical_value(&self, field_name: &str) -> Result> { + let Some(mapped_value) = self.try_get_mapped_value(field_name)? else { return Ok(None); }; - let mapping_lookup = self.lookup_mapping(mapped_name)?; + let mapping_lookup = self.lookup_mapping(field_name)?; Ok(mapping_lookup .mapping_entry @@ -426,78 +432,32 @@ impl<'a> NodeManager<'a> { } /// Convenience wrapper around `try_get_logical_value` that treats errors as missing values. - pub fn get_logical_value(&self, mapped_name: &str) -> Option { - self.try_get_logical_value(mapped_name).ok().flatten() - } - - /// Sends a `FieldGetReq` for the raw field behind a mapped name. - /// - /// The response is processed asynchronously by the normal CAN message handler and updates the - /// cached value read by `get_raw_value`, `get_mapped_value`, and `get_logical_value`. - pub fn request_value(&self, mapped_name: &str) -> Result<()> { - let (_, target) = self.resolve_mapping_by_name(mapped_name)?; - - self.event_dispatcher - .dispatch(events::Event::SendCanMessage { - receiver_node_id: target.node_id, - message: CanMessage::FieldGetReq { - payload: FieldGetReqPayload { - field_id: target.field_id, - }, - }, - }); - - Ok(()) - } - - /// Writes a mapped value to a mapped parameter field. - /// - /// The value is converted back to the raw CAN type using the inverse of the configured linear - /// mapping, then sent as a `ParameterSetReq`. - pub fn set_mapped_value(&self, mapped_name: &str, mapped_value: f64) -> Result<()> { - let (mapping_lookup, target) = self.resolve_mapping_by_name(mapped_name)?; - - if mapping_lookup.mapping_entry.field_type != mapping::FieldType::Parameter { - bail!("mapped field {mapped_name} is not writable because it is not a parameter"); - } - - let raw_value = mapping_lookup - .mapping_entry - .raw_value_from_mapped(mapped_value, target.data_type)?; - self.dispatch_parameter_set(target, raw_value); - - Ok(()) + pub fn get_logical_value(&self, field_name: &str) -> Option { + self.try_get_logical_value(field_name).ok().flatten() } - /// Writes a raw CAN value to a mapped parameter field. - /// - /// The value is sent as a `ParameterSetReq`. - pub fn set_raw_value(&self, mapped_name: &str, raw_value: CanDataValue) -> Result<()> { - let (mapping_lookup, target) = self.resolve_mapping_by_name(mapped_name)?; - - if mapping_lookup.mapping_entry.field_type != mapping::FieldType::Parameter { - bail!("mapped field {mapped_name} is not writable because it is not a parameter"); + fn lookup_mapping(&self, field_name: &str) -> Result> { + if field_name.contains(':') { + // This is a raw name + let (node_name, raw_field_name) = field_name.split_once(':').unwrap(); + self.mapping + .get_mapping_for_raw(node_name, raw_field_name) + .with_context(|| format!("no mapping exists for {field_name}")) + } else { + self.mapping + .get_mapping_for_name(field_name) + .with_context(|| format!("no mapping exists for {field_name}")) } - - self.dispatch_parameter_set(target, raw_value); - - Ok(()) - } - - fn lookup_mapping(&self, mapped_name: &str) -> Result> { - self.mapping - .get_mapping_for_name(mapped_name) - .with_context(|| format!("no mapping exists for {mapped_name}")) } fn resolve_mapping_by_name( &self, - mapped_name: &str, + field_name: &str, ) -> Result<(MappingLookupResult<'_>, ResolvedMappingTarget)> { - let mapping_lookup = self.lookup_mapping(mapped_name)?; + let mapping_lookup = self.lookup_mapping(field_name)?; let target = self .resolve_mapping_target(&mapping_lookup) - .with_context(|| format!("mapped field {mapped_name} is not registered"))?; + .with_context(|| format!("mapped field {field_name} is not registered"))?; Ok((mapping_lookup, target)) } @@ -510,19 +470,6 @@ impl<'a> NodeManager<'a> { }) } - fn dispatch_parameter_set(&self, target: ResolvedMappingTarget, raw_value: CanDataValue) { - self.event_dispatcher - .dispatch(events::Event::SendCanMessage { - receiver_node_id: target.node_id, - message: CanMessage::ParameterSetReq { - payload: ParameterSetReqPayload { - parameter_id: target.field_id, - value: raw_value, - }, - }, - }); - } - /// Resolves a mapping entry to the currently registered node id, field id, and field type. /// /// Mappings are written against stable device/field names, but LiquidCAN requests need numeric @@ -564,7 +511,8 @@ mod tests { use std::{sync::mpsc, time::Duration}; use chrono::Utc; - use liquidcan::payloads::{CanDataType, CanDataValue}; + use liquidcan::payloads::{CanDataType, CanDataValue, ParameterSetReqPayload}; + use serde_json::json; use toml::Value; use crate::events::{Event, EventDispatcher, EventKind}; @@ -677,7 +625,7 @@ mod tests { insert_test_node(&manager); manager - .set_raw_value("valve_opening", CanDataValue::UInt8(42)) + .set_raw_value("valve_opening", json!(42)) .expect("raw parameter should be writable"); assert_eq!(receive_parameter_set(&rx), (5, 20, CanDataValue::UInt8(42))); @@ -732,7 +680,7 @@ mod tests { )); let err = manager - .set_raw_value("tank_pressure", CanDataValue::UInt16(1)) + .set_raw_value("tank_pressure", json!(1)) .expect_err("telemetry mappings should not be writable"); assert!(err.to_string().contains("is not writable")); diff --git a/src/nodes/node_manager/command.rs b/src/nodes/node_manager/command.rs new file mode 100644 index 0000000..0bb330b --- /dev/null +++ b/src/nodes/node_manager/command.rs @@ -0,0 +1,123 @@ +use anyhow::{Context, Result, anyhow}; +use liquidcan::{ + CanMessage, + payloads::{CanDataType, CanDataValue, FieldGetReqPayload, ParameterSetReqPayload}, +}; +use serde_json::Value; + +use crate::{events, nodes::mapping}; + +use super::{NodeManager, ResolvedMappingTarget}; + +impl<'a> NodeManager<'a> { + /// Sends a `FieldGetReq` for the raw field. + /// + /// The response is processed asynchronously by the normal CAN message handler. + pub fn request_value(&self, field_name: &str) -> Result<()> { + let (_, target) = self.resolve_mapping_by_name(field_name)?; + + self.event_dispatcher + .dispatch(events::Event::SendCanMessage { + receiver_node_id: target.node_id, + message: CanMessage::FieldGetReq { + payload: FieldGetReqPayload { + field_id: target.field_id, + }, + }, + }); + + Ok(()) + } + + /// Writes a mapped value. + /// + /// The value is converted back to the raw CAN type using the inverse of the configured linear + /// mapping, then sent as a `ParameterSetReq`. + pub fn set_mapped_value(&self, field_name: &str, mapped_value: f64) -> Result<()> { + let (mapping_lookup, target) = self.resolve_mapping_by_name(field_name)?; + + if mapping_lookup.mapping_entry.field_type != mapping::FieldType::Parameter { + anyhow::bail!( + "mapped field {field_name} is not writable because it is not a parameter" + ); + } + + let raw_value = mapping_lookup + .mapping_entry + .raw_value_from_mapped(mapped_value, target.data_type)?; + self.dispatch_parameter_set(target, raw_value); + + Ok(()) + } + + /// Writes a raw CAN value. + pub fn set_raw_value(&self, field_name: &str, value: Value) -> Result<()> { + let (mapping_lookup, target) = self.resolve_mapping_by_name(field_name)?; + + if mapping_lookup.mapping_entry.field_type != mapping::FieldType::Parameter { + anyhow::bail!( + "mapped field {field_name} is not writable because it is not a parameter" + ); + } + + let raw_value = json_value_to_can_data_value(value, target.data_type)?; + + self.dispatch_parameter_set(target, raw_value); + + Ok(()) + } + + fn dispatch_parameter_set(&self, target: ResolvedMappingTarget, raw_value: CanDataValue) { + self.event_dispatcher + .dispatch(events::Event::SendCanMessage { + receiver_node_id: target.node_id, + message: CanMessage::ParameterSetReq { + payload: ParameterSetReqPayload { + parameter_id: target.field_id, + value: raw_value, + }, + }, + }); + } +} + +pub fn json_value_to_can_data_value( + value: serde_json::Value, + data_type: CanDataType, +) -> Result { + match data_type { + CanDataType::Float32 => Ok(CanDataValue::Float32(json_value_to_f64(&value)? as f32)), + CanDataType::Int32 => Ok(CanDataValue::Int32(json_value_to_integer(&value)?)), + CanDataType::Int16 => Ok(CanDataValue::Int16(json_value_to_integer(&value)?)), + CanDataType::Int8 => Ok(CanDataValue::Int8(json_value_to_integer(&value)?)), + CanDataType::UInt32 => Ok(CanDataValue::UInt32(json_value_to_integer(&value)?)), + CanDataType::UInt16 => Ok(CanDataValue::UInt16(json_value_to_integer(&value)?)), + CanDataType::UInt8 => Ok(CanDataValue::UInt8(json_value_to_integer(&value)?)), + CanDataType::Boolean => value + .as_bool() + .map(CanDataValue::Boolean) + .or_else(|| { + value + .as_i64() + .map(|value| CanDataValue::Boolean(value != 0)) + }) + .with_context(|| format!("expected boolean-compatible value, got {value}")), + } +} + +fn json_value_to_f64(value: &serde_json::Value) -> Result { + value + .as_f64() + .with_context(|| format!("expected numeric value, got {value}")) +} + +fn json_value_to_integer(value: &serde_json::Value) -> Result +where + T: TryFrom, + >::Error: std::fmt::Debug, +{ + let raw = value + .as_i64() + .with_context(|| format!("expected integer value, got {value}"))?; + T::try_from(raw).map_err(|_| anyhow!("integer value {raw} is out of range")) +} From c404d8742fb7e8dc7edc743e8c79c9f613258f2f Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 7 Jun 2026 23:38:32 +0200 Subject: [PATCH 13/19] add deny_unknown_fields to mapping structs --- src/nodes/mapping.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index 0e75486..4892661 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -9,12 +9,13 @@ use std::{ use toml::Value; #[derive(Debug, Clone, Default, Deserialize)] -#[serde(default)] +#[serde(default, deny_unknown_fields)] pub struct Mapping { pub mapping: BTreeMap>, } #[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] pub struct MappingEntry { pub name: String, #[serde(rename = "type")] @@ -27,6 +28,7 @@ pub struct MappingEntry { } #[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] pub struct ValueParams { pub slope: f64, pub offset: f64, @@ -45,6 +47,7 @@ impl Default for ValueParams { } #[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] pub struct LogicalRule { pub range: LogicalRange, pub value: Value, From 66314d076a79b29692398581d943af05be3175ef Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sun, 7 Jun 2026 23:39:28 +0200 Subject: [PATCH 14/19] fix clippy --- src/sequence/sequence_definition.rs | 2 +- src/sequence/sequence_runner.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sequence/sequence_definition.rs b/src/sequence/sequence_definition.rs index 3c50fcc..f543ed4 100644 --- a/src/sequence/sequence_definition.rs +++ b/src/sequence/sequence_definition.rs @@ -177,7 +177,7 @@ pub struct HoldCondition { impl HoldCondition { pub fn evaluate(&self, node_manager: &nodes::NodeManager) -> bool { let Some(actual) = node_manager.get_mapped_value(&self.field) else { - eprintln!("Value for field '{}' missing", &self.field); + eprintln!("Value for field '{}' missing", self.field); return false; }; let eps = 1e-6; diff --git a/src/sequence/sequence_runner.rs b/src/sequence/sequence_runner.rs index 2de3311..007ff28 100644 --- a/src/sequence/sequence_runner.rs +++ b/src/sequence/sequence_runner.rs @@ -160,7 +160,7 @@ impl<'scope, 'env> SequenceRunner<'scope, 'env> { if let Err(err) = result { eprintln!( "Failed to set value '{}' for param '{}': {:#?}", - ¶m_state.value, ¶m_state.param, err + param_state.value, param_state.param, err ); } } From e7ae22fc8bd0ad6365ea0cee4f0313bdd246831c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 8 Jun 2026 02:53:02 +0200 Subject: [PATCH 15/19] allow converting between numeric and boolean values since the mapping works with numeric values (f64's) we have to convert between booleans and numeric values. this should also help when interacting with js. --- src/nodes/node_manager/command.rs | 41 ++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/nodes/node_manager/command.rs b/src/nodes/node_manager/command.rs index 0bb330b..c64bfc6 100644 --- a/src/nodes/node_manager/command.rs +++ b/src/nodes/node_manager/command.rs @@ -93,22 +93,18 @@ pub fn json_value_to_can_data_value( CanDataType::UInt32 => Ok(CanDataValue::UInt32(json_value_to_integer(&value)?)), CanDataType::UInt16 => Ok(CanDataValue::UInt16(json_value_to_integer(&value)?)), CanDataType::UInt8 => Ok(CanDataValue::UInt8(json_value_to_integer(&value)?)), - CanDataType::Boolean => value - .as_bool() - .map(CanDataValue::Boolean) - .or_else(|| { - value - .as_i64() - .map(|value| CanDataValue::Boolean(value != 0)) - }) - .with_context(|| format!("expected boolean-compatible value, got {value}")), + CanDataType::Boolean => Ok(CanDataValue::Boolean(json_value_as_bool(&value)?)), } } fn json_value_to_f64(value: &serde_json::Value) -> Result { - value - .as_f64() - .with_context(|| format!("expected numeric value, got {value}")) + match value { + Value::Number(num) => num + .as_f64() + .with_context(|| format!("expected numeric value, got {value}")), + Value::Bool(b) => Ok(if *b { 1.0 } else { 0.0 }), + _ => Err(anyhow!("expected numeric or boolean value, got {value}")), + } } fn json_value_to_integer(value: &serde_json::Value) -> Result @@ -116,8 +112,23 @@ where T: TryFrom, >::Error: std::fmt::Debug, { - let raw = value - .as_i64() - .with_context(|| format!("expected integer value, got {value}"))?; + let raw = match value { + Value::Number(num) => num + .as_i64() + .with_context(|| format!("expected integer value, got {value}")), + Value::Bool(b) => Ok(if *b { 1 } else { 0 }), + _ => Err(anyhow!("expected integer or boolean value, got {value}")), + }?; T::try_from(raw).map_err(|_| anyhow!("integer value {raw} is out of range")) } + +fn json_value_as_bool(value: &serde_json::Value) -> Result { + match value { + Value::Bool(b) => Ok(*b), + Value::Number(num) => num + .as_i64() + .map(|value| value != 0) + .with_context(|| format!("expected boolean-compatible value, got {value}")), + _ => Err(anyhow!("expected boolean-compatible value, got {value}")), + } +} From b265d3e900892ab039f356f4a02d04a9bbf2f885 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 8 Jun 2026 02:55:56 +0200 Subject: [PATCH 16/19] add set_value and get_value that return mapped or raw value depending on name --- src/nodes/mapping.rs | 4 +- src/nodes/node_manager.rs | 176 +++++++++++++++++++++++++++++- src/nodes/node_manager/command.rs | 13 +++ 3 files changed, 185 insertions(+), 8 deletions(-) diff --git a/src/nodes/mapping.rs b/src/nodes/mapping.rs index 4892661..be3f0b3 100644 --- a/src/nodes/mapping.rs +++ b/src/nodes/mapping.rs @@ -429,7 +429,7 @@ impl LogicalRange { } } -fn can_data_value_to_f64(value: &CanDataValue) -> anyhow::Result { +pub fn can_data_value_to_f64(value: &CanDataValue) -> anyhow::Result { match value { CanDataValue::Float32(value) => Ok(*value as f64), CanDataValue::Int32(value) => Ok(*value as f64), @@ -444,7 +444,7 @@ fn can_data_value_to_f64(value: &CanDataValue) -> anyhow::Result { } /// Converts a mapped numeric value back into a typed CAN payload value. -fn can_data_value_from_f64(value: f64, data_type: CanDataType) -> anyhow::Result { +pub fn can_data_value_from_f64(value: f64, data_type: CanDataType) -> anyhow::Result { ensure!(value.is_finite(), "raw value must be finite"); match data_type { diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index e1d8f14..24a3f68 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -14,7 +14,9 @@ use liquidcan::{ }, }; -use crate::nodes::mapping::{self, LogicalValue, MappedValue, Mapping, MappingLookupResult}; +use crate::nodes::mapping::{ + self, LogicalValue, MappedValue, Mapping, MappingLookupResult, can_data_value_to_f64, +}; use crate::{db::FieldLog, events}; use super::can_node::{CanNode, FieldInfo, RegistrationInfo, TelemetryGroupDefinition}; @@ -384,6 +386,44 @@ impl<'a> NodeManager<'a> { } } + fn is_mapped_name(field_name: &str) -> bool { + !field_name.contains(':') + } + + /// Takes a raw or mapped field name, and returns the raw or mapped value, respectively. + /// + /// That is, for raw field names, this returns the latest cached raw CAN value. + /// For mapped field names, this returns the latest cached value after applying the mapping's slope/offset conversion. + pub fn try_get_value(&self, field_name: &str) -> Result> { + if Self::is_mapped_name(field_name) { + self.try_get_mapped_value(field_name).with_context(|| { + format!("failed to get mapped value for field name '{field_name}'") + }) + } else { + let Some(raw_value) = self.try_get_raw_value(field_name).with_context(|| { + format!("failed to get raw value for field name '{field_name}'") + })? + else { + return Ok(None); + }; + + let numerical_representation = + can_data_value_to_f64(&raw_value).with_context(|| { + format!("failed to convert raw value for field name '{field_name}' to f64") + })?; + + Ok(Some(MappedValue { + value: numerical_representation, + unit: String::new(), // this is a converted raw value which has no unit + })) + } + } + + /// Convenience wrapper around `try_get_value` that treats errors as missing values. + pub fn get_value(&self, field_name: &str) -> Option { + self.try_get_value(field_name).ok().flatten() + } + /// Returns the latest cached raw CAN value. /// /// This does not send a CAN request. Call `request_value` first if a fresh value is needed. @@ -440,15 +480,14 @@ impl<'a> NodeManager<'a> { } fn lookup_mapping(&self, field_name: &str) -> Result> { - if field_name.contains(':') { - // This is a raw name - let (node_name, raw_field_name) = field_name.split_once(':').unwrap(); + if Self::is_mapped_name(field_name) { self.mapping - .get_mapping_for_raw(node_name, raw_field_name) + .get_mapping_for_name(field_name) .with_context(|| format!("no mapping exists for {field_name}")) } else { + let (node_name, raw_field_name) = field_name.split_once(':').unwrap(); self.mapping - .get_mapping_for_name(field_name) + .get_mapping_for_raw(node_name, raw_field_name) .with_context(|| format!("no mapping exists for {field_name}")) } } @@ -583,6 +622,51 @@ mod tests { ); } + #[test] + fn get_value_reads_mapped_names_as_mapped_and_raw_names_as_numeric_raw() { + let dispatcher = EventDispatcher::new(); + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + let mapped = manager + .try_get_value("tank_pressure") + .expect("mapped lookup should succeed") + .expect("mapped value should be cached"); + assert_eq!(mapped.value, 100.0); + assert_eq!(mapped.unit, "bar"); + + let raw = manager + .try_get_value("ECU:pressure_adc") + .expect("raw lookup should succeed") + .expect("raw value should be cached"); + assert_eq!(raw.value, 198.0); + assert_eq!(raw.unit, ""); + + assert_eq!(manager.try_get_value("ECU:valve_raw").unwrap(), None); + assert_eq!(manager.get_value("non_existent"), None); + assert_eq!(manager.get_value("ECU:missing_raw"), None); + } + + #[test] + fn try_get_value_reports_raw_values_that_cannot_be_converted_to_numbers() { + let dispatcher = EventDispatcher::new(); + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + manager + .can_nodes + .get(&5) + .expect("node should exist") + .values + .insert(10, (Utc::now(), CanDataValue::Raw(vec![1, 2, 3]))); + + let err = manager + .try_get_value("ECU:pressure_adc") + .expect_err("raw bytes should not convert to a generic numeric value"); + assert!(format!("{err:#}").contains("failed to convert raw value")); + assert_eq!(manager.get_value("ECU:pressure_adc"), None); + } + #[test] fn get_returns_none_on_missing_mapping_or_unregistered() { let dispatcher = EventDispatcher::new(); @@ -634,6 +718,55 @@ mod tests { assert_eq!(receive_parameter_set(&rx), (5, 20, CanDataValue::UInt8(42))); } + #[test] + fn set_value_routes_mapped_and_raw_names() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, test_mapping()); + insert_test_node(&manager); + + manager + .set_value("valve_opening", json!(60.0)) + .expect("mapped parameter should be writable through generic setter"); + assert_eq!( + receive_parameter_set(&rx), + (5, 20, CanDataValue::UInt8(100)) + ); + + manager + .set_value("ECU:valve_raw", json!(42)) + .expect("raw parameter should be writable through generic setter"); + assert_eq!(receive_parameter_set(&rx), (5, 20, CanDataValue::UInt8(42))); + } + + #[test] + fn set_value_accepts_boolean_mapped_input() { + let dispatcher = EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); + + let manager = NodeManager::new(&dispatcher, boolean_test_mapping()); + insert_boolean_test_node(&manager); + + manager + .set_value("pump_enabled", json!(true)) + .expect("boolean mapped value should be accepted"); + assert_eq!( + receive_parameter_set(&rx), + (5, 30, CanDataValue::Boolean(true)) + ); + + manager + .set_value("pump_enabled", json!(false)) + .expect("boolean mapped value should be accepted"); + assert_eq!( + receive_parameter_set(&rx), + (5, 30, CanDataValue::Boolean(false)) + ); + } + #[test] fn requests_field_get_for_mapped_values() { let dispatcher = EventDispatcher::new(); @@ -897,6 +1030,18 @@ value = { slope = 0.5, offset = 10.0, unit = "%" } .expect("mapping should parse") } + fn boolean_test_mapping() -> Mapping { + Mapping::parse_mapping( + r##" +[[mapping.ECU]] +name = "pump_enabled" +type = "parameter" +raw_field = "pump_enable_raw" +"##, + ) + .expect("mapping should parse") + } + fn receive_parameter_set(rx: &mpsc::Receiver) -> (u8, u8, CanDataValue) { let event = rx .recv_timeout(Duration::from_millis(200)) @@ -979,4 +1124,23 @@ value = { slope = 0.5, offset = 10.0, unit = "%" } manager.can_nodes.insert(5, node); } + + fn insert_boolean_test_node(manager: &NodeManager<'_>) { + let mut node = CanNode::new(RegistrationInfo { + telemetry_count: 0, + parameter_count: 1, + firmware_hash: 0, + protocol_hash: 0, + device_name: "ECU".to_string(), + }); + node.parameter_fields.insert( + 30, + FieldInfo { + data_type: CanDataType::Boolean, + name: "pump_enable_raw".to_string(), + }, + ); + + manager.can_nodes.insert(5, node); + } } diff --git a/src/nodes/node_manager/command.rs b/src/nodes/node_manager/command.rs index c64bfc6..14649c4 100644 --- a/src/nodes/node_manager/command.rs +++ b/src/nodes/node_manager/command.rs @@ -67,6 +67,19 @@ impl<'a> NodeManager<'a> { Ok(()) } + /// Writes a value, either raw or mapped depending on the field name format. + /// + /// If `field_name` is a raw name, `value` is converted directly to a `CanDataValue` and sent as-is. + /// If `field_name` is a mapped name, `value` is converted using the inverse of the configured linear mapping before being sent. + pub fn set_value(&self, field_name: &str, value: Value) -> Result<()> { + if Self::is_mapped_name(field_name) { + self.set_mapped_value(field_name, json_value_to_f64(&value)?)?; + } else { + self.set_raw_value(field_name, value)?; + } + Ok(()) + } + fn dispatch_parameter_set(&self, target: ResolvedMappingTarget, raw_value: CanDataValue) { self.event_dispatcher .dispatch(events::Event::SendCanMessage { From ab214c0ea8b71ca054ce19f7e2f9cf835f62d6a5 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 8 Jun 2026 03:04:25 +0200 Subject: [PATCH 17/19] use set_value/get_value in sequence runner --- src/sequence/sequence_definition.rs | 2 +- src/sequence/sequence_runner.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sequence/sequence_definition.rs b/src/sequence/sequence_definition.rs index f543ed4..a3f16f1 100644 --- a/src/sequence/sequence_definition.rs +++ b/src/sequence/sequence_definition.rs @@ -176,7 +176,7 @@ pub struct HoldCondition { impl HoldCondition { pub fn evaluate(&self, node_manager: &nodes::NodeManager) -> bool { - let Some(actual) = node_manager.get_mapped_value(&self.field) else { + let Some(actual) = node_manager.get_value(&self.field) else { eprintln!("Value for field '{}' missing", self.field); return false; }; diff --git a/src/sequence/sequence_runner.rs b/src/sequence/sequence_runner.rs index 007ff28..1813055 100644 --- a/src/sequence/sequence_runner.rs +++ b/src/sequence/sequence_runner.rs @@ -155,8 +155,8 @@ impl<'scope, 'env> SequenceRunner<'scope, 'env> { } Action::SetParam(param_state) => { - let result = - node_manager.set_mapped_value(¶m_state.param, param_state.value); + let result = node_manager + .set_value(¶m_state.param, serde_json::json!(param_state.value)); if let Err(err) = result { eprintln!( "Failed to set value '{}' for param '{}': {:#?}", From 5a374ae82f2a4ecb37b60f5cb7566e32070da138 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 8 Jun 2026 03:04:37 +0200 Subject: [PATCH 18/19] update and enable sequence runner tests --- src/sequence/sequence_runner.rs | 89 +++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/src/sequence/sequence_runner.rs b/src/sequence/sequence_runner.rs index 1813055..f7bfe35 100644 --- a/src/sequence/sequence_runner.rs +++ b/src/sequence/sequence_runner.rs @@ -204,11 +204,18 @@ impl Drop for SequencePanicGuard { #[cfg(test)] mod tests { - use crate::{events, nodes::mapping::Mapping}; + use crate::{ + events::{self, EventKind}, + nodes::mapping::Mapping, + }; use super::*; + use liquidcan::{ + CanMessage, CanMessageId, + payloads::{CanDataType, CanDataValue, FieldRegistrationPayload, NodeInfoResPayload}, + }; use ntest::timeout; - use std::{path::Path, thread, time::Duration}; + use std::{path::Path, sync::mpsc, thread, time::Duration}; fn load_seq(name: &str) -> Sequence { let seq_dir = Path::new(env!("CARGO_MANIFEST_DIR")) @@ -230,12 +237,14 @@ mod tests { .expect("failed to load test mapping") } - #[ignore] #[test] #[timeout(2000)] fn test_run_sequence_execution_completes() { let event_dispatcher = events::EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + event_dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); let node_manager = nodes::NodeManager::new(&event_dispatcher, load_mapping()); + register_sequence_test_node(&node_manager); thread::scope(|scope| { let mut runner = SequenceRunner::new(&node_manager, scope); @@ -255,16 +264,19 @@ mod tests { let join_result = handle.thread_handle.join(); assert!(join_result.is_ok()); let sequence_result = join_result.unwrap(); - assert!(sequence_result.is_ok()) + assert!(sequence_result.is_ok()); + + assert_eq!(receive_parameter_set(&rx), (5, 1, CanDataValue::UInt8(12))); + assert_eq!(receive_parameter_set(&rx), (5, 2, CanDataValue::UInt8(12))); }); } - #[ignore] #[test] #[timeout(2000)] fn test_run_sequence_hold_and_resume_completes() { let event_dispatcher = events::EventDispatcher::new(); let node_manager = nodes::NodeManager::new(&event_dispatcher, load_mapping()); + register_sequence_test_node(&node_manager); thread::scope(|scope| { let mut runner = SequenceRunner::new(&node_manager, scope); @@ -292,12 +304,14 @@ mod tests { }); } - #[ignore] #[test] #[timeout(2000)] fn test_run_sequence_abort() { let event_dispatcher = events::EventDispatcher::new(); + let (tx, rx) = mpsc::channel(); + event_dispatcher.subscribe(tx, vec![EventKind::SendCanMessage], "test-send-listener"); let node_manager = nodes::NodeManager::new(&event_dispatcher, load_mapping()); + register_sequence_test_node(&node_manager); thread::scope(|scope| { let mut runner = SequenceRunner::new(&node_manager, scope); @@ -321,11 +335,64 @@ mod tests { let join_result = handle.thread_handle.join(); assert!(join_result.is_ok()); let sequence_result = join_result.unwrap(); - assert!(sequence_result.is_err()); - assert!(matches!( - sequence_result.unwrap_err(), - SequenceRunError::Aborted - )); + assert!(sequence_result.is_ok()); + assert_eq!(receive_parameter_set(&rx), (5, 3, CanDataValue::UInt8(1))); }); } + + fn register_sequence_test_node(node_manager: &nodes::NodeManager<'_>) { + let msg_id = CanMessageId::new() + .with_sender_id(5) + .with_receiver_id(liquidcan::NODE_ID_SERVER); + + node_manager + .handle_node_info_announcement( + msg_id, + NodeInfoResPayload { + tel_count: 0, + par_count: 3, + firmware_hash: 0, + liquid_hash: 0, + device_name: "SequenceTestNode".try_into().unwrap(), + }, + ) + .expect("test node info should register"); + + for (field_id, field_name) in [(1, "servo1"), (2, "valve1"), (3, "abort")] { + node_manager + .handle_field_registration( + msg_id, + FieldRegistrationPayload { + field_id, + field_type: CanDataType::UInt8, + field_name: field_name.try_into().unwrap(), + }, + false, + ) + .expect("test parameter should register"); + } + + assert_eq!(node_manager.get_nodes().len(), 1); + } + + fn receive_parameter_set(rx: &mpsc::Receiver) -> (u8, u8, CanDataValue) { + let event = rx + .recv_timeout(Duration::from_millis(200)) + .expect("send event should be dispatched"); + + match event { + events::Event::SendCanMessage { + receiver_node_id, + message: + CanMessage::ParameterSetReq { + payload: + liquidcan::payloads::ParameterSetReqPayload { + parameter_id, + value, + }, + }, + } => (receiver_node_id, parameter_id, value), + other => panic!("unexpected event: {other:?}"), + } + } } From 072eb988459c2db7498f14317e2f50a87766ed1a Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 8 Jun 2026 20:21:47 +0200 Subject: [PATCH 19/19] clarify return values of try_get_value --- src/nodes/node_manager.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/nodes/node_manager.rs b/src/nodes/node_manager.rs index 24a3f68..844b477 100644 --- a/src/nodes/node_manager.rs +++ b/src/nodes/node_manager.rs @@ -394,6 +394,9 @@ impl<'a> NodeManager<'a> { /// /// That is, for raw field names, this returns the latest cached raw CAN value. /// For mapped field names, this returns the latest cached value after applying the mapping's slope/offset conversion. + /// + /// Returns Err if the field name is invalid or applying the mapping failed. + /// Returns Ok(None) if the field name is valid but no value has been received yet. pub fn try_get_value(&self, field_name: &str) -> Result> { if Self::is_mapped_name(field_name) { self.try_get_mapped_value(field_name).with_context(|| {