Dashboard: support custom fan names on Airflow tile#2662
Dashboard: support custom fan names on Airflow tile#2662mrjacobarussell wants to merge 2 commits into
Conversation
The Airflow tile labels each detected fan sequentially as "FAN 1", "FAN 2", etc. based on `sensors -uA` enumeration order, which often doesn't match the physical header labels printed on the motherboard (e.g. dashboard FAN 1 may actually be the motherboard's PWM2 header). This adds an optional /boot/config/plugins/dynamix/fan_names.json (JSON array, indexed in the same order as `sensors -uA`) whose values override the generic "FAN N" label when present. Falls back to the existing behavior if the file doesn't exist or an entry is empty. A companion plugin (https://github.com/mrjacobarussell/dashboard-fan-labels) provides a settings page to manage this file - it lists each detected fan sensor (chip, sensor, live RPM) in Dashboard order so users can assign meaningful names. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughDashboard fan labels now optionally use per-fan names from ChangesCustom fan labels
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@emhttp/plugins/dynamix/DashStats.page`:
- Around line 809-814: The code uses $fan_names[$fan] without ensuring it's a
string before calling htmlspecialchars, which can cause runtime errors for
non-string JSON values; update the logic around $fan_label in the loop (the
handling that reads $fan_names_file, $fan_names and assigns $fan_label) to first
validate the value is a non-empty string (e.g. is_string($fan_names[$fan]) &&
$fan_names[$fan] !== '') and only then call htmlspecialchars on it, otherwise
fall back to the default _('FAN')." ".($fan+1); ensure the check replaces the
current !== '' test so malformed fan_names.json entries won't reach
htmlspecialchars.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7c0ab7ce-1493-466b-a4de-c93beb075493
📒 Files selected for processing (1)
emhttp/plugins/dynamix/DashStats.page
| $fan_names = file_exists($fan_names_file) ? (json_decode(file_get_contents($fan_names_file), true) ?: []) : []; | ||
| for ($fan=0; $fan<$fans; $fan++) { | ||
| if ($fan > 0 && $fan % 3 == 0) $i++; | ||
| $class = $fan % 3 == 2 ? "" : " class='fan'"; | ||
| $label[$i][] = "<span{$class}>"._('FAN')." ".($fan+1)."</span>"; | ||
| $fan_label = ($fan_names[$fan] ?? '') !== '' ? htmlspecialchars($fan_names[$fan]) : _('FAN')." ".($fan+1); | ||
| $label[$i][] = "<span{$class}>$fan_label</span>"; |
There was a problem hiding this comment.
Validate custom fan-name type before escaping.
At Line 813, non-string JSON values can pass the !== '' check and then reach htmlspecialchars(...), which can trigger runtime errors and break Airflow tile rendering for malformed fan_names.json.
Suggested fix
-$fan_names = file_exists($fan_names_file) ? (json_decode(file_get_contents($fan_names_file), true) ?: []) : [];
+$fan_names = file_exists($fan_names_file) ? json_decode(file_get_contents($fan_names_file), true) : [];
+if (!is_array($fan_names)) $fan_names = [];
...
- $fan_label = ($fan_names[$fan] ?? '') !== '' ? htmlspecialchars($fan_names[$fan]) : _('FAN')." ".($fan+1);
+ $custom_name = $fan_names[$fan] ?? null;
+ $fan_label = (is_string($custom_name) && trim($custom_name) !== '')
+ ? htmlspecialchars($custom_name)
+ : _('FAN')." ".($fan+1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $fan_names = file_exists($fan_names_file) ? (json_decode(file_get_contents($fan_names_file), true) ?: []) : []; | |
| for ($fan=0; $fan<$fans; $fan++) { | |
| if ($fan > 0 && $fan % 3 == 0) $i++; | |
| $class = $fan % 3 == 2 ? "" : " class='fan'"; | |
| $label[$i][] = "<span{$class}>"._('FAN')." ".($fan+1)."</span>"; | |
| $fan_label = ($fan_names[$fan] ?? '') !== '' ? htmlspecialchars($fan_names[$fan]) : _('FAN')." ".($fan+1); | |
| $label[$i][] = "<span{$class}>$fan_label</span>"; | |
| $fan_names = file_exists($fan_names_file) ? json_decode(file_get_contents($fan_names_file), true) : []; | |
| if (!is_array($fan_names)) $fan_names = []; | |
| for ($fan=0; $fan<$fans; $fan++) { | |
| if ($fan > 0 && $fan % 3 == 0) $i++; | |
| $class = $fan % 3 == 2 ? "" : " class='fan'"; | |
| $custom_name = $fan_names[$fan] ?? null; | |
| $fan_label = (is_string($custom_name) && trim($custom_name) !== '') | |
| ? htmlspecialchars($custom_name) | |
| : _('FAN')." ".($fan+1); | |
| $label[$i][] = "<span{$class}>$fan_label</span>"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@emhttp/plugins/dynamix/DashStats.page` around lines 809 - 814, The code uses
$fan_names[$fan] without ensuring it's a string before calling htmlspecialchars,
which can cause runtime errors for non-string JSON values; update the logic
around $fan_label in the loop (the handling that reads $fan_names_file,
$fan_names and assigns $fan_label) to first validate the value is a non-empty
string (e.g. is_string($fan_names[$fan]) && $fan_names[$fan] !== '') and only
then call htmlspecialchars on it, otherwise fall back to the default _('FAN')."
".($fan+1); ensure the check replaces the current !== '' test so malformed
fan_names.json entries won't reach htmlspecialchars.
Summary
sensors -uAenumeration order. This order frequently doesn't match the physical header labels printed on the motherboard (e.g. dashboard "FAN 1" may actually be the "PWM2" header)./boot/config/plugins/dynamix/fan_names.jsonfile (a JSON array, indexed in the same ordersensors -uAreports fans) whose values override the generic "FAN N" label when present and non-empty. If the file doesn't exist, behavior is unchanged.Companion plugin
A small plugin (https://github.com/mrjacobarussell/dashboard-fan-labels) provides a settings page (Settings -> Dashboard Fan Labels) that lists each detected fan sensor (chip, sensor name, live RPM) in the same order as the Dashboard, with a text field to assign a custom name. It writes to the
fan_names.jsonpath this PR reads.Test plan
fan_names.json, dashboard shows "FAN 1".."FAN 6" as before.fan_names.jsonpopulated (e.g.["CPU","Case Top","","Pump"]), tile shows custom names for indices 0,1,3 and falls back to "FAN 3" for the empty entry.get.fan[k]via nchan) are unaffected - only the static label changes.Summary by CodeRabbit