Use NetworkManager::DeviceStatistics instead of Plasma data engine
ClosedPublic

Authored by volkov on Aug 14 2018, 2:09 PM.

Details

Summary

It's more correct to get information about devices from a single
source, i.e. from NetworkManager. Besides it allows to show the
total number of received/transmitted bytes instead of the current
speed, which is more useful for the user. The current speed can
still be seen on the graph.

This change requires NetworkManagerQt 5.50, because of the fixed
DevicesStatistics::setRefreshRateMs().

Diff Detail

Repository
R116 Plasma Network Management Applet
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1874
Build 1892: arc lint + arc unit
volkov created this revision.Aug 14 2018, 2:09 PM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 14 2018, 2:09 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
volkov requested review of this revision.Aug 14 2018, 2:09 PM
broulik added inline comments.
libs/models/networkmodel.cpp
220

Would it be better to only request the data when the statistics are open to avoid wakeups and processing?

volkov added inline comments.Aug 14 2018, 2:26 PM
libs/models/networkmodel.cpp
220

Yes, good point.

volkov updated this revision to Diff 39790.Aug 15 2018, 11:50 AM

update device statistics only when it's shown

volkov marked an inline comment as done.Aug 15 2018, 11:52 AM
jgrulich accepted this revision.Aug 16 2018, 10:16 AM

Looks good to me. I was thinking about using it already some time ago, but didn't really find time to change that so thank you for implementing it. Actually, I think it was not possible before as we supported older NM versions where device statistics interface was not available yet.

This revision is now accepted and ready to land.Aug 16 2018, 10:16 AM
jgrulich requested changes to this revision.Aug 16 2018, 10:19 AM

Hmm, taking it back, actually this cannot be used as it requires NM 1.4 and newer and we require NM 1.0 and newer.

This revision now requires changes to proceed.Aug 16 2018, 10:19 AM

I assume we can bump requirements to NM 1.4, even Debian stable is now using NM 1.6.

Unfortunately KDE Neon is based on Ubuntu 16.04 (Xenial) which is using NM 1.2: https://packages.ubuntu.com/source/xenial/network-manager

volkov updated this revision to Diff 40632.Aug 29 2018, 11:13 AM

minimize the number of setDeviceStatisticsRefreshRateMs() callings

Can we get some opinions from KDE Neon people? I have older review to bump NM requirements to 1.4.0 here https://phabricator.kde.org/D12493.

Thanks for asking. This is fine with us, the ubuntu 16.04 base is going away in due time.

Another thing is, can Plasma 5.14 depend on KDE Frameworks 5.50?

Another thing is, can Plasma 5.14 depend on KDE Frameworks 5.50?

Yes. According to [1] the dependency is 5.50

[1] https://community.kde.org/Schedules/Plasma_5

jgrulich accepted this revision.Aug 30 2018, 11:19 AM
This revision is now accepted and ready to land.Aug 30 2018, 11:19 AM
volkov closed this revision.Aug 31 2018, 11:07 AM