<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5850/">http://svn.reviewboard.kde.org/r/5850/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Solid and Dario Freddi.</div>
<div>By Alex Merry.</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">First of all, note that while I am sure this is the right thing to do, I'm not sure the architectural details in the patch are correct (using an isValid() method - emitting a signal like backendUnavailable might be better, for example).
The current backend loading for the powerdevil daemon is clearly broken - it loads all the backends, and ends up just initialising the lowest-priority one. If that one doesn't work, it gives up (although, in fact, none of the backends actually report failing to initialise, even if the infrastructure they require isn't available).
What should happen (IMO) is that it should take the first backend and try to initialise it. If that backend reports a failure, it should try the next backend, and so on. If all the backends fail to load (or it can't find any backends), only at that point should it give up. It should probably then load a dummy backend so that it won't crash kded (I haven't included this part in the patch).
The patch has an extra stage - it does a quick immediate check to a method on the backend called isValid() before trying to initialise the backend. This is meant to check whether the infrastructure for the backend is available. If this returns false, it moves on to the next backend, but without reporting an error to the user (they don't want to be told that UPower is not available every time they log in if HAL is available).
The UPower backend checks whether the UPower service is available, and sets the "valid" property appropriately. The hal backend (which could more accurately be called the Solid backend) just assumes that Solid has a working backend of some description, so never reports being invalid. This is mainly because there seems to be no way to check that Solid can give sensible information.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested to make sure it does the Right Thing(TM) with IPower installed and with UPower uninstalled (ie: loads the UPower backend if and only if UPower is available).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp <span style="color: grey">(1196931)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilbackendinterface.h <span style="color: grey">(1196931)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilbackendinterface.cpp <span style="color: grey">(1196931)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilcore.h <span style="color: grey">(1196931)</span></li>
<li>/trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilcore.cpp <span style="color: grey">(1196931)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5850/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>