From 48f06cd543e1a139306c56e854955e2c17bd3da1 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 30 Mar 2026 16:40:11 -0700 Subject: [PATCH 1/2] Disambiguate hwmon chips with duplicate names (#33) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple hwmon sysfs devices share the same name (e.g. jc42 DIMM temp sensors, multiple NVMe drives), they produced identical SensorIds and collided in the HashMap — only one reading survived. Detect duplicate chip names during discovery and append the sysfs device symlink basename to disambiguate (e.g. jc42-9-0018, nvme-nvme0). Falls back to the hwmon directory index when no device symlink exists. Also fixes is_gpu_hwmon_chip to use prefix matching so multi-GPU systems still get "GPU " label prefixes, and pre-expands board template label overrides so unqualified names continue to match disambiguated chips. https://github.com/level1techs/siomon/issues/33 --- src/sensors/hwmon.rs | 89 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 73 insertions(+), 16 deletions(-) diff --git a/src/sensors/hwmon.rs b/src/sensors/hwmon.rs index 843c3b8..9c98d20 100644 --- a/src/sensors/hwmon.rs +++ b/src/sensors/hwmon.rs @@ -9,7 +9,9 @@ use std::path::Path; const GPU_HWMON_CHIPS: &[&str] = &["amdgpu", "nouveau", "i915", "xe"]; fn is_gpu_hwmon_chip(chip_name: &str) -> bool { - GPU_HWMON_CHIPS.contains(&chip_name) + GPU_HWMON_CHIPS + .iter() + .any(|&gpu| chip_name == gpu || chip_name.starts_with(&format!("{gpu}-"))) } /// Prefix a GPU hwmon label with "GPU " if it doesn't already start with "GPU". @@ -42,55 +44,107 @@ impl HwmonSource { pub fn discover(label_overrides: &HashMap) -> Self { let mut chips = Vec::new(); - for hwmon_dir in sysfs::glob_paths("/sys/class/hwmon/hwmon*") { - let chip_name = sysfs::read_string_optional(&hwmon_dir.join("name")) - .unwrap_or_else(|| "unknown".into()); + // First pass: collect hwmon dirs with their chip names to detect duplicates + let hwmon_dirs: Vec<_> = sysfs::glob_paths("/sys/class/hwmon/hwmon*") + .into_iter() + .map(|dir| { + let chip_name = sysfs::read_string_optional(&dir.join("name")) + .unwrap_or_else(|| "unknown".into()); + (dir, chip_name) + }) + .collect(); + + // Count occurrences of each chip name + let mut name_counts: HashMap = HashMap::new(); + for (_, name) in &hwmon_dirs { + *name_counts.entry(name.clone()).or_default() += 1; + } + + // Compute display names and expand label overrides for disambiguated chips. + // Board templates use unqualified names like "hwmon/jc42/temp1"; when a chip + // is disambiguated to "jc42-9-0018", we copy matching overrides so they + // still apply without changing discover_type/discover_power signatures. + let hwmon_entries: Vec<_> = hwmon_dirs + .into_iter() + .map(|(dir, chip_name)| { + let display_name = if name_counts[&chip_name] > 1 { + let suffix = sysfs::read_link_basename(&dir.join("device")) + .or_else(|| { + // Last resort: use hwmon sysfs index (unstable across + // reboots, but avoids collisions within a session) + dir.file_name() + .and_then(|n| n.to_str()) + .map(|s| s.to_string()) + }) + .unwrap_or_else(|| "unknown".into()); + format!("{chip_name}-{suffix}") + } else { + chip_name.clone() + }; + (dir, chip_name, display_name) + }) + .collect(); + + let mut effective_overrides = label_overrides.clone(); + for (_, chip_name, display_name) in &hwmon_entries { + if chip_name != display_name { + let prefix = format!("hwmon/{chip_name}/"); + for (key, value) in label_overrides { + if let Some(sensor) = key.strip_prefix(&prefix) { + effective_overrides + .entry(format!("hwmon/{display_name}/{sensor}")) + .or_insert_with(|| value.clone()); + } + } + } + } + for (hwmon_dir, _, display_name) in &hwmon_entries { let mut entries = Vec::new(); // Temperature sensors discover_type( - &hwmon_dir, - &chip_name, + hwmon_dir, + display_name, "temp", SensorCategory::Temperature, SensorUnit::Celsius, 1000.0, - label_overrides, + &effective_overrides, &mut entries, ); // Fan sensors discover_type( - &hwmon_dir, - &chip_name, + hwmon_dir, + display_name, "fan", SensorCategory::Fan, SensorUnit::Rpm, 1.0, - label_overrides, + &effective_overrides, &mut entries, ); // Voltage sensors discover_type( - &hwmon_dir, - &chip_name, + hwmon_dir, + display_name, "in", SensorCategory::Voltage, SensorUnit::Volts, 1000.0, - label_overrides, + &effective_overrides, &mut entries, ); // Power sensors - discover_power(&hwmon_dir, &chip_name, label_overrides, &mut entries); + discover_power(hwmon_dir, display_name, &effective_overrides, &mut entries); // Current sensors discover_type( - &hwmon_dir, - &chip_name, + hwmon_dir, + display_name, "curr", SensorCategory::Current, SensorUnit::Amps, @@ -285,6 +339,9 @@ mod tests { assert!(is_gpu_hwmon_chip("nouveau")); assert!(is_gpu_hwmon_chip("i915")); assert!(is_gpu_hwmon_chip("xe")); + // Disambiguated multi-GPU names must still match + assert!(is_gpu_hwmon_chip("amdgpu-0000:41:00.0")); + assert!(is_gpu_hwmon_chip("nouveau-0000:01:00.0")); assert!(!is_gpu_hwmon_chip("nct6798")); assert!(!is_gpu_hwmon_chip("coretemp")); assert!(!is_gpu_hwmon_chip("k10temp")); From 1026a0789c6a2b6f87da2e8f529c6c5047fc1e0c Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 30 Mar 2026 16:49:51 -0700 Subject: [PATCH 2/2] Address review feedback on hwmon disambiguation - Avoid allocation in is_gpu_hwmon_chip by using strip_prefix instead of format! - Fix curr sensor discovery passing original label_overrides instead of effective_overrides - Extract expand_label_overrides as a pure testable function - Add unit tests for label override expansion (no duplicates, with duplicates, qualified precedence) --- src/sensors/hwmon.rs | 107 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 17 deletions(-) diff --git a/src/sensors/hwmon.rs b/src/sensors/hwmon.rs index 9c98d20..6d77a76 100644 --- a/src/sensors/hwmon.rs +++ b/src/sensors/hwmon.rs @@ -9,9 +9,12 @@ use std::path::Path; const GPU_HWMON_CHIPS: &[&str] = &["amdgpu", "nouveau", "i915", "xe"]; fn is_gpu_hwmon_chip(chip_name: &str) -> bool { - GPU_HWMON_CHIPS - .iter() - .any(|&gpu| chip_name == gpu || chip_name.starts_with(&format!("{gpu}-"))) + GPU_HWMON_CHIPS.iter().any(|&gpu| { + chip_name == gpu + || chip_name + .strip_prefix(gpu) + .is_some_and(|rest| rest.starts_with('-')) + }) } /// Prefix a GPU hwmon label with "GPU " if it doesn't already start with "GPU". @@ -85,19 +88,7 @@ impl HwmonSource { }) .collect(); - let mut effective_overrides = label_overrides.clone(); - for (_, chip_name, display_name) in &hwmon_entries { - if chip_name != display_name { - let prefix = format!("hwmon/{chip_name}/"); - for (key, value) in label_overrides { - if let Some(sensor) = key.strip_prefix(&prefix) { - effective_overrides - .entry(format!("hwmon/{display_name}/{sensor}")) - .or_insert_with(|| value.clone()); - } - } - } - } + let effective_overrides = expand_label_overrides(label_overrides, &hwmon_entries); for (hwmon_dir, _, display_name) in &hwmon_entries { let mut entries = Vec::new(); @@ -149,7 +140,7 @@ impl HwmonSource { SensorCategory::Current, SensorUnit::Amps, 1000.0, - label_overrides, + &effective_overrides, &mut entries, ); @@ -188,6 +179,30 @@ impl HwmonSource { } } +/// Expand label overrides for disambiguated chip names. Board templates use +/// unqualified names like `hwmon/jc42/temp1`; when a chip is disambiguated to +/// `jc42-9-0018`, copy matching overrides so they still apply. Qualified +/// overrides (if any) take precedence via `or_insert`. +fn expand_label_overrides( + base: &HashMap, + entries: &[(std::path::PathBuf, String, String)], +) -> HashMap { + let mut expanded = base.clone(); + for (_, chip_name, display_name) in entries { + if chip_name != display_name { + let prefix = format!("hwmon/{chip_name}/"); + for (key, value) in base { + if let Some(sensor) = key.strip_prefix(&prefix) { + expanded + .entry(format!("hwmon/{display_name}/{sensor}")) + .or_insert_with(|| value.clone()); + } + } + } + } + expanded +} + #[allow(clippy::too_many_arguments)] fn discover_type( hwmon_dir: &Path, @@ -358,4 +373,62 @@ mod tests { "GPU Temperature" ); } + + #[test] + fn test_expand_label_overrides_no_duplicates() { + let base: HashMap = [("hwmon/nct6798/temp1".into(), "SYSTIN".into())] + .into_iter() + .collect(); + // Unique chip name — display_name == chip_name, no expansion + let entries = vec![( + std::path::PathBuf::from("/sys/class/hwmon/hwmon0"), + "nct6798".into(), + "nct6798".into(), + )]; + let result = expand_label_overrides(&base, &entries); + assert_eq!(result.len(), 1); + assert_eq!(result["hwmon/nct6798/temp1"], "SYSTIN"); + } + + #[test] + fn test_expand_label_overrides_with_duplicates() { + let base: HashMap = [("hwmon/jc42/temp1".into(), "DIMM Temp".into())] + .into_iter() + .collect(); + let entries = vec![ + ( + std::path::PathBuf::from("/sys/class/hwmon/hwmon0"), + "jc42".into(), + "jc42-9-0018".into(), + ), + ( + std::path::PathBuf::from("/sys/class/hwmon/hwmon1"), + "jc42".into(), + "jc42-9-0019".into(), + ), + ]; + let result = expand_label_overrides(&base, &entries); + assert_eq!(result.len(), 3); + assert_eq!(result["hwmon/jc42/temp1"], "DIMM Temp"); + assert_eq!(result["hwmon/jc42-9-0018/temp1"], "DIMM Temp"); + assert_eq!(result["hwmon/jc42-9-0019/temp1"], "DIMM Temp"); + } + + #[test] + fn test_expand_label_overrides_qualified_takes_precedence() { + let base: HashMap = [ + ("hwmon/jc42/temp1".into(), "DIMM Temp".into()), + ("hwmon/jc42-9-0018/temp1".into(), "DIMM A1".into()), + ] + .into_iter() + .collect(); + let entries = vec![( + std::path::PathBuf::from("/sys/class/hwmon/hwmon0"), + "jc42".into(), + "jc42-9-0018".into(), + )]; + let result = expand_label_overrides(&base, &entries); + // Qualified override takes precedence over expanded unqualified + assert_eq!(result["hwmon/jc42-9-0018/temp1"], "DIMM A1"); + } }