> On Oct. 2, 2012, 3:14 p.m., Dario Freddi wrote: > > powerdevil/daemon/powerdevilcore.cpp, lines 589-611 > > <http://git.reviewboard.kde.org/r/106686/diff/2/?file=88099#file88099line589> > > > > You don't need that much logic - when batteryChargeStateChanged is > > emitted it is guaranteed the state is not the same as it was before. So you > > could simply fire the notification if the state went to NoCharge and AC is > > plugged. Test this solution - if you get false positives we might add more > > checks, but I am pretty sure the AC plugged check is more than enough for > > making it reliable. > > Kai Uwe Broulik wrote: > I tried it again and it seems that (m_backend->acAdapterState() == > BackendInterface::Plugged) is always false when this slot is triggered. > Without it my new logic works fine. > > Dario Freddi wrote: > Indeed, I am not saying your logic is wrong (apart from the comment > below) - it's simply not needed, as the checks you are doing are already > implied in the emission of the slot. It really surprises me it works though, > the problem I outlined below should prevent it from working in any situation > in theory, or maybe I completely overlooked something.
I mean, I cleaned up the logic and made it simpler (with the loop, so it checks if *all* batteries are charged). It works, but the check for AC adapter plugged in always fails, so the notification is not issued. But that should really only show up if the adapter is plugged in, to prevent false positives (e.g. NoCharge can also mean error, or sth else went wrong) - Kai Uwe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106686/#review19760 ----------------------------------------------------------- On Oct. 2, 2012, 11:21 a.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106686/ > ----------------------------------------------------------- > > (Updated Oct. 2, 2012, 11:21 a.m.) > > > Review request for Solid and Dario Freddi. > > > Description > ------- > > This patch fixes battery notification not being emitted due to early return, > and makes it watch for a change in ChargeState rather than the charge > percentage which might not be reached at all. > > > Diffs > ----- > > powerdevil/daemon/powerdevilcore.h 12e4b2c > powerdevil/daemon/powerdevilcore.cpp b968d21 > > Diff: http://git.reviewboard.kde.org/r/106686/diff/ > > > Testing > ------- > > Compiles. > Did not test if notification is properly emited, will do. Just wanted to know > if this is the right approach. > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Kde-hardware-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-hardware-devel
