----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116481/#review51546 -----------------------------------------------------------
Thanks for looking into this, I have a few code-styling related issues, see below. In the future I want to get rid of all of this, especially the magic ints instead of enums, and move that stuff to Solid - where it's already sort-of duplicated - and especially provide charge/discharge time reporting for individual batteries. a/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp <https://git.reviewboard.kde.org/r/116481/#comment36389> Pedantic, but please use camelCase for those variables a/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp <https://git.reviewboard.kde.org/r/116481/#comment36391> Please always enclose, even one-liners, in curly braces: if (state == 1) { and below a/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp <https://git.reviewboard.kde.org/r/116481/#comment36394> if (…) { - Kai Uwe Broulik On Feb. 28, 2014, 1:53 p.m., Joschi Brauchle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116481/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2014, 1:53 p.m.) > > > Review request for kde-workspace and Kai Uwe Broulik. > > > Bugs: https://bugs.kde.org/show_bug.cgi?id=253453 > > http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=253453 > > > Repository: kde-workspace > > > Description > ------- > > Currently powerdevil uses the "time-to-empty" property provided by upower to > add up battery life for all installed batteries. > Unfortunately, upower 0.9.x does not provide a "time-to-empty" for fully > charged batteries. > Hence, the remaining battery lifetime displayed by powerdevil considers only > batteries which are currently being discharged and ignores all fully charged > batteries. > > The proposed patch is taken from upower 0.99.x and sums up the total energy > available in all installed batteries (discharging and charged) and divides it > by the current total discharge rate over all batteries (this is for lifetime, > similarly for charge time). > > > Diffs > ----- > > a/powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp eae5436 > > Diff: https://git.reviewboard.kde.org/r/116481/diff/ > > > Testing > ------- > > Tested with KDE 4.11.6 on Thinkpad T440s with > - two batteries installed > - one battery installed > > > Thanks, > > Joschi Brauchle > >
