Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.9.9 - don't work "network" {bandwidth*} #1392

Closed
ghost opened this issue Jan 14, 2022 · 24 comments
Closed

0.9.9 - don't work "network" {bandwidth*} #1392

ghost opened this issue Jan 14, 2022 · 24 comments

Comments

@ghost
Copy link

ghost commented Jan 14, 2022

'Network speed' is working on 0.9.8.
I don't know why. Now (0.9.9) always static 0 bit/s (or octet). Maybe I can try something step-by-step? Let me know, please.
My config:

"network": {
    "format-wifi": " {bandwidthDowbBits}  {bandwidthUpBits} ",
    "format-ethernet": " {bandwidthDownBits}  {bandwidthUpBits} ",
    "format-linked": "(No IP)",
    "format-disconnected": "",
    "on-click": "alacritty -e nmtui",
    "tooltip": false,
    "interval": 5
}

And, and, and. When "format-wifi" - nothing is displayed (empty, even without icons).

@ibrokemypie
Copy link

I believe it was caused by commit 9d9f959 which states that setting interface to "*" should result in the same as the old auto implementation. This isn't documented in the wiki though and should probably be the default since the old default no longer works.

@bd-g
Copy link
Contributor

bd-g commented Jan 18, 2022

I've done testing on that commit and setting the interface to "*" does not have the same behavior as the old auto implementation. There probably wasn't enough testing done on different configurations prior to this commit, and I'm working on a fix. In the meantime, a PR to fix the wiki and reset the default would be helpful.

@ghost
Copy link
Author

ghost commented Jan 20, 2022

I've done testing on that commit and setting the interface to "*" does not have the same behavior as the old auto implementation.

How to set up multiple interfaces?

@mvdan
Copy link
Contributor

mvdan commented Jan 25, 2022

I don't think it's just the "auto" or wildcard detection; even when I set up the interface to enp5s0, my wired connection, I still get 0o/s even though I can clearly see large numbers in /dev:

$ cat /proc/net/dev | grep enp5s0
enp5s0: 3793211018 3216162    0    4    0     0          0      1262 325850572 1492065    0    0    0     0       0          0

@bd-g
Copy link
Contributor

bd-g commented Jan 25, 2022

#610 was the initial issue that the commit was trying to fix
#1327 is a duplicate issue

@ciarand Thanks for taking up the initial issue and pushing a fix! Before anyone else dives into the code, do you have an idea of why the above behavior would be happening?

@droc12345
Copy link

I had another type problem (network related) so I looked at the code.
and between 0.98 and 0.99 only 2 changes were made to network.cpp (1 of which was a comment)
the other is line 81 want_route_dump was changed from false to true, changing it back fixed my problem.

I think that the change was done to try and show initial parameters (bandwidth doesn't show immediately, but after 1st refresh)
But evidently setting want_route_dump to true creates some other issue(s).

@bd-g
Copy link
Contributor

bd-g commented Feb 5, 2022 via email

@droc12345
Copy link

droc12345 commented Feb 7, 2022

I have tried a few different things (config file)

"interface": "eth0", -- shows eth0 and data
"interface": "eth*", -- shows eth0 and data
"interface": "e*", -- shows eth0 and data
all normal

"interface": "*", -- shows lo and no data
no "interface" line -- shows lo and no data

With the "lo" being what I would expect as it's the first interface shown (on my system) in /proc/net/dev
and waybar doesn't know that I don't care about the lo interface.

Edit to add: IMO, interface name should always be in config file
Take my laptop, I sometimes have both wired and wireless on at the same time, no interface name, it doesn't know what to pick.

Edit to add 2: You can sort of figure out the interface, if you ignore the "lo" line and you don't have more than one network connection. But if I have eth0, tun0 and wifi0 how does it determine which one to use?

@bd-g
Copy link
Contributor

bd-g commented Feb 7, 2022

I agree - it appears with regex you can ignore interfaces you don't care about, but there isn't logic to determine prioritization of networks. Maybe we could add some logic to find all interfaces matching the config file, then sort by volume of network traffic?

@tombh
Copy link

tombh commented Feb 7, 2022

Ohhh, thanks to @droc12345, I've just realised what was meant by "interface" in this thread! It's a waybar config setting. So now adding the interface line makes my bandwidth show up again :)

"network": {
    "interface": "wlan0", // This is the magic line
    "format-wifi": " {bandwidthDowbBits}  {bandwidthUpBits} ",
    "format-ethernet": " {bandwidthDownBits}  {bandwidthUpBits} ",
    "format-linked": "(No IP)",
    "format-disconnected": "",
    "on-click": "alacritty -e nmtui",
    "tooltip": false,
    "interval": 5
}

@droc12345
Copy link

I agree - it appears with regex you can ignore interfaces you don't care about, but there isn't logic to determine prioritization of networks. Maybe we could add some logic to find all interfaces matching the config file, then sort by volume of network traffic?

It would be hard (to satisfy everyone) by trying to choose which interface to use, when there is more than one interface.
Interface should probably be a required parameter.
Reasons:
Wildcards could be allowed, but it needs to be made clear in the documentation, that what you get
might not be what you want, ie you might get eth0 all the time, where eth1 would never show (with wildcard)

Trying to determine by usage would be difficult, as that number is in constant flux.

@bd-g
Copy link
Contributor

bd-g commented Feb 7, 2022

Great point. Just brainstorming here. What about this - building in scroll functionality for the network module, where when you scroll on the module, it cycles through all the interfaces that match the regex? This would remove the need for interface to be a required parameter, or for us to try and guess a default.

@droc12345
Copy link

If there is no interface specified (not sure why we really want that) then is there a way to provide an ability to show all interfaces and allow the user to select one? prog/script pops up when no interface is specified or there is more than 2 interfaces (lo and another) . To me this would be easier than trying to fit scrolling/cycing.
The reason I don't disregard lo completely is that someone, somewhere will want to see it. lol

Lets look at my case, I have lo, eth0 and when I turn on the vpn I have tun0, and when I turn the vpn off tun0 goes away.
Could that be handled by scrolling/cycling, ie would the choices be static (from when I started waybar)?

@bd-g
Copy link
Contributor

bd-g commented Feb 7, 2022

I believe that the networks would be polled and matched against the "interface" parameter on every refresh. The current default is 60s.

So it would be a matter of storing an array of interfaces with associated network information and updating the entire array each refresh period. Then having way bar select one interface from that array to display, and building a functionality to change which is displayed (whether the built in scroll ability, the click functionality, or something new) and persisting that selection across refreshes.

@droc12345
Copy link

That might work (mine is 2 sec refresh as I like to see more or less current usage) and as long as the interface parm is still being recognized, then I see no harm.

You might want to keep a flag for interface/no-interface(or wildcard) so that easy choices could be made if this extends into more than one or two functions though.

@bd-g
Copy link
Contributor

bd-g commented Feb 7, 2022

Mine is set to 2s as well actually.

You might want to keep a flag for interface/no-interface(or wildcard) so that easy choices could be made if this extends into more than one or two functions though.

What do you mean by this?

@droc12345
Copy link

If a function calls some other function or the code changes extends into multiple functions, and it has to make choices based on being given an interface name or finding them out, it would be nice to have a flag saying it's been given an interface vs figure out the interfaces, kind of like the want_route_dump flag, just something that's easy to use for making choices.
I could just be overthinking it too. lol

I'll mention the interface (config file) once again, whether using the interface tag or making a choice because they're offered multiple interfaces (scrolling, etc), they HAVE to make a choice. To me it's easiest to do it in the config file.

Another possible option (just off the top of my head) is if no interface specified, put all that data into something like a tooltip that shows all interfaces (and data) when hovered, rather than letting them select from scrolling/menu selection

@bd-g
Copy link
Contributor

bd-g commented Feb 7, 2022

Another possible option (just off the top of my head) is if no interface specified, put all that data into something like a tooltip that shows all interfaces (and data) when hovered, rather than letting them select from scrolling/menu selection

So this would entail changing the default tooltip format to being a list of possible interfaces and data. I think it would only be feasible to list the interfaces, not the data. That would be a ton of text for a tooltip otherwise. But that is a good idea - could format it like the default CPU tooltip?

Could we combine the two ideas? If no interface is specified use * as the default, have the default tooltip be a list of interfaces, and include scroll functionality by default as well? Could update the wiki with a description of all of this.

@droc12345
Copy link

That sounds feasible

@droc12345
Copy link

Might mention in the docs/wiki that the interface name (config option) is the same as what is returned from
ifconfig, ip addr or netstat -i

@bd-g
Copy link
Contributor

bd-g commented Feb 7, 2022

That's really helpful, I agree.

I'm going to start work on this at https://github.com/bd-g/Waybar, you're welcome to contribute/collaborate

@droc12345
Copy link

line 108-117 of network.cpp has an if condition to check whether the interface is up (want_route_dump_)
then either sets the flag OR sets it for link and addr. I think this is wrong.
I think that link and addr should always be set (remove them from the else) and just toggle the route flag if interface is not specified. I'm basing this on the fact that want_* are supposed to handle multiple calls.

This is off the top of my head, so I haven't tried it.

@siikamiika
Copy link
Contributor

siikamiika commented Apr 2, 2022

I'm not sure about compatibility with old behavior but got mine to work without explicit interface with this change to Network::checkInterface

diff --git a/src/modules/network.cpp b/src/modules/network.cpp
index 74b54d9..3b88011 100644
--- a/src/modules/network.cpp
+++ b/src/modules/network.cpp
@@ -395,7 +395,8 @@ bool waybar::modules::Network::checkInterface(std::string name) {
     return config_["interface"].asString() == name ||
            wildcardMatch(config_["interface"].asString(), name);
   }
-  return false;
+  spdlog::debug("ifname_: {}; name: {}", ifname_, name);
+  return ifname_ == name;
 }

 void waybar::modules::Network::clearIface() {

alternatively to avoid overloading checkInterface

diff --git a/src/modules/network.cpp b/src/modules/network.cpp
index 74b54d9..b9a0e16 100644
--- a/src/modules/network.cpp
+++ b/src/modules/network.cpp
@@ -41,7 +41,7 @@ waybar::modules::Network::readBandwidthUsage() {
     std::string ifacename;
     iss >> ifacename;  // ifacename contains "eth0:"
     ifacename.pop_back();  // remove trailing ':'
-    if (!checkInterface(ifacename)) {
+    if (ifacename != ifname_) {
       continue;
     }

siikamiika added a commit to siikamiika/os-setup that referenced this issue Apr 3, 2022
@cmprmsd
Copy link

cmprmsd commented Apr 7, 2022

For me the bandwidth did not show up correctly on wlan0 as described here and #610.

I built a go custom app based on i3-netusage and uploaded it to Github. Maybe it helps you as well :)

I tried to add autodetection of the gateway interface. Not sure if it does work correctly. Just had some late night coding time :D

https://github.com/cmprmsd/sway-netusage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants