Fan point temperature no longer require a prefix ('['); fixed fahrenh…

…eit temperature support regression; narrowed point search space; changed in-config documentation; fixed suggestions not being outputted; exit failure when launching a competing instance; refactoring; CMakeLists fix; updated README install instructions
hbriese · Apr 14, 2017 · a67f56b · a67f56b
1 parent b1440c9
commit a67f56b
Show file tree

Hide file tree

Showing 11 changed files with 243 additions and 233 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -94,7 +94,7 @@ target_link_libraries(${PROJECT_NAME} ${LIBS})
 
 ## Run lint if debug build
 option(LINT "Run linter with debug build" OFF)
-if (${CMAKE_BUILD_TYPE} STREQUAL "Debug" AND LINT)
+if (CMAKE_BUILD_TYPE STREQUAL "Debug" AND LINT)
     set(LINT_CXX_FLAGS "${CMAKE_CXX_FLAGS_DEBUG} -std=c++${CMAKE_CXX_STANDARD}")
     set(LINT_CHECKS "*,-clang-diagnostic-unused-command-line-argument,\
     -*readability-braces-around-statements,-google-runtime-int")

diff --git a/README.md b/README.md
@@ -18,31 +18,30 @@ Low overhead and easy, meaningful configuration are the main goals of fancon, th
 
 ### Installation
 
+Download latest appropriate release from github.com/hbriese/fancon/releases
+
 ###### Debian/Ubuntu
 
 ```sh
-$ wget https://github.com/hbriese/fancon/releases/download/0.10.1/fancon_0.10.1_amd64.deb
-$ sudo dpkg -i ./fancon_0.10.1_amd64.deb
+$ sudo dpkg -i ./fancon*.deb
 ```
 
 ###### Fedora
 ```sh
-$ wget https://github.com/hbriese/fancon/releases/download/0.10.1/fancon-0.10.1-2.x86_64.rpm
-$ sudo yum –nogpgcheck install ./fancon-0.10.1-2.x86_64.rpm
+$ sudo yum –nogpgcheck install ./fancon*.rpm
 ```
 
 ##### Build from source:
 Tested with both gcc & clang
 
 ```sh
-$ sudo apt-get install gcc cmake libgcc-6-dev libc6-dev linux-libc-dev libc++-helpers lm-sensors libsensors4-dev libboost-system-dev libboost-filesystem-dev libboost-log-dev libpthread-stubs0-dev libpstreams-dev libsm-dev
-$ sudo apt-get install libxnvctrl-dev libx11-dev
-$ git clone https://github.com/HBriese/fancon.git && cd fancon
+$ sudo apt-get install gcc cmake libgcc-6-dev libc6-dev linux-libc-dev libc++-helpers lm-sensors libsensors4-dev libboost-system-dev libboost-filesystem-dev libboost-log-dev libpthread-stubs0-dev libpstreams-dev
+$ git clone https://github.com/hbriese/fancon.git && cd fancon
 $ mkdir build; cd build && cmake -DCMAKE_BUILD_TYPE=Release .. && make -j && sudo make install
 ```
 
 | CMake Option     | Default | Description                                                                                 |
-|------------------|---------|---------------------------------------------------------------------------------------------|
+|:-----------------|:-------:| :-------------------------------------------------------------------------------------------|
 | NVIDIA_SUPPORT   | ON      | Support for NVIDIA GPUs                                                                     |
 | STATIC_LIBSTDC++ | OFF     | Statically link libstdc++ - useful for binary distribution                                  |
 | OPTIMIZE_DEBUG   | OFF     | Enable compiler optimizations on debug build                                                |

diff --git a/TODO.md b/TODO.md
@@ -4,7 +4,7 @@
 
 ### Bugs
 
-- Get xauth file & xdisplay from files in /etc/fancon.d/
+- Fan PWM writes fail once after reload
 
 ### Features
 
@@ -17,8 +17,8 @@
 - Remove fan when control has been lost (maybe??)
 - Add 'precise' option for more accurate RPM control
 - Auto-config option - copy current controller configuration
-- Remove '[' and ']' requirement from fan point
 - Check for NVIDIA hardware, and recommend libs for nvidia support
+- Update interval as a float
 
 ### Performance 
 
@@ -36,5 +36,5 @@
 
 ## Watch
 
-- NVML API - several functions missing before it can be used to replace XNVCtrl
-- CMake 3.8 release
+- NVML: docs.nvidia.com/deploy/nvml-api/change-log.html#change-log for XNVCtrl functionality
+- C++17: GCC 7 & Clang 4
diff --git a/src/Config.cpp b/src/Config.cpp
@@ -4,99 +4,96 @@ using fancon::InputValue;
 namespace controller = fancon::controller;
 namespace fan = fancon::fan;
 
-InputValue::InputValue(string &input, const string &sep, std::function<bool(const char &ch)> predicate) {
-  beg = search(input.begin(), input.end(), sep.begin(), sep.end());
-  if (beg != input.end())
-    beg = next(beg, sep.size());
+// TODO: review beg != end
+InputValue::InputValue(string &input, string::iterator &&begin, std::function<bool(const char &ch)> predicate)
+    : beg(begin), end(std::find_if_not(beg, input.end(), predicate)), found(beg != input.end() && beg != end) {}
 
-  finishConstruction(input, move(predicate));
-}
+InputValue::InputValue(string &input, const string &sep, std::function<bool(const char &ch)> predicate)
+    : InputValue(input, afterSeperator(input.begin(), input.end(), sep), move(predicate)) {}
+
+InputValue::InputValue(string &input, const char &sep, std::function<bool(const char &ch)> predicate)
+    : InputValue(input, afterSeperator(input.begin(), input.end(), sep), move(predicate)) {}
 
-InputValue::InputValue(string &input, const char &sep, std::function<bool(const char &ch)> predicate) {
-  beg = find(input.begin(), input.end(), sep);
-  if (beg != input.end())
-    ++beg;
+string::iterator
+InputValue::afterSeperator(const string::iterator &&beg, const string::iterator &&end, const char &sep) {
+  auto ret = find(beg, end, sep);
+  if (ret != end)
+    ++ret;
 
-  finishConstruction(input, move(predicate));
+  return ret;
 }
 
-void InputValue::finishConstruction(string &input, std::function<bool(const char &ch)> predicate) {
-  end = find_if(beg, input.end(), predicate);
-  found = (beg != input.end() && beg != end);
+string::iterator
+InputValue::afterSeperator(const string::iterator &&beg, const string::iterator &&end, const string &sep) {
+  auto ret = search(beg, end, sep.begin(), sep.end());
+  if (ret != end)
+    std::advance(ret, sep.size());
+
+  return ret;
 }
 
 ostream &controller::operator<<(ostream &os, const controller::Config &c) {
   using namespace fancon::serialization_constants::controller_config;
-  os << update_prefix << c.update_interval.count() << ' ' << threads_prefix << c.max_threads
-     << ' ' << dynamic_prefix << ((c.dynamic) ? "true" : "false");
+  os << interval_prefix << c.update_interval.count() << ' '
+     << threads_prefix << c.max_threads << ' '
+     << dynamic_prefix << ((c.dynamic) ? "true" : "false");
+
   return os;
 }
 
 istream &controller::operator>>(istream &is, controller::Config &c) {
-  // Read whole line from istream
-  std::istreambuf_iterator<char> eos;
-  string in(std::istreambuf_iterator<char>(is), eos);
+  string in;
+  std::getline(is, in);
 
   using namespace fancon::serialization_constants::controller_config;
-  InputValue dynamicVal(in, dynamic_prefix, [](const char &ch) { return !std::isalpha(ch); });
-  InputValue updateVal(in, update_prefix, [](const char &ch) { return !std::isdigit(ch); });
-  InputValue threadsVal(in, threads_prefix, [](const char &ch) { return !std::isdigit(ch); });
-  InputValue updateValDeprecated(in, update_prefix_deprecated, [](const char &ch) { return !std::isdigit(ch); });
+  InputValue dynamic(in, dynamic_prefix, ::isalpha);
+  InputValue interval(in, interval_prefix, ::isdigit);
+  InputValue threads(in, threads_prefix, ::isdigit);
+  InputValue update(in, update_prefix_deprecated, ::isdigit);  /// <\deprecated Use interval
 
   // Fail if no values are found
-  if (!dynamicVal.found && !updateVal.found && !threadsVal.found) {
+  if (!dynamic.found && !interval.found && !threads.found && !update.found) {
     // Set invalid values - see valid()
     c.update_interval = seconds(0);
     c.max_threads = 0;
     return is;
   }
 
-  if (dynamicVal.found) {
+  if (dynamic.found) {
     // Convert string to bool
     string dynamicStr;
-    dynamicVal.setIfValid(dynamicStr);
+    dynamic.setIfValid(dynamicStr);
     c.dynamic = (dynamicStr != "false" || dynamicStr != "0");
   }
 
-  if (updateVal.found || updateValDeprecated.found) {
+  if (interval.found || update.found) {
     // chrono::duration doesn't define operator>>
     decltype(c.update_interval.count()) update_interval{0};
 
-    if (updateVal.found)
-      updateVal.setIfValid(update_interval);
+    if (interval.found)
+      interval.setIfValid(update_interval);
     else {
-      updateValDeprecated.setIfValid(update_interval);
+      update.setIfValid(update_interval);
       LOG(llvl::warning) << update_prefix_deprecated << " in " << Util::config_path
-                         << " is deprecated, and WILL BE REMOVED. Use " << update_prefix;
+                         << " is deprecated, and WILL BE REMOVED. Use " << interval_prefix;
     }
 
     if (update_interval > 0)
       c.update_interval = seconds(update_interval);
   }
 
-  if (threadsVal.found)
-    threadsVal.setIfValid(c.max_threads);
+  if (threads.found)
+    threads.setIfValid(c.max_threads);
 
   return is;
 }
 
-fan::Point &fan::Point::operator=(const fan::Point &other) {
-  temp = other.temp;
-  rpm = other.rpm;
-  pwm = other.pwm;
-
-  return *this;
-}
-
 ostream &fan::operator<<(ostream &os, const fan::Point &p) {
   using namespace fancon::serialization_constants::point;
-  string rpmOut, pwmOut;
-  if (p.validRPM())
-    rpmOut += rpm_separator + to_string(p.rpm);
-  if (p.validPWM())
-    pwmOut += pwm_separator + to_string(p.pwm);
+  os << p.temp
+     << (p.validRPM() ? string() + rpm_separator + to_string(p.rpm) : "")
+     << (p.validPWM() ? string() + pwm_separator + to_string(p.pwm) : "");
 
-  os << temp_separator << p.temp << rpmOut << pwmOut << end_separator;
   return os;
 }
 
@@ -106,28 +103,33 @@ istream &fan::operator>>(istream &is, fan::Point &p) {
   is >> std::skipws >> in;
   if (in.empty())
     return is;
-//  std::remove_if(in.begin(), in.end(), [](auto &c) { return isspace(c); });
 
-  auto notDigit = [](const char &c) { return !std::isdigit(c); };
-  InputValue tempVal(in, temp_separator, notDigit);
-  InputValue pwmVal(in, pwm_separator, notDigit);
-  InputValue rpmVal(in, rpm_separator, notDigit);
+  InputValue pwm(in, pwm_separator, ::isdigit);
+  InputValue rpm(in, rpm_separator, ::isdigit);
+
+  // Temp must be before (first of) PWM & RPM
+  auto &&tempEnd = std::prev((rpm.found && rpm.beg < pwm.beg) ? rpm.beg : pwm.beg);
+  InputValue temp(in, find_if(in.begin(), tempEnd, ::isdigit), ::isdigit);
 
   // Must contain temp, and either a rpm or pwm value
-  if (!tempVal.found || (!rpmVal.found & !pwmVal.found)) {
+  if (!temp.found || (!rpm.found & !pwm.found)) {
     LOG(llvl::error) << "Invalid fan config: " << in;
     return is;
   }
 
-  if (tempVal.found)
-    tempVal.setIfValid(p.temp);
+  // Set values if they are found & valid
+  if (temp.found) {
+    temp.setIfValid(p.temp);
+    if (temp.end != in.end() && std::tolower(*temp.end) == fahrenheit)
+      p.temp = static_cast<temp_t>((p.temp - 32) / 1.8);
+  }
 
-  if (pwmVal.found)
-    pwmVal.setIfValid(p.pwm);
-  else if (rpmVal.found) {
-    rpmVal.setIfValid(p.rpm);
-    if (rpmVal.end != in.end())
-      p.is_rpm_percent = (std::tolower(*rpmVal.end) == percent);
+  if (pwm.found)
+    pwm.setIfValid(p.pwm);
+  else if (rpm.found) {
+    rpm.setIfValid(p.rpm);
+    if (rpm.end != in.end())
+      p.is_rpm_percent = (std::tolower(*rpm.end) == percent);
   }
 
   return is;

diff --git a/src/Config.hpp b/src/Config.hpp
@@ -29,11 +29,12 @@ using temp_t = int;
 
 class InputValue {
 public:
+  InputValue(string &input, string::iterator &&begin, std::function<bool(const char &ch)> predicate);
   InputValue(string &input, const string &sep, std::function<bool(const char &ch)> predicate);
   InputValue(string &input, const char &sep, std::function<bool(const char &ch)> predicate);
 
-  string::iterator beg, end;
-  bool found;
+  const string::iterator beg, end;
+  const bool found;
 
   template<typename T>
   void setIfValid(T &value) {
@@ -48,7 +49,8 @@ class InputValue {
   }
 
 private:
-  void finishConstruction(string &input, std::function<bool(const char &ch)> predicate);
+  string::iterator afterSeperator(const string::iterator &&beg, const string::iterator &&end, const char &sep);
+  string::iterator afterSeperator(const string::iterator &&beg, const string::iterator &&end, const string &sep);
 };
 
 namespace controller {
@@ -96,8 +98,6 @@ class Point {
   Point(temp_t temp = 0, rpm_t rpm = (rpm_min_abs - 1), pwm_t pwm = (pwm_min_abs - 1), bool isRpmPercent = false)
       : temp(temp), rpm(rpm), pwm(pwm), is_rpm_percent(isRpmPercent) {}
 
-  Point &operator=(const Point &other);
-
   temp_t temp;
   rpm_t rpm;    // TODO: C++17: replace with std::variant (tagged union)
   pwm_t pwm;
@@ -137,17 +137,15 @@ namespace serialization_constants {   // TODO: Review name
 namespace controller_config {
 const string
     dynamic_prefix = "dynamic=",
-    update_prefix = "interval=",
+    interval_prefix = "interval=",
     threads_prefix = "threads=";
-const string update_prefix_deprecated = "update=";  /// <\deprecated Use update_prefix  // TODO: remove 08/17
+const string update_prefix_deprecated = "update=";  /// <\deprecated Use interval_prefix  // TODO: remove 08/17
 }
 
 namespace point {
 constexpr const char
-    temp_separator = '[',    // TODO: Rename, and refactor
     rpm_separator = ':',
     pwm_separator = ';',
-    end_separator = ']',
     fahrenheit = 'f',
     percent = '%';
 }