Feat:Add power, temps and Split physical CPUS #2407
Conversation
WalkthroughAdds per-physical-CPU-package handling: new get_cpu_packages() helper, per-package rendering (headers, total power, optional temperature), per-core power/temp updates via updateCPUPower(), and a retry-enabled GraphQL subscription for CPU power/temperature with proper subscription lifecycle management. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant Dash as DashStats.page
participant Helper as get_cpu_packages()
participant Sys as /sys/devices/system/cpu/...
User->>Dash: Open dashboard
Dash->>Helper: get_cpu_packages(',')
Helper->>Sys: read thread_siblings_list + physical_package_id
Sys-->>Helper: sibling lists + package IDs
Helper-->>Dash: package -> siblings mapping
rect rgb(240,250,240)
note right of Dash: Per-package render & telemetry setup
loop for each package
Dash->>Dash: Render "Physical CPU N Power" header and `cpu-total-power`
Dash->>Dash: Render per-core / HT rows and `cpu-powerN` / `cpu-tempN`
Dash->>Dash: subscribe GraphQL power/temp (retry/backoff)
Dash->>Dash: on telemetry -> updateCPUPower() updates DOM
end
Dash->>Dash: subscribe existing percentTotal usage telemetry
end
User->>Dash: Close page
Dash->>Dash: Unsubscribe/cleanup telemetry subscriptions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix/DashStats.page(3 hunks)emhttp/plugins/dynamix/include/Helpers.php(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
emhttp/plugins/dynamix/DashStats.page (1)
403-423: Fix malformed HTML structure in per-package header (flagged in previous review).The per-package header has two critical HTML issues that were flagged in the previous review but remain unfixed:
- Missing opening
<tr>tag: Line 405 opens a<td>without a preceding<tr>, breaking table structure.- Wrong closing order: Line 407 closes
</td></span></tr>, but since the<span>is inside the<td>, the correct order must be</span></td></tr>.These issues will cause rendering problems and DOM manipulation failures.
Apply this diff:
foreach ($cpus as $cpu_index=>$package) { if (count($cpus) > 1) { - echo "<td><span class='cpu_open w72'>"._("Physical")." CPU $cpu_index "._("Power").": <span id='cpu-power$cpu_index'>N/A </span> "; + echo "<tr class='cpu_open'><td><span class='w72'>"._("Physical")." CPU $cpu_index "._("Power").": <span id='cpu-power$cpu_index'>N/A</span> "; if (count($cpus)>1) echo _("Temp").": <span id='cpu-temp$cpu_index'>N/A</span>"; - echo "</td></span></tr>"; + echo "</span></td></tr>"; } foreach ($package as $pair) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/DashStats.page(5 hunks)
🔇 Additional comments (2)
emhttp/plugins/dynamix/DashStats.page (2)
97-97: LGTM!The call to
get_cpu_packages()is clean and straightforward.
349-355: LGTM!The conditional logic correctly shows temperature in the header only for single-CPU-package systems, while multi-package systems display per-package temperatures below.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
emhttp/plugins/dynamix/DashStats.page (2)
410-414: Fix markup for per-package header row.The header is still missing its opening
<tr>and closes elements out of order (</td></span>), leaving invalid table structure. Wrap the header in its own<tr>and close the spans before the cell so browsers don’t have to guess.- echo "<td><span class='cpu_open w72'><i class='fa fa-plug'></i> "._("Physical")." CPU $cpu_index "._("Power").": <span id='cpu-power$cpu_index'>N/A </span> "; - if (count($cpus)>1) echo "<i class='fa fa-thermometer'></i> "._("Temperature").": <span id='cpu-temp$cpu_index'>N/A</span>"; - echo "</td></span></tr>"; + echo "<tr class='cpu_open'><td><span class='cpu_open w72'><i class='fa fa-plug'></i> "._("Physical")." CPU $cpu_index "._("Power").": <span id='cpu-power$cpu_index'>N/A</span> "; + if (count($cpus) > 1) echo "<i class='fa fa-thermometer'></i> "._("Temperature").": <span id='cpu-temp$cpu_index'>N/A</span>"; + echo "</span></td></tr>";
2825-2864: Fix power subscription retry counter typo.The retry branch increments
cpuPInitPWRAttempts, but the variable is declared ascpuInitPWRAttempts. The first retry throws a ReferenceError and halts the subscription, so power data never updates when Apollo isn’t ready on load. Use the correct identifier.- cpuPInitPWRAttempts++; + cpuInitPWRAttempts++;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix/DashStats.page(6 hunks)emhttp/plugins/dynamix/include/Helpers.php(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
emhttp/plugins/dynamix/DashStats.page (1)
406-428: Critical: Fix malformed package header markup and orphaned tags.The per-package header rows have several critical markup issues:
- Line 406: The
<tr>opened here becomes an orphan whencount($cpus) == 1(never closed).- Line 410-412: For multi-CPU systems, only the first package header uses the
<tr>from line 406; subsequent package headers (2nd, 3rd, etc.) have no opening<tr>tag.- Line 412: Wrong closing tag order—
</td></span></tr>should be</span></td></tr>.- Line 411: Redundant condition
if (count($cpus)>1)—already inside the same check from line 409.Apply this diff:
- <tr> <? foreach ($cpus as $cpu_index=>$package) { if (count($cpus) > 1) { - echo "<td><span class='cpu_open w72'><i class='fa fa-plug'></i> "._("Physical")." CPU $cpu_index "._("Power").": <span id='cpu-power$cpu_index'>N/A </span> "; - if (count($cpus)>1) echo "<i class='fa fa-thermometer'></i> "._("Temperature").": <span id='cpu-temp$cpu_index'>N/A</span>"; - echo "</td></span></tr>"; + echo "<tr class='cpu_open'><td><span class='w72'><i class='fa fa-plug'></i> "._("Physical")." CPU $cpu_index "._("Power").": <span id='cpu-power$cpu_index'>N/A</span> "; + echo "<i class='fa fa-thermometer'></i> "._("Temperature").": <span id='cpu-temp$cpu_index'>N/A</span>"; + echo "</span></td></tr>"; } foreach ($package as $pair) {
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/DashStats.page (1)
2819-2872: Optional: Remove extraneous blank lines.The CPU power subscription logic is correct, but there are extra blank lines at 2820, 2825, 2843, and 2871 that can be removed for consistency.
Apply this diff:
},500); - - - // Start GraphQL CPU power subscription with retry logic let cpuInitPWRAttempts = 0, cpuPWRRetryMs = 100; function initPwrCpuSubscription() { - - if (window.gql && window.apolloClient) { // Define the subscription query when GraphQL is available ... }).subscribe({ next: (result) => { - - if (result.data?.systemMetricsCpuTelemetry){ cpupower = result.data.systemMetricsCpuTelemetry; updateCPUPower(); } }, ... }); } }); - - // Cleanup GraphQL subscription on page unload
| function updateCPUPower() { | ||
| if (!cpupower) return; | ||
|
|
||
| // Update total power | ||
| const totalEl = document.getElementById('cpu-total-power'); | ||
| const totalPower = cpupower.totalPower ?? 0; | ||
| if (totalEl) { | ||
| totalEl.innerHTML = `<i class="fa fa-fw fa-plug"></i> _(Total)_ _(Power)_: ${totalPower.toFixed(2)} W`; | ||
| } | ||
|
|
||
| // Update each core's span | ||
| const cpuspower = cpupower.power ?? []; | ||
| cpuspower.forEach((power, index) => { | ||
| const coreEl = document.getElementById(`cpu-power${index}`); | ||
| if (coreEl) { | ||
| coreEl.innerHTML = `${power.toFixed(2)} W`; | ||
| } | ||
| }); | ||
|
|
||
| const cpustemps = cpupower.temp ?? []; | ||
| cpustemps.forEach((temp, index) => { | ||
| const coreTempEl = document.getElementById(`cpu-temp${index}`); | ||
| if (coreTempEl) { | ||
| tempdisplay = temp.toFixed(0); | ||
| if (tempunit === "F") { | ||
| tempdisplay = ((temp.toFixed(0))* 9 / 5) + 32; | ||
| } | ||
| coreTempEl.innerHTML = Math.round(tempdisplay)+` °`+tempunit;; | ||
| } | ||
| }); | ||
|
|
||
| } |
There was a problem hiding this comment.
Fix translation markers, variable declaration, and temperature conversion logic.
Several issues in updateCPUPower():
- Line 1718: The translation markers
_(Total)_and_(Power)_inside the JavaScript template literal will not be processed by PHP—they'll appear as literal text. You need to pre-translate these server-side or use a different approach for localization. - Line 1734:
tempdisplayis assigned withoutlet,const, orvar, creating an unintended global variable. - Line 1736:
temp.toFixed(0)returns a string; while JavaScript coerces it to a number in arithmetic, it's cleaner to convert the raw number:temp * 9 / 5 + 32, then round once. - Line 1738: Double semicolon at end (
tempunit;;) and inconsistent indentation.
Apply this diff:
const totalEl = document.getElementById('cpu-total-power');
const totalPower = cpupower.totalPower ?? 0;
if (totalEl) {
- totalEl.innerHTML = `<i class="fa fa-fw fa-plug"></i> _(Total)_ _(Power)_: ${totalPower.toFixed(2)} W`;
+ totalEl.innerHTML = `<i class="fa fa-fw fa-plug"></i> <?=_('Total')?> <?=_('Power')?>: ${totalPower.toFixed(2)} W`;
}
...
const cpustemps = cpupower.temp ?? [];
cpustemps.forEach((temp, index) => {
const coreTempEl = document.getElementById(`cpu-temp${index}`);
- if (coreTempEl) {
- tempdisplay = temp.toFixed(0);
- if (tempunit === "F") {
- tempdisplay = ((temp.toFixed(0))* 9 / 5) + 32;
- }
- coreTempEl.innerHTML = Math.round(tempdisplay)+` °`+tempunit;;
+ if (coreTempEl) {
+ let tempdisplay = temp;
+ if (tempunit === "F") {
+ tempdisplay = (temp * 9 / 5) + 32;
+ }
+ coreTempEl.innerHTML = Math.round(tempdisplay) + ` °` + tempunit;
}
});
SimonFair
changed the title
3 participants
Add CPU Total Power
Change CPU layout to split physical CPUs.
Add per physical CPU power.
Mockup on 6.12 Dual CPU.
Summary by CodeRabbit
New Features
Improvements