From 9f520dea7a7faef3fc9af5c02e3406c3e8eae7ac Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Tue, 31 May 2016 11:49:33 +0200 Subject: [PATCH] fix(network): Proper handling of connectivity testing --- README.md | 37 +++++++--- config | 18 ++--- include/interfaces/net.hpp | 15 +++-- include/modules/network.hpp | 19 +++--- src/modules/network.cpp | 131 +++++++++++++++++++----------------- 5 files changed, 127 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index 1b401bf7..1fecca61 100644 --- a/README.md +++ b/README.md @@ -575,10 +575,7 @@ See [the bspwm module](#user-content-dependencies) for details on `label:dimmed` ~~~ -### 🟊 Module `internal/network` - -The module is still marked as WIP since the packetloss testing is not -fully functional yet. If you notice any other anomalies, please [create an issue](https://github.com/jaagr/lemonbuddy/issues). +### Module `internal/network` **NOTE:** If you use both a wired and a wireless network, just add 2 module definitions. For example: @@ -599,12 +596,14 @@ fully functional yet. If you notice any other anomalies, please [create an issue interface = wlan1 ; Seconds to sleep between updates - interval = 2.0 + interval = 3.0 - ; Seconds to sleep between connectivity tests - ; A value of 0 disables the testing + ; Test connectivity every Nth update + ; A value of 0 disables the feature + ; Recommended minimum value: round(10 / interval) + ; - which would test the connection approx. every 10th sec. ; Default: 0 - ;connectivity_test_interval = 10 + ;ping_interval = 3 ~~~ ##### Extra formatting (example) @@ -618,6 +617,12 @@ fully functional yet. If you notice any other anomalies, please [create an issue ; (default) ;format:disconnected = + ; Available tags: + ; (default) + ; + ; + format:packetloss = + ; Available tokens: ; %ifname% [wireless+wired] ; %local_ip% [wireless+wired] @@ -634,12 +639,28 @@ fully functional yet. If you notice any other anomalies, please [create an issue ;label:disconnected = not connected ;label:disconnected:foreground = #66ffffff + ; Available tokens: + ; %ifname% [wireless+wired] + ; %local_ip% [wireless+wired] + ; %essid% [wireless] + ; %signal% [wireless] + ; %linkspeed% [wired] + ; Default: (none) + ;label:packetloss = %essid% + ;label:packetloss:foreground = #eefafafa + ramp:signal:0 = 😱 ramp:signal:1 = 😠 ramp:signal:2 = 😒 ramp:signal:3 = 😊 ramp:signal:4 = 😃 ramp:signal:5 = 😈 + + animation:packetloss:0 = ⚠ + animation:packetloss:0:foreground = #ffa64c + animation:packetloss:1 = 📶 + animation:packetloss:1:foreground = #000000 + animation:packetloss:framerate_ms = 500 ~~~ diff --git a/config b/config index ef8ce6d6..a37b014e 100644 --- a/config +++ b/config @@ -632,12 +632,14 @@ type = internal/network interface = net1 ; Seconds to sleep between updates -interval = 2.0 +interval = 3.0 -; Seconds to sleep between connectivity tests -; A value of 0 disables the testing +; Test connectivity every Nth update +; A value of 0 disables the feature ; Default: 0 -connectivity_test_interval = 10 +; Recommended minimum value: round(10 / interval) +; - which would test the connection approx. every 10th sec. +ping_interval = 3 ; Available tags: ; (default) @@ -649,9 +651,10 @@ format:connected = ;format:disconnected = ; Available tags: -; (default) +; (default) +; ; -; format:packetloss = +;format:packetloss = ; Available tokens: ; %ifname% [wireless+wired] @@ -675,8 +678,7 @@ label:disconnected:foreground = #66ffffff ; %essid% [wireless] ; %signal% [wireless] ; %linkspeed% [wired] -; Default: %ifname% %local_ip% -; ------------------------- NOT ACTIVATED (Needs more testing) +; Default: (none) ;label:packetloss = %essid% ;label:packetloss:foreground = #eefafafa diff --git a/include/interfaces/net.hpp b/include/interfaces/net.hpp index 24d438a3..91ddff98 100644 --- a/include/interfaces/net.hpp +++ b/include/interfaces/net.hpp @@ -16,8 +16,9 @@ namespace net { bool is_wireless_interface(const std::string& ifname); + // // Network - + // class NetworkException : public Exception { public: @@ -46,9 +47,9 @@ namespace net std::string get_ip() throw(NetworkException); }; - + // // WiredNetwork - + // class WiredNetworkException : public NetworkException { using NetworkException::NetworkException; }; @@ -58,14 +59,14 @@ namespace net int linkspeed = 0; public: - WiredNetwork(const std::string& interface); + WiredNetwork(const std::string& interface); - std::string get_link_speed(); + std::string get_link_speed(); }; - + // // WirelessNetwork - + // class WirelessNetworkException : public NetworkException { using NetworkException::NetworkException; }; diff --git a/include/modules/network.hpp b/include/modules/network.hpp index 6f44fc75..10f217ec 100644 --- a/include/modules/network.hpp +++ b/include/modules/network.hpp @@ -28,8 +28,6 @@ namespace modules const char *TAG_LABEL_PACKETLOSS = ""; const char *TAG_ANIMATION_PACKETLOSS = ""; - const int PING_EVERY_NTH_UPDATE = 10; - std::unique_ptr wired_network; std::unique_ptr wireless_network; @@ -42,19 +40,20 @@ namespace modules std::unique_ptr label_packetloss_tokenized; std::string interface; - bool connected = false; - int signal_quality = 0; - bool conseq_packetloss = false; - int counter = -1; // -1 to avoid ping the first run - int connectivity_test_interval; - // std::thread t_animation; - // void animation_thread_runner(); + concurrency::Atomic connected; + concurrency::Atomic conseq_packetloss; + concurrency::Atomic signal_quality; + + int ping_nth_update; + int counter = -1; // Set to -1 to ignore the first run + + void subthread_routine(); public: NetworkModule(const std::string& name); - ~NetworkModule(); + void start(); bool update(); std::string get_format(); diff --git a/src/modules/network.cpp b/src/modules/network.cpp index a1fb91da..2b8825c4 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -14,35 +14,49 @@ using namespace modules; NetworkModule::NetworkModule(const std::string& name_) : TimerModule(name_, 1s) { - this->interval = std::chrono::duration( - config::get(name(), "interval", 1)); - this->connectivity_test_interval = config::get( - name(), "connectivity_test_interval", 0); - this->interface = config::get(name(), "interface"); + static const auto DEFAULT_FORMAT_CONNECTED = TAG_LABEL_CONNECTED; + static const auto DEFAULT_FORMAT_DISCONNECTED = TAG_LABEL_DISCONNECTED; + static const auto DEFAULT_FORMAT_PACKETLOSS = TAG_LABEL_CONNECTED; + + static const auto DEFAULT_LABEL_CONNECTED = "%ifname% %local_ip%"; + static const auto DEFAULT_LABEL_DISCONNECTED = ""; + static const auto DEFAULT_LABEL_PACKETLOSS = ""; + this->connected = false; + this->conseq_packetloss = false; - this->formatter->add(FORMAT_CONNECTED, TAG_LABEL_CONNECTED, { TAG_RAMP_SIGNAL, TAG_LABEL_CONNECTED }); - this->formatter->add(FORMAT_DISCONNECTED, TAG_LABEL_DISCONNECTED, { TAG_LABEL_DISCONNECTED }); + // Load configuration values + this->interface = config::get(name(), "interface"); + this->interval = std::chrono::duration(config::get(name(), "interval", 1)); + this->ping_nth_update = config::get(name(), "ping_interval", 0); + // Add formats + this->formatter->add(FORMAT_CONNECTED, DEFAULT_FORMAT_CONNECTED, { TAG_RAMP_SIGNAL, TAG_LABEL_CONNECTED }); + this->formatter->add(FORMAT_DISCONNECTED, DEFAULT_FORMAT_DISCONNECTED, { TAG_LABEL_DISCONNECTED }); + + // Create elements for format-connected if (this->formatter->has(TAG_RAMP_SIGNAL, FORMAT_CONNECTED)) this->ramp_signal = drawtypes::get_config_ramp(name(), get_tag_name(TAG_RAMP_SIGNAL)); if (this->formatter->has(TAG_LABEL_CONNECTED, FORMAT_CONNECTED)) - this->label_connected = drawtypes::get_optional_config_label(name(), get_tag_name(TAG_LABEL_CONNECTED), "%ifname% %local_ip%"); + this->label_connected = drawtypes::get_optional_config_label(name(), get_tag_name(TAG_LABEL_CONNECTED), DEFAULT_LABEL_CONNECTED); + // Create elements for format-disconnected if (this->formatter->has(TAG_LABEL_DISCONNECTED, FORMAT_DISCONNECTED)) { - this->label_disconnected = drawtypes::get_optional_config_label(name(), get_tag_name(TAG_LABEL_DISCONNECTED), ""); + this->label_disconnected = drawtypes::get_optional_config_label(name(), get_tag_name(TAG_LABEL_DISCONNECTED), DEFAULT_LABEL_DISCONNECTED); this->label_disconnected->replace_token("%ifname%", this->interface); } - if (this->connectivity_test_interval > 0) { - this->formatter->add(FORMAT_PACKETLOSS, "", { TAG_ANIMATION_PACKETLOSS, TAG_LABEL_PACKETLOSS }); + // Create elements for format-packetloss if we are told to test connectivity + if (this->ping_nth_update > 0) { + this->formatter->add(FORMAT_PACKETLOSS, DEFAULT_FORMAT_PACKETLOSS, { TAG_ANIMATION_PACKETLOSS, TAG_LABEL_PACKETLOSS, TAG_LABEL_CONNECTED }); if (this->formatter->has(TAG_LABEL_PACKETLOSS, FORMAT_PACKETLOSS)) - this->label_packetloss = drawtypes::get_optional_config_label(name(), get_tag_name(TAG_LABEL_PACKETLOSS), "%ifname% %local_ip%"); + this->label_packetloss = drawtypes::get_optional_config_label(name(), get_tag_name(TAG_LABEL_PACKETLOSS), DEFAULT_LABEL_PACKETLOSS); if (this->formatter->has(TAG_ANIMATION_PACKETLOSS, FORMAT_PACKETLOSS)) this->animation_packetloss = drawtypes::get_config_animation(name(), get_tag_name(TAG_ANIMATION_PACKETLOSS)); } + // Get an intstance of the network interface try { if (net::is_wireless_interface(this->interface)) { this->wireless_network = std::make_unique(this->interface); @@ -54,54 +68,46 @@ NetworkModule::NetworkModule(const std::string& name_) : TimerModule(name_, 1s) } } -NetworkModule::~NetworkModule() +void NetworkModule::start() { - std::lock_guard lck(this->update_lock); - // if (this->t_animation.joinable()) - // this->t_animation.join(); + this->TimerModule::start(); + + // We only need to start the subthread if the packetloss animation is used + if (this->animation_packetloss) + this->threads.emplace_back(std::thread(&NetworkModule::subthread_routine, this)); } -// void NetworkModule::dispatch() -// { -// this->EventModule::dispatch(); -// -// // if (this->animation_packetloss) -// // this->t_animation = std::thread(&NetworkModule::animation_thread_runner, this); -// } +void NetworkModule::subthread_routine() +{ + std::this_thread::yield(); -// bool NetworkModule::has_event() -// { -// std::this_thread::sleep_for(this->interval); -// return true; -// } + const auto dur = std::chrono::duration( + float(this->animation_packetloss->get_framerate()) / 1000.0f); -// void NetworkModule::animation_thread_runner() -// { -// while (this->enabled()) { -// std::unique_lock lck(this->mtx); -// -// if (this->connected && this->conseq_packetloss) -// this->Module::notify_change(); -// else -// this->cv.wait(lck, [&]{ -// return this->connected && this->conseq_packetloss; }); -// -// std::this_thread::sleep_for(std::chrono::duration( -// float(this->animation_packetloss->get_framerate()) / 1000)); -// } -// } + while (this->enabled()) { + std::unique_lock lck(this->broadcast_lock); + + if (this->connected && this->conseq_packetloss) { + lck.unlock(); + this->broadcast(); + } + + std::this_thread::sleep_for(dur); + } + + log_debug("Reached end of network subthread"); +} bool NetworkModule::update() { std::string ip, essid, linkspeed; int signal_quality = 0; + net::Network *network = nullptr; + + // Process data for wireless network interfaces if (this->wireless_network) { - try { - ip = this->wireless_network->get_ip(); - } catch (net::NetworkException &e) { - get_logger()->debug(e.what()); - } + network = this->wireless_network.get(); try { essid = this->wireless_network->get_essid(); @@ -110,26 +116,33 @@ bool NetworkModule::update() get_logger()->debug(e.what()); } - this->connected = this->wireless_network->connected(); this->signal_quality = signal_quality; - if (this->connectivity_test_interval > 0 && this->connected && this->counter++ % PING_EVERY_NTH_UPDATE == 0) - this->conseq_packetloss = !this->wireless_network->test(); + // Process data for wired network interfaces } else if (this->wired_network) { + network = this->wired_network.get(); + linkspeed = this->wired_network->get_link_speed(); + } + + if (network != nullptr) { try { - ip = this->wired_network->get_ip(); + ip = network->get_ip(); } catch (net::NetworkException &e) { get_logger()->debug(e.what()); } - linkspeed = this->wired_network->get_link_speed(); + this->connected = network->connected(); - this->connected = this->wired_network->connected(); - - if (this->connectivity_test_interval > 0 && this->connected && this->counter++ % PING_EVERY_NTH_UPDATE == 0) - this->conseq_packetloss = !this->wired_network->test(); + // Ignore the first run + if (this->counter == -1) { + this->counter = 0; + } else if (this->ping_nth_update > 0 && this->connected && (++this->counter % this->ping_nth_update) == 0) { + this->conseq_packetloss = !network->test(); + this->counter = 0; + } } + // Update label contents if (this->label_connected || this->label_packetloss) { auto replace_tokens = [&](std::unique_ptr &label){ label->replace_token("%ifname%", this->interface); @@ -161,8 +174,6 @@ bool NetworkModule::update() } } - // this->cv.notify_all(); - return true; } @@ -170,7 +181,7 @@ std::string NetworkModule::get_format() { if (!this->connected) return FORMAT_DISCONNECTED; - else if (this->conseq_packetloss) + else if (this->conseq_packetloss && this->ping_nth_update > 0) return FORMAT_PACKETLOSS; else return FORMAT_CONNECTED;