[Kde-hardware-devel] Review Request: Make threshold for when the battery is considered full configurable (like you can with crtiical/low)

[Kai Uwe Broulik]

> On Oct. 2, 2012, 7:23 a.m., Dario Freddi wrote:
> > I am afraid to say I don't really understand why this patch would be useful - we've been working quite hard to remove useless config options and frankly speaking, this seems to be one of them. Could you please explain why you would need such a switch? If it is because some batteries got broken and don't charge up to 100% anymore, we can detect this and trigger the notification at the right threshold. But I am afraid to say unless there is an actual good reason for adding such an option, the patch is a no-go - it is also very, very confusing to the average user, who might understand the option in the wrongest way.

Okay, you’re right. It would probably better monitoring when the ChargeState turns from Charging to NoCharge while the AC adapter is plugged in?


> On Oct. 2, 2012, 7:23 a.m., Dario Freddi wrote:
> > powerdevil/daemon/powerdevilcore.cpp, lines 543-545
> > <http://git.reviewboard.kde.org/r/106677/diff/1/?file=88046#file88046line543>
> >
> >     That's the reason why your patch doesn't work :)

Ah, sure. :)


- Kai Uwe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106677/#review19726
-----------------------------------------------------------


On Oct. 1, 2012, 10:30 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106677/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2012, 10:30 p.m.)
> 
> 
> Review request for Solid.
> 
> 
> Description
> -------
> 
> This patch adds the config option to set the threshold when the "Battery is full" notification should appear. Default is 100%.
> 
> 
> This addresses bug 223553.
>     http://bugs.kde.org/show_bug.cgi?id=223553
> 
> 
> Diffs
> -----
> 
>   powerdevil/PowerDevilSettings.kcfg cd103c6 
>   powerdevil/daemon/powerdevilcore.cpp b968d21 
>   powerdevil/kcmodule/global/GeneralPage.cpp 5ff3f4d 
>   powerdevil/kcmodule/global/generalPage.ui 7840526 
> 
> Diff: http://git.reviewboard.kde.org/r/106677/diff/
> 
> 
> Testing
> -------
> 
> Config option works.
> But the notification doesnt’t get triggered (also not without that patch). What did I miss in the if that checks for the battery percentage? Any idea?
> 
> 
> Screenshots
> -----------
> 
> KCM with third option (ordered descending)
>   http://git.reviewboard.kde.org/r/106677/s/751/
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20121002/df5168bb/attachment.html>