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"

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]

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:

landscape/lib/tests/test_sysstats.py

@@ -1,7 +1,7 @@
 import os
 import re
-import unittest
 from datetime import datetime
+from unittest import TestCase, mock
 
 from landscape.lib import testing
 from landscape.lib.sysstats import BootTimes
@@ -44,7 +44,7 @@
 class BaseTestCase(
     testing.TwistedTestCase,
     testing.FSTestCase,
-    unittest.TestCase,
+    TestCase,
 ):
     pass
 
@@ -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")
@@ -161,6 +162,14 @@ def write_thermal_zone(self, name, temperature):
 
 
 class GetProcfsThermalZonesTest(ProcfsThermalZoneTest):
+    @mock.patch("glob.glob")
+    @mock.patch("os.path.isdir")
+    def test_default_thermal_zone_path(self, isdir_mock, glob_mock):
+        isdir_mock.side_effect = (lambda dir: dir == "/proc/acpi/thermal_zone")
+        thermal_zones = list(get_thermal_zones(None))
+        self.assertEqual(thermal_zones, [])
+        glob_mock.assert_called_with("/proc/acpi/thermal_zone/*/temperature")
+
     def test_non_existent_thermal_zone_directory(self):
         thermal_zones = list(get_thermal_zones("/non-existent/thermal_zone"))
         self.assertEqual(thermal_zones, [])
@@ -179,7 +188,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 +221,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,24 +232,33 @@ 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")
         file.write(temperature)
         file.close()
 
 
-class GetSysfsThermalZonesTest(ThermalZoneTest):
+class GetSysfsThermalZonesTest(SysfsThermalZoneTest):
+    @mock.patch("glob.glob")
+    @mock.patch("os.path.isdir")
+    def test_default_thermal_zone_path(self, isdir_mock, glob_mock):
+        isdir_mock.side_effect = (lambda dir: dir == "/sys/class/thermal")
+        thermal_zones = list(get_thermal_zones(None))
+        self.assertEqual(thermal_zones, [])
+        glob_mock.assert_called_with("/sys/class/thermal/*/temp")
+
     def test_non_existent_thermal_zone_directory(self):
         thermal_zones = list(get_thermal_zones("/non-existent/thermal_zone"))
         self.assertEqual(thermal_zones, [])
@@ -262,7 +277,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 +303,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 +316,104 @@ 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):
+    @mock.patch("glob.glob")
+    @mock.patch("os.path.isdir")
+    def test_default_thermal_zone_path(self, isdir_mock, glob_mock):
+        isdir_mock.side_effect = (lambda dir: dir == "/sys/class/hwmon")
+        thermal_zones = list(get_thermal_zones(None))
+        self.assertEqual(thermal_zones, [])
+        glob_mock.assert_called_with("/sys/class/hwmon/*/temp*_input")
+
+    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)

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,31 +16,31 @@ 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(),
             [("Temperature", "51.0 C")],
         )
 
     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()
         self.temperature.run()
         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(),