From ad0af86a7b43397fb62b1e1bd28506c106544623 Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Sat, 31 Dec 2016 04:20:46 +0100 Subject: [PATCH] refactor(battery): Abstract value readers Refs #263, #245 --- include/components/config.hpp | 2 +- include/modules/battery.hpp | 71 +++++-- include/modules/meta/inotify_module.hpp | 11 +- include/utils/file.hpp | 3 +- src/adapters/net.cpp | 3 +- src/modules/backlight.cpp | 2 +- src/modules/battery.cpp | 268 +++++++++++------------- src/modules/temperature.cpp | 2 +- src/utils/file.cpp | 17 +- 9 files changed, 190 insertions(+), 189 deletions(-) diff --git a/include/components/config.hpp b/include/components/config.hpp index e54dc96d..d5ea951d 100644 --- a/include/components/config.hpp +++ b/include/components/config.hpp @@ -282,7 +282,7 @@ class config { string filename{move(var)}; if (file_util::exists(filename)) { - return convert(string_util::trim(file_util::get_contents(filename), '\n')); + return convert(string_util::trim(file_util::contents(filename), '\n')); } return fallback; diff --git a/include/modules/battery.hpp b/include/modules/battery.hpp index f6b473bd..5ccefc36 100644 --- a/include/modules/battery.hpp +++ b/include/modules/battery.hpp @@ -1,7 +1,6 @@ #pragma once #include "common.hpp" -#include "config.hpp" #include "modules/meta/inotify_module.hpp" POLYBAR_NS @@ -25,6 +24,25 @@ namespace modules { RATE, }; + template + struct value_reader { + using return_type = ReturnType; + + explicit value_reader() = default; + explicit value_reader(function&& fn) : m_fn(forward(fn)) {} + + ReturnType read() const { + return m_fn(); + } + + private: + const function m_fn; + }; + + using state_reader = mutex_wrapper>; + using capacity_reader = mutex_wrapper>; + using rate_reader = mutex_wrapper>; + public: explicit battery_module(const bar_settings&, string); @@ -36,40 +54,51 @@ namespace modules { bool build(builder* builder, const string& tag) const; protected: - int current_percentage(); - battery_module::state current_state(); + state current_state(); + int current_percentage(state state); string current_time(); void subthread(); private: - static constexpr auto FORMAT_CHARGING = "format-charging"; - static constexpr auto FORMAT_DISCHARGING = "format-discharging"; - static constexpr auto FORMAT_FULL = "format-full"; + static constexpr const char* FORMAT_CHARGING{"format-charging"}; + static constexpr const char* FORMAT_DISCHARGING{"format-discharging"}; + static constexpr const char* FORMAT_FULL{"format-full"}; - static constexpr auto TAG_ANIMATION_CHARGING = ""; - static constexpr auto TAG_BAR_CAPACITY = ""; - static constexpr auto TAG_RAMP_CAPACITY = ""; - static constexpr auto TAG_LABEL_CHARGING = ""; - static constexpr auto TAG_LABEL_DISCHARGING = ""; - static constexpr auto TAG_LABEL_FULL = ""; + static constexpr const char* TAG_ANIMATION_CHARGING{""}; + static constexpr const char* TAG_BAR_CAPACITY{""}; + static constexpr const char* TAG_RAMP_CAPACITY{""}; + static constexpr const char* TAG_LABEL_CHARGING{""}; + static constexpr const char* TAG_LABEL_DISCHARGING{""}; + static constexpr const char* TAG_LABEL_FULL{""}; - static const int SKIP_N_UNCHANGED{3}; + static const size_t SKIP_N_UNCHANGED{3_z}; + + unique_ptr m_state_reader; + unique_ptr m_capacity_reader; + unique_ptr m_rate_reader; - animation_t m_animation_charging; - ramp_t m_ramp_capacity; - progressbar_t m_bar_capacity; label_t m_label_charging; label_t m_label_discharging; label_t m_label_full; + animation_t m_animation_charging; + progressbar_t m_bar_capacity; + ramp_t m_ramp_capacity; + + string m_fstate; + string m_fcapnow; + string m_fcapfull; + string m_frate; + string m_fvoltage; + + state m_state{state::DISCHARGING}; + int m_percentage{0}; - battery_module::state m_state{battery_module::state::DISCHARGING}; - map m_valuepath; - std::atomic m_percentage{0}; int m_fullat{100}; + string m_timeformat; + size_t m_unchanged{SKIP_N_UNCHANGED}; chrono::duration m_interval{}; chrono::system_clock::time_point m_lastpoll; - string m_timeformat; - int m_unchanged{SKIP_N_UNCHANGED}; + thread m_subthread; }; } diff --git a/include/modules/meta/inotify_module.hpp b/include/modules/meta/inotify_module.hpp index 2c95535f..dfd3bbac 100644 --- a/include/modules/meta/inotify_module.hpp +++ b/include/modules/meta/inotify_module.hpp @@ -23,6 +23,7 @@ namespace modules { CAST_MOD(Impl)->broadcast(); while (this->running()) { + std::lock_guard guard(this->m_updatelock); CAST_MOD(Impl)->poll_events(); } } catch (const module_error& err) { @@ -57,8 +58,6 @@ namespace modules { } while (this->running()) { - std::unique_lock guard(this->m_updatelock); - for (auto&& w : watches) { this->m_log.trace_x("%s: Poll inotify watch %s", this->name(), w->path()); @@ -66,11 +65,7 @@ namespace modules { auto event = w->get_event(); for (auto&& w : watches) { - try { - w->remove(); - } catch (const system_error&) { - // ignore - } + w->remove(true); } if (CAST_MOD(Impl)->on_event(event.get())) { @@ -83,8 +78,6 @@ namespace modules { if (!this->running()) break; } - - guard.unlock(); CAST_MOD(Impl)->idle(); } } diff --git a/include/utils/file.hpp b/include/utils/file.hpp index 3b90d87b..b8a5eb3e 100644 --- a/include/utils/file.hpp +++ b/include/utils/file.hpp @@ -93,7 +93,8 @@ class fd_stream : public StreamType { namespace file_util { bool exists(const string& filename); - string get_contents(const string& filename); + string pick(const vector& filenames); + string contents(const string& filename); bool is_fifo(string filename); template diff --git a/src/adapters/net.cpp b/src/adapters/net.cpp index 33ed22d4..73af1cfa 100644 --- a/src/adapters/net.cpp +++ b/src/adapters/net.cpp @@ -182,8 +182,7 @@ namespace net { * Test if the network interface is in a valid state */ bool network::test_interface() const { - auto operstate = file_util::get_contents("/sys/class/net/" + m_interface + "/operstate"); - return operstate.compare(0, 2, "up") == 0; + return file_util::contents("/sys/class/net/" + m_interface + "/operstate").compare(0, 2, "up") == 0; } /** diff --git a/src/modules/backlight.cpp b/src/modules/backlight.cpp index 71ebc914..076a020c 100644 --- a/src/modules/backlight.cpp +++ b/src/modules/backlight.cpp @@ -20,7 +20,7 @@ namespace modules { } float backlight_module::brightness_handle::read() const { - return std::strtof(file_util::get_contents(m_path).c_str(), nullptr); + return std::strtof(file_util::contents(m_path).c_str(), nullptr); } backlight_module::backlight_module(const bar_settings& bar, string name_) diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index 641dae8e..e9257677 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -1,10 +1,8 @@ #include "modules/battery.hpp" - #include "drawtypes/animation.hpp" #include "drawtypes/label.hpp" #include "drawtypes/progressbar.hpp" #include "drawtypes/ramp.hpp" - #include "utils/file.hpp" #include "utils/math.hpp" @@ -15,60 +13,71 @@ POLYBAR_NS namespace modules { template class module; + template + typename ValueReader::return_type read(ValueReader& reader) { + std::lock_guard guard(reader); + return reader.read(); + } + /** * Bootstrap module by setting up required components */ battery_module::battery_module(const bar_settings& bar, string name_) : inotify_module(bar, move(name_)) { - auto battery = m_conf.get(name(), "battery", "BAT0"s); - auto adapter = m_conf.get(name(), "adapter", "ADP1"s); - - auto path_adapter = string_util::replace(PATH_ADAPTER, "%adapter%", adapter) + "/"; - auto path_battery = string_util::replace(PATH_BATTERY, "%battery%", battery) + "/"; - - if (!file_util::exists(path_adapter + "online")) { - throw module_error("The file '" + path_adapter + "online' does not exist"); - } - m_valuepath[battery_module::value::ADAPTER] = path_adapter + "online"; - - if (!file_util::exists(path_battery + "voltage_now")) { - throw module_error("The file '" + path_battery + "voltage_now' does not exist"); - } - m_valuepath[battery_module::value::VOLTAGE] = path_battery + "voltage_now"; - - for (auto&& file : vector{"charge", "energy"}) { - if (file_util::exists(path_battery + file + "_now")) { - m_valuepath[battery_module::value::CAPACITY] = path_battery + file + "_now"; - } - if (file_util::exists(path_battery + file + "_full")) { - m_valuepath[battery_module::value::CAPACITY_MAX] = path_battery + file + "_full"; - } - } - - if (m_valuepath[battery_module::value::CAPACITY].empty()) { - throw module_error("The file '" + path_battery + "[charge|energy]_now' does not exist"); - } - if (m_valuepath[battery_module::value::CAPACITY_MAX].empty()) { - throw module_error("The file '" + path_battery + "[charge|energy]_full' does not exist"); - } - - for (auto&& file : vector{"current", "power"}) { - if (file_util::exists(path_battery + file + "_now")) { - m_valuepath[battery_module::value::RATE] = path_battery + file + "_now"; - } - } - - if (m_valuepath[battery_module::value::RATE].empty()) { - throw module_error("The file '" + path_battery + "[current|power]_now' does not exist"); - } - - m_fullat = m_conf.get(name(), "full-at", 100); - m_interval = m_conf.get(name(), "poll-interval", 5s); + // Load configuration values + m_fullat = m_conf.get(name(), "full-at", m_fullat); + m_interval = m_conf.get(name(), "poll-interval", 5s); m_lastpoll = chrono::system_clock::now(); + auto path_adapter = string_util::replace(PATH_ADAPTER, "%adapter%", m_conf.get(name(), "adapter", "ADP1"s)) + "/"; + auto path_battery = string_util::replace(PATH_BATTERY, "%battery%", m_conf.get(name(), "battery", "BAT0"s)) + "/"; + + // Make state reader + if (file_util::exists((m_fstate = path_adapter + "online"))) { + m_state_reader = make_unique([=] { return file_util::contents(m_fstate).erase(1) == "1"; }); + } else if (file_util::exists((m_fstate = path_battery + "status"))) { + m_state_reader = make_unique([=] { return file_util::contents(m_fstate).erase(8) == "Charging"; }); + } else { + throw module_error("No suitable way to get current charge state"); + } + + // Make capacity reader + if ((m_fcapnow = file_util::pick({path_battery + "charge_now", path_battery + "energy_now"})).empty()) { + throw module_error("No suitable way to get current capacity value"); + } else if ((m_fcapfull = file_util::pick({path_battery + "charge_full", path_battery + "energy_full"})).empty()) { + throw module_error("No suitable way to get max capacity value"); + } + + m_capacity_reader = make_unique([=] { + auto cap_now = std::strtoul(file_util::contents(m_fcapnow).c_str(), nullptr, 10); + auto cap_max = std::strtoul(file_util::contents(m_fcapfull).c_str(), nullptr, 10); + return math_util::percentage(cap_now, 0UL, cap_max); + }); + + // Make rate reader + if ((m_fvoltage = file_util::pick({path_battery + "voltage_now"})).empty()) { + throw module_error("No suitable way to get current voltage value"); + } else if ((m_frate = file_util::pick({path_battery + "current_now", path_battery + "power_now"})).empty()) { + throw module_error("No suitable way to get current charge rate value"); + } + + m_rate_reader = make_unique([this] { + unsigned long rate{std::strtoul(file_util::contents(m_frate).c_str(), nullptr, 10) / 1000UL}; + unsigned long volt{std::strtoul(file_util::contents(m_fvoltage).c_str(), nullptr, 10) / 1000UL}; + unsigned long now{std::strtoul(file_util::contents(m_fcapnow).c_str(), nullptr, 10) / 1000UL}; + unsigned long max{std::strtoul(file_util::contents(m_fcapfull).c_str(), nullptr, 10) / 1000UL}; + unsigned long cap{read(*m_state_reader) ? max - now : now}; + + if (rate && volt && cap) { + return 3600UL * (cap * 1000UL / volt) / (rate * 1000UL / volt); + } else { + return 0UL; + } + }); + // Load state and capacity level - m_percentage = current_percentage(); m_state = current_state(); + m_percentage = current_percentage(m_state); // Add formats and elements m_formatter->add(FORMAT_CHARGING, TAG_LABEL_CHARGING, @@ -97,11 +106,12 @@ namespace modules { } // Create inotify watches - watch(m_valuepath[battery_module::value::CAPACITY], IN_ACCESS); - watch(m_valuepath[battery_module::value::ADAPTER], IN_ACCESS); + watch(m_fcapnow, IN_ACCESS); + watch(m_fstate, IN_ACCESS); // Setup time if token is used - if ((m_label_charging && m_label_charging->has_token("%time%")) || (m_label_discharging && m_label_discharging->has_token("%time%"))) { + if ((m_label_charging && m_label_charging->has_token("%time%")) || + (m_label_discharging && m_label_discharging->has_token("%time%"))) { if (!m_bar.locale.empty()) { setlocale(LC_TIME, m_bar.locale.c_str()); } @@ -115,14 +125,16 @@ namespace modules { */ void battery_module::start() { this->inotify_module::start(); - m_threads.emplace_back(thread(&battery_module::subthread, this)); + m_subthread = thread(&battery_module::subthread, this); } /** * Release wake lock when stopping the module */ void battery_module::teardown() { - wakeup(); + if (m_subthread.joinable()) { + m_subthread.join(); + } } /** @@ -137,11 +149,10 @@ namespace modules { void battery_module::idle() { if (m_interval.count() > 0) { auto now = chrono::system_clock::now(); - if (chrono::duration_cast(now - m_lastpoll) > m_interval) { m_lastpoll = now; m_log.info("%s: Polling values (inotify fallback)", name()); - file_util::get_contents(m_valuepath[battery_module::value::CAPACITY]); + read(*m_capacity_reader); } } @@ -152,47 +163,40 @@ namespace modules { * Update values when tracked files have changed */ bool battery_module::on_event(inotify_event* event) { - if (event != nullptr) { - m_log.trace("%s: Inotify event reported for %s", name(), event->filename); - } + auto state = current_state(); + auto percentage = current_percentage(state); // Reset timer to avoid unnecessary polling m_lastpoll = chrono::system_clock::now(); - auto state = current_state(); - int percentage = m_percentage; + if (event != nullptr) { + m_log.trace("%s: Inotify event reported for %s", name(), event->filename); - if (state != battery_module::state::FULL) { - percentage = current_percentage(); + if (state == m_state && percentage == m_percentage && m_unchanged--) { + return false; + } + + m_unchanged = SKIP_N_UNCHANGED; } - if (event != nullptr && state == m_state && percentage == m_percentage && m_unchanged--) { - return false; - } - - m_percentage = percentage; m_state = state; - m_unchanged = SKIP_N_UNCHANGED; + m_percentage = percentage; - string time_remaining; + const auto label = [this] { + if (m_state == state::FULL) { + return m_label_full; + } else if (m_state == state::DISCHARGING) { + return m_label_discharging; + } else { + return m_label_charging; + } + }(); - if (m_state == battery_module::state::CHARGING && m_label_charging) { - if (!m_timeformat.empty()) { - time_remaining = current_time(); - } - m_label_charging->reset_tokens(); - m_label_charging->replace_token("%percentage%", to_string(m_percentage) + "%"); - m_label_charging->replace_token("%time%", time_remaining); - } else if (m_state == battery_module::state::DISCHARGING && m_label_discharging) { - if (!m_timeformat.empty()) { - time_remaining = current_time(); - } - m_label_discharging->reset_tokens(); - m_label_discharging->replace_token("%percentage%", to_string(m_percentage) + "%"); - m_label_discharging->replace_token("%time%", time_remaining); - } else if (m_state == battery_module::state::FULL && m_label_full) { - m_label_full->reset_tokens(); - m_label_full->replace_token("%percentage%", to_string(m_percentage) + "%"); + label->reset_tokens(); + label->replace_token("%percentage%", to_string(m_percentage) + "%"); + + if (m_state != state::FULL && !m_timeformat.empty()) { + label->replace_token("%time%", current_time()); } return true; @@ -202,17 +206,17 @@ namespace modules { * Get the output format based on state */ string battery_module::get_format() const { - if (m_state == battery_module::state::FULL) { - return FORMAT_FULL; - } else if (m_state == battery_module::state::CHARGING) { + if (m_state == state::CHARGING) { return FORMAT_CHARGING; - } else { + } else if (m_state == state::DISCHARGING) { return FORMAT_DISCHARGING; + } else { + return FORMAT_FULL; } } /** - * Generate the module output using defined drawtypes + * Generate module output using defined drawtypes */ bool battery_module::build(builder* builder, const string& tag) const { if (tag == TAG_ANIMATION_CHARGING) { @@ -230,6 +234,7 @@ namespace modules { } else { return false; } + return true; } @@ -237,83 +242,45 @@ namespace modules { * Get the current battery state */ battery_module::state battery_module::current_state() { - auto adapter_status = file_util::get_contents(m_valuepath[battery_module::value::ADAPTER]); - - if (adapter_status.empty()) { - return battery_module::state::DISCHARGING; - } else if (adapter_status[0] == '0') { - return battery_module::state::DISCHARGING; - } else if (adapter_status[0] != '1') { - return battery_module::state::DISCHARGING; - } else if (m_percentage < m_fullat) { - return battery_module::state::CHARGING; + if (!read(*m_state_reader)) { + return state::DISCHARGING; + } else if (read(*m_capacity_reader) < m_fullat) { + return state::CHARGING; } else { - return battery_module::state::FULL; + return state::FULL; } } /** * Get the current capacity level */ - int battery_module::current_percentage() { - auto capacity_now = - std::strtoul(file_util::get_contents(m_valuepath[battery_module::value::CAPACITY]).c_str(), nullptr, 10); - auto capacity_max = - std::strtoul(file_util::get_contents(m_valuepath[battery_module::value::CAPACITY_MAX]).c_str(), nullptr, 10); - auto percentage = math_util::percentage(capacity_now, 0UL, capacity_max); - - return percentage < m_fullat ? percentage : 100; + int battery_module::current_percentage(state state) { + int percentage{read(*m_capacity_reader)}; + if (state == state::FULL && percentage >= m_fullat) { + percentage = 100; + } + return percentage; } /** * Get estimate of remaining time until fully dis-/charged */ string battery_module::current_time() { - if (m_state == battery_module::state::FULL) { - return ""; - } - - int rate{atoi(file_util::get_contents(m_valuepath[battery_module::value::RATE]).c_str()) / 1000}; - int volt{atoi(file_util::get_contents(m_valuepath[battery_module::value::VOLTAGE]).c_str()) / 1000}; - int now{atoi(file_util::get_contents(m_valuepath[battery_module::value::CAPACITY]).c_str()) / 1000}; - int max{atoi(file_util::get_contents(m_valuepath[battery_module::value::CAPACITY_MAX]).c_str()) / 1000}; - int cap{0}; - - if (m_state == battery_module::state::CHARGING) { - cap = max - now; - } else if (m_state == battery_module::state::DISCHARGING) { - cap = now; - } - struct tm t { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr }; - if (rate && volt && cap) { - cap = cap * 1000 / volt; - rate = rate * 1000 / volt; - - if (!rate) { - rate = -1; - } - - chrono::seconds sec{3600 * cap / rate}; - - m_log.trace("%s: sec=%d %d%% cap=%lu rate=%lu volt=%lu", name(), sec.count(), static_cast(m_percentage), cap, - rate, volt); - - if (sec.count() > 0) { - t.tm_hour = chrono::duration_cast(sec).count(); - sec -= chrono::seconds{3600 * t.tm_hour}; - t.tm_min = chrono::duration_cast(sec).count(); - sec -= chrono::seconds{60 * t.tm_min}; - t.tm_sec = chrono::duration_cast(sec).count(); - } + chrono::seconds sec{read(*m_rate_reader)}; + if (sec.count() > 0) { + t.tm_hour = chrono::duration_cast(sec).count(); + sec -= chrono::seconds{3600 * t.tm_hour}; + t.tm_min = chrono::duration_cast(sec).count(); + sec -= chrono::seconds{60 * t.tm_min}; + t.tm_sec = chrono::duration_cast(sec).count(); } char buffer[256]{0}; strftime(buffer, sizeof(buffer), m_timeformat.c_str(), &t); - return {buffer}; } @@ -322,18 +289,19 @@ namespace modules { * to refresh in case it is used. */ void battery_module::subthread() { - chrono::duration dur = 1s; + chrono::duration dur{0.0}; if (m_animation_charging) { - dur = chrono::duration(float(m_animation_charging->framerate()) / 1000.0f); + dur += chrono::milliseconds{m_animation_charging->framerate()}; + } else { + dur += 1s; } while (running()) { for (int i = 0; running() && i < dur.count(); ++i) { - if (m_state == battery_module::state::CHARGING) { + if (m_state == state::CHARGING) { broadcast(); } - sleep(dur); } } diff --git a/src/modules/temperature.cpp b/src/modules/temperature.cpp index 0b451b5c..db466516 100644 --- a/src/modules/temperature.cpp +++ b/src/modules/temperature.cpp @@ -39,7 +39,7 @@ namespace modules { } bool temperature_module::update() { - m_temp = std::atoi(file_util::get_contents(m_path).c_str()) / 1000.0f + 0.5f; + m_temp = std::atoi(file_util::contents(m_path).c_str()) / 1000.0f + 0.5f; m_perc = math_util::cap(math_util::percentage(m_temp, 0, m_tempwarn), 0, 100); const auto replace_tokens = [&](label_t& label) { diff --git a/src/utils/file.cpp b/src/utils/file.cpp index 0387ab62..ebf6158c 100644 --- a/src/utils/file.cpp +++ b/src/utils/file.cpp @@ -169,14 +169,25 @@ namespace file_util { return stat(filename.c_str(), &buffer) == 0; } + /** + * Picks the first existing file out of given entries + */ + string pick(const vector& filenames) { + for (auto&& f : filenames) { + if (exists(f)) { + return f; + } + } + return ""; + } + /** * Gets the contents of the given file */ - string get_contents(const string& filename) { + string contents(const string& filename) { try { std::ifstream ifs(filename); - string contents((std::istreambuf_iterator(ifs)), (std::istreambuf_iterator())); - return contents; + return string((std::istreambuf_iterator(ifs)), (std::istreambuf_iterator())); } catch (std::ios_base::failure& e) { return ""; }