search

LeeVD / plasma-applet-net-bandwidth-monitor

Network bandwidth monitor for plasma using dbus
GNU General Public License v3.0
25 stars 4 forks source link

[Bug] Update interval doesn't work. #2

Closed tazihad closed 1 year ago

tazihad commented 1 year ago

Plasma Version: 5.26.4

Update interval is stuck at 0.5 second. Doesn't work other options.

Thanks for this widget. It's amazing work man.

LeeVD commented 1 year ago

Thanks Tazihad. Did you build from Github or install from Plasma desktop widget section? I created it in the Manjaro Plasma version, im sure it was the latest. Installing from Plasma desktop widget section seemed to work ok for me. I have noticed some issues in the Github code since i uploaded it. I'll check against Fedora and build via Github and correct any issues i find.

luisbocanegra commented 1 year ago

I'm still facing this in v0.1.1, both from Plasma Widgets and manual built, the update interval option seems to be ignored as it keeps updating at the same rate, this is on Plasma 5.26.5 (latest)

LeeVD commented 1 year ago

Yes, i noticed this last night just after i had uploaded the new version. I fixed one issue with time interval but it wasnt the main thing, annoyingly. Its an issue with the 2 code bases and my understand on the dbus code. Its high priority on my fix list.

luisbocanegra commented 1 year ago

Yeah looked at the code and couldn't figure it out with my little qml/plasma api knowledge :laughing: and yeah dbus can be tricky to work with

LeeVD commented 1 year ago

Yes, tricky is an accurate term! Ok, i have a 'working' version now. Its ugly as sin and i'll need to revisit to improve the code but i'll try and get a new fix out soon. Here's a question I've pondered while working on this, on how you would expect it to work. Let me know which option you would pick? : 1, set to 2 second update interval. When triggered the display shows the bandwidth used at that moment. 2, set to 2 second update interval. When triggered, the display shows the accumulated bandwidth over the entire 2 seconds.

luisbocanegra commented 1 year ago

I think option 1 makes more sense, 2 will be the average of that interval and may not represent the current speed? :thinking: Also wouldn't option 1 have the benefit of making less dbus calls/calculations since it is already an averaged value?, correct me if I'm wrong here as haven't looked of how the speed is obtained/calculated

LeeVD commented 1 year ago

Yep, it would show how much traffic passed in the last 2 seconds also displayed for 2 seconds. From what I've found, the dbus code is triggered every 0.5 seconds (no idea if that can changed). the update interval triggers a separate method that updates the display. so if you set update interval to 2 seconds, the dbus data updates 4 times. The display method updates the display with the last dbus data. It will also just sit there with that data displayed for another 2 seconds until it updates again. That's how it currently works. Could be another checkbox in settings for the 2 options?

luisbocanegra commented 1 year ago

Yep, it would show how much traffic passed in the last 2 seconds also displayed for 2 seconds. From what I've found, the dbus code is triggered every 0.5 seconds (no idea if that can changed). the update interval triggers a separate method that updates the display. so if you set update interval to 2 seconds, the dbus data updates 4 times. The display method updates the display with the last dbus data. It will also just sit there with > that data displayed for another 2 seconds until it updates again. That's how it currently works. Could be another checkbox in settings for the 2 options?

I think it would be less confusing having only one interval option, I think most users expect to see the current rate.

Then it could have in a tooltip for example the consumed traffic for the whole session or since x amount of time, similar to what NetSpeed widget has.

As for how fast it should trigger if it can't be changed or is too complicated you could resort to just display the last updated value

LeeVD commented 1 year ago

@luisbocanegra Version 0.2 just released. The interval issue now fixed. Based on our conversation, i added both option. if you choose any interval above 0.5 sec, you get a second option to show the exact data at each of the interval or an accumulated figure over the length of the last update. Just another option in the spirit of KDE's many options :) let me know how you get on.

luisbocanegra commented 1 year ago

Thanks, can confirm the update interval is now working as expected, same with the accumulated bandwidth (haven't tested it thoroughly)

At first I couldn't add the widget to the panel/desktop so had to uninstall it first, I see you changed its name, so probably that was the cause as it was fine running it with plasmoidviewer

tazihad commented 1 year ago

Ya. Seems like it works now. closing this issue. Thanks for all the works. Amazing tool btw.

CatEricka commented 1 year ago

The calculation of the accumulated bandwidth seems to be incorrect, e.g. when the interval to 1 seconds, the value will be about twice the correct value. There is a fix: #8

The algorithm of option 2 (accumulator) now is to add up all consumed traffic and finally divide by the updateInterval.

CatEricka commented 1 year ago

I think it would be less confusing having only one interval option, I think most users expect to see the current rate.

I found a similar issue with KDE's System Monitor (plasma-systemmonitor/bookworm, 5.26.90-1 amd64): if set interval to 2 seconds, it will only update the most recent rate every 2 seconds (like the option 1). In other words, it's ignoring samples.

If one of the samples has a large value (for my example, the samba video client will download the buffer every few seconds), this will definitely confuse the user.

LeeVD commented 1 year ago

@CatEricka I've moved your comments and suggestions to a new 'issue tracker' for further discussion. Lets continue this there. Ta.

LeeVD commented 1 year ago

I see you changed its name,

@luisbocanegra Hello Luis, I didn't change its name. Where did you see a difference, I'll have to go back and check. I was interested myself in how the update process would go and it seemed to work for me, i could have been using a newer version than what was released to update over though. I did see issues in past attempts to update, I'm wondering if it had to do with serialized variables. Something I'll keep an eye on for future release.

CatEricka commented 1 year ago

@LeeVD

Hello Luis, I didn't change its name. Where did you see a difference, I'll have to go back and check.

https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/blob/62e5c9476774057ef2dc4bcd191ee1beb23f0d8f/package/metadata.desktop#L10

$ pwd
/home/user_name/.local/share/plasma/plasmoids/org.kde.nsw_dbus