<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://git.reviewboard.kde.org/r/106859/">http://git.reviewboard.kde.org/r/106859/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/106859/diff/1/?file=89940#file89940line58" style="color: black; font-weight: bold; text-decoration: underline;">powerdevil/daemon/powerdevilprofilegenerator.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace ProfileGenerator</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">58</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">upgradeProfilesv2</span><span class="p">();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">58</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">upgradeProfilesv2</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">for the sake of making the code more readable I'd either add toRam and toDisk and Q_UNUSED them or add a comment as to why they are not passed to the v2 upgrade.
otherwise one needs to look at the function to understand why the arguments are not needed (and that they are in fact not needed...)</pre>
</div>
<br />
<p>- Harald</p>
<br />
<p>On October 14th, 2012, 11:20 p.m., Alex Fiestas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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.</div>
<div>By Alex Fiestas.</div>
<p style="color: grey;"><i>Updated Oct. 14, 2012, 11:20 p.m.</i></p>
<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;">The bug is produced because of the following code:
if (methods.contains(Solid::PowerManagement::SuspendState)) {
handleButtonEvents.writeEntry< uint >("lidAction", ToRamMode);
} else {
handleButtonEvents.writeEntry< uint >("lidAction", TurnOffScreenMode);
}
There, SuspendState will NOT be contained in method, so we will fallback to TurnOffScreenMode.
LibSolid is always returning an empty "methods" (QList of SuspendMethods). LibSolid uses powerdevil
dbus api to know which suspend methods are available:
libsolid---dbus--->powerdevl-----backend---->UDisk
The problem is that the DBus interface of powerdevil is being setup AFTER generateProfile is executed, so libsolid
will fail to query powerdevil for the available suspend methods because powerdevil haven't made them public yet.
The approach in the fix:
There are many ways of fixing this, the simpler one is to execute the dbus registering before generateProfiles is executed,
the others are based on the same principle of moving the current code around to make libsolid work.
The reason why I went for a different approach is because it is architecturally wrong to have an interdependence:
PowerDevil------->libsolid------->PowerDevil.
No matter how careful we are, having this interdependence is going to cause problems sooner or later.
So, what I have done to keep the patch small is add 2 new arguments to generateProfiles (toRam and toDisk) so we don't
have to call libsolid in order to get this info.
I had to add these arguments instead of using PowerDevil::BackendInfo because generateProfiles is also used in EditPage.</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>powerdevil/daemon/powerdevilcore.cpp <span style="color: grey">(b968d21)</span></li>
<li>powerdevil/daemon/powerdevilprofilegenerator.h <span style="color: grey">(a909294)</span></li>
<li>powerdevil/daemon/powerdevilprofilegenerator.cpp <span style="color: grey">(060fc30)</span></li>
<li>powerdevil/kcmodule/profiles/EditPage.cpp <span style="color: grey">(95a98d9)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106859/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>