- CLI option to monitor current fan speeds

- Fixed multi fan testing
- Forced testing option recommendation
- Fixed tests always reporting 0% completion before the real completion %, when monitoring an on-going test
- Better status text alignment
- Performance improvements by using from_chars rather than stou

Signed-off-by: Hayden Briese <[email protected]>

README.md

@@ -14,6 +14,7 @@ A Linux user-space fan control daemon
 - [Usage](#usage)
 - [Managing service](#managing-service)
 - [Debugging issues](#debugging-issues)
+    - [Monitoring fans and their status](#monitoring-fans-and-their-status)
     - [Logs](#logs)
     - [Undetected fans](#undetected-fans)
     - [Mis-configured or unsupported devices](#mis-configured-or-unsupported-devices)
@@ -116,14 +117,17 @@ e  enable  [fan]  Enable control of the fan
 d  disable        Disable control of all fans
 d  disable [fan]  Disable control of the fans
 t  test           Test all (untested) fans
-t  test    [fan]  Test the fan if untested
+t  test    [fan]  Test the fan (forced)
 f  force          Test even already tested fans (default: false)
+m  monitor        Monitor all fans
+m  monitor [fan]  Monitor the fan
 r  reload         Reload config
 c  config  [file] Config path (default: /etc/fancon.conf)
    service        Start as service
    daemon         Daemonize the process (default: false)
    stop-service   Stop the service
 i  sysinfo [file] Save system info to file (default: sysinfo.txt)
+   recover        Recover control of enabled devices
    nv-init        Init nvidia devices
 v  verbose        Debug logging level
 a  trace          Trace logging level
@@ -142,6 +146,9 @@ a  trace          Trace logging level
 
 
 ### Debugging issues
+#### Monitoring fans and their status
+```fancon monitor```
+
 #### Logs
 ```journalctl -u fancon```
 

debian/changelog

@@ -1,4 +1,4 @@
-fancon (0.23.4) focal; urgency=low
+fancon (0.23.5) focal; urgency=low
 
   * Initial release. Closes: #00000
 

src/Args.cpp

@@ -1,13 +1,23 @@
 #include "Args.hpp"
 
-fc::Arg::Arg(string name, string short_name, bool has_value, bool needs_value,
-             string value, bool triggered)
+fc::Arg::Arg(string name, string short_name, bool potential_value,
+             bool needs_value, string value, bool triggered)
     : key(move(name)), short_key(move(short_name)), value(move(value)),
-      triggered(triggered), potential_value(has_value),
+      triggered(triggered), potential_value(potential_value),
       needs_value(needs_value) {}
 
 bool fc::Arg::has_value() const { return !value.empty(); }
 
 fc::Arg::operator bool() const { return triggered; }
 
+map<string, string> fc::Args::short_to_key() const {
+  map<string, string> short_to_key;
+  for (const auto &[key, a] : from_key) {
+    if (!a.short_key.empty())
+      short_to_key.insert_or_assign(a.short_key, a.key);
+  }
+
+  return short_to_key;
+}
+
 pair<string, fc::Arg &> fc::Args::a(Arg &arg) { return {arg.key, arg}; }

src/Args.hpp

@@ -15,7 +15,7 @@ static const char *DEFAULT_CONF_PATH(FANCON_SYSCONFDIR "/fancon.conf"),
 
 class Arg {
 public:
-  Arg(string name, string short_name = "", bool has_value = false,
+  Arg(string name, string short_name = "", bool potential_value = false,
       bool needs_value = false, string value = "", bool triggered = false);
 
   string key, short_key, value;
@@ -31,7 +31,7 @@ class Args {
       enable = {"enable", "e", true, false},
       disable = {"disable", "d", true, false},
       test = {"test", "t", true, false}, force = {"force", "f"},
-      reload = {"reload", "r"},
+      monitor = {"monitor", "m", true, false}, reload = {"reload", "r"},
       config = {"config", "c", true, true, DEFAULT_CONF_PATH, true},
       service = {"service"}, daemon = {"daemon"},
       stop_service = {"stop-service"},
@@ -40,10 +40,12 @@ class Args {
       trace = {"trace", "a"};
 
   map<string, Arg &> from_key = {
-      a(help),    a(status),  a(enable),       a(disable),
-      a(test),    a(force),   a(reload),       a(config),
-      a(service), a(daemon),  a(stop_service), a(sysinfo),
-      a(recover), a(nv_init), a(verbose),      a(trace)};
+      a(help),    a(status),       a(enable),  a(disable), a(test),
+      a(force),   a(monitor),      a(reload),  a(config),  a(service),
+      a(daemon),  a(stop_service), a(sysinfo), a(recover), a(nv_init),
+      a(verbose), a(trace)};
+
+  map<string, string> short_to_key() const;
 
 private:
   static pair<string, Arg &> a(Arg &arg);

src/Client.cpp

@@ -35,6 +35,8 @@ void fc::Client::run(Args &args) {
       test(args.test.value, true);
     else
       test(args.force);
+  } else if (args.monitor) {
+    monitor(args.monitor.value);
   } else if (args.reload) {
     reload();
   } else if (args.stop_service) {
@@ -95,39 +97,46 @@ void fc::Client::status() {
   }
 
   // Find the status of all devices
-  size_t longest_label = 0;
+  size_t longest_label = 0, longest_status = 0;
   vector<fc_pb::FanStatus> statuses;
   for (const auto &f : devices->fan()) {
     ClientContext context;
-    fc_pb::FanStatus status;
-    if (check(client->GetFanStatus(&context, from(f.label()), &status))) {
-      if (const auto l = status.label().length(); l > longest_label)
+    fc_pb::FanStatus s;
+    if (check(client->GetFanStatus(&context, from(f.label()), &s))) {
+      if (const auto l = s.label().length(); l > longest_label)
         longest_label = l;
-      statuses.push_back(move(status));
+      if (const auto l = status_text(s.status()).length(); l > longest_status)
+        longest_status = l;
+      statuses.push_back(move(s));
     }
   }
 
   // Write out all the collected status, width adjusting the outputs
   for (const auto &s : statuses) {
     stringstream extras;
     if (s.status() != FanStatus::FanStatus_Status_DISABLED)
-      extras << " " << setw(5) << s.rpm() << "rpm " << setw(3) << s.pwm()
-             << "pwm";
+      extras << setw(4) << s.rpm() << "rpm, " << setw(3) << s.pwm() << "pwm";
 
-    cout << setw(longest_label) << s.label() << ": " << setw(8)
-         << status_text(s.status()) << extras.rdbuf() << endl;
+    cout << setw(longest_label) << s.label() << ": " << extras.rdbuf() << " "
+         << setw(longest_status) << status_text(s.status()) << endl;
   }
 }
 
+void fc::Client::enable() {
+  ClientContext context;
+  if (check(client->EnableAll(&context, empty, &empty)))
+    status();
+}
+
 void fc::Client::enable(const string &flabel) {
   ClientContext context;
   if (check(client->Enable(&context, from(flabel), &empty)))
     LOG(llvl::info) << flabel << ": enabled";
 }
 
-void fc::Client::enable() {
+void fc::Client::disable() {
   ClientContext context;
-  if (check(client->EnableAll(&context, empty, &empty)))
+  if (check(client->DisableAll(&context, empty, &empty)))
     status();
 }
 
@@ -137,38 +146,20 @@ void fc::Client::disable(const string &flabel) {
     LOG(llvl::info) << flabel << ": disabled";
 }
 
-void fc::Client::disable() {
-  ClientContext context;
-  if (check(client->DisableAll(&context, empty, &empty)))
-    status();
-}
-
 void fc::Client::test(bool forced) {
   const auto devices = get_devices();
   if (!devices || devices->fan_size() == 0) {
     LOG(llvl::info) << "No devices found";
     return;
   }
 
-  mutex write_mutex;
+  if (!forced)
+    LOG(llvl::info) << "Add 'force' option to test already tested fans";
+
   vector<thread> threads;
   for (const auto &f : devices->fan()) {
     const string &flabel = f.label();
-    threads.emplace_back([&, flabel] {
-      ClientContext context;
-      fc_pb::TestRequest req;
-      req.set_device_label(flabel);
-      req.set_forced(forced);
-
-      auto reader = client->Test(&context, req);
-      fc_pb::TestResponse resp;
-      while (reader->Read(&resp)) {
-        const lock_guard<mutex> lock(write_mutex);
-        LOG(llvl::info) << flabel << ": " << resp.status() << "%";
-      }
-      if (!check(reader->Finish()))
-        LOG(llvl::error) << flabel << ": test failed";
-    });
+    threads.emplace_back([=] { test(flabel, forced); });
   }
 
   for (auto &t : threads) {
@@ -193,6 +184,27 @@ void fc::Client::test(const string &flabel, bool forced) {
     LOG(llvl::error) << flabel << ": test failed";
 }
 
+void fc::Client::monitor(const string &flabel) {
+  status();
+
+  ClientContext context;
+  const auto reader = client->SubscribeFanStatus(&context, empty);
+  fc_pb::FanStatus r;
+  while (reader->Read(&r)) {
+    if (!flabel.empty() && flabel != r.label())
+      continue;
+
+    const string pwm_rpm =
+        (r.status() != fc_pb::FanStatus_Status_DISABLED)
+            ? to_string(r.rpm()) + "rpm, " + to_string(r.pwm()) + "pwm"
+            : "";
+    const string status = (r.status() == fc_pb::FanStatus_Status_TESTING)
+                              ? (string("(") + status_text(r.status()) + ")")
+                              : "";
+    cout << r.label() << ": " << pwm_rpm << status << endl;
+  }
+}
+
 void fc::Client::reload() {
   ClientContext context;
   if (check(client->Reload(&context, empty, &empty)))
@@ -266,6 +278,8 @@ void fc::Client::print_help(const string &conf) {
                   << "t  test    [fan]  Test the fan (forced)" << endl
                   << "f  force          Test even already tested fans "
                   << "(default: false)" << endl
+                  << "m  monitor        Monitor all fans" << endl
+                  << "m  monitor [fan]  Monitor the fan" << endl
                   << "r  reload         Reload config" << endl
                   << "c  config  [file] Config path (default: "
                   << log::fmt_green_bold << conf << log::fmt_reset << ")"

src/Client.hpp

@@ -28,12 +28,13 @@ class Client {
   optional<fc_pb::Devices> get_enumerated_devices();
 
   void status();
-  void enable(const string &flabel);
   void enable();
-  void disable(const string &flabel);
+  void enable(const string &flabel);
   void disable();
+  void disable(const string &flabel);
   void test(bool forced);
   void test(const string &flabel, bool forced);
+  void monitor(const string &flabel);
   void reload();
   void recover();
   void nv_init();

src/Controller.cpp

@@ -62,7 +62,7 @@ void fc::Controller::disable(const string &flabel, bool disable_all_dell) {
   if (devices.fans.at(flabel)->type() == DevType::DELL && disable_all_dell)
     return disable_dell_fans();
 
-  tasks.erase(it);
+  tasks.erase(flabel);
   if (const auto fit = devices.fans.find(flabel); fit != devices.fans.end())
     fit->second->disable_control();
 
@@ -128,23 +128,21 @@ void fc::Controller::test(fc::FanInterface &fan, bool forced, bool blocking,
   if (auto it = tasks.find(fan.label); it->second.is_testing()) {
     // Add test_status observers to existing test_status
     for (const auto &cb : test_status->observers)
-      it->second.test_status->register_observer(cb);
+      it->second.test_status->register_observer(cb, true);
     if (blocking)
       it->second.join();
     return;
   }
 
-  LOG(llvl::info) << fan << ": testing";
-
   // Remove any running thread before testing
-  if (const auto it = tasks.find(fan.label); it != tasks.end())
-    tasks.erase(it);
+  tasks.erase(fan.label);
 
   auto test_func = [&] {
     // Test fan, then remove thread from map
+    LOG(llvl::info) << fan << ": testing";
     fan.test(*test_status);
-    tasks.erase(fan.label);
     LOG(llvl::info) << fan << ": test complete";
+    const thread t([&] { tasks.erase(fan.label); });
 
     // Only write to file when no other fan are still testing
     if (tests_running() == 0)
@@ -153,7 +151,7 @@ void fc::Controller::test(fc::FanInterface &fan, bool forced, bool blocking,
     enable(fan);
   };
 
-  auto [it, success] =
+  const auto [it, success] =
       tasks.try_emplace(fan.label, move(test_func), test_status);
 
   if (!success)