i3 / i3status

Generates status bar to use with i3bar, dzen2 or xmobar
BSD 3-Clause "New" or "Revised" License
602 stars 254 forks source link

battery: set status to full when battery is not charging #461

Closed leonixyz closed 1 year ago

leonixyz commented 2 years ago

As reported in this comment, when the reported battery status is:

POWER_SUPPLY_STATUS=Not charging

The battery is actually full, therefore not charging anymore. Currently i3status shows the battery as discharging.

leonixyz commented 2 years ago

The test needs to be changed as well

Fixed the test (I hope) and force-pushed.

erbth commented 2 years ago

I think POWER_SUPPLY_STATUS=Not charging means that the battery is not full and a power supply (e.g. AC adapter) is connected, but the supply does not charge the battery. Such conditions can be found in different charger/fuel gauge drivers in the Linux kernel: https://elixir.bootlin.com/linux/v5.16.13/source/drivers/power/supply/bq2515x_charger.c#L615 or https://elixir.bootlin.com/linux/v5.16.13/source/drivers/power/supply/ds2781_battery.c#L311 (note how they use the combination of power supply connected, not-full capacity and negligible charging current to determine that state). The internal enum-value POWER_SUPPLY_STATUS_NOT_CHARGING used in the drivers' code is translated to "Not charging" for use as context (? - not sure if this is the correct term) for uevents (and therefore /sys/.../uevent) through this array: https://elixir.bootlin.com/linux/v5.16.13/source/drivers/power/supply/power_supply_sysfs.c#L79.

Since this commit: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=44cbd2a16a07cbc02a4921853d9f17f1ffaaa97a the "Not charging" status is also reported correctly on ThinkPad laptops. (That commit made it into Debian's stable kernel with the last update.) If one has configured a charging-stop threshold on a ThinkPad, i3status previously showed UNK when that threshold was reached - now it shows BAT (with 0 watts power draw), which is in my opinion not right because the laptop is actually not powered from battery.

Therefore I would agree with https://github.com/i3/i3status/issues/304#issuecomment-962655488 that "Not charging" should not be treated as on battery, but I would also disagree with the changes in this PR, because it should not mean that the battery is full - that's what the status "Full" is for. Of course I see that the battery in https://github.com/i3/i3status/issues/304#issuecomment-962655488 is supposed to be full given the POWER_SUPPLY_CHARGE_* values. I would claim that the kernel driver of that charger/battery gauge of that laptop reports the "wrong" status (however one could argue that "Full" is stronger than "Not charging" and hence "Not charging" could be viewed as correct).

Referring to https://github.com/i3/i3status/issues/304#issue-352286101 I would claim that the "Not charging" status should only happen while the battery is not discharging either, hence only if a power supply is connected. However looking at the kernel code one could probably find all sorts of broken ACPI firmwares etc. out there...

Long story short I would propose adding another state to i3status's battery implementation which describes that a battery is not being charged but not full either (because it is configured to be not fully charged or because conditions prevent charging, like too high temperature). One could then map Linux's "Not charging" onto this new state. Maybe one should make this configurable to some extent (and maybe keep the behavior introduced in !305 as default) to make it easy to deal with broken firmware/driver implementations...

Of course, this relates only to Linux, I don't know about e.g. BSD.

erbth commented 2 years ago

I've created #486 with my proposed changes; @leonixyz, @orestisfl would be great if you could have a look whether this PR might work for you/might look acceptable to you. @leonixyz with #486 it would be possible for you to specify 'FULL' for 'status_idle' in i3status.conf.

orestisfl commented 1 year ago

Closed in favor of https://github.com/i3/i3status/pull/486