Add another battery status called 'idle' #486
Conversation
|
I am not sure how this interacts with #304; one would have needed more information on the original problem. If you have any doubts regarding that or anything else in this PR, please request a change. |
On some systems, a battery's charging regulator may not charge a battery even though it is not full. This might be the case because it was configured to stop charging at a capacity threshold, or e.g. because environmental conditions do not allow for charging the battery. This commit adds this status (called 'idle') and adds support for detecting the status on Linux.
erbth
force-pushed
the
battery_status_idle
branch
from
2a24521
to
3272abc
Compare
|
fyi just rebased to current main |
|
I have a system76 laptop which also lets me set a charging threshold. Mine ends up reporting UNK/Unknown along with 0.00W when it reaches this threshold. The current patch doesn't appear to make things better for me, but it also clearly doesn't make them worse. |
|
@thrasymache thanks for testing! - Yeah, unfortunately the hardware and kernel driver need to report that state as 'Not charging', too. It looks like yours does not - if you like, you could confirm that with |
|
@erbth The contents of the BAT0/status pseudo-file are |
|
@thrasymache Then your driver (or hardware) does not support reporting the idle-state correctly. Good to know. Mine (ThinkPad) shows e.g. "Not charging". Of course that depends on the kernel version, too (e.g. the acpi-driver used by mine got support only recently, currently I'm on Debian's 5.10.149-1). |
| @@ -129,6 +130,13 @@ static void add_battery_info(struct battery_info *acc, const struct battery_info | |||
| acc->status = batt_info->status; | |||
| /* else: retain FULL, since it is more specific than UNKNOWN */ | |||
| break; | |||
|
|
|||
| case CS_IDLE: | |||
| if (batt_info->status != CS_UNKNOWN && batt_info->status != CS_FULL) | |||
There was a problem hiding this comment.
Not sure about this case, if the driver reports full, wouldn't it mean that the batter is now at full capacity (for some reason) and not just idle?
There was a problem hiding this comment.
My turn to apologize for the delayed reply ;-). After thinking about this again, my reasoning would be like 'if all batteries are full but one is at, say, 50% and idle, set idle as aggregated battery-state as the accumulated capacity is not fully filled with charge'. Back then I probably simply thought that 'idle' is correct for all batteries in such a state (assuming that top-charging counts as idle...) but full would be wrong for the 50%-one.
Hence I decided to leave acc->status idle if the status of the battery to add to the aggregated status is - according to the driver - full (batt_info->status == CS_FULL). But of course this assumes that all 'virtual' batteries reported by e.g. the Linux kernel correspond to different physical batteries and that the drivers do only return CS_IDLE if the battery is not full. And this edge-case can even only be problematic if one has (a) multiple batteries and (b) their resp. charging regulators are configured differently.
But I would be perfectly fine with reporting full in that CS_IDLE-case. Shall I change the line to if (batt_info->status != CS_UNKNOWN) (+adapt the comment)?
|
thanks for merging |
Hi everyone,
I would like to add another state to the battery-module called 'idle'. My ThinkPad has a configurable charging regulator that can be instructed to stop charging before the battery is full. Recently a Linux-kernel patch got merged s.t. this state is now correctly reported as "Not charging" by the kernel (before it was "Unknown").
However since #305 this status is mapped to "Discharging". Please have a look at #461 (comment), where I tried to describe why I think my proposed behavior would be better, and why I think that it would in principle also be better than the proposed change in #461. In my comment I wrote that a mechanism to customize i3status's behavior for "Not charging" might be needed, however I think the mechanism for changing the texts mapped to states through i3status.conf might be enough.
Of course, this PR covers only the foundation and Linux-specific implementation. Support for other OSs would have to be added afterwards if possible, however I cannot assist in that.
Regards,
Thomas