Redesign of system monitor plasmoids
ClosedPublic

Authored by mart on Apr 1 2020, 2:57 PM.

Details

Summary

Those plasmoids are intended to replace the old systemmonitor plasmoids
They are based upon the new ksysguard daemon: see D28333 and D28141

It has pluggable presets and sensor "faces" which are available from the KDE store

Every preset is available as a separate plasmoid.
By default are installed ones to replace
roughly one by one the existing systemmonitor plasmoids so systems that use it
will get the new ones in the updates

Depends on D28922

Test Plan

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Apr 1 2020, 2:57 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 1 2020, 2:57 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart requested review of this revision.Apr 1 2020, 2:57 PM
mart planned changes to this revision.Apr 1 2020, 2:58 PM
mart updated this revision to Diff 79058.Apr 1 2020, 3:07 PM
  • remove useless debug
mart edited the test plan for this revision. (Show Details)Apr 1 2020, 3:13 PM
mart retitled this revision from upstream of the ksysguard based plasmoids to Upstream of the ksysguard based plasmoids.
mart added a reviewer: Plasma.
mart added inline comments.Apr 1 2020, 3:17 PM
applets/systemmonitor/systemmonitor/faces/barchart/metadata.desktop
11 ↗(On Diff #79058)

still org.kde.ksysguard plugin name prefix, or org.kde.plasma.systemmonitor as well?

mart added a comment.Apr 1 2020, 3:37 PM

should faces be still installed under .local/share/ksysguard/sensorapplet/ or under share/plasma?

In D28487#639476, @mart wrote:

should faces be still installed under .local/share/ksysguard/sensorapplet/ or under share/plasma?

Ideally, I'd like to see them installed into share/ksysguard/faces or something similar. I want to try and reuse them for custom pages in the new KSysGuard UI.

mmustac added a subscriber: mmustac.Apr 1 2020, 7:55 PM

Those look very lovely.

mart added a comment.Apr 2 2020, 7:58 AM
In D28487#639476, @mart wrote:

should faces be still installed under .local/share/ksysguard/sensorapplet/ or under share/plasma?

Ideally, I'd like to see them installed into share/ksysguard/faces or something similar. I want to try and reuse them for custom pages in the new KSysGuard UI.

a problem in there is that faces do access plasmoid.*.. it may require quite some refactorto be able to do that but i may gie a try

davidedmundson added inline comments.
shell/scripting/applet.h
29 ↗(On Diff #79058)

Can we split all the changes in shell/*

mart updated this revision to Diff 79120.Apr 2 2020, 9:42 AM
  • remove changes in shell/
mart marked an inline comment as done.Apr 2 2020, 9:43 AM
mart retitled this revision from Upstream of the ksysguard based plasmoids to REdesign of system monitor plasmoids.Apr 2 2020, 12:22 PM
mart retitled this revision from REdesign of system monitor plasmoids to Redesign of system monitor plasmoids.
mart updated this revision to Diff 80403.Apr 17 2020, 3:23 PM
  • port to the new library
mart updated this revision to Diff 81020.Apr 23 2020, 4:46 PM
  • use the new face library
mart updated this revision to Diff 81077.Apr 24 2020, 9:32 AM
  • move faces and packagestructure to libksysguard
ksmanis added a subscriber: ksmanis.May 1 2020, 4:48 PM

Is there an ETA for this? Is it worth fixing any bugs in the old applets in the meantime?

ngraham added a subscriber: ngraham.May 1 2020, 4:52 PM

Is there an ETA for this?

Hopefully Plasma 5.19! But given the closeness to the release, we might end up punting it until 5.20. Either way, one of those two.

Is it worth fixing any bugs in the old applets in the meantime?

Doubtful IMO.

Is there an ETA for this? Is it worth fixing any bugs in the old applets in the meantime?

We'd really like this in 5.19, but it's a pretty large set of changes in multiple parts of the stack. If you want it sooner extra reviews would be nice. :)

Looks fantastic!

However I've applied the dependent patches, but this doesn't compile for me:

 In member function ‘virtual void SystemMonitor::init()’:
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:63:78: error: invalid use of incomplete type ‘class SensorFaceController’
   63 |     m_sensorFaceController = new SensorFaceController(cg, qmlObject->engine());
      |                                                                              ^
In file included from /home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:20:
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.h:33:7: note: forward declaration of ‘class SensorFaceController’
   33 | class SensorFaceController;
      |       ^~~~~~~~~~~~~~~~~~~~
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:67:31: error: invalid use of incomplete type ‘class SensorFaceController’
   67 |         m_sensorFaceController->loadPreset(m_pendingStartupPreset);
      |                               ^~
In file included from /home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:20:
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.h:33:7: note: forward declaration of ‘class SensorFaceController’
   33 | class SensorFaceController;
      |       ^~~~~~~~~~~~~~~~~~~~
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:71:31: error: invalid use of incomplete type ‘class SensorFaceController’
   71 |         m_sensorFaceController->loadPreset(preset);
      |                               ^~
In file included from /home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:20:
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.h:33:7: note: forward declaration of ‘class SensorFaceController’
   33 | class SensorFaceController;
      |       ^~~~~~~~~~~~~~~~~~~~
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp: In member function ‘virtual void SystemMonitor::configChanged()’:
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:83:31: error: invalid use of incomplete type ‘class SensorFaceController’
   83 |         m_sensorFaceController->reloadConfig();
      |                               ^~
In file included from /home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.cpp:20:
/home/nate/kde/src/plasma-workspace/applets/systemmonitor/systemmonitor/systemmonitor.h:33:7: note: forward declaration of ‘class SensorFaceController’
   33 | class SensorFaceController;
      |       ^~~~~~~~~~~~~~~~~~~~

Also the test plan screenshot shows a string overlap issue:

alexde added a subscriber: alexde.May 4 2020, 3:28 PM

Not sure, if this is part of the scope of this work, but do you also plan to add some meaningful labels and ticks to the axis?

ngraham added inline comments.May 4 2020, 4:46 PM
applets/systemmonitor/systemmonitor/package/contents/ui/config/ConfigAppearance.qml
24

as QQC2

applets/systemmonitor/systemmonitor/package/contents/ui/config/ConfigSensors.qml
25

as QQC2

applets/systemmonitor/systemmonitor/package/contents/ui/config/FaceDetails.qml
24

as QQC2

mart updated this revision to Diff 82117.May 6 2020, 3:59 PM
  • adapt to api changes
  • move faces config in own file
mart updated this revision to Diff 82118.May 6 2020, 4:03 PM
  • Controls QQC2
ksmanis removed a subscriber: ksmanis.Wed, May 6, 8:44 PM
mart updated this revision to Diff 82187.Thu, May 7, 10:07 AM
mart marked 3 inline comments as done.
  • make disk usage a bar chart
mart updated this revision to Diff 82224.Thu, May 7, 5:11 PM
  • adapt to api changes
mart updated this revision to Diff 82287.Fri, May 8, 4:35 PM
  • Merge branch 'master' into mart/mewSystemMonitor

When I create a new System Monitor Sensor widget, it's totally blank:

Needs to have a Configure... button on it in this case.

This revision was not accepted when it landed; it landed in state Needs Review.Mon, May 11, 11:09 AM
This revision was automatically updated to reflect the committed changes.