battery shrinking problem in battery plasmoid

[Sebastian Kügler]

On Monday 02 February 2009 23:55:25 matthias sweertvaegher wrote:
> dear plasma devs,

Hi and welcome to Plasma development!

> First, thanks for having the guts to create a whole new
> rearchitectured desktop environment from scratch. :)
> also I guess for devs, it must be extra motivating to be able to
> contribute to still such a "pure" project. 

Yup, the pristine code base and not having to deal with a lot of old cruft is 
very nice to have. But that's true for most of KDE actually, since 4.0 allowed 
us to clean up big time. :)

> I for one, certainly do feel some itch ;)

Goooood :>

> anyway, in trying to get involved with my number one favorite FOSS
> project (KDE of course ;)), I am starting an effort in trying to fix
> my own pet bugs =) One down in the ksysguard, next is battery applet
>
> :)

two down :)

> Here is the bug report:
> https://bugs.kde.org/show_bug.cgi?id=178116
>
> around new year it got reassigned to plasma-devel, so here I am :)
>
> When running the svn version, you will notice a warning about a wrong
> way of disconnecting a SLOT. i.e.
> Object::disconnect: Attempt to unbind non-signal
> Battery::sourceAdded(QString) Object::disconnect: Attempt to unbind
> non-signal Battery::sourceRemoved(QString)
>
> this happens in battery.cpp, void Battery::disconnectSources(), which
> is called when the configuration dialog is closed.
>
> So, first thing I did, was correcting
>
>     disconnect(SLOT(sourceAdded(QString)));
>     disconnect(SLOT(sourceRemoved(QString)));
>
> to
>
>     disconnect(this, SLOT(sourceAdded(QString)));
>     disconnect(this, SLOT(sourceRemoved(QString)));
>
> which I think what was meant?

Yes, good catch. I've committed that change.

> So, no warning anymore. But still a bug! I tried figuring it out, but
> since I am totally new to plasma, let alone, KDE development, I
> thought I shouldn't be embarassed to ask some help ;) So here I am.

Surely not, sharing knowledge is good :)

> Here are my current findings:
> - the bug only occurs after having used the configuration dialog
> - seems disconnecting doesn't work? the plasmoid thinks there are more
> batteries ( I printed the battery_num var), but only 2 valid ones are
> detected (which is correct of course)
>
> why does the reconnection have to happen anyway? so, of course, when I
> disable the disconnect/reconnect code after the configuration is done,
> everything is fine (except maybe some memory leak I'm not aware off
> ;)). I tried debugging the events, but I don't manage to pull a lot of
> information out of it. So, I'm stuck.

Apparently the sourceAdded() connection wasn't disconnected correctly for all 
battery sources. The reconnect thing is probably some relic of the past, and 
it shouldn't be needed anymore. Your solution of removing the disconnect magic 
sounds right to me, and it indeed fixes #178116. It also results in a patch 
with

 workspace/plasma/applets/battery/battery.cpp |   19 -------------------
 workspace/plasma/applets/battery/battery.h   |    1 -
 2 files changed, 0 insertions(+), 20 deletions(-)

Which is the best kind of bugfix (without losing functionality) there is. :)

I've committed (and effectively undid your previous change), thanks for 
catching this. We should probably backport it, but I'd first like to run with 
it in trunk and see if anyone notes problems with this cleanup.

> please provide some insight

Hope I did, thanks for providing insight as well :)

> greets
> matthias
> btw, can somebody explain what's the deal with applets? I thought they
> were called plasmoids today and that the word applet was forbidden now
> ;) but in the sourcetree there are all kinds of references to applet.
> Is it just fashion?

I'll leave that one to someone else :P
-- 
sebas

 http://www.kde.org | http://vizZzion.org |  GPG Key ID: 9119 0EF9