From 71f5329af427db4abac66010ebccb035377dd16e Mon Sep 17 00:00:00 2001 From: Justin Klaassen <6700251+jck112@users.noreply.github.com> Date: Mon, 22 Jan 2024 12:30:07 -0800 Subject: [PATCH] Read temperature from hwmon devices 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" --- .../client/monitor/tests/test_temperature.py | 4 +- landscape/lib/sysstats.py | 62 +++++----- landscape/lib/tests/test_sysstats.py | 116 +++++++++++++++--- landscape/sysinfo/tests/test_temperature.py | 18 +-- 4 files changed, 141 insertions(+), 59 deletions(-) diff --git a/landscape/client/monitor/tests/test_temperature.py b/landscape/client/monitor/tests/test_temperature.py index 93e3b6f17..472fa4689 100644 --- a/landscape/client/monitor/tests/test_temperature.py +++ b/landscape/client/monitor/tests/test_temperature.py @@ -5,10 +5,10 @@ from landscape.client.monitor.temperature import Temperature from landscape.client.tests.helpers import LandscapeTest from landscape.client.tests.helpers import MonitorHelper -from landscape.lib.tests.test_sysstats import ThermalZoneTest +from landscape.lib.tests.test_sysstats import SysfsThermalZoneTest -class TemperatureTestWithSampleData(ThermalZoneTest, LandscapeTest): +class TemperatureTestWithSampleData(SysfsThermalZoneTest, LandscapeTest): """Tests for the temperature plugin.""" helpers = [MonitorHelper] diff --git a/landscape/lib/sysstats.py b/landscape/lib/sysstats.py index d1a270429..d0815f7b1 100644 --- a/landscape/lib/sysstats.py +++ b/landscape/lib/sysstats.py @@ -1,3 +1,4 @@ +import glob import os.path import struct import time @@ -95,13 +96,14 @@ def get_uptime(uptime_file="/proc/uptime"): def get_thermal_zones(thermal_zone_path=None): if thermal_zone_path is None: - if os.path.isdir("/sys/class/thermal"): - thermal_zone_path = "/sys/class/thermal" + if os.path.isdir("/sys/class/hwmon"): + thermal_zone_path = "/sys/class/hwmon/*/temp*_input" + elif os.path.isdir("/sys/class/thermal"): + thermal_zone_path = "/sys/class/thermal/*/temp" else: - thermal_zone_path = "/proc/acpi/thermal_zone" - if os.path.isdir(thermal_zone_path): - for zone_name in sorted(os.listdir(thermal_zone_path)): - yield ThermalZone(thermal_zone_path, zone_name) + thermal_zone_path = "/proc/acpi/thermal_zone/*/temperature" + for temperature_path in sorted(glob.glob(thermal_zone_path)): + yield ThermalZone(temperature_path) class ThermalZone: @@ -110,37 +112,29 @@ class ThermalZone: temperature_value = None temperature_unit = None - def __init__(self, base_path, name): - self.name = name - self.path = os.path.join(base_path, name) - temperature_path = os.path.join(self.path, "temp") - if os.path.isfile(temperature_path): - try: - with open(temperature_path) as f: - line = f.readline() - try: - self.temperature_value = int(line.strip()) / 1000.0 - self.temperature_unit = "C" - self.temperature = "{:.1f} {}".format( - self.temperature_value, - self.temperature_unit, - ) - except ValueError: - pass - except OSError: - pass - else: - temperature_path = os.path.join(self.path, "temperature") - if os.path.isfile(temperature_path): - for line in open(temperature_path): - if line.startswith("temperature:"): - self.temperature = line[12:].strip() - try: + def __init__(self, temperature_path): + self.path = os.path.dirname(temperature_path) + self.name = os.path.basename(self.path) + try: + with open(temperature_path) as f: + if os.path.basename(temperature_path) == "temperature": + for line in f: + if line.startswith("temperature:"): + self.temperature = line[12:].strip() value, unit = self.temperature.split() self.temperature_value = int(value) self.temperature_unit = unit - except ValueError: - pass + break + else: + line = f.readline() + self.temperature_value = int(line.strip()) / 1000.0 + self.temperature_unit = "C" + self.temperature = "{:.1f} {}".format( + self.temperature_value, + self.temperature_unit, + ) + except (ValueError, OSError): + pass class LoginInfo: diff --git a/landscape/lib/tests/test_sysstats.py b/landscape/lib/tests/test_sysstats.py index 52886ccbf..02ef8788f 100644 --- a/landscape/lib/tests/test_sysstats.py +++ b/landscape/lib/tests/test_sysstats.py @@ -146,13 +146,14 @@ def test_valid_uptime_file(self): class ProcfsThermalZoneTest(BaseTestCase): def setUp(self): super().setUp() - self.thermal_zone_path = self.makeDir() + self.base_path = self.makeDir() + self.thermal_zone_path = os.path.join(self.base_path, "*/temperature") def get_thermal_zones(self): return list(get_thermal_zones(self.thermal_zone_path)) def write_thermal_zone(self, name, temperature): - zone_path = os.path.join(self.thermal_zone_path, name) + zone_path = os.path.join(self.base_path, name) if not os.path.isdir(zone_path): os.mkdir(zone_path) file = open(os.path.join(zone_path, "temperature"), "w") @@ -179,7 +180,7 @@ def test_one_thermal_zone(self): self.assertEqual(thermal_zones[0].temperature_unit, "C") self.assertEqual( thermal_zones[0].path, - os.path.join(self.thermal_zone_path, "THM0"), + os.path.join(self.base_path, "THM0"), ) def test_two_thermal_zones(self): @@ -212,10 +213,7 @@ def test_badly_formatted_with_missing_space(self): def test_temperature_file_with_missing_label(self): self.write_thermal_zone("THM0", "SOMETHINGBAD") - temperature_path = os.path.join( - self.thermal_zone_path, - "THM0/temperature", - ) + temperature_path = os.path.join(self.base_path, "THM0/temperature") file = open(temperature_path, "w") file.write("bad-label: foo bar\n") file.close() @@ -226,16 +224,17 @@ def test_temperature_file_with_missing_label(self): self.assertEqual(thermal_zones[0].temperature_unit, None) -class ThermalZoneTest(BaseTestCase): +class SysfsThermalZoneTest(BaseTestCase): def setUp(self): super().setUp() - self.thermal_zone_path = self.makeDir() + self.base_path = self.makeDir() + self.thermal_zone_path = os.path.join(self.base_path, "*/temp") def get_thermal_zones(self): return list(get_thermal_zones(self.thermal_zone_path)) def write_thermal_zone(self, name, temperature): - zone_path = os.path.join(self.thermal_zone_path, name) + zone_path = os.path.join(self.base_path, name) if not os.path.isdir(zone_path): os.mkdir(zone_path) file = open(os.path.join(zone_path, "temp"), "w") @@ -243,7 +242,7 @@ def write_thermal_zone(self, name, temperature): file.close() -class GetSysfsThermalZonesTest(ThermalZoneTest): +class GetSysfsThermalZonesTest(SysfsThermalZoneTest): def test_non_existent_thermal_zone_directory(self): thermal_zones = list(get_thermal_zones("/non-existent/thermal_zone")) self.assertEqual(thermal_zones, []) @@ -262,7 +261,7 @@ def test_one_thermal_zone(self): self.assertEqual(thermal_zones[0].temperature_unit, "C") self.assertEqual( thermal_zones[0].path, - os.path.join(self.thermal_zone_path, "THM0"), + os.path.join(self.base_path, "THM0"), ) def test_two_thermal_zones(self): @@ -288,7 +287,7 @@ def test_non_int_temperature(self): self.assertEqual(thermal_zones[0].temperature_unit, "C") self.assertEqual( thermal_zones[0].path, - os.path.join(self.thermal_zone_path, "THM0"), + os.path.join(self.base_path, "THM0"), ) def test_badly_formatted_temperature(self): @@ -301,7 +300,96 @@ def test_badly_formatted_temperature(self): def test_read_error(self): self.write_thermal_zone("THM0", "50000") - temperature_path = os.path.join(self.thermal_zone_path, "THM0/temp") + temperature_path = os.path.join(self.base_path, "THM0/temp") + os.chmod(temperature_path, 0o200) # --w------- + thermal_zones = self.get_thermal_zones() + self.assertEqual(len(thermal_zones), 1) + self.assertEqual(thermal_zones[0].temperature, None) + self.assertEqual(thermal_zones[0].temperature_value, None) + self.assertEqual(thermal_zones[0].temperature_unit, None) + + +class HwmonThermalZoneTest(BaseTestCase): + def setUp(self): + super().setUp() + self.base_path = self.makeDir() + self.thermal_zone_path = os.path.join(self.base_path, "*/temp*_input") + + def get_thermal_zones(self): + return list(get_thermal_zones(self.thermal_zone_path)) + + def write_thermal_zone(self, name, number, temperature): + zone_path = os.path.join(self.base_path, name) + if not os.path.isdir(zone_path): + os.mkdir(zone_path) + file = open(os.path.join(zone_path, f"temp{number}_input"), "w") + file.write(temperature) + file.close() + + +class GetHwmonThermalZonesTest(HwmonThermalZoneTest): + def test_non_existent_thermal_zone_directory(self): + thermal_zones = list(get_thermal_zones("/non-existent/thermal_zone")) + self.assertEqual(thermal_zones, []) + + def test_empty_thermal_zone_directory(self): + self.assertEqual(self.get_thermal_zones(), []) + + def test_one_thermal_zone(self): + self.write_thermal_zone("THM0", "1", "50000") + thermal_zones = self.get_thermal_zones() + self.assertEqual(len(thermal_zones), 1) + + self.assertEqual(thermal_zones[0].name, "THM0") + self.assertEqual(thermal_zones[0].temperature, "50.0 C") + self.assertEqual(thermal_zones[0].temperature_value, 50.0) + self.assertEqual(thermal_zones[0].temperature_unit, "C") + self.assertEqual( + thermal_zones[0].path, + os.path.join(self.base_path, "THM0"), + ) + + def test_three_thermal_zones(self): + self.write_thermal_zone("THM0", "1", "50000") + self.write_thermal_zone("THM0", "2", "51000") + self.write_thermal_zone("THM1", "1", "52000") + thermal_zones = self.get_thermal_zones() + self.assertEqual(len(thermal_zones), 3) + self.assertEqual(thermal_zones[0].temperature, "50.0 C") + self.assertEqual(thermal_zones[0].temperature_value, 50.0) + self.assertEqual(thermal_zones[0].temperature_unit, "C") + self.assertEqual(thermal_zones[1].temperature, "51.0 C") + self.assertEqual(thermal_zones[1].temperature_value, 51.0) + self.assertEqual(thermal_zones[1].temperature_unit, "C") + self.assertEqual(thermal_zones[2].temperature, "52.0 C") + self.assertEqual(thermal_zones[2].temperature_value, 52.0) + self.assertEqual(thermal_zones[2].temperature_unit, "C") + + def test_non_int_temperature(self): + self.write_thermal_zone("THM0", "1", "50432") + thermal_zones = self.get_thermal_zones() + self.assertEqual(len(thermal_zones), 1) + + self.assertEqual(thermal_zones[0].name, "THM0") + self.assertEqual(thermal_zones[0].temperature, "50.4 C") + self.assertEqual(thermal_zones[0].temperature_value, 50.432) + self.assertEqual(thermal_zones[0].temperature_unit, "C") + self.assertEqual( + thermal_zones[0].path, + os.path.join(self.base_path, "THM0"), + ) + + def test_badly_formatted_temperature(self): + self.write_thermal_zone("THM0", "1", "SOMETHING BAD") + thermal_zones = self.get_thermal_zones() + self.assertEqual(len(thermal_zones), 1) + self.assertEqual(thermal_zones[0].temperature, None) + self.assertEqual(thermal_zones[0].temperature_value, None) + self.assertEqual(thermal_zones[0].temperature_unit, None) + + def test_read_error(self): + self.write_thermal_zone("THM0", "1", "50000") + temperature_path = os.path.join(self.base_path, "THM0/temp1_input") os.chmod(temperature_path, 0o200) # --w------- thermal_zones = self.get_thermal_zones() self.assertEqual(len(thermal_zones), 1) diff --git a/landscape/sysinfo/tests/test_temperature.py b/landscape/sysinfo/tests/test_temperature.py index b72aa2f53..97a3cdf9b 100644 --- a/landscape/sysinfo/tests/test_temperature.py +++ b/landscape/sysinfo/tests/test_temperature.py @@ -1,11 +1,11 @@ import os -from landscape.lib.tests.test_sysstats import ThermalZoneTest +from landscape.lib.tests.test_sysstats import HwmonThermalZoneTest from landscape.sysinfo.sysinfo import SysInfoPluginRegistry from landscape.sysinfo.temperature import Temperature -class TemperatureTest(ThermalZoneTest): +class TemperatureTest(HwmonThermalZoneTest): def setUp(self): super().setUp() self.temperature = Temperature(self.thermal_zone_path) @@ -16,7 +16,7 @@ def test_run_returns_succeeded_deferred(self): self.assertIs(None, self.successResultOf(self.temperature.run())) def test_run_adds_header(self): - self.write_thermal_zone("THM0", "51000") + self.write_thermal_zone("THM0", "1", "51000") self.temperature.run() self.assertEqual( self.sysinfo.get_headers(), @@ -24,8 +24,8 @@ def test_run_adds_header(self): ) def test_ignores_bad_files(self): - self.write_thermal_zone("THM0", "") - temperature_path = os.path.join(self.thermal_zone_path, "THM0/temp") + self.write_thermal_zone("THM0", "1", "") + temperature_path = os.path.join(self.base_path, "THM0/temp1_input") file = open(temperature_path, "w") file.write("bad-label: 51 C") file.close() @@ -33,14 +33,14 @@ def test_ignores_bad_files(self): self.assertEqual(self.sysinfo.get_headers(), []) def test_ignores_unknown_formats(self): - self.write_thermal_zone("THM0", "FOO") + self.write_thermal_zone("THM0", "1", "FOO") self.temperature.run() self.assertEqual(self.sysinfo.get_headers(), []) def test_picks_highest_temperature(self): - self.write_thermal_zone("THM0", "51000") - self.write_thermal_zone("THM1", "53000") - self.write_thermal_zone("THM2", "52000") + self.write_thermal_zone("THM0", "1", "51000") + self.write_thermal_zone("THM0", "2", "53000") + self.write_thermal_zone("THM1", "1", "52000") self.temperature.run() self.assertEqual( self.sysinfo.get_headers(),