extend support for setting battery charge threshold#470
extend support for setting battery charge threshold#470hepp3n wants to merge 1 commit intopop-os:masterfrom
Conversation
| Path::new("/sys/devices/platform/huawei-wmi/charge_control_thresholds").exists() | ||
| Path::new("/sys/devices/platform/huawei-wmi/charge_control_thresholds").exists() || | ||
| // and Lenovo ThinkPads and probably some more | ||
| Path::new("/sys/class/power_supply/BAT0/charge_control_end_threshold").exists() |
There was a problem hiding this comment.
This is just the standard path that anything that supports thresholds would have, including System76 (and probably Huawei) devices that passed the above checks. I'm pretty sure it's also what Path::new(END_THRESHOLD).exists() checks in the supports_thresholds() function just a few lines after this.
So, I don't think it makes sense to add this redundant check to this function. If we want to stop filtering by vendors, it'd be better to just get rid of the is_supported() function and rely solely on the supports_thresholds() function.
Short of that, the correct addition to the per-vendor check function would be checking for a device only present in Lenovo/ThinkPad devices.
There was a problem hiding this comment.
Oh yeah, that make sense :D
However after some deeper test it seems for Lenovo ThinkPad it would be good to make some additional changes which are out of scope of this PR.
Sorry for bothering tho :)
There was a problem hiding this comment.
What are the additional changes that would be needed for the ThinkPad? It would be helpful to know if the brand check is still required.
There was a problem hiding this comment.
Not sure exactly but looking into TLP library, we have something like this: https://github.com/linrunner/TLP/blob/main/bat.d/05-thinkpad#L788-L813
And after setting charging threshold, and even resetting value to 100, my battery did not charge to full capacity. I think Lenovo has something else to control.
There was a problem hiding this comment.
And after setting charging threshold, and even resetting value to 100, my battery did not charge to full capacity.
Did you also unplug and re-plug the power after setting the threshold setting?
I'm not sure what the batdrv_write_force_discharge function you linked to would do. Does it make the laptop start discharging without having to unplug the AC adapter?
There was a problem hiding this comment.
I'm not sure what the batdrv_write_force_discharge function you linked to would do. Does it make the laptop start discharging without having to unplug the AC adapter?
I guess.
Did not dig this topic too much.
Did you also unplug and re-plug the power after setting the threshold setting?
Yes, tried different things even reboots etc but nothing helped. Like my battery stuck on this threshold which was weird.
I found somewhere on the new similar issue on ThinkPad and fully discharging helped. So I did the same. After my laptop was turned off by discharged battery, it started charging too 100%.
Today also had some weird issue with battery in my ThinkPad. I had to reset it by pin hole on its back. So... Maybe thinkpads are just thinkpads ^^
2 participants
This commit extends support for setting battery charge threshold for more laptops. Just an easy check but we should assume (I guess?) if this file exists we can set this setting. Otherwise it would not be presented?
Tested on my ThinkPad Z16 Gen 1 and it definitely works.