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

Read temperature from hwmon devices #204

Merged
merged 1 commit into from Jan 30, 2024
Merged

Read temperature from hwmon devices #204

merged 1 commit into from Jan 30, 2024

Conversation

jck112
Copy link
Contributor

@jck112 jck112 commented Jan 22, 2024

On most systems hwmon provides access to the temperature information on more devices, including the CPU and GPU on most systems. These additional temperature sources yield a more accurate maximum temperature for the overall system.

With this change, the following precedence is used for reading thermal zones (depending on availability):

  1. /sys/class/hwmon/*/temp*_input
  2. /sys/class/thermal/*/temp
  3. /proc/acpi/thermal_zone/*/temperature

Copy link
Contributor

@Perfect5th Perfect5th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor inline comment. There's also a test that needs to be fixed.

landscape/lib/sysstats.py Outdated Show resolved Hide resolved
@jck112
Copy link
Contributor Author

jck112 commented Jan 30, 2024

@Perfect5th, I fixed the broken test and made the change you requested. Thanks for the review!

@Perfect5th
Copy link
Contributor

Thanks for fixing the test! Unfortunately it looks like we're lacking test coverage on a few lines. Do you mind improving that as well?

On most systems hwmon provides access to the temperature information on
more devices, including the CPU and GPU on most systems. These
additional temperature sources yield a more accurate maximum temperature
for the overall system.

With this change, the following precedence is used for reading thermal
zones (depending on availability):
1. "/sys/class/hwmon/*/temp*_input"
2. "/sys/class/thermal/*/temp"
3. "/proc/acpi/thermal_zone/*/temperature"
@jck112
Copy link
Contributor Author

jck112 commented Jan 30, 2024

@Perfect5th, There should be 100% coverage now for the change.

Copy link
Contributor

@Perfect5th Perfect5th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks much for the contribution!

@Perfect5th Perfect5th merged commit 8df12c7 into canonical:master Jan 30, 2024
5 checks passed
@jck112 jck112 deleted the temp-hwmon branch January 30, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants