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

[Commit Hook]

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


This review has been submitted with commit 86465c2c88e110f85bd5f750663944d827a8869d by Alex Fiestas to branch master.

- Commit Hook


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/20121023/72ac02b6/attachment.html>