Review Request: various fix for battery plasmoid

[Alex Fiestas]

On Saturday 12 January 2013 00:53:10 Xuetian Weng wrote:
> > On Jan. 12, 2013, 12:18 a.m., Kai Uwe Broulik wrote:
> > > plasma/generic/applets/batterymonitor/contents/code/logic.js, line 97
> > > <http://git.reviewboard.kde.org/r/108355/diff/2/?file=106715#file106715l
> > > ine97>> > 
> > >     But isn't it the dataengine that prematurely triggers a brightnes
> > >     change and OSD? When I restart plasma or use plasmaengineexplorer
> > >     and access the PM dataengine, the screen brightness changes and the
> > >     OSD appears. Shouldn't it be fixed inside the dataengine?
> Restart Plasma shows OSD because you have battery plasmoid.
> I don't see plasmaengineexplorer shows osd, unless you use setBrightness
> service.
> 
> The reason of this bug is, Slider have a value, when it's not equal to
> current brightness, it will trigger valueChanged, and then trigger
> brightnessChanged and send a call to dataengine service. If brightness
> value is not changed from user operation on slider, it should never trigger
> this call.
> 
> If the fix in dataengine you mentioned is:
> dataengine check the current brightness and argument of setBrightness
> service is same or not, to decide whether to call powerdevil or not.
> 
> I don't think it's the right approach, even if it can solve the problem.
> Reason is, real brightness is saved on powerdevil side, which is a
> different process. The value in dataengine is only cached value. If
> setBrightness comes, there is no guarantee that this value is up-to-date
> (even it's just theoretically possible), so if setBrightness comes, it's
> always safe to send a real request to powerdevil. And the value send to
> powerdevil will cause powerdevil to set current brightness is explicit set,
> and do a lot of corresponding process, which should never be ignored just
> because a cached value is the current value.

Please, backport the fix of the race condition to 4.9 and send an email to 
kde-packagers indicating that they should include this patch, it is quite 
nasty having an infinite loop of brightness change.

Thanks.