LeeVD / plasma-applet-net-bandwidth-monitor

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

Update Interval: additional Options #9

Closed LeeVD closed 1 year ago

LeeVD commented 1 year ago

image https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/issues/2#issuecomment-1425348183 https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/issues/2#issuecomment-1425417573

Moved to new Issue for further discussion. @CatEricka Is this an issue with expected behaviour? I didn't intend it to 'average out' the bandwidth polling, i wanted to show the total that had traversed the network connections between the triggering of UI updates. I'll need to check again as to if my code if functioning as expected, when i tested it last, before release, it seemed to. I can add an additional option for providing the average based on the code you supplied? I am aware that that section could probably do with a tooltip or something to explain what its meant to do. Will hopefully have something like that in the next few releases.

CatEricka commented 1 year ago

Is this an issue with expected behaviour? I didn't intend it to 'average out' the bandwidth polling, i wanted to show the total that had traversed the network connections between the triggering of UI updates.

The issue is base unit. Now no matter what kind of option, the unit is bits/s or Bytes/s

CatEricka commented 1 year ago

I'll need to check again as to if my code if functioning as expected, when i tested it last, before release, it seemed to.

Kind of...

https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/blob/62e5c9476774057ef2dc4bcd191ee1beb23f0d8f/package/contents/ui/CompactRepresentation.qml#L825

This branch never executed, but yes, it's works as your wish: show the total that had traversed the network connections between the triggering of UI updates; but with a wrong unit: bit/s.

It's really a difference between different flavors of UX, and while I think my design is more in line with the expectations of the average user, but yah, it's just a matter of taste.

CatEricka commented 1 year ago

and while I think my design is more in line with the expectations of the average user

Default setting update every 0.5 seconds, I just downsample it. This is reasonable for a time series 🤔

LeeVD commented 1 year ago

Is this an issue with expected behaviour? I didn't intend it to 'average out' the bandwidth polling, i wanted to show the total that had traversed the network connections between the triggering of UI updates.

The issue is base unit. Now no matter what kind of option, the unit is bits/s or Bytes/s

So i understand, is it the bits / bytes you are focused on or the /s (seconds)?

LeeVD commented 1 year ago

I'll need to check again as to if my code if functioning as expected, when i tested it last, before release, it seemed to.

Kind of...

https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/blob/62e5c9476774057ef2dc4bcd191ee1beb23f0d8f/package/contents/ui/CompactRepresentation.qml#L825

This branch never executed, but yes, it's works as your wish: show the total that had traversed the network connections between the triggering of UI updates; but with a wrong unit: bit/s.

It's really a difference between different flavors of UX, and while I think my design is more in line with the expectations of the average user, but yah, it's just a matter of taste.

I'll run some tests and see what output i get. If you're referencing the comment "//netDataBits[i].down", that is left over from testing/ rewrites, I should have cleaned that up. The getAccumulator function is fed by multiNetAddition function which gets values based on which unit is selected by the user, bits or bytes.

CatEricka commented 1 year ago

I can add an additional option for providing the average based on the code you supplied? I am aware that that section could probably do with a tooltip or something to explain what its meant to do.

I agree with you because of KDE style and tooltips are useful for explaining what options mean.

Back to the topic. The discussion below is based on the current implementation: https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/blob/62e5c9476774057ef2dc4bcd191ee1beb23f0d8f/package/contents/ui/CompactRepresentation.qml#L811-L836

If I set updateInterval to 1000 ms, I got this:

getAccumulator() called 1:
    downSpeed = 1000 bits/s
    netDataAccumulator.down = 1000 bits/s

getAccumulator() called 2:
    downSpeed = 2000 bits/s
    netDataAccumulator.down = 3000 bits/s

then it will now display:

▼ 3000 bits/s

What is the meaning of 3000 bits/s? Is it the traffic of this second divided by the time? No, This is simply adding up the two values, these two values ​​are not dimensionless numbers, you can't simply add them together and get a new value, and still use the same units and meaning.

LeeVD commented 1 year ago

0.5 s

The DBUS call is made every 0.5 seconds, that happens no matter what update Interval the user sets. If activated, the accumulator will/should then grab bandwidth data every 0.5 secs, add it up and then display it based on user interval time, then reset to 0 and repeat. Is that how you think it should work?

CatEricka commented 1 year ago

So i understand, is it the bits / bytes you are focused on or the /s (seconds)?

/s (seconds) :)

CatEricka commented 1 year ago

The DBUS call is made every 0.5 seconds, that happens no matter what update Interval the user sets.

Is this value calculated from samples that are 0.5 seconds long? According to my observation it should be true.

LeeVD commented 1 year ago

So i understand, is it the bits / bytes you are focused on or the /s (seconds)?

/s (seconds) :)

Yes! I getchya! Im not a bright man :D I did ponder something similar when i saw how the DBUS section worked as it polls every 500ms. If the user update interval is set to 0.5sec, should the displayed unit (bits for this example) be 'bits'/s. That would be wrong right? For it to be accurate the user set update interval would need to be 1sec. But! if the user interval is 1 second and there is no accumulation / addition going on, only 0.5sec of traffic is being displayed to the user. Is that what the user would want?

LeeVD commented 1 year ago

The DBUS call is made every 0.5 seconds, that happens no matter what update Interval the user sets.

Is this value calculated from samples that are 0.5 seconds long? According to my observation it should be true.

Yes, that's correct. I'm not sure if this is standard DBUS behaviour. When I've used command line to get DBUS data it seems to update every 0.5 seconds as well.

CatEricka commented 1 year ago

Simple summary.

You can say:

But you can't say:

CatEricka commented 1 year ago

I did ponder something similar when i saw how the DBUS section worked as it polls every 500ms. If the user update interval is set to 0.5sec, should the displayed unit (bits for this example) be 'bits'/s. That would be wrong right?

If the developer has learned mathematics, he should divide the collected traffic by 0.5s...

By comparing with the plasma-systemmonitor, I believe he is right, this value is the network speed obtained by sampling for 0.5 seconds, and the unit is bits/s (or bytes/s). Maybe I should do more check, apparently calculating network traffic is never a simple matter 😺

CatEricka commented 1 year ago

These values ​​should be viewed as a time series, and the sampling interval is definitely a factor that must be taken care of.

In my opinion, the developers of plasma-systemmonitor never thought about this issue. They use option 1 (i.e current). If the update interval is set to be a big value and your network traffic contains many large spikes, you will notice the area of ​​​​the line graph (i.e. the integral of speed value) is much lower than normal. Their downsampling is wrong.

LeeVD commented 1 year ago

But you can't say: After 1s, the traverse of network connections between the triggering of UI updates is 1000 bits/s + 2000bits/s * = 3000bits/s

You could if the output was 3000 bits right? In this case the output would be total traffic over a user set update interval and dropping the time series measurement unit?

CatEricka commented 1 year ago

You could if the output was 3000 bits right?

No it's not.

In this case the output would be total traffic over a user set update interval and dropping the time series measurement unit?

In my opinion, the only right way to display somethings use unit bits/s with modifiable updateInterval and meet requirement “i wanted to show the total that had traversed the network connections between the triggering of UI updates” is this ⬇️:

  • After 1s (because now updateInterval set to 1s, just like DBUS calls but longer), the average speed of this second is (1000bits/s * 0.5s + 2000bits/s * 0.5s) / 1s = 1500bits/s, because I downsample it from the sampling interval that once every 0.5s to the sampling interval that once every 1s, and keep the unit bits/s

This can satisfy the integral value of speed to time before and after downsampling remains unchanged. Both consume 1500bits in 1s.

CatEricka commented 1 year ago

I drew a picture (yes it's excel):

图片

LeeVD commented 1 year ago

ok, say i set the user interval to 5 seconds. DBUS grabs traffic bandwidth 10 times. For simplicity, lets say the below is what DBUS grabbed in that 5 second interval:

0 1 2 3 4 5 6 7 8 9 [1000], [2000], [3000], [4000], [5000], [6000], [7000], [8000], [9000], [10000]

To measure this accurately over bits per second the final result would be 5500 bits/s. (total * 0.5) / 5 right? The actual traffic that had traversed the link in the 5 second interval would have been 55000 bits.

I think both these are valuable results... another user settings option? Let the people decide what they want to see for themselves. It is a plasma widget after all :) I would have to correct for the unit value.

LeeVD commented 1 year ago

I drew a picture (yes it's excel):

图片

I do like a bit of excel :) Yes, i completely understand your point. I guess even the non-accumulated value would be wrong then? DBUS provides values based on 0.5sec intervals. if user interval is set to 0.5sec, showing this value with bits/s or b/s is incorrect as its only half a second. How should something like that be displayed?

CatEricka commented 1 year ago

To measure this accurately over bits per second the final result would be 27500 bits/s.

No, it's (1000bits/s * 0.5s + 2000bits/s * 0.5s + .... + 10000bits/s * 0.5s) / 5s = 5500 bits/s, and it's means over the elapsed 5 seconds, the average speed is 5500bits/s

LeeVD commented 1 year ago

To measure this accurately over bits per second the final result would be 27500 bits/s.

No, it's (1000bits/s * 0.5s + 2000bits/s * 0.5s + .... + 10000bits/s * 0.5s) / 5s = 5500 bits/s, and it's means over the elapsed 5 seconds, the average speed is 5500bits/s

yep, i just saw i messed up there and corrected it :)

CatEricka commented 1 year ago

I guess even the non-accumulated value would be wrong then? DBUS provides values based on 0.5sec intervals. if user interval is set to 0.5sec, showing this value with bits/s or b/s is incorrect as its only half a second. How should something like that be displayed?

I don't know :(

The correct way is to count the total traffic in the interval time first, and then divide it by the interval time (in this case, it's 0.5s) I can only assume that the developer knows what he's doing.

CatEricka commented 1 year ago

By the way, where is the source code of this?

https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/blob/main/package/contents/ui/imports/DbusModel/libqmldbusmodelplugin.so

I noticed that the license of this project is gpl3.0, and putting a blob without source code in the project does not seem to be the best practice for gpl3.0

LeeVD commented 1 year ago

I guess even the non-accumulated value would be wrong then? DBUS provides values based on 0.5sec intervals. if user interval is set to 0.5sec, showing this value with bits/s or b/s is incorrect as its only half a second. How should something like that be displayed?

I don't know :(

The correct way is to count the total traffic in the interval time first, and then divide it by the interval time (in this case, it's 0.5s) I can only assume that the developer knows what he's doing.

Ha! Dont be so sure, I'm making this up as i go along :D To me its a challenge, just seeing if i can do it.

LeeVD commented 1 year ago

By the way, where is the source code of this?

https://github.com/LeeVD/plasma-applet-net-bandwidth-monitor/blob/main/package/contents/ui/imports/DbusModel/libqmldbusmodelplugin.so

I noticed that the license of this project is gpl3.0, and putting a blob without source code in the project does not seem to be the best practice for gpl3.0

That is part of the second widget i based mine of, source code for that is on https://www.opencode.net/bstrong5280/system-monitor-plasmoid site.

CatEricka commented 1 year ago

DBUS provides values based on 0.5sec intervals. if user interval is set to 0.5sec, showing this value with bits/s or b/s is incorrect as its only half a second

图片

It looks like it works the right way. The values which provided by DBUS is indeed obtained by:

count the total traffic in the interval time (which is 0.5s) first, and then divide it by the interval time (in this case, it's 0.5s)

CatEricka commented 1 year ago

Aha. I found it.

https://github.com/KDE/ksystemstats/blob/master/src/plasma-ksystemstats.service.in

This is the daemon of this DBUS interface.

Edited:

And there a tool kstatsviewer:

https://github.com/KDE/ksystemstats/tree/master/tools/kstatsviewer

Yet another undocumented kde component :(

LeeVD commented 1 year ago

DBUS provides values based on 0.5sec intervals. if user interval is set to 0.5sec, showing this value with bits/s or b/s is incorrect as its only half a second

图片

It looks like it works the right way. The values which provided by DBUS is indeed obtained by:

count the total traffic in the interval time (which is 0.5s) first, and then divide it by the interval time (in this case, it's 0.5s)

Are the widgets shown both from 'system-monitor-plasmoid' by bstrong5280?
When looking at his code originally i saw him moving values around and doing math somethings with a count value. I swapped that work out for my own. I'll look at adding what we've discussed here into it.

LeeVD commented 1 year ago

kstatsviewer

Yes, kstatsviewer is good for getting dbus values. Used it when i was trying to work out dbus. bstrong5280 did most of the hard work for me already.

CatEricka commented 1 year ago

Are the widgets shown both from 'system-monitor-plasmoid' by bstrong5280?

It's plasma-systemmonitor, a part of standard KDE install.

CatEricka commented 1 year ago

https://github.com/KDE/ksystemstats/blob/ef1a5332f06c3c1f81441919cda7c457b6312dbf/plugins/network/NetworkManagerBackend.cpp#L20

This is the answer to the ultimate question of life, the universe, and everything. It's 500. Source code is documentation, I hate this.

CatEricka commented 1 year ago

Look at this:

https://github.com/KDE/ksystemstats/blob/ef1a5332f06c3c1f81441919cda7c457b6312dbf/plugins/network/NetworkManagerBackend.cpp#L68-L71

Someone definitely didn't expect someone to changed the cosmological constant:

github.com/KDE/ksystemstats/commit/1b8d4ee95ddb1797b315bd3470efe1b7a97fa346

I can't help but worry about what will happen if the speed of light changes again...

CatEricka commented 1 year ago

This also has a hardcoded:

constexpr auto UpdateRate = std::chrono::milliseconds{500};

https://github.com/KDE/ksystemstats/blob/ef1a5332f06c3c1f81441919cda7c457b6312dbf/src/daemon.cpp#L54-L55

The newSensorData signal is sent every UpdateRate milliseconds.

LeeVD commented 1 year ago

That would explain what i see in the DBUS data collection in this widget. What would you make it, 1000ms ?

CatEricka commented 1 year ago

It cannot be modified (maybe many codes rely on this behavior), just keep it as it is