Possible to get Intel PCH/Chipset temperature to Thermal Sensor Widget?
-
Added a patch there you can test.
You'll need to reset the RRD data to see the new pchtherm value graphed.
Let me know if you see any issues.
-
Widget seems to be working, didn't reset RRD data yet.
Could add warning and critical limits also for PCH in the widget :)EDIT: Reset the RRD data, graphs also seem to work.
-
Adding the limits could be done but it's significantly more complex. Could be added later though.
-
I dabbled a bit with the files and I think I managed to add the limits for PCH temps.
Can you possibly review/check my changes? -
Sure. The best way is to make a pull request in github if you can.
-
I have never used github and have no idea how to do a pull request...
-
Ok, no worries, you have a patch? Or some other list of your changes? Or just the file we can diff against?
-
I have the modified php and js files
-
Upload them here: https://nc.netgate.com/nextcloud/index.php/s/qoK6XmdA2ffAiRH
-
Files uploaded
-
Ok I reworked the formatting slightly and added it as a diff:
https://redmine.pfsense.org/issues/14255#note-4Please test that if you can. Looks good here though.
-
Restored original files and applied your diffs, seems to be working.
Formatting rework was mostly spaces to tabs?Though js-file seems to have extra tabs at the end of lines 85 and 141 (criticalTemp = tsParams.pchCriticalTempThreshold;)
Might not really be relevant, but in the widget settings, the treshold range is "hardcoded" in the text and not using variables MIN/MAX_THRESHOLD_VALUE, so if you change those values, the change isn't reflected in the description text.
Also the pchtherm driver basically already provides the warning (t0temp) and critical (t2temp) limits, but that goes beyond my coding knowledge
-
Yup, you're right. Not sure how those crept in. I'll remove them.
-
Just went through the code once more and noticed the there are lines with "pulsate" in the variable names (like "thermal_sensors_widget_pulsate_warning").
The variable values are saved in the settings, but there seem to have no function that use them and the widget doesn't have checkbox to set this value.Are those line part of a code for a function/feature that was left unfinished or discarded in the widget final version?
-
Probably. I have no insight there. I imagine the intention was to have the widget display flash in some way to alert the user.
