Review Request: A usable system monitor plasmoid

[rhn rhn]

> On Feb. 19, 2012, 9:26 a.m., Aaron J. Seigo wrote:
> > aside from the memory leak you've already spotted and documented, i will need sometime to review this as it's not a small review request. buuut ... i like the idea. it's an "old school" monitor that i just know many of our desktop power users will love. :)
> > 
> > igve me a few days to play with it on my laptop, look over the code, etc and i'll post a thorough review then. cheers..

The memory leak note applies to m_signal_plotter. I "patched" the leak quick and dirty by deleting widgets inside layout on removal, but there is still a mistake somewhere there.
The problem is that I was getting different behavior when a LinearLayout was used. The LinearLayout deleted my widget and caused a double free if I deleted the widget together with CPULoad, so I stopped doing it. IconGridLayout OTOH (predecessor to my RigidGridLayout) didn't delete them. I don't know how to solve that situation :(


- rhn


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104015/#review10739
-----------------------------------------------------------


On Feb. 18, 2012, 6:16 p.m., rhn rhn wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104015/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2012, 6:16 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> A plasmoid intended to be used on a panel. Inspired by gnome-multiload-applet and gkrellm.
> 
> Before I decided to write this plasmoid, I considered other monitors. Here's what I found:
> - system-monitor:
>    * not pixel-perfect: it uses antialiasing and Bezier curves
>    * in stacking mode, it adds a few pixels to all values, so a 0 network throughput still shows 2 belts.
>    * impossible to read at a glance due to low contrast and no custom colors
>    * shows only basic properties
>    * graphs change features depending on size
>    * deal-breaker: impossible to set custom height or width
> - system-load-viewer: almost there, but no history display
> - The Only Right RAM Monitor: too slow and RAM only
> 
> This plasmoid attempts to solve all those problems while introducing new ones :)
> What's fixed in this one:
> - many graphs in one applet
> - graphs laid out on a grid
> - forces graph width or height (depending on mode)
> - can force row or column number (depending on mode)
> - arbitrary colors for all graph types
> - pixel-perfect
> - handles runtime CPU/network addition/removal (CPU untested)
> 
> Other stuff:
> - labels only for network interfaces
> - resizable labels
> - network transmit graph stacks on top
> - C++, a monitor should measure resources, not use them
> - no way to disable graphs :)
> - no way to reorder graphs
> - network interfaces are not sorted
> - all CPUs are displayed
> - can't change colors for a distinct graph
> - will not fit in container if the wrong layout params are selected (forcing a dimension to be more than available)
> - the original idea was to synchronize all readings, but it seems impossible with plasma data engines
> 
> It's my first project in Qt, so the code will not be as nice as it could be. I rewrote the plotter and the layout, so those are the most likely to contain errors/memory leaks. Also, I don't know which of the above problems need to be solved and which decisions are acceptable, but I'll happily apply all fixes to make this accepted.
> 
> 
> Diffs
> -----
> 
>   applets/CMakeLists.txt 1e1ef75 
>   applets/graphmon/CMakeLists.txt PRE-CREATION 
>   applets/graphmon/color-config.ui PRE-CREATION 
>   applets/graphmon/cpuload.h PRE-CREATION 
>   applets/graphmon/cpuload.cpp PRE-CREATION 
>   applets/graphmon/general-config.ui PRE-CREATION 
>   applets/graphmon/graphmon.h PRE-CREATION 
>   applets/graphmon/graphmon.cpp PRE-CREATION 
>   applets/graphmon/netthroughput.h PRE-CREATION 
>   applets/graphmon/netthroughput.cpp PRE-CREATION 
>   applets/graphmon/plasma-applet-graphmon.desktop PRE-CREATION 
>   applets/graphmon/ramusage.h PRE-CREATION 
>   applets/graphmon/ramusage.cpp PRE-CREATION 
>   applets/graphmon/rigidgridlayout.h PRE-CREATION 
>   applets/graphmon/rigidgridlayout.cpp PRE-CREATION 
>   applets/graphmon/simpleplotter.h PRE-CREATION 
>   applets/graphmon/simpleplotter.cpp PRE-CREATION 
>   applets/graphmon/swapusage.h PRE-CREATION 
>   applets/graphmon/swapusage.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104015/diff/
> 
> 
> Testing
> -------
> 
> Display, layout, configuration, persistence.
> 
> 
> Screenshots
> -----------
> 
> Graph on panel and general config section
>   http://git.reviewboard.kde.org/r/104015/s/437/
> 
> 
> Thanks,
> 
> rhn rhn
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120219/c92601bd/attachment.html>