[Kde-hardware-devel] Review Request: Fix backend loading for powerdevil

[Alex Merry]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5850/
-----------------------------------------------------------

Review request for Solid and Dario Freddi.


Summary
-------

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.


Diffs
-----

  /trunk/KDE/kdebase/workspace/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp 1196931 
  /trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilbackendinterface.h 1196931 
  /trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilbackendinterface.cpp 1196931 
  /trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilcore.h 1196931 
  /trunk/KDE/kdebase/workspace/powerdevil/daemon/powerdevilcore.cpp 1196931 

Diff: http://svn.reviewboard.kde.org/r/5850/diff


Testing
-------

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


Thanks,

Alex

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20101115/f298c5c3/attachment.htm