From 669ee0a10f5ce66213df89e637e85e0ae721d42b Mon Sep 17 00:00:00 2001 From: Jacob Russell Date: Sat, 19 Sep 2026 13:48:19 -0500 Subject: [PATCH 1/5] Fix PWM10+ discovery and sort order in Common.php Two bugs, both hit by any hwmon device with 10+ PWM channels (e.g. ARCTIC Fan Controller, 10 channels): 1. glob("pwm[0-9]") and find -iname 'pwm[0-9]' match exactly one digit, so pwm10 (and above) are silently dropped from both build_pwm_map() and list_pwm(). Fixed by globbing broadly (pwm*) and filtering with a strict ^pwm\d+$ regex, which also avoids matching auxiliary attributes like pwm1_enable, pwm1_auto_point1_pwm, etc. 2. list_pwm()'s usort() used strcmp(), which sorts alphabetically ("pwm10" < "pwm2" as strings) rather than numerically. Fixed by switching to strnatcmp(). Verified against a live 10-channel ARCTIC Fan Controller on Unraid: before the fix, channel 10 was missing from the plugin UI entirely and pwm10 sorted between pwm1 and pwm2; after, all 10 channels appear in correct numeric order. --- .../emhttp/plugins/fanctrlplus2/include/Common.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php index 23a7dee..db43dbe 100644 --- a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php +++ b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php @@ -79,7 +79,8 @@ function build_pwm_map(): array { $chip = normalize_chip_name(trim(file_get_contents($name_file))); - foreach (glob("$dir/pwm[0-9]") as $pwm_path) { + foreach (glob("$dir/pwm*") as $pwm_path) { + if (!preg_match('/^pwm\d+$/', basename($pwm_path))) continue; $pwmN = basename($pwm_path); $real = realpath($pwm_path) ?: $pwm_path; $map["$chip:$pwmN"] = $real; @@ -234,15 +235,16 @@ function migrate_cfg_and_labels(string $plugin): void { function list_pwm() { $out = []; - exec("find /sys/devices -type f -iname 'pwm[0-9]' -exec dirname \"{}\" + | uniq", $chips); + exec("find /sys/devices -type f -regextype posix-extended -regex '.*/pwm[0-9]+' -exec dirname \"{}\" + | uniq", $chips); foreach ($chips as $chip) { $name = is_file("$chip/name") ? trim(file_get_contents("$chip/name")) : ''; - foreach (glob("$chip/pwm[0-9]") as $pwm) { + foreach (glob("$chip/pwm*") as $pwm) { + if (!preg_match('/^pwm\d+$/', basename($pwm))) continue; $out[] = ['chip' => $name, 'name' => basename($pwm), 'sensor' => $pwm]; } } - usort($out, fn($a, $b) => strcmp($a['name'], $b['name'])); + usort($out, fn($a, $b) => strnatcmp($a['name'], $b['name'])); return $out; } From 75e0dcd9fa2f19d3ebea0e697a1b159f8897be64 Mon Sep 17 00:00:00 2001 From: Jacob Russell Date: Sun, 20 Sep 2026 17:21:18 -0500 Subject: [PATCH 2/5] Address review: drop find/regextype, avoid pwm[0-9]* auxiliary-attribute match - Replaced the find(1) + GNU-only -regextype chip discovery in list_pwm() with the same glob('/sys/class/hwmon/hwmon*') approach build_pwm_map() already uses. No more shell exec() for this, and no portability concern about -regextype being a GNU find extension. - Kept the strict ^pwm\d+$ regex filter rather than switching to a bare "pwm[0-9]*" glob as suggested in review. Verified directly: a glob char class only consumes one character, so "pwm[0-9]*" still matches "pwm1_enable", "pwm10_enable", "pwm1_auto_point1_pwm", etc. -- the same auxiliary-attribute flooding the regex filter was added to prevent. fnmatch() confirms this: pwm1_enable vs pwm[0-9]*: MATCHES pwm10_enable vs pwm[0-9]*: MATCHES pwm1_auto_point1_pwm vs pwm[0-9]*: MATCHES Happy to go with a different structure if preferred, but "pwm[0-9]*" specifically doesn't fix the bug this PR is for. --- .../emhttp/plugins/fanctrlplus2/include/Common.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php index db43dbe..602c7c2 100644 --- a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php +++ b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php @@ -235,8 +235,15 @@ function migrate_cfg_and_labels(string $plugin): void { function list_pwm() { $out = []; - exec("find /sys/devices -type f -regextype posix-extended -regex '.*/pwm[0-9]+' -exec dirname \"{}\" + | uniq", $chips); - foreach ($chips as $chip) { + // Enumerate via /sys/class/hwmon, same as build_pwm_map() above -- avoids + // a shell exec() + GNU-only `find -regextype` (unavailable on some find + // implementations), and glob("pwm*") + a strict regex filter correctly + // excludes per-channel attributes (pwm1_enable, pwm1_auto_point1_pwm, + // etc.) that a bare "pwm[0-9]*" pattern would also match, since a glob + // char class only consumes one character before the trailing "*" takes + // over -- "pwm[0-9]*" still matches "pwm1_enable" the same way "pwm*" + // does. + foreach (glob('/sys/class/hwmon/hwmon*') as $chip) { $name = is_file("$chip/name") ? trim(file_get_contents("$chip/name")) : ''; foreach (glob("$chip/pwm*") as $pwm) { if (!preg_match('/^pwm\d+$/', basename($pwm))) continue; From 763f86a4939f7b799bcbe35d535101767aa7d369 Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Sun, 4 Oct 2026 03:26:03 +0000 Subject: [PATCH 3/5] fanctrlplus2: list PWM channels by their device path Enumerating /sys/class/hwmon returned /sys/class/hwmon/hwmonN/pwmX paths, but saved labels and controller settings are keyed by the resolved /sys/devices path, so every existing label and fan assignment would have stopped matching. Resolve each channel, and cover discovery with a test. --- .../plugins/fanctrlplus2/include/Common.php | 17 ++++----- tests/pwm_discovery_test.php | 35 +++++++++++++++++++ 2 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 tests/pwm_discovery_test.php diff --git a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php index 602c7c2..afb50e8 100644 --- a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php +++ b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php @@ -233,21 +233,16 @@ function migrate_cfg_and_labels(string $plugin): void { // END: Migrate hwmonX (cfg+labels) // ================================ -function list_pwm() { +// The sensor is the resolved /sys/devices path: saved labels and controller +// settings use it, and the /sys/class/hwmon/hwmonN link follows probe order. +// "pwm[0-9]*" would also match pwm1_enable and friends, hence the regex. +function list_pwm(string $hwmon_glob = '/sys/class/hwmon/hwmon*') { $out = []; - // Enumerate via /sys/class/hwmon, same as build_pwm_map() above -- avoids - // a shell exec() + GNU-only `find -regextype` (unavailable on some find - // implementations), and glob("pwm*") + a strict regex filter correctly - // excludes per-channel attributes (pwm1_enable, pwm1_auto_point1_pwm, - // etc.) that a bare "pwm[0-9]*" pattern would also match, since a glob - // char class only consumes one character before the trailing "*" takes - // over -- "pwm[0-9]*" still matches "pwm1_enable" the same way "pwm*" - // does. - foreach (glob('/sys/class/hwmon/hwmon*') as $chip) { + foreach (glob($hwmon_glob) as $chip) { $name = is_file("$chip/name") ? trim(file_get_contents("$chip/name")) : ''; foreach (glob("$chip/pwm*") as $pwm) { if (!preg_match('/^pwm\d+$/', basename($pwm))) continue; - $out[] = ['chip' => $name, 'name' => basename($pwm), 'sensor' => $pwm]; + $out[] = ['chip' => $name, 'name' => basename($pwm), 'sensor' => realpath($pwm) ?: $pwm]; } } diff --git a/tests/pwm_discovery_test.php b/tests/pwm_discovery_test.php new file mode 100644 index 0000000..7565be1 --- /dev/null +++ b/tests/pwm_discovery_test.php @@ -0,0 +1,35 @@ + Date: Sun, 4 Oct 2026 03:29:02 +0000 Subject: [PATCH 4/5] fanctrlplus2: share PWM enumeration and keep legacy-layout channels Older drivers keep pwmN on the parent device, where the removed find over /sys/devices still found them. Scan hwmonN/device too, de-duplicate by the resolved path, break sort ties by path, and build the migration map from list_pwm so both use the same discovery. --- .../plugins/fanctrlplus2/include/Common.php | 32 ++++++------- tests/pwm_discovery_test.php | 47 ++++++++++++------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php index afb50e8..8b372bb 100644 --- a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php +++ b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php @@ -73,18 +73,9 @@ function normalize_chip_name(string $chip): string { function build_pwm_map(): array { $map = []; - foreach (glob("/sys/class/hwmon/hwmon*") as $dir) { - $name_file = "$dir/name"; - if (!is_file($name_file)) continue; - - $chip = normalize_chip_name(trim(file_get_contents($name_file))); - - foreach (glob("$dir/pwm*") as $pwm_path) { - if (!preg_match('/^pwm\d+$/', basename($pwm_path))) continue; - $pwmN = basename($pwm_path); - $real = realpath($pwm_path) ?: $pwm_path; - $map["$chip:$pwmN"] = $real; - } + foreach (list_pwm() as $pwm) { + if ($pwm['chip'] === '') continue; + $map[normalize_chip_name($pwm['chip']).':'.$pwm['name']] = $pwm['sensor']; } return $map; } @@ -235,18 +226,23 @@ function migrate_cfg_and_labels(string $plugin): void { // The sensor is the resolved /sys/devices path: saved labels and controller // settings use it, and the /sys/class/hwmon/hwmonN link follows probe order. +// Older drivers keep their attributes on the parent device instead. // "pwm[0-9]*" would also match pwm1_enable and friends, hence the regex. function list_pwm(string $hwmon_glob = '/sys/class/hwmon/hwmon*') { $out = []; - foreach (glob($hwmon_glob) as $chip) { - $name = is_file("$chip/name") ? trim(file_get_contents("$chip/name")) : ''; - foreach (glob("$chip/pwm*") as $pwm) { - if (!preg_match('/^pwm\d+$/', basename($pwm))) continue; - $out[] = ['chip' => $name, 'name' => basename($pwm), 'sensor' => realpath($pwm) ?: $pwm]; + foreach (glob($hwmon_glob) ?: [] as $hwmon) { + foreach ([$hwmon, "$hwmon/device"] as $dir) { + $name = is_file("$dir/name") ? trim(file_get_contents("$dir/name")) : ''; + foreach (glob("$dir/pwm*") ?: [] as $pwm) { + if (!preg_match('/^pwm\d+$/', basename($pwm)) || !is_file($pwm)) continue; + $sensor = realpath($pwm) ?: $pwm; + $out[$sensor] = ['chip' => $name, 'name' => basename($pwm), 'sensor' => $sensor]; + } } } - usort($out, fn($a, $b) => strnatcmp($a['name'], $b['name'])); + $out = array_values($out); + usort($out, fn($a, $b) => strnatcmp($a['name'], $b['name']) ?: strcmp($a['sensor'], $b['sensor'])); return $out; } diff --git a/tests/pwm_discovery_test.php b/tests/pwm_discovery_test.php index 7565be1..3819a4f 100644 --- a/tests/pwm_discovery_test.php +++ b/tests/pwm_discovery_test.php @@ -3,26 +3,41 @@ require_once __DIR__ . '/../src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php'; $failures = []; -$root = sys_get_temp_dir() . '/fcp_pwm_discovery_' . getmypid(); -$device = "$root/devices/usb1/1-14/1-14:1.0/0003:3904:F001.0005/hwmon/hwmon5"; -@mkdir($device, 0777, true); -@mkdir("$root/class/hwmon", 0777, true); -symlink($device, "$root/class/hwmon/hwmon5"); -file_put_contents("$device/name", "arctic_fan_controller\n"); -foreach (['pwm1', 'pwm2', 'pwm10', 'pwm1_enable', 'pwm10_enable', 'pwm1_auto_point1_pwm'] as $f) { - touch("$device/$f"); -} +$root = realpath(sys_get_temp_dir()) . '/fcp_pwm_discovery_' . getmypid(); + +$chip = function (string $hwmon, string $device, string $name, array $files) use ($root): void { + @mkdir("$root/devices/$device", 0777, true); + @mkdir("$root/class/hwmon", 0777, true); + symlink("$root/devices/$device", "$root/class/hwmon/$hwmon"); + file_put_contents("$root/devices/$device/name", "$name\n"); + foreach ($files as $f) touch("$root/devices/$device/$f"); +}; +$usb = 'usb1/1-14/1-14:1.0/0003:3904:F001.0005/hwmon/hwmon5'; +$chip('hwmon5', $usb, 'arctic_fan_controller', + ['pwm1', 'pwm2', 'pwm10', 'pwm1_enable', 'pwm10_enable', 'pwm1_auto_point1_pwm']); + +// Older drivers put the attributes on the parent device; hwmonN is a stub. +$legacy = 'platform/w83627hf.656'; +@mkdir("$root/devices/$legacy/hwmon/hwmon2", 0777, true); +symlink("$root/devices/$legacy", "$root/devices/$legacy/hwmon/hwmon2/device"); +$chip('hwmon2', "$legacy/hwmon/hwmon2", 'w83627hf', []); +unlink("$root/devices/$legacy/hwmon/hwmon2/name"); +file_put_contents("$root/devices/$legacy/name", "w83627hf\n"); +touch("$root/devices/$legacy/pwm1"); $pwms = list_pwm("$root/class/hwmon/hwmon*"); -$names = array_column($pwms, 'name'); -if ($names !== ['pwm1', 'pwm2', 'pwm10']) { - $failures[] = 'Channels must include pwm10, exclude per-channel attributes and sort numerically, got ' . implode(',', $names); -} -// Labels and controller settings are keyed by the /sys/devices path; a +$expected = [ + ['chip' => 'w83627hf', 'name' => 'pwm1', 'sensor' => "$root/devices/$legacy/pwm1"], + ['chip' => 'arctic_fan_controller', 'name' => 'pwm1', 'sensor' => "$root/devices/$usb/pwm1"], + ['chip' => 'arctic_fan_controller', 'name' => 'pwm2', 'sensor' => "$root/devices/$usb/pwm2"], + ['chip' => 'arctic_fan_controller', 'name' => 'pwm10', 'sensor' => "$root/devices/$usb/pwm10"], +]; +// Labels and controller settings are keyed by the /sys/devices path, so a // /sys/class/hwmon/hwmonN path would orphan every saved one. -if (($pwms[0]['sensor'] ?? null) !== "$device/pwm1") { - $failures[] = 'The sensor must be the resolved device path, got ' . var_export($pwms[0]['sensor'] ?? null, true); +if ($pwms !== $expected) { + $failures[] = "PWM channels must include pwm10 and legacy-layout channels, exclude per-channel attributes,\n" + . "sort numerically and use the resolved device path. Got:\n" . var_export($pwms, true); } exec('rm -rf ' . escapeshellarg($root)); From 30c12745ca046ff0173084c02e8a9b8bd3421eec Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Sun, 4 Oct 2026 04:28:12 +0000 Subject: [PATCH 5/5] fanctrlplus2: name legacy-layout channels after their hwmon device A parent device without its own name file left the chip empty, which build_pwm_map skips. Fall back to the hwmonN name. --- .../plugins/fanctrlplus2/include/Common.php | 3 ++- tests/pwm_discovery_test.php | 25 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php index 8b372bb..2e60d7e 100644 --- a/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php +++ b/src/usr/local/emhttp/plugins/fanctrlplus2/include/Common.php @@ -231,8 +231,9 @@ function migrate_cfg_and_labels(string $plugin): void { function list_pwm(string $hwmon_glob = '/sys/class/hwmon/hwmon*') { $out = []; foreach (glob($hwmon_glob) ?: [] as $hwmon) { + $name = ''; foreach ([$hwmon, "$hwmon/device"] as $dir) { - $name = is_file("$dir/name") ? trim(file_get_contents("$dir/name")) : ''; + if (is_file("$dir/name")) $name = trim(file_get_contents("$dir/name")); foreach (glob("$dir/pwm*") ?: [] as $pwm) { if (!preg_match('/^pwm\d+$/', basename($pwm)) || !is_file($pwm)) continue; $sensor = realpath($pwm) ?: $pwm; diff --git a/tests/pwm_discovery_test.php b/tests/pwm_discovery_test.php index 3819a4f..b1e73de 100644 --- a/tests/pwm_discovery_test.php +++ b/tests/pwm_discovery_test.php @@ -16,19 +16,26 @@ $chip('hwmon5', $usb, 'arctic_fan_controller', ['pwm1', 'pwm2', 'pwm10', 'pwm1_enable', 'pwm10_enable', 'pwm1_auto_point1_pwm']); -// Older drivers put the attributes on the parent device; hwmonN is a stub. -$legacy = 'platform/w83627hf.656'; -@mkdir("$root/devices/$legacy/hwmon/hwmon2", 0777, true); -symlink("$root/devices/$legacy", "$root/devices/$legacy/hwmon/hwmon2/device"); -$chip('hwmon2', "$legacy/hwmon/hwmon2", 'w83627hf', []); -unlink("$root/devices/$legacy/hwmon/hwmon2/name"); -file_put_contents("$root/devices/$legacy/name", "w83627hf\n"); -touch("$root/devices/$legacy/pwm1"); +// Older drivers put the attributes on the parent device; hwmonN is a stub +// that may or may not carry the name. +$legacy = function (string $hwmon, string $device, string $name, bool $nameOnDevice) use ($root, $chip): void { + @mkdir("$root/devices/$device/hwmon/$hwmon", 0777, true); + symlink("$root/devices/$device", "$root/devices/$device/hwmon/$hwmon/device"); + $chip($hwmon, "$device/hwmon/$hwmon", $name, []); + if ($nameOnDevice) { + unlink("$root/devices/$device/hwmon/$hwmon/name"); + file_put_contents("$root/devices/$device/name", "$name\n"); + } + touch("$root/devices/$device/pwm1"); +}; +$legacy('hwmon2', 'platform/w83627hf.656', 'w83627hf', true); +$legacy('hwmon3', 'platform/it87.552', 'it8728', false); $pwms = list_pwm("$root/class/hwmon/hwmon*"); $expected = [ - ['chip' => 'w83627hf', 'name' => 'pwm1', 'sensor' => "$root/devices/$legacy/pwm1"], + ['chip' => 'it8728', 'name' => 'pwm1', 'sensor' => "$root/devices/platform/it87.552/pwm1"], + ['chip' => 'w83627hf', 'name' => 'pwm1', 'sensor' => "$root/devices/platform/w83627hf.656/pwm1"], ['chip' => 'arctic_fan_controller', 'name' => 'pwm1', 'sensor' => "$root/devices/$usb/pwm1"], ['chip' => 'arctic_fan_controller', 'name' => 'pwm2', 'sensor' => "$root/devices/$usb/pwm2"], ['chip' => 'arctic_fan_controller', 'name' => 'pwm10', 'sensor' => "$root/devices/$usb/pwm10"],