D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R106:4c6c5638ef15: Add some new battery sensors : energy_now, 
energy_full and power_now. (authored by jjorge, committed by ngraham).

REPOSITORY
  R106 KSysguard

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28127?vs=79665=80124

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[Nathaniel Graham]

ngraham added a comment.


  I've landed this for you.
  
  Keep up the great work!

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: ngraham, anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge added a comment.


  In D28127#647747 , @ahiemstra 
wrote:
  
  > Seems fine to me now. You didn't have a developer account right?
  
  
  Yes, I don't.

REPOSITORY
  R106 KSysguard

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[Arjen Hiemstra]

ahiemstra accepted this revision.
ahiemstra added a comment.
This revision is now accepted and ready to land.


  Seems fine to me now. You didn't have a developer account right?

REPOSITORY
  R106 KSysguard

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge added a comment.


  I have finished my corrections. Is it nice now? Thanks.

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge updated this revision to Diff 79665.
jjorge added a comment.


  - A better cast to float

REPOSITORY
  R106 KSysguard

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28127?vs=79655=79665

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge updated this revision to Diff 79655.
jjorge added a comment.


  I should learn to better use arc...
  
  - typo

REPOSITORY
  R106 KSysguard

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28127?vs=79653=79655

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge updated this revision to Diff 79653.
jjorge added a comment.


  - Oops fix bad named functions

REPOSITORY
  R106 KSysguard

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28127?vs=79629=79653

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> acpi.c:147
>  if ( maximum > 0) {
> -state = charge * 100 / maximum;
> +state = charge/(float)(maximum/100);/* to get 0.1% changes */
>  }

`(float)(var_int/var_int)` does not what you want, it should be 
`((float)var_int / var_int)`

> acpi.c:179
>  if (maximum > 0) {
> -state = charge * 100 / maximum;
> +state = charge/(float)(maximum/100);/* to get 0.1% changes */
>  }

ditto

> acpi.c:211
> +if ( maximum > 0) {
> +state = charge/(float)(maximum/100);/* to get 0.1% changes */
> +}

ditto

> acpi.c:243
> +if (maximum > 0) {
> +state = charge/(float)(maximum/100);/* to get 0.1% changes */
> +}

ditto

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge added a comment.


  Hem, you are right, I missed it because I was testing old binary.

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge updated this revision to Diff 79629.
jjorge added a comment.


  - fix pointer

REPOSITORY
  R106 KSysguard

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28127?vs=79321=79629

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[Arjen Hiemstra]

ahiemstra added inline comments.

INLINE COMMENTS

> acpi.c:73
>  
> -void registerBatteryRate(int number, struct SensorModul *sm)
> +void registerBatteryEnergy(char name, int number, struct SensorModul *sm)
>  {

Uhm, this should be `const char *name` right? I don't really see how this can 
work otherwise.

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge updated this revision to Diff 79321.
jjorge added a comment.


  - Follow ahiemstra suggestions :
- cast to float directly
- move 2 repeated lines to initAcpiBattery

REPOSITORY
  R106 KSysguard

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28127?vs=77923=79321

BRANCH
  acpi-energy

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[Arjen Hiemstra]

ahiemstra added a comment.


  Good stuff. Just two small comments.
  
  Additionally, you may be interested in https://phabricator.kde.org/D28333 , 
which will eventually replace ksysguardd and has a better API for sensors.

INLINE COMMENTS

> acpi.c:82
> +char name[ ACPIFILENAMELENGTHMAX ];
> +readTypeFile("/sys/class/power_supply/BAT%d/type", number, name, 
> sizeof(name));
> +

These two lines are now repeated in all "registerX" functions. Maybe move this 
to initAcpiBattery and pass it to each function as a parameter?

> acpi.c:163
>  if ( maximum > 0) {
> -state = charge * 100 / maximum;
> +state = charge/(double)(maximum/100);/* to get 0.1% changes */
>  }

You cast to double but then store it in a float, which implies a cast to float. 
Probably better to just cast to float directly. (Also applies to the functions 
below.)

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28127

To: jjorge, #plasma, davidedmundson, ahiemstra
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28127: Add some new battery sensors : energy_now, energy_full and power_now.

[José JORGE]

jjorge created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
jjorge requested review of this revision.

REVISION SUMMARY
  Depending on hardware, Linux kernel exports battery state in
  different files. Previously only charge_now, charge_full and current_now
  were available in ksysguard. With this commit all are selectable.
  
  While we have now screens with more than 1000 pixels height,
  a percentage with 100 steps will show a ladder. This commit also
  make the created graphs nicer by giving 1000 steps.

REPOSITORY
  R106 KSysguard

BRANCH
  AddSysBatteryInfo (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28127

AFFECTED FILES
  ksysguardd/Linux/acpi.c
  ksysguardd/Linux/acpi.h

To: jjorge
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart