j7126 / OctoPrint-Dashboard

A dashboard for Octoprint
GNU Affero General Public License v3.0
288 stars 39 forks source link

SystemInfo Widget CPU Temperature always shows as 0°C #327

Closed gpread closed 2 years ago

gpread commented 2 years ago

Describe the bug The SystemInfo Widget does not report/display CPU Temperature correctly - it always shows 0°C

Expected behavior A temperature value displayed and which updates in line with what the OS reports, or an error value.

Screenshots OctoPrint-Dashboard - CPU Temp Issue

Files (please attach the following Files or screenshots when applicable):

OctoPrint server and Plugins:

Desktop (please complete the following information):

Logs octoprint.log.txt

Additional context I am new to Octoprint Dashboard, so I currently have no idea how/where it gets its information from to present in the widgets. I can appreciate much of it probably comes from Octoprint API (and/or g-code but that's definitely not relevant in this issue) but perhaps not that for SystemInfo Widget (data from system on which Octoprint is running) which is the issue here.

Since I am not running OctoPi nor Octoprint on a standard RPi with Raspbian, its perhaps helpful to provide simple working example for retrieving CPU/SYS temperature in my case. Then perhaps, if nothing else, someone can at least point me in the right direction so I could perhaps make changes myself to ensure the Dashboard SystemInfo Widget gets the appropriate value for display.......

The standard Armbian welcome screen displays text for CPU/System Temperature as an integer °C value. Attached below is a bash script which does that, its an extract of only the relevant parts from the standard full system-info Armbian script (so its simple but still somewhat hardware neutral as there are cases to cover some hardware permutations other than mine). armbian_cputemp.txt

This produces an output which looks like this: ssh_armbian_systemp

StefanCohen commented 2 years ago

Hi. Dashboard uses psUtil to read the cpu temp. You can see how it does it in the psUtilGetStats method in init.py in the plugin. Different systems report temperatures differently and your rockpi is most likely not covered yet. I would suggest that you fire up python on the device, import psutil and see what psutil.sensors_temperatures(fahrenheit=False) returns. You should then be able to modify your init.py accordingly and possibly contribute a PR so that others may benefit from it. The psutil documentation is available here: https://psutil.readthedocs.io/en/latest/#psutil.sensors_temperatures

gpread commented 2 years ago

Hi. Dashboard uses psUtil to read the cpu temp. You can see how it does it in the psUtilGetStats method in init.py in the plugin. Different systems report temperatures differently and your rockpi is most likely not covered yet. I would suggest that you fire up python on the device, import psutil and see what psutil.sensors_temperatures(fahrenheit=False) returns. You should then be able to modify your init.py accordingly and possibly contribute a PR so that others may benefit from it. The psutil documentation is available here: https://psutil.readthedocs.io/en/latest/#psutil.sensors_temperatures

Thanks, I'll look into it

gpread commented 2 years ago

Hi. Dashboard uses psUtil to read the cpu temp. You can see how it does it in the psUtilGetStats method in init.py in the plugin. Different systems report temperatures differently and your rockpi is most likely not covered yet. I would suggest that you fire up python on the device, import psutil and see what psutil.sensors_temperatures(fahrenheit=False) returns. You should then be able to modify your init.py accordingly and possibly contribute a PR so that others may benefit from it. The psutil documentation is available here: https://psutil.readthedocs.io/en/latest/#psutil.sensors_temperatures

@StefanCohen - Ok, so I did that and even with my limited programming skills I found it easy to get a working python script using psUtil to get the cpu temp. Script and output shown in a putty screen capture below:

WorkingPyScriptforRockPi4_CPUTemp_psUtil

So, then I went ahead and modified accordingly the __init__.py installed on my Octoprint. I'm pretty sure the change I have made is good/correct.

However, then I could not get Octoprint to accept it, when the modified version is installed/active Octoprint determines the plugin is incompatible (lightning flash next to name in plugin manager and an asterisk (*) prefix shown by octoprint.plugin.core in the Octoprint log file).

Presumably there is some cksum or other such checking to detect if an installed plugin is trustworthy or something ?

Attached below is a copy of the modified __init__.py, any assistance with getting that to be accepted by Octoprint so I can fully test my change will be much appreciated.

init.py_fixed_CpuTemp.txt

StefanCohen commented 2 years ago

There are afaik no checksums to be evaluated by OP to verify the integrity of a plugin. I think it will be enough to simply restart OP.

gpread commented 2 years ago

I did that in the very first place. I backed up the original (added suffix to filename), made a copy, modified it. Then copied modified script file to __init__.py and restarted Octoprint. Dashboard tab is not there after the restart and clearly Octoprint detects something it does not like, showing its now got "incompatible" status.

I know its not a simple syntax error in the/my python code additions because I did have one initially and in such case Octoprint reports it as such in its log.

So if its not local OP integrity checking (cksum or similar against a original manifest) then my guess is its a check against the repository or something like that.

I currently have no experience of modifying installed OP plugins either downloaded by OP plugin manager or installed manually, so nothing to go on really.

StefanCohen commented 2 years ago

Hang on. I haven't touched my OP in like a year so I have a bunch of updates to go thru in order to verify this.

gpread commented 2 years ago

Hang on. I haven't touched my OP in like a year so I have a bunch of updates to go thru in order to verify this.

👍

gpread commented 2 years ago

Ah ha, seems like standard log level does not report all python validation issues. Seems like there is a formating issue in my edits, I found it in the log after setting debug level of logging for plugins and dashboard.

Now perhaps🙏 I just need to figure out exactly what it does not like and change it.

StefanCohen commented 2 years ago

It looks like you have mixed tabs and spaces in your edit. Python doesn't like that. Stick with four spaces instead of a tab and you are golden. ...And you have a pair of redundant paranthesis in there too. Fix that and it looks like it may work. My OP doesn't complain now but I can't verify that it works as I don't have a RockPI.

gpread commented 2 years ago

Yay! After fixing whitespace format issues (invisible death! lol) it works as expected/hoped.

Thanks for your assistance @StefanCohen, apologies if I triggered you to do some OP housekeeping you had been avoiding.

Attached is a modified __init__.py that now works for RockPi (presumably most RockChip CPUs/SOCs in fact) RockPiinit.py.txt

(I didn't change anything with parenthesis since I had duplicated syntax used for other platforms which surely works fine)

StefanCohen commented 2 years ago

Perfect. All that is left is to push a PR to the repo and you can put "Open Source Contributor" in your CV 😃

gpread commented 2 years ago

Perfect. All that is left is to push a PR to the repo and you can put "Open Source Contributor" in your CV 😃

I have created a fork, modified it and created a PR. I also tried to link it to this issue.

Since I am a noob to most of this, I have little idea if I did it successfully (meaning it will work easily for the maintainers) or if I did it the easiest/cleanest way. Therefore I am wide open to constructive feedback 😁

gpread commented 2 years ago

Just tested 1.19.7, I can confirm it seems to be all good. My CPU temp is now reporting correctly.

Thanks for the help and your good work @j7126 and @StefanCohen