[Kde-hardware-devel] Review Request: Fix bug: generateProfiles generates a config where lidClose event is turn of screen.

[Harald Sitter]

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



powerdevil/daemon/powerdevilprofilegenerator.h
<http://git.reviewboard.kde.org/r/106859/#comment16091>

    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...)


- Harald Sitter


On Oct. 14, 2012, 11:20 p.m., Alex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106859/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2012, 11:20 p.m.)
> 
> 
> Review request for Solid.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   powerdevil/daemon/powerdevilcore.cpp b968d21 
>   powerdevil/daemon/powerdevilprofilegenerator.h a909294 
>   powerdevil/daemon/powerdevilprofilegenerator.cpp 060fc30 
>   powerdevil/kcmodule/profiles/EditPage.cpp 95a98d9 
> 
> Diff: http://git.reviewboard.kde.org/r/106859/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Fiestas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20121014/856a0abb/attachment-0001.html>