[Kde-hardware-devel] PowerManager Library

[Lamarque V. Souza]

Em Wednesday 20 June 2012, Daniel Nicoletti escreveu:
> Hi,
> 
> recently I reported a bug as the PowerManager library
> was reporting the wrong PowerSaveStatus, Lukas took
> a look, saw nothing wrong but proposed a patch, that well
> sort of worked. After doing several tests with Apped KDED
> module I realized the problem was when the DBus
> interface was not available since Apper is loaded before
> PM kded module.
> 
> So I decided to look at the code to find the issue, and
> well there are actually some:
> * QDBusPendingReply doesn't provide a default value
> in case the call fails, which is why it returned true
> when PM interface wasn't available, which in case the
> best default would of course be false.
> * If KDED dies for some reason (we can't think it must
> work, bugs happen), it doesn't watch for service registration
> so it won't update the client if the power save mode
> has changed.
> 
> To the first I have two ways of fixing it, create a function
> for cold plug which checks if the reply is valid otherwise
> defaults to false. OR actually have the values at
> the private class declared as QDBusPendingReply, which
> when the user calls appShouldConserveResources() we
> check if the reply is valid and if not we try getting the
> value again, otherwise just return.
> 
> For the second it's a bit more complex, since the current
> service watcher need to watch another path, and will
> need to cold plug everything if the service is registered.
> 
> I started coding a cold plug function but well the code will
> be quite big, so depending on the preferred approach I'll
> do my codings
> 
> QDBusPendingReply<bool> psReply = managerIface.GetPowerSaveStatus();
>     reply.waitForFinished();

	Avoid using waitForFinished or QDBusReply, they can cause freezes in the 
desktop. Always try the asynchronous approach first. Unfortunately in this 
case we will need them to retrieve the initial status of some variables. 
However, restrict the usage of waitForFinished and QDBusReply to early 
initialization. Do not use them in slots that can be called several times 
during the lifetime of the class.

>     bool psValue = psReply.isValid() ? psReply.value() : false;
>     if (psValue != powerSaveStatus) {
>         slotPowerSaveStatusChanged(psValue);
>     }
> 
> this is for only one of the values, there are more 2,
> so well, the patch will be quite intrusive...

	Well, I created the attached patch to fix the problem Lukas described 
yesterday on #solid. Can you test it for me? If you wish you can add it to 
git.reviewboard.kde.org too.

-- 
Lamarque V. Souza
KDE's Network Management maintainer
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20120620/05f22708/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdelibs-4.8.4-fix_powermangement.diff
Type: text/x-patch
Size: 10334 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20120620/05f22708/attachment-0001.diff>