tshakalekholoane / bat

Battery management utility for Linux laptops.
https://tshaka.dev/x/bat
MIT License
286 stars 23 forks source link

feat: battery health command #57

Closed HarshDobariya79 closed 1 year ago

HarshDobariya79 commented 1 year ago

Subject of the issue

There should be a command to check battery health which is the maximum storage capacity in percentage.

Eg.

Your system

OS: Ubuntu 22.04 Linux x86_64 Host: Asus TUF A15 FA506IH Kernel: 5.19.0-45-generic

Version

0.12 (Latest)

tshakalekholoane commented 1 year ago

This is interesting. Would you like to help with this, @HarshDobariya79?

HarshDobariya79 commented 1 year ago

Thank you for the opportunity @tshakalekholoane, I would love to but unfortunately I don't have any experience in GO language.

tshakalekholoane commented 1 year ago

Okay, @HarshDobariya79. I will leave it open in case you or someone else wants to pick this up in future.

pepa65 commented 1 year ago

I implemented this health feature in my fork of this repo, but I think the repos have already diverged quite a bit... https://github.com/pepa65/bat

HarshDobariya79 commented 1 year ago

I implemented this health feature in my fork of this repo, but I think the repos have already diverged quite a bit... https://github.com/pepa65/bat

Ah.. That's bad. Seems like the entire repo has been re-fractored.

pepa65 commented 1 year ago

This repo has recently been refactored a lot, and since I liked to give a different CLI experience, it's divergent. But the core ideas are exactly the same.

tshakalekholoane commented 1 year ago

Yes, it seemed overly complex so I did a rewrite.

@pepa65, is that the charge_full_design variable? I have none of those on my device.

pepa65 commented 1 year ago

I thought health would be charge_full divided by charge_full_design, but if that last one doesn't exist on some ASUS models, than that's sad... Are those /sys entries provided through a module? Perhaps version dependent??

tshakalekholoane commented 1 year ago

I believe so. For ASUS devices the charge threshold seems to be in linux/drivers/platform/x86/asus-wmi.c. The others i.e. charge_full, I am not sure. Maybe in linux/drivers/power/supply depending on the model 🤷.

pepa65 commented 1 year ago

On 5.19.0-43-generic #44~22.04.1-Ubuntu and on 5.15.0-75-generic charge_full_design is exposed here (and perhaps it also depends on the battery model?). It seems to be in asus-wmi: https://www.spinics.net/lists/platform-driver-x86/msg19387.html and https://www.spinics.net/lists/platform-driver-x86/msg19537.html

tshakalekholoane commented 1 year ago

I see. You're right, it probably also depends on the battery model.

sravan-s commented 1 year ago

Hello @tshakalekholoane Thanks for the program ~ I use it on my laptop to set charge threshold

I was looking at battery health too. health is same as capacity, correct? https://upower.freedesktop.org/docs/Device.html#id-1.2.4.8.92

Upower calculates it using energy_full / energy_design https://gitlab.freedesktop.org/upower/upower/-/blob/master/src/up-device-battery.c#L307

https://github.com/tshakalekholoane/bat/compare/main...sravan-s:bat:feat/health This worked for me

pepa65 commented 1 year ago

A note about the implementation of @sravan-s: it uses energy_full and energy_full_design. Maybe you have those on your system, @tshakalekholoane ? I now implemented both, so you can try it out (or just merge @sravan-s's work).

sravan-s commented 1 year ago

@pepa65 I found something interesting in this doc -> https://www.kernel.org/doc/Documentation/power/power_supply_class.txt

~ ~ ~ ~ ~ ~ ~  Charge/Energy/Capacity - how to not confuse  ~ ~ ~ ~ ~ ~ ~
~                                                                       ~
~ Because both "charge" (µAh) and "energy" (µWh) represents "capacity"  ~
~ of battery, this class distinguish these terms. Don't mix them!       ~
~                                                                       ~
~ CHARGE_* attributes represents capacity in µAh only.                  ~
~ ENERGY_* attributes represents capacity in µWh only.                  ~
~ CAPACITY attribute represents capacity in *percents*, from 0 to 100.  ~
~                                                                       ~
~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~
CHARGE_FULL_DESIGN, CHARGE_EMPTY_DESIGN - design charge values, when
battery considered full/empty.

ENERGY_FULL_DESIGN, ENERGY_EMPTY_DESIGN - same as above but for energy.

And from upower docs ->

 The capacity of the power source expressed as a percentage between 0 and 100.
The capacity of the battery will reduce with age.
A capacity value less than 75% is usually a sign that you should renew your battery.
Typically this value is the same as (full-design / full) * 100.
However, some primitive power sources are not capable reporting capacity and in this
case the capacity property will be unset. 

So, as you said, energy/capacity either should be fine. .If neither exist, we can assume power source have some issue Yeah, we can try/merge your solution or mine, all good ~

pepa65 commented 1 year ago

Interesting about the Ampere vs. Watt, so yes, don't mix them..! How I have implemented it, they don't, so that's good. I wonder if that depends on the battery, which are used.

tshakalekholoane commented 1 year ago

Hello @tshakalekholoane Thanks for the program ~ I use it on my laptop to set charge threshold

I was looking at battery health too. health is same as capacity, correct? https://upower.freedesktop.org/docs/Device.html#id-1.2.4.8.92

Upower calculates it using energy_full / energy_design https://gitlab.freedesktop.org/upower/upower/-/blob/master/src/up-device-battery.c#L307

main...sravan-s:bat:feat/health This worked for me

Hi @sravan-s. It appears so! Thank you for sharing.

A note about the implementation of @sravan-s: it uses energy_full and energy_full_design. Maybe you have those on your system, @tshakalekholoane ? I now implemented both, so you can try it out (or just merge @sravan-s's work).

@pepa65, I do have those energy_full and energy_full_design but not the others so I will probably go with those. Both implementation look great!

Although I do have a preference for something more minimal so something like what @sravan-s did except with fmt.Sscanf instead of defining a function.

var v, w int
_, err := fmt.Sscanf(mustRead("energy_full"), "%d\n", &v)
// ...

I also like @pepa65's implementation of the calculation, v*100/w, which avoids floating-point arithmetic.

You are both welcome to file a pull requests or I can just do it and credit you both since you did the bulk of the research.

Also, the upower documentation you linked seems to say (full-design / full) * 100 which would be > 1, unless I am missing something?

But either way, thank you all for you contribution!

sravan-s commented 1 year ago

@pepa65 Feel free to make PR 😃

pepa65 commented 1 year ago

@sravan-s Don't you already have a ready-to-go implementation? (I guess you don't handle charge_ entries...)

In any case, my fork is too divergent. 🤷🏼‍♂️