From a7056f7ccec0126f3b2a1c86b09a4d6dacef8cce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Thu, 26 Nov 2020 01:02:06 +0000 Subject: [PATCH 1/7] Calculate battery state from just energy values The energy values are all that's needed to calculate the battery state. Using other values for the total capacity results in broken results in some cases. This matches the output of TLP and i3status, while also being more straightforward. --- src/modules/battery.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index beb0554f..93fcc8bd 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -80,18 +80,15 @@ void waybar::modules::Battery::getBatteries() { const std::tuple waybar::modules::Battery::getInfos() const { try { - uint16_t total = 0; uint32_t total_power = 0; // μW uint32_t total_energy = 0; // μWh uint32_t total_energy_full = 0; std::string status = "Unknown"; for (auto const& bat : batteries_) { - uint16_t capacity; uint32_t power_now; uint32_t energy_full; uint32_t energy_now; std::string _status; - std::ifstream(bat / "capacity") >> capacity; std::ifstream(bat / "status") >> _status; auto rate_path = fs::exists(bat / "current_now") ? "current_now" : "power_now"; std::ifstream(bat / rate_path) >> power_now; @@ -102,7 +99,6 @@ const std::tuple waybar::modules::Battery::getInfos if (_status != "Unknown") { status = _status; } - total += capacity; total_power += power_now; total_energy += energy_now; total_energy_full += energy_full; @@ -120,7 +116,7 @@ const std::tuple waybar::modules::Battery::getInfos } else if (status == "Charging" && total_power != 0) { time_remaining = -(float)(total_energy_full - total_energy) / total_power; } - uint16_t capacity = total / batteries_.size(); + float capacity = ((float)total_energy * 100.0f / (float) total_energy_full); // Handle full-at if (config_["full-at"].isUInt()) { auto full_at = config_["full-at"].asUInt(); From e0cdcb6e30efbf56a862b0b4dfb7baa5677bf35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Thu, 26 Nov 2020 02:16:57 +0000 Subject: [PATCH 2/7] Handle charging above 100% gracefully When calibrating a battery it's possible to go above 100%. Handle that gracefully by just presenting the battery as full and 100%. --- src/modules/battery.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index 93fcc8bd..02e05b79 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -115,6 +115,11 @@ const std::tuple waybar::modules::Battery::getInfos time_remaining = (float)total_energy / total_power; } else if (status == "Charging" && total_power != 0) { time_remaining = -(float)(total_energy_full - total_energy) / total_power; + if (time_remaining > 0.0f) { + // If we've turned positive it means the battery is past 100% and so + // just report that as no time remaining + time_remaining = 0.0f; + } } float capacity = ((float)total_energy * 100.0f / (float) total_energy_full); // Handle full-at @@ -127,6 +132,12 @@ const std::tuple waybar::modules::Battery::getInfos } } } + if (capacity > 100.f) { + // This can happen when the battery is calibrating and goes above 100% + // Handle it gracefully by clamping at 100% and presenting it as full + capacity = 100.f; + status = "Full"; + } return {capacity, time_remaining, status}; } catch (const std::exception& e) { spdlog::error("Battery: {}", e.what()); From eb3f4216d402903b18c41b38cf4ddb8f0a72ad3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Thu, 26 Nov 2020 02:20:53 +0000 Subject: [PATCH 3/7] Show battery state as rounded number Round the battery charge state so that 99.9% shows as 100%. --- src/modules/battery.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index 02e05b79..eece435a 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -138,7 +138,7 @@ const std::tuple waybar::modules::Battery::getInfos capacity = 100.f; status = "Full"; } - return {capacity, time_remaining, status}; + return {round(capacity), time_remaining, status}; } catch (const std::exception& e) { spdlog::error("Battery: {}", e.what()); return {0, 0, "Unknown"}; From f45d5829577d59eb9980a31793f2feb2f0c15eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Thu, 26 Nov 2020 02:34:36 +0000 Subject: [PATCH 4/7] Always mark battery as full at 100% Since we're now clamping at 100% and rounding, mark as full at that point. Some batteries will stay in charging state for a long time while stuck at the same charge level. Just mark them as full to not be confusing. --- src/modules/battery.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index eece435a..2d553705 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -134,11 +134,17 @@ const std::tuple waybar::modules::Battery::getInfos } if (capacity > 100.f) { // This can happen when the battery is calibrating and goes above 100% - // Handle it gracefully by clamping at 100% and presenting it as full + // Handle it gracefully by clamping at 100% capacity = 100.f; + } + uint8_t cap = round(capacity); + if (cap == 100) { + // If we've reached 100% just mark as full as some batteries can stay + // stuck reporting they're still charging but not yet done status = "Full"; } - return {round(capacity), time_remaining, status}; + + return {cap, time_remaining, status}; } catch (const std::exception& e) { spdlog::error("Battery: {}", e.what()); return {0, 0, "Unknown"}; From 908fa2c6c239af7a27c53658940f1f20bc5fa5c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Fri, 27 Nov 2020 10:55:27 +0000 Subject: [PATCH 5/7] Make the battery full-at go to 100% full-at was capped at the value instead of allowing the battery to show 100% when you were at the full-at value. Uncapping makes more sense as it makes the full-at value the new 100% and the scale goes down from there. Whereas before the battery would stay at the full-at value until it went down enough which is unrealistic. --- man/waybar-battery.5.scd | 2 +- src/modules/battery.cpp | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/man/waybar-battery.5.scd b/man/waybar-battery.5.scd index 917a03d7..c9e6e78a 100644 --- a/man/waybar-battery.5.scd +++ b/man/waybar-battery.5.scd @@ -20,7 +20,7 @@ The *battery* module displays the current capacity and state (eg. charging) of y *full-at*: ++ typeof: integer ++ - Define the max percentage of the battery, useful for an old battery, e.g. 96 + Define the max percentage of the battery, for when you've set the battery to stop charging at a lower level to save it. For example, if you've set the battery to stop at 80% that will become the new 100%. *interval*: ++ typeof: integer ++ diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index 2d553705..031c949f 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -127,9 +127,6 @@ const std::tuple waybar::modules::Battery::getInfos auto full_at = config_["full-at"].asUInt(); if (full_at < 100) { capacity = 100.f * capacity / full_at; - if (capacity > full_at) { - capacity = full_at; - } } } if (capacity > 100.f) { From 89ca155c43eea73593cacfa4f58e687851a0989c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Fri, 27 Nov 2020 13:56:04 +0000 Subject: [PATCH 6/7] Support hotplugging of batteries Refresh the list of batteries on update to handle hotplug correctly. Probably fixes #490. --- include/modules/battery.hpp | 6 ++-- src/modules/battery.cpp | 69 +++++++++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/include/modules/battery.hpp b/include/modules/battery.hpp index d4d20d1e..f0e3c39d 100644 --- a/include/modules/battery.hpp +++ b/include/modules/battery.hpp @@ -31,16 +31,16 @@ class Battery : public ALabel { private: static inline const fs::path data_dir_ = "/sys/class/power_supply/"; - void getBatteries(); + void refreshBatteries(); void worker(); const std::string getAdapterStatus(uint8_t capacity) const; const std::tuple getInfos() const; const std::string formatTimeRemaining(float hoursRemaining); - std::vector batteries_; + int global_watch; + std::map batteries_; fs::path adapter_; int fd_; - std::vector wds_; std::string old_status_; util::SleeperThread thread_; diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index 031c949f..84bc973b 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -4,23 +4,31 @@ waybar::modules::Battery::Battery(const std::string& id, const Json::Value& config) : ALabel(config, "battery", id, "{capacity}%", 60) { - getBatteries(); fd_ = inotify_init1(IN_CLOEXEC); if (fd_ == -1) { throw std::runtime_error("Unable to listen batteries."); } - for (auto const& bat : batteries_) { - auto wd = inotify_add_watch(fd_, (bat / "uevent").c_str(), IN_ACCESS); - if (wd != -1) { - wds_.push_back(wd); - } + + // Watch the directory for any added or removed batteries + global_watch = inotify_add_watch(fd_, data_dir_.c_str(), IN_CREATE | IN_DELETE); + if (global_watch < 0) { + throw std::runtime_error("Could not watch for battery plug/unplug"); } + + refreshBatteries(); worker(); } waybar::modules::Battery::~Battery() { - for (auto wd : wds_) { - inotify_rm_watch(fd_, wd); + if (global_watch >= 0) { + inotify_rm_watch(fd_, global_watch); + } + for (auto it = batteries_.cbegin(); it != batteries_.cend(); it++) { + auto watch_id = (*it).second; + if (watch_id >= 0) { + inotify_rm_watch(fd_, watch_id); + } + batteries_.erase(it); } close(fd_); } @@ -43,7 +51,13 @@ void waybar::modules::Battery::worker() { }; } -void waybar::modules::Battery::getBatteries() { +void waybar::modules::Battery::refreshBatteries() { + // Mark existing list of batteries as not necessarily found + std::map check_map; + for (auto const& bat : batteries_) { + check_map[bat.first] = false; + } + try { for (auto& node : fs::directory_iterator(data_dir_)) { if (!fs::is_directory(node)) { @@ -54,12 +68,22 @@ void waybar::modules::Battery::getBatteries() { if (((bat_defined && dir_name == config_["bat"].asString()) || !bat_defined) && fs::exists(node.path() / "capacity") && fs::exists(node.path() / "uevent") && fs::exists(node.path() / "status") && fs::exists(node.path() / "type")) { - std::string type; - std::ifstream(node.path() / "type") >> type; + std::string type; + std::ifstream(node.path() / "type") >> type; - if (!type.compare("Battery")){ - batteries_.push_back(node.path()); + if (!type.compare("Battery")){ + check_map[node.path()] = true; + auto search = batteries_.find(node.path()); + if (search == batteries_.end()) { + // We've found a new battery save it and start listening for events + auto event_path = (node.path() / "uevent"); + auto wd = inotify_add_watch(fd_, event_path.c_str(), IN_ACCESS); + if (wd < 0) { + throw std::runtime_error("Could not watch events for " + node.path().string()); } + batteries_[node.path()] = wd; + } + } } auto adap_defined = config_["adapter"].isString(); if (((adap_defined && dir_name == config_["adapter"].asString()) || !adap_defined) && @@ -76,6 +100,17 @@ void waybar::modules::Battery::getBatteries() { } throw std::runtime_error("No batteries."); } + + // Remove any batteries that are no longer present and unwatch them + for (auto const& check : check_map) { + if (!check.second) { + auto watch_id = batteries_[check.first]; + if (watch_id >= 0) { + inotify_rm_watch(fd_, watch_id); + } + batteries_.erase(check.first); + } + } } const std::tuple waybar::modules::Battery::getInfos() const { @@ -84,7 +119,8 @@ const std::tuple waybar::modules::Battery::getInfos uint32_t total_energy = 0; // μWh uint32_t total_energy_full = 0; std::string status = "Unknown"; - for (auto const& bat : batteries_) { + for (auto const& item : batteries_) { + auto bat = item.first; uint32_t power_now; uint32_t energy_full; uint32_t energy_now; @@ -175,6 +211,11 @@ const std::string waybar::modules::Battery::formatTimeRemaining(float hoursRemai } auto waybar::modules::Battery::update() -> void { + // Make sure we have the correct set of batteries, in case of hotplug + // TODO: split the global watch into it's own event and only run the refresh + // when there's been a CREATE/DELETE event + refreshBatteries(); + auto [capacity, time_remaining, status] = getInfos(); if (status == "Unknown") { status = getAdapterStatus(capacity); From 31a4aff1f8328037ad8986e2ffd1a4a90af33114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20C=C3=B4rte-Real?= Date: Fri, 27 Nov 2020 14:23:37 +0000 Subject: [PATCH 7/7] Don't show battery estimate at 0 If we think we're done might as well not show 0h 0min as the estimate and just not show anything. --- src/modules/battery.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/modules/battery.cpp b/src/modules/battery.cpp index 84bc973b..c506171f 100644 --- a/src/modules/battery.cpp +++ b/src/modules/battery.cpp @@ -200,10 +200,14 @@ const std::string waybar::modules::Battery::getAdapterStatus(uint8_t capacity) c } const std::string waybar::modules::Battery::formatTimeRemaining(float hoursRemaining) { - hoursRemaining = std::fabs(hoursRemaining); + hoursRemaining = std::fabs(hoursRemaining); uint16_t full_hours = static_cast(hoursRemaining); uint16_t minutes = static_cast(60 * (hoursRemaining - full_hours)); auto format = std::string("{H} h {M} min"); + if (full_hours == 0 && minutes == 0) { + // Migh as well not show "0h 0min" + return ""; + } if (config_["format-time"].isString()) { format = config_["format-time"].asString(); }