search

valpackett / systemstat

Rust library for getting system information | also on https://codeberg.org/valpackett/systemstat
https://crates.io/crates/systemstat
The Unlicense
616 stars 71 forks source link

Improve CPU temp discovery via hwmon #119

Closed dangreco closed 9 months ago

dangreco commented 1 year ago

This PR improves CPU temperature detection on Linux. Previously, it relied on either thermal_zone0 or hwmon0 for CPU temperature. However this yields inconsistent results across different machines.

Instead, we now look for hwmonX such that /sys/class/hwmon/hwmonX/name contains coretemp, indicating that it this is the for the CPU itself. Our readings are subsequently based off of /sys/class/hwmon/hwmonX/temp1_input, which seems to be the convention for the package temp of the entire CPU.

valpackett commented 1 year ago

Makes sense. Is it always coretemp on any CPU, i.e. core not meaning Intel Core?

dangreco commented 1 year ago

@valpackett You're right re: Intel-specific --- should have read the documentation first ;)

That being said, it looks like the name file in sysfs is different per-CPU thermal driver implementation. Not sure what the best way to handle this is -- will look into a better solution tomorrow

Edit: I took a look at Freon, a popular Gnome Shell extension that displays CPU temperature (among other sensors). It looks like it just takes the average of all of the thermal sensors by default.

It doesn't look like there's a standard way of grabbing CPU temp that would work cross-machine setup. Perhaps it might be a good idea to expose hwmon devices through a new API in systemstat? We could then include other sensor data supported through hwmon (fan speed, current, etc.). If this is the direction you'd like to go, I can get a PR rolled out for that.

valpackett commented 1 year ago

It looks like it just takes the average of all of the thermal sensors by default

Hmmmm. That definitely seems like the best possible thing to do in a simple API, but would that result in a weird average of Tctl and Tdie (one of which is just a stupid offset) on Ryzen SoCs that have that stupid offset?

Perhaps it might be a good idea to expose hwmon devices through a new API in systemstat?

Well, the whole point of this crate is to expose identical API across platforms… The only platform-specific thing we expose is PlatformMemory. But that's a very simple case with just one flat struct. I think creating something similar for something as complicated as hardware sensors would be way too much.