Review Request: Add suspend-signal to powerdevil and make plasma time dataengine detecting clock skrews

[Sebastian Kügler]

On Monday, September 13, 2010 18:52:03 Björn Ruberg wrote:
> > On 2010-09-13 16:44:03, Dario Freddi wrote:
> > > First of all, thanks for looking into this and most of all for
> > > submitting a patch.
> > > 
> > > Unfortunately, just like Sebastian said, I'm afraid I'm not willing to
> > > let this patch in. However, in the upcoming solid sprint I plan to
> > > address this. Luckily, UPower has a signal which does exactly what you
> > > tried to achieve here. I will get in touch with you as soon as I will
> > > implement the needed part in PowerDevil/Solid so you can code another
> > > patch against the new method. How does this sound to you?
> 
> That's sounds good. But this patch does not mean that the problem can never
> be fixed correctly. Maybe this can be seen as a fix for the 4.5 branch
> while you add the correct functionality for 4.6?

I'd rather not have it in high-level Plasma components since it's essentially 
working around a limitation in HAL. There are a couple of problems with 
putting it in powerdevil as is:

- the DBus method you expose is only valid for the HAL backend, if someone 
uses the (future) upower backend, you'd still use this workaround. Also, app 
developers might start to rely on this behaviour.
- DBus interfaces are to be regarded as public API, therefore we cannot just 
change them
- the workaround is in the wrong layer of the stack, the app (timeengine in 
this case) shouldn't work around limitations which we need to fix below in the 
stack (i.e. in Solid)

Maybe we can put this workaround into the hal backend itself, and have it emit 
resumed() when it detects this clock skew. The upower backend would then emit 
the same signal, but triggered in the correct way. The applications get the 
hotness in a transparant manner and can adapt (i.e. our timeengine fires 
updates). 

This doesn't solve the problem that we're adding public API (the DBus method 
in powerdevil), but doing it in this future-proof (yey, right!) way would make 
it more acceptable IMO. You'll need to get it past Kevin, though. >:)

> On 2010-09-13 11:20:38, Björn Ruberg wrote:
> > http://svn.reviewboard.kde.org/r/5320/

> > NOTE: This a little bit ugly as I have to workaround the missing "going
> > to suspend"-signal in the linux userland
> > 
> > This patch adds a signal to powerdevil that is emitted when a
> > suspend/hibernate/standby is requested there. The signal is caught by
> > the time engine what starts to look for an unexpected change in system
> > time for two minutes by polling. If a clock skrew is detected, all
> > sources are updated immediatly.
> > 
> > This fixes the bug that the plasma clocks are showing old time after
> > suspending your machine up until 59 seconds (whenever the normal update
> > is scheduled). This is not exactly a patch for bug 181380. But the clock
> > skrew detection code can be used quite similar for detecting normal time
> > changes. I'll look into that as soon this is accepted.
> > 
> > Please tell me whether I can commit this to 4.5 trunk as it is a bugfix
> > for a nasty glitch.
> > 
> > 
> > This addresses bug 181380.
> > 
> >     https://bugs.kde.org/show_bug.cgi?id=181380

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9