Merge pull request #16097 from lrusak/sysfs-cleanup

SysfsUtils: cleanup and improve interface according to the documentation

cmake/treedata/linux/tests.txt

@@ -0,0 +1 @@
+xbmc/platform/linux/test platform/linux/test

xbmc/platform/linux/CMakeLists.txt

@@ -1,11 +1,13 @@
 set(SOURCES CPUInfoLinux.cpp
             MemUtils.cpp
             OptionalsReg.cpp
+            SysfsPath.cpp
             TimeUtils.cpp)
 
 set(HEADERS CPUInfoLinux.h
             OptionalsReg.h
             PlatformConstants.h
+            SysfsPath.h
             TimeUtils.h)
 
 if(ALSA_FOUND)

xbmc/platform/linux/CPUInfoLinux.cpp

@@ -9,9 +9,10 @@
 #include "CPUInfoLinux.h"
 
 #include "utils/StringUtils.h"
-#include "utils/SysfsUtils.h"
 #include "utils/Temperature.h"
 
+#include "platform/linux/SysfsPath.h"
+
 #include <fstream>
 #include <regex>
 #include <sstream>
@@ -68,23 +69,17 @@ std::shared_ptr<CCPUInfo> CCPUInfo::GetCPUInfo()
 
 CCPUInfoLinux::CCPUInfoLinux()
 {
-  // new socs use the sysfs soc interface to describe the hardware
-  if (SysfsUtils::Has("/sys/bus/soc/devices/soc0"))
-  {
-    std::string machine;
-    std::string family;
-    std::string socId;
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/machine"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/machine", machine);
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/family"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/family", family);
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/soc_id"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/soc_id", socId);
-    if (m_cpuHardware.empty() && !machine.empty())
-      m_cpuHardware = machine;
-    if (!family.empty() && !socId.empty())
-      m_cpuSoC = family + " " + socId;
-  }
+  CSysfsPath machinePath{"/sys/bus/soc/devices/soc0/machine"};
+  if (machinePath.Exists())
+    m_cpuHardware = machinePath.Get<std::string>();
+
+  CSysfsPath familyPath{"/sys/bus/soc/devices/soc0/family"};
+  if (familyPath.Exists())
+    m_cpuSoC = familyPath.Get<std::string>();
+
+  CSysfsPath socPath{"/sys/bus/soc/devices/soc0/soc_id"};
+  if (socPath.Exists())
+    m_cpuSoC += " " + socPath.Get<std::string>();
 
   m_cpuCount = sysconf(_SC_NPROCESSORS_ONLN);
 
@@ -284,34 +279,26 @@ int CCPUInfoLinux::GetUsedPercentage()
 
 float CCPUInfoLinux::GetCPUFrequency()
 {
-  int value{-1};
-  if (SysfsUtils::Has("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq"))
-    SysfsUtils::GetInt("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq", value);
+  float value{0};
+  CSysfsPath path("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq");
 
-  value /= 1000.0;
+  if (path.Exists())
+    value = path.Get<float>() / 1000.0;
 
   return value;
 }
 
 bool CCPUInfoLinux::GetTemperature(CTemperature& temperature)
 {
-  if (!SysfsUtils::Has("/sys/class/hwmon/hwmon0/temp1_input"))
-    return CCPUInfoPosix::GetTemperature(temperature);
+  int value{0};
+  CSysfsPath path("/sys/class/hwmon/hwmon0/temp1_input");
 
-  int value{-1};
-  char scale{'c'};
-
-  SysfsUtils::GetInt("/sys/class/hwmon/hwmon0/temp1_input", value);
-  value = value / 1000.0;
-  scale = 'c';
+  if (!path.Exists())
+    return CCPUInfoPosix::GetTemperature(temperature);
 
-  if (scale == 'C' || scale == 'c')
-    temperature = CTemperature::CreateFromCelsius(value);
-  else if (scale == 'F' || scale == 'f')
-    temperature = CTemperature::CreateFromFahrenheit(value);
-  else
-    return false;
+  value = path.Get<int>() / 1000.0;
 
+  temperature = CTemperature::CreateFromCelsius(value);
   temperature.SetValid(true);
 
   return true;

xbmc/platform/linux/SysfsPath.cpp

@@ -0,0 +1,19 @@
+/*
+ *  Copyright (C) 2011-2018 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#include "SysfsPath.h"
+
+bool CSysfsPath::Exists()
+{
+  std::ifstream file(m_path);
+
+  if (!file.is_open())
+    return false;
+
+  return true;
+}

xbmc/platform/linux/SysfsPath.h

@@ -0,0 +1,45 @@
+/*
+ *  Copyright (C) 2011-2018 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#pragma once
+
+#include "utils/log.h"
+
+#include <fstream>
+#include <string>
+
+class CSysfsPath
+{
+public:
+  CSysfsPath() = default;
+  CSysfsPath(const std::string& path) : m_path(path) {}
+  ~CSysfsPath() = default;
+
+  bool Exists();
+
+  template<typename T>
+  T Get()
+  {
+    std::ifstream file(m_path);
+
+    T value;
+
+    file >> value;
+
+    if (file.bad())
+    {
+      CLog::LogF(LOGERROR, "error reading from '{}'", m_path);
+      throw std::runtime_error("error reading from " + m_path);
+    }
+
+    return value;
+  }
+
+private:
+  std::string m_path;
+};

xbmc/platform/linux/test/CMakeLists.txt

@@ -0,0 +1,3 @@
+list(APPEND SOURCES TestSysfsPath.cpp)
+
+core_add_test_library(linux_test)

xbmc/platform/linux/test/TestSysfsPath.cpp

@@ -0,0 +1,40 @@
+/*
+ *  Copyright (C) 2005-2018 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#include "platform/linux/SysfsPath.h"
+
+#include <fstream>
+
+#include <gtest/gtest.h>
+
+struct TestSysfsPath : public ::testing::Test
+{
+  ~TestSysfsPath() { std::remove("/tmp/kodi-test"); }
+};
+
+TEST_F(TestSysfsPath, SysfsPathTest)
+{
+  int temp{1234};
+  std::ofstream output{"/tmp/kodi-test"};
+  output << temp;
+  output.close();
+
+  CSysfsPath path("/tmp/kodi-test");
+  ASSERT_TRUE(path.Exists());
+  ASSERT_TRUE(path.Get<int>() == 1234);
+  ASSERT_TRUE(path.Get<float>() == 1234);
+  ASSERT_TRUE(path.Get<double>() == 1234);
+  ASSERT_TRUE(path.Get<uint64_t>() == 1234);
+  ASSERT_TRUE(path.Get<uint16_t>() == 1234);
+  ASSERT_TRUE(path.Get<unsigned int>() == 1234);
+  ASSERT_TRUE(path.Get<unsigned long int>() == 1234);
+  ASSERT_TRUE(path.Get<std::string>() == "1234");
+
+  CSysfsPath otherPath{"/thispathdoesnotexist"};
+  ASSERT_FALSE(otherPath.Exists());
+}

xbmc/platform/win10/CPUInfoWin10.cpp

@@ -12,7 +12,6 @@
 #include "settings/AdvancedSettings.h"
 #include "settings/SettingsComponent.h"
 #include "utils/StringUtils.h"
-#include "utils/SysfsUtils.h"
 #include "utils/Temperature.h"
 
 #include <winrt/Windows.Foundation.Metadata.h>

xbmc/platform/win32/CPUInfoWin32.cpp

@@ -12,7 +12,6 @@
 #include "settings/AdvancedSettings.h"
 #include "settings/SettingsComponent.h"
 #include "utils/StringUtils.h"
-#include "utils/SysfsUtils.h"
 #include "utils/Temperature.h"
 
 #include "platform/win32/CharsetConverter.h"

xbmc/utils/CMakeLists.txt

@@ -59,7 +59,6 @@ set(SOURCES ActorProtocol.cpp
             StreamUtils.cpp
             StringUtils.cpp
             StringValidation.cpp
-            SysfsUtils.cpp
             SystemInfo.cpp
             Temperature.cpp
             TextSearch.cpp
@@ -151,7 +150,6 @@ set(HEADERS ActorProtocol.h
             StreamUtils.h
             StringUtils.h
             StringValidation.h
-            SysfsUtils.h
             SystemInfo.h
             Temperature.h
             TextSearch.h