----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5850/#review8866 -----------------------------------------------------------
Thanks for the patch, however, while I was away I coded an almost identical one, which also uses static backends as discussed with Kevin. I'll commit it in the weekend. - Dario On 2010-11-15 13:17:54, Alex Merry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/5850/ > ----------------------------------------------------------- > > (Updated 2010-11-15 13:17:54) > > > 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 > >
_______________________________________________ Kde-hardware-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-hardware-devel
