Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: battery health command #57

Closed
HarshDobariya79 opened this issue Jun 25, 2023 · 18 comments
Closed

feat: battery health command #57

HarshDobariya79 opened this issue Jun 25, 2023 · 18 comments
Assignees
Labels
enhancement New feature or request

Comments

@HarshDobariya79
Copy link

HarshDobariya79 commented Jun 25, 2023

Subject of the issue

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

Eg.

  • health
    • Print the current battery health in percentage.

Your system

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

Version

0.12 (Latest)

@HarshDobariya79 HarshDobariya79 added the bug Something isn't working label Jun 25, 2023
@tshakalekholoane tshakalekholoane added enhancement New feature or request and removed bug Something isn't working labels Jun 26, 2023
@tshakalekholoane
Copy link
Owner

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

@HarshDobariya79
Copy link
Author

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

@tshakalekholoane tshakalekholoane changed the title [Feature] Battery health command feat: battery health command Jun 28, 2023
@tshakalekholoane
Copy link
Owner

tshakalekholoane commented Jun 28, 2023

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

@pepa65
Copy link
Collaborator

pepa65 commented Jun 28, 2023

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
Copy link
Author

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
Copy link
Collaborator

pepa65 commented Jun 28, 2023

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
Copy link
Owner

tshakalekholoane commented Jun 28, 2023

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
Copy link
Collaborator

pepa65 commented Jun 28, 2023

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
Copy link
Owner

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
Copy link
Collaborator

pepa65 commented Jun 29, 2023

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
Copy link
Owner

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

@sravan-s
Copy link

sravan-s commented Aug 3, 2023

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

@pepa65
Copy link
Collaborator

pepa65 commented Aug 3, 2023

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
Copy link

sravan-s commented Aug 3, 2023

@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
Copy link
Collaborator

pepa65 commented Aug 3, 2023

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
Copy link
Owner

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
Copy link

sravan-s commented Aug 5, 2023

@pepa65 Feel free to make PR 😃

@pepa65
Copy link
Collaborator

pepa65 commented Aug 5, 2023

@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. 🤷🏼‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants