Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No temperatures displayed #130

Closed
alicektx opened this issue May 18, 2020 · 7 comments
Closed

No temperatures displayed #130

alicektx opened this issue May 18, 2020 · 7 comments
Assignees
Labels
area:gopsutil Issues associated with the gopsutil library bug Something isn't working
Milestone

Comments

@alicektx
Copy link

$ git rev-parse HEAD
b47acdc

$ ./gotop --list devices
Temperatures:

./gotop --test
PASS

~/.local/state/gotop/errors.log -> doesn't exist

Hi. I had this issue with gotop-3.0, and it appears it's still here sadly
(looks like it still hasn't been fixed in gopsutil) :-/

Looking under /sys/class/hwmon, i see...
/sys/class/hwmon/hwmon0/name -> ADP0
/sys/class/hwmon/hwmon1/name -> BAT0
/sys/class/hwmon/hwmon2/name -> amdgpu
/sys/class/hwmon/hwmon3/name -> coretemp
/sys/class/hwmon/hwmon4/name -> pch_skylake

With v3.0, i was able to fix such using a patched gopsutil from cjbassi.
Please see comment & accompanying patch back then here.
v3 0_with_cjbassi_patched_gopsutil

Since that patch doesn't work for newer gopsutil / gotop, if possible,
I'd still like to see that worked around somehow at some point
(well, i don't wanna stay on older v3.0 for...eternity) :-)

Anything else, please ask me to test...
All the best

@xxxserxxx xxxserxxx self-assigned this May 18, 2020
@xxxserxxx xxxserxxx added the bug Something isn't working label May 18, 2020
@xxxserxxx
Copy link
Owner

Thanks for the report, and especially the link.

@xxxserxxx xxxserxxx added the area:gopsutil Issues associated with the gopsutil library label May 18, 2020
@xxxserxxx
Copy link
Owner

xxxserxxx commented May 18, 2020

@alicektx , the version you're running is using the newest gopsutil, and also contains the changes referenced in the comment you linked. We may have to do some investigation. I have to make a couple of notes here, because when I had to fork the issues it confused the ticket histories and makes looking back in time harder :-)

  1. Every comment up to Feb 14 was from the original repo (cjbassi) and the authors are inaccurate do to how the issue cloning had to be done.
  2. The issue referenced had to do with gopsutil returning errors and gotop (v3.0) breaking when it got the first error. The "fix" for that was for gotop to continue on errors and report whatever information it could. That logic appears to have gotten lost (mislaid?) in the restructuring.
  3. master is using a current minor revision of gopsutil, and that code has changed enough that the source code references in the original ticket aren't useful any more.

@alicektx , you mentioned you patched v3.0. Can you link that patch? Also, I'm not certain this is the same issue; errors returned by gopsutil are dumped out to the error log, and you say your error log doesn't exist. Did you get the error log location from gotop --list paths?

@alicektx
Copy link
Author

alicektx commented May 19, 2020

Thank you for your quick reply xxxserxxx!
It's been more than a year since, but i remember manually replacing host_linux.go & common.go from here.
I also remember clearly that some versions of gopsutil
(depending on the...stars for all i know)...
would either not return cpu numbers correctly, or temps at all, grrr.
But as you say however, it's likely not the same issue after all this time.

Indeed the log exists (--list paths, doh - how did i miss this?),
and it was under .cache/gotop/errors.log instead:

08:39:14 temp.go:18: error updating temp for psHost: Number of warnings: 1
08:39:19 temp.go:18: error updating temp for psHost: Number of warnings: 1
08:39:24 temp.go:18: error updating temp for psHost: Number of warnings: 1

Thanks in advance

@xxxserxxx xxxserxxx modified the milestones: v4.1.0, v4.0.2 Jun 11, 2020
@xxxserxxx
Copy link
Owner

@alicektx The command line arguments have been evolving as I try to clean up and provide more clarity. I'm sure some of them are flying under a lot of people's radars!

Can you tell me more about your system? You're on some *nix variant -- Linux, or a BSD? Can you tell me what uname -a shows?

Finally, would you mind downloading one of the v4.0.1 binaries for your system and seeing if it does the same thing? I don't think it will, but it's worth a shot.

@xxxserxxx xxxserxxx modified the milestones: v4.0.2, v4.1.1 Aug 25, 2020
@alicektx
Copy link
Author

alicektx commented Sep 9, 2020

Hi!

Sorry for being late in my reply - i really didn't wanted to further bother you above,
well, at least not until i could first try my best in finding a workaround on my own...

Found some spare time today & decided to have an extra shot at it,
by examining / trying to re-apply cjbassi's gopsutil modifications from early 2019...
I really can't believe gopsutil has been in that state for so much time...
digging in the repo, it seems to have various issues with linux temp detection :-/

Anyway - indeed, by copy paste trial & error...eventually, i did got it working 🚀 🌠
No probs or warnings that i see in gotop's errors.log as well.

$ gotop --list devices
 Temperatures:
 	amdgpu_edge_crit
 	amdgpu_edge_crithyst
 	coretemp_packageid0_crit
 	coretemp_packageid0_critalarm
 	coretemp_packageid0
 	coretemp_packageid0_max
 	coretemp_core0_crit
 	coretemp_core0_critalarm
 	coretemp_core0
 	coretemp_core0_max
 	coretemp_core1_crit
 	coretemp_core1_critalarm
 	coretemp_core1
 	coretemp_core1_max
 	pch_skylake

I've added in the attachment below,
the 2 slightly modified files that were needed here,
for the temps to be properly recognized.
Now if only i actually understood what they do exactly, heh...
i'd happily further ask / make a pull request to the gopsutil guys themselves :-)

All the best
gopsutil-tweaked.zip

@xxxserxxx
Copy link
Owner

Hi @alicektx -- thank you! I'll get this merged in ASAP. We moved house a few weeks back and it'll probably be a couple more weeks before we finish with the move and I can get back to the project, but I'll look at your changes and get it merged ASAP. Thanks again, and sorry for the delayed response.

@xxxserxxx xxxserxxx modified the milestones: v4.1.1, v4.1.0 Sep 15, 2020
@xxxserxxx
Copy link
Owner

@alicektx -- I think what's happening is that gopsutil is getting errors from your temperature subsystem, and gotop is seeing the errors which causes it to skip the temps. Your code essentially ignores the errors: it creates an errors variable which it doesn't assign any value to, then later checks to see if it's empty -- which it always is. This causes gotop to think there are no errors, so it displays what it can get.

gotop should be doing this anyway: any errors should be reported but ignored, because gopsutils can still return useful data even if the subsystem returns errors. This is a change to gotop. I've put it in the queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gopsutil Issues associated with the gopsutil library bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants