fix: start/stop thresholds not being set because of initial values

[CasperVM]

Background

I had this issue on my thinkpad where sometimes the start/stop thresholds of the battery charge were not being applied and failed with this;

write error: Invalid argument

This can happen when e.g. our start value is higher than the current stop value. In those cases we should lower the stop value to below our start.

What this change does

• Rewrites the battery scripts more cleanly to share more code.
• Adjusts the write to first set 0 and 100 for start/stop respectively.
• Sleeps 100ms before then applying actual settings.

Impact

If writes somehow fail user might end up with incorrect settings, but this was the case before anyway.
The 100ms sleep is to ensure settings are applied by the driver, but this might not be needed?

[AdnanHodzic]

Sounds good, please let me know once it's complete.

Thanks!

[CasperVM]

Sounds good, please let me know once it's complete.

Thanks!

Done 👍

Have a look when you can :)

[AdnanHodzic]

@PurpleWazard since you originally implemented this feature, please let me know if you have any comments.

In meantime, @CasperVM please give me some time to further review and test this.

[PurpleWazard]

@AdnanHodzic @CasperVM. this PR adds a batterydevice class which i like makes the code a bit cleaner. however, thinkpad_acpi and ideapad_acpi were removed for unknown reason and they dont seem to be implemented with the changes.

I had this issue on my thinkpad where sometimes the start/stop thresholds of the battery charge were not being applied and failed with this;

write error: Invalid argument
This can happen when e.g. our start value is higher than the current stop value. In those cases we should lower the stop value to below our start.

which is backwards the start shouldn't be higher then the stop. the reason for the error is becuase the kernel module wont work with those values.

the start value is what % at or under for the battery to start charging. the stop value is that value should charging at. ie. start: 70, stop: 80. the battery will maintain % between 70 and 80.

[CasperVM]

the start value is what % at or under for the battery to start charging. the stop value is that value should charging at. ie. start: 70, stop: 80. the battery will maintain % between 70 and 80.

Let me elaborate a little bit here, because I was probably a bit too unclear with my changes:

First of the class is an abstraction, both the thinkpad_acpi and ideapad_acpi had the exact same behavior, thus, we just use the parent class that implements the 'default'. I could add these back for clarity, but that just feels like unnecessary boilerplate to me.

For the battery percent, yes, the start shouldn't be higher than the stop. That was never in my config, but if you change the config it might become an issue:

• E.g. you initially set start/stop to 50-60 using the config.
• You then decide, hey, I want it to be 70-80 instead.
• The script tries to first set the start value to 70, which the kernel doesn't allow, because your old CURRENT stop value is 60.
• So this is not a config issue, but an error in how we handle config changes.

[CasperVM]

@AdnanHodzic @PurpleWazard

[Copilot]

Pull Request Overview

This PR refactors the battery threshold management scripts to fix a critical issue where start/stop thresholds fail to apply due to conflicting initial values. The solution sets temporary safe values (0/100) before applying the actual configured thresholds, with a 100ms delay to allow driver processing.

• Consolidates common battery management logic into a shared BatteryDevice base class
• Implements a two-step threshold setting process to avoid "Invalid argument" errors
• Refactors Ideapad and Asus battery scripts to use class-based inheritance

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
auto_cpufreq/battery_scripts/shared.py	New shared base class implementing common battery threshold management logic with two-step threshold setting
auto_cpufreq/battery_scripts/ideapad_laptop.py	Refactored to extend BatteryDevice class with Ideapad-specific conservation mode handling
auto_cpufreq/battery_scripts/asus.py	Refactored to extend BatteryDevice class with Asus-specific fallback paths
auto_cpufreq/battery_scripts/battery.py	Updated to instantiate device classes instead of calling standalone functions
auto_cpufreq/battery_scripts/thinkpad.py	Removed - functionality now provided by shared BatteryDevice class
auto_cpufreq/battery_scripts/ideapad_acpi.py	Removed - functionality now provided by shared BatteryDevice class

Comments suppressed due to low confidence (2)

auto_cpufreq/battery_scripts/ideapad_laptop.py:27

• Normal methods should have 'self', rather than 'value', as their first parameter.

    def set_conservation_mode(value):

auto_cpufreq/battery_scripts/asus.py:9

• This class does not call BatteryDevice.init during initialization. (AsusBatteryDevice.init may be missing a call to a base class init)

class AsusBatteryDevice(BatteryDevice):

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[AdnanHodzic]

Code changes (logic) generally make sense to me and from my testing everything worked as it should on ThinkPad X1 Carbon. I ran Copilot review also since there are a lot of changes and refactoring so please take a look at those, as it made some good remarks.

1. Things that could be improved and also addressed is that changes will not automatically picked up from config file:

# battery charging threshold
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds
enable_thresholds = true
#start_threshold = 20
stop_threshold = 90  

instead I had to restart the auto-cpufreq daemon in order for them to be picked up (or simply auto-cpufreq --remove && auto-cpufreq --install)

2. As I suggested in a comment in code review, I think Battery charging thresholds of docs could be updated.

3. Changes in my case worked as intended, e.g: I purposely charged laptop to 91% and after I set the stop_threshold = 90 it didn't continue to charge. Same case is with charging stopping to charge at 90% if it was below that level.

With that said I think, after mentioned things above are addressed I think we're good to merge this, unless you have any other major concerns @PurpleWazard

[AdnanHodzic]

In meantime, I ran into a major issue, after I disabled stop_threshold and enable_thresholds = true and removed auto-cpufreq in meantime, and reverted everything with changes that are on master branch.

# battery charging threshold
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds
#enable_thresholds = true
#start_threshold = 20
#stop_threshold = 90 

I realized that my laptop was still not charging above 90% because permanent changes were made to my /sys/class/power_supply/BAT0/charge_control_end_threshold file which still returns 90 ...

After I manually edited it back to 100, it charges past 90%.

[CasperVM]

@AdnanHodzic Fixed some of the issues that the auto-review caught. Should this also perhaps be tested before merge?

Added some clarification in the Readme as well

As for the thresholds not resetting, we could add that to the --remove option when uninstalling auto-cpufreq. But perhaps in another PR?

[CasperVM]

One small oversight still, but unsure what would be the proper way to go around it here:

If we only have e.g. the stop threshold, the start will be at 0. Meaning we wont start charging until the battery is dead.

We should probably keep some sane defaults instead? Or just ignore the config if only one of the values is set?

Edit: made it so we actually validate now, and both start/stop are needed. I don't think it makes sense to apply anything if either one is not set.

[AdnanHodzic]

Fixed some of the issues that the auto-review caught. Should this also perhaps be tested before merge?
Added some clarification in the Readme as well

Thanks, I can also test this if changes are finalized, just please note it might not happen until end of the week.

As for the thresholds not resetting, we could add that to the --remove option when uninstalling auto-cpufreq. But perhaps in another PR?

Option 1: Ideally, changes would be updated immediately as soon as they are written to config file. Some kind of hook could be created to trigger it, but would need to think more about this. As this will be the problem with enabling and disabling the changes.

Option 2: Alternatively, we could update config part (and README) and below # battery charging threshold state that after each change to fields:

#enable_thresholds = true
#start_threshold = 20
#stop_threshold = 90

auto-cpufreq daemon should be restart or simply removed and enabled again (`auto-cpufreq --remove && auto-cpufreq --install). It's not pretty, but it's the simplest thing I can think of now.

As for the thresholds not resetting, we could add that to the --remove option when uninstalling auto-cpufreq. But perhaps in another PR?

If we're going for Option 2, then it can be part of this PR.

One small oversight still, but unsure what would be the proper way to go around it here:

If we only have e.g. the stop threshold, the start will be at 0. Meaning we wont start charging until the battery is dead.

We should probably keep some sane defaults instead? Or just ignore the config if only one of the values is set?

Edit: made it so we actually validate now, and both start/stop are needed. I don't think it makes sense to apply anything if either one is not set.

Not sure if I understand this part, as during my testing I used following config:

# battery charging threshold
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds
enable_thresholds = true
#start_threshold = 20
stop_threshold = 90

and charging worked fine?

[CasperVM]

I think that option 2 for now would be the best and a proper hook should be implemented later. So we just add more documentation.

It's odd that your charging worked fine, as in the previous commit the default for start = 0, and stop = 100.

As per the kernel docs:
https://docs.kernel.org/admin-guide/laptops/thinkpad-acpi.html#battery-charge-control

charge_control_start_threshold accepts an integer between 0 and 99 (inclusive); this value represents a battery percentage level, below which charging will begin.

and

charge_control_end_threshold accepts an integer between 1 and 100 (inclusive); this value represents a battery percentage level, above which charging will stop.

I'll make one last commit to properly enforce this + add documentation/comments. Again, I don't think it make sense to have only 1 of the values set up. It might just end up in someone having issues and not understanding why. Additionally, it's difficult to determine 'sane' defaults.

[AdnanHodzic]

I think that option 2 for now would be the best and a proper hook should be implemented later. So we just add more documentation.

Agreed.

It's odd that your charging worked fine, as in the previous commit the default for start = 0, and stop = 100.

As per the kernel docs: https://docs.kernel.org/admin-guide/laptops/thinkpad-acpi.html#battery-charge-control

charge_control_start_threshold accepts an integer between 0 and 99 (inclusive); this value represents a battery percentage level, below which charging will begin.

and

charge_control_end_threshold accepts an integer between 1 and 100 (inclusive); this value represents a battery percentage level, above which charging will stop.

I think it worked because by not setting #start_threshold = 20 it left it at 0 by default and didn't change anything?

I'll make one last commit to properly enforce this + add documentation/comments. Again, I don't think it make sense to have only 1 of the values set up. It might just end up in someone having issues and not understanding why. Additionally, it's difficult to determine 'sane' defaults.

Sounds good, better to relate them to avoid unnecessary new issues and questions.

[AdnanHodzic]

@CasperVM Once you're done making all the changes and testing from your side, please let me know so I know when I can do the final review and test. Thanks

P.S: reference from #898 how users setup battery charging thresholds.

sudo modprobe thinkpad_acpi force_load=true
echo 85 > /sys/class/power_supply/BAT0/charge_control_end_threshold

[CasperVM]

@AdnanHodzic I'm happy with the current changes, you can do the final review and test when you have the time

[AdnanHodzic]

What does "(THIS SHOULD BE LEFT AS-IS IN MOST CASES)" mean in this case? Left uncommitted, or left as is it's in example, which is true?

If that's the case I would rephrase it like:

(THIS SHOULD BE LEFT AS-IS IN MOST CASES, e.g: true)

[AdnanHodzic]

Let's also make sure these changes are reflected everywhere else where it's written, e.g: auto-cpufreq.conf-example.nix

[AdnanHodzic]

Finally, let's make the example more how it was before

current:

# battery charging threshold
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds
#enable_thresholds = true
#start_threshold = 20 
stop_threshold = 80

Your proposed changes:

# experimental

# Add battery charging threshold (currently only available to Lenovo)
# checkout README.md for more info

# enable thresholds true or false
#enable_thresholds = true
#
# whether to check if thresholds are valid (true or false). (THIS SHOULD BE LEFT AS-IS IN MOST CASES)
#check_thresholds = true                                                   
#
# start threshold (0 is off ) can be 0-99
#start_threshold = 0
#
# stop threshold (100 is off) can be 1-100
#stop_threshold = 100

How I think it should be as it looks cleaner

# Add battery charging threshold 
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds

# enable thresholds true or false
#enable_thresholds = true

# whether to check if thresholds are valid (true or false). Recommended in most cases "true"
#check_thresholds = true     

# start threshold (0 is off ) can be 0-99
#start_threshold = 0

# stop threshold (100 is off) can be 1-100
#stop_threshold = 100
`

Btw, regarding:
>  Add battery charging threshold 

You proposed:
> # Add battery charging threshold  (currently only available to Lenovo) 

do we want to limit it only to Lenovo, because there is other models, I mean we have asus.py ... it might be easier to leave that part so it's all listed [on Threshold part of readme.](https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds)

[CasperVM]

Left 'as-is' means as-is, so the example value. Generally speaking you wouldn't want to skip the check for invalid start/stop thresholds. I only added this option because of the exception with fixed thresholds. I didn't want to break existing functionality.

Would be good to find someone to test it however..

[AdnanHodzic]

I'm not sure where you got this information, but I never heard about this before. Have you also verified this works?

[CasperVM]

I have not, I do not have these devices. This was added to the readme quite a while ago: 7862a3f

[AdnanHodzic]

Thanks,

I added some more comments in documentation, and let's please get rid of auto-cpufreq.conf example contents under "Example config file contents" because user should refer to auto-cpufreq.conf file instead.

and let's make "Example config file contents" a link to auto-cpufreq.conf-example file

Also during my testing, setting following values in /etc/auto-cpufreq.conf file:

# enable thresholds true or false
enable_thresholds = true

# whether to check if thresholds are valid (true or false). (THIS SHOULD BE LEFT AS-IS IN MOST CASES)
check_thresholds = true

# start threshold (0 is off ) can be 0-99
start_threshold = 80
                                                                                                                                                       
# stop threshold (100 is off) can be 1-100
stop_threshold = 90

Even after as instructed in readme, systemctl restart auto-cpufreq.service and when that didn't change anything auto-cpufreq --remove && auto-cpufreq --install made no difference and values weren't picked up from auto-cpufreq.conf file and these values remained the same as set by system:

cat /sys/class/power_supply/BAT0/charge_stop_threshold
100
cat /sys/class/power_supply/BAT0/charge_start_threshold
0 

So not sure what happened but this feature is not working as it should anymore on my side.

[CasperVM]

I added some debug prints for further testing/investigation. I cannot test this myself, so help would be appreciated. I added some comments for clarification, I'll update the branch more once people can test/give some more feedback as to why this doesn't work in it's current state.

edit: I used to have a Thinkpad, where I experienced some issues that I'm trying to fix in this branch. But since a short while ago I do not have that anymore, so testing is difficult.

[CasperVM]

I have not, I do not have these devices. This was added to the readme quite a while ago: 7862a3f

[CasperVM]

Left 'as-is' means as-is, so the example value. Generally speaking you wouldn't want to skip the check for invalid start/stop thresholds. I only added this option because of the exception with fixed thresholds. I didn't want to break existing functionality.

Would be good to find someone to test it however..

[AdnanHodzic]

1. I started testing these changes again and I came across another weird issue.

Which is, if in /etc/auto-cpufreq.conf I leave these values commented, e.g:

# battery charging threshold
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds


# enable thresholds true or false
#enable_thresholds = true                                                                                                                                                                                             

# whether to check if thresholds are valid (true or false). (THIS SHOULD BE LEFT AS-IS IN MOST CASES)
#check_thresholds = true

# start threshold (0 is off ) can be 0-99
#start_threshold = 0 

# stop threshold (100 is off) can be 1-100
#stop_threshold = 80

install auto-cpufreq using auto-cpufreq-installer from changes with this PR, I'll install auto-cpufreq daemon and --stats will work fine.

But, if I remove auto-cpufreq using auto-cpufreq-installer, install it again using the same, enable threshold values in: /etc/auto-cpufreq.conf`

# battery charging threshold
# reference: https://github.com/AdnanHodzic/auto-cpufreq/#battery-charging-thresholds


# enable thresholds true or false
enable_thresholds = true                                                                                                                                                                                             

# whether to check if thresholds are valid (true or false). (THIS SHOULD BE LEFT AS-IS IN MOST CASES)
check_thresholds = true

# start threshold (0 is off ) can be 0-99
start_threshold = 0 

# stop threshold (100 is off) can be 1-100
stop_threshold = 80

and install auto-cpufreq daemon, running stats will return the following:

sudo auto-cpufreq --stats              

------------------------ auto-cpufreq not running ------------------------------

ERROR: auto-cpufreq is not running in daemon mode.

Make sure to run "sudo auto-cpufreq --install" first

-------------------------------------------------------------------------------

2. Besides that new issue, as I said in my latest comment, these values no longer work for me unlike with initial commit. So I can't even test anything anymore.

3. Furthermore, I also noticed that my laptop is no longer charging when connected to my external monitor via usb-c (which was always the case before). Not sure if this is related to testing these changes, but with all this said as much as I appreciate the effort, I cannot merge this PR and its changes in this state.

Update: reboot fixed the issue and my laptop is charging again when connected to external monitor with changes on master branch.

Especially since I wan't to make a big new release in next couple of weeks.

If anyone would like to further test these changes or new ones are made, and is in working state again I'm also willing to test it again. But right now I'm facing these 3 major issues.

[be-west]

Hi @AdnanHodzic,

I would be willing to contribute to this PR. I own a ThinkPad, so I could run a few tests.
After taking a look at the current code, I identified a couple of potential issues:

1. The main problem you're having, as mentioned in your first point, seems to be a type error while parsing the config, which makes the daemon crash.

2. The --monitor option enables the configured battery thresholds. I think this should be fixed, considering the README mentions No changes are made to the system.

3. Instead of the charge_ {start/end}_threshold, maybe the charge_control_{start,end}_threshold VFS files should be used if they exist. I had a very quick look at how TLP does this, and they use the former only if the latter wasn't found. Plus, I could only find official documentation about the charge_control_* attributes.

[CasperVM]

Hi @AdnanHodzic,

I would be willing to contribute to this PR. I own a ThinkPad, so I could run a few tests. After taking a look at the current code, I identified a couple of potential issues:

1. The main problem you're having, as mentioned in your first point, seems to be a type error while parsing the config, which makes the daemon crash.

2. The `--monitor` option enables the configured battery thresholds. I think this should be fixed, considering the README mentions `No changes are made to the system.`

3. Instead of the `charge_ {start/end}_threshold`, maybe the `charge_control_{start,end}_threshold` VFS files should be used if they exist. I had a very quick look at how TLP does this, and they use the former only if the latter wasn't found. Plus, I could only find official documentation about the `charge_control_*` attributes.

@be-west I wouldn't mind to collaborate. I'd be completely fine if you finish it up, as I don't have the hardware to test this change.

[AdnanHodzic]

@be-west and @CasperVM please give it a try, especially since @CasperVM is already an existing contributor, if you need to sync you can use auto-cpufreq Discord community.

As always, you will be credited for your work as part of future release :)

[be-west]

@CasperVM
Hi. I've made some changes, and now the thresholds are working as expected on my ThinkPad.
Main changes:

• The --monitor flag no longer makes changes to the thresholds
• The type errors are fixed.
• Moved some code around for easier test writing (I wrote some unit tests for myself)
• The charge_control_{start,end}_threshold files are now preferred over the charge_{start,stop}_threshold files

Should I commit this to your branch, or should I create my own fork?

Edit: you can also reach me on the Discord: bene_w

[CasperVM]

@CasperVM Hi. I've made some changes, and now the thresholds are working as expected on my ThinkPad. Main changes:

* The `--monitor` flag no longer makes changes to the thresholds

* The type errors are fixed.

* Moved some code around for easier test writing (I wrote some unit tests for myself)

* The `charge_control_{start,end}_threshold` files are now preferred over the `charge_{start,stop}_threshold` files

Should I commit this to your branch, or should I create my own fork?

Edit: you can also reach me on the Discord: bene_w

Hi! Sorry I've been quite busy with life. You can commit it to my branch if you like, I'll add you as a collaborator on my fork. Please commit it somewhere at least so we can look at the changes so far :).

[CasperVM]

@be-west

[be-west]

@CasperVM I committed the preliminary changes.
In the meantime, I had a look at the various vendor-specific settings in the great TLP documentation

It seems that Asus only uses the charge_control_end_threshold file. The code did not/does not reflect that.
Maybe we could fix that in another PR and also add other vendors according to the TLP docs?

[CasperVM]

@be-west Yes we should perhaps take a closer look in these vendor specific settings in another PR. Right now I think this should be finished on its own, since these changes were more for a general refactoring and fixing the core functionality.

[CasperVM]

Would you also be able to quickly explain why the thresholds were not applying in my version? This is still somewhat confusing for me. What did you change to make that part work?

[be-west]

@be-west Yes we should perhaps take a closer look in these vendor specific settings in another PR. Right now I think this should be finished on its own, since these changes were more for a general refactoring and fixing the core functionality.

I think so too. Just wanted to mention it after I looked through the TLP documentation.

Would you also be able to quickly explain why the thresholds were not applying in my version? This is still somewhat confusing for me. What did you change to make that part work?

If you are referencing the issue @AdnanHodzic had, that was a type error. The config values for the start and stop thresholds were never cast to integers, and so the sanity check failed.

Can you run your tests again @AdnanHodzic, and tell us if it now works?

[AdnanHodzic]

Can you run your tests again @AdnanHodzic, and tell us if it now works?

Still doesn't work and it failed on first test on my ThinkPad Carbon, after setting:

enable_thresholds = true
start_threshold = 20
stop_threshold = 80   

as part of /etc/auto-cpufreq.conf

While threshold start/stop was recognized at 20 & 80 and AC plugged was reported as "yes". My battery status was not charging and battery percentage kept dropping, although I tried charging at 62%

I also still had to manually revert changes made to /sys/class/power_supply/BAT0/charge_control_end_threshold and /sys/class/power_supply/BAT0/charge_control_start_threshold after I commented these fieds in auto-cpufreq.conf and even after auto-cpufreq daemon removal.

P.S: Although there are no detected conflicts, I would also suggest pulling and rebasing this PR on latest changes from amster, since I get a popup there's update for 3.0.0 and that I'm running on 2.6.0

[be-west]

@AdnanHodzic
I rebased to master.
Thanks for running the tests. That's weird that the thresholds are correctly written/read, but your battery is losing charge. On my L16, the charge stays on the same level when I plug in AC while the level is above the start value. That means the Not Charging output is actually correct in that case.

This is a screenshot from my system where the thresholds were correctly applied, and AC was plugged at 54%:

Did you test if your ThinkPad starts charging when you set the start threshold above your current charge level?
Also, can you give the output of these files (if present) while applying the thresholds?

/sys/class/power_supply/BAT0/charge_behaviour
/sys/devices/platform/smapi/BAT0/force_discharge

I also still had to manually revert changes made to /sys/class/power_supply/BAT0/charge_control_end_threshold and /sys/class/power_supply/BAT0/charge_control_start_threshold after I commented these fieds in auto-cpufreq.conf and even after auto-cpufreq daemon removal.

Yeah, I guess the thresholds should be set to the vendor default when enable_thresholds = false. Do you want this to be part of this PR?

[AdnanHodzic]

Ran these before applying threshold:

cat /sys/class/power_supply/BAT0/charge_behaviour
[auto] inhibit-charge force-discharge

But this might be the problem, because:

cat /sys/devices/platform/smapi/BAT0/force_discharge
cat: /sys/devices/platform/smapi/BAT0/force_discharge: No such file or directory (os error 2)

Even /sys/devices/platform/smapi doesn't exist:

ll /sys/devices/platform/     
total 0
drwxr-xr-x 3 root root    0 Feb 10 18:55 ACPI000C:00
drwxr-xr-x 4 root root    0 Feb 10 18:55 ACPI000E:00
drwxr-xr-x 3 root root    0 Feb 10 18:55 INT0E0C:00
drwxr-xr-x 4 root root    0 Feb 10 18:55 INT3400:00
drwxr-xr-x 3 root root    0 Feb 10 18:55 PNP0C0D:00
drwxr-xr-x 3 root root    0 Feb 10 18:55 PNP0C0E:00
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:00
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:01
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:02
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:03
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:04
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:05
drwxr-xr-x 4 root root    0 Feb 10 18:55 PNP0C14:06
drwxr-xr-x 5 root root    0 Feb 10 18:55 STM7364:00
drwxr-xr-x 6 root root    0 Feb 10 18:55 USBC000:00
drwxr-xr-x 3 root root    0 Feb 10 18:55 acpi-cpufreq
drwxr-xr-x 4 root root    0 Feb 10 18:55 coretemp.0
drwxr-xr-x 3 root root    0 Feb 10 18:55 efivars.0
drwxr-xr-x 5 root root    0 Feb 10 18:55 i8042
drwxr-xr-x 3 root root    0 Feb 10 18:55 intel_pmc_core.0
drwxr-xr-x 3 root root    0 Feb 10 18:55 intel_rapl_msr.0
drwxr-xr-x 3 root root    0 Feb 10 18:55 kgdboc
drwxr-xr-x 3 root root    0 Feb 10 18:55 pcspkr
drwxr-xr-x 2 root root    0 Feb 10 18:55 power
drwxr-xr-x 3 root root    0 Feb 10 18:55 rtc-efi.0
drwxr-xr-x 6 root root    0 Feb 10 18:55 rtc_cmos
drwxr-xr-x 4 root root    0 Feb 10 18:55 serial8250
drwxr-xr-x 4 root root    0 Feb 10 18:55 snd_aloop.0
drwxr-xr-x 7 root root    0 Feb 10 18:55 thinkpad_acpi
drwxr-xr-x 4 root root    0 Feb 10 18:55 thinkpad_hwmon
-rw-r--r-- 1 root root 4.0K Feb 10 18:55 uevent

[be-west]

That's fine. /sys/devices/platform/smapi would only be present on older ThinkPads.
/sys/class/power_supply/BAT0/charge_behaviour also seems to be the default value.
Have you checked if your ThinkPad charges when you set your start value above your current charge level?

[AdnanHodzic]

Have you checked if your ThinkPad charges when you set your start value above your current charge level?

Setting:

enable_thresholds = true
start_threshold = 80
stop_threshold = 100                                                                                               

While my battery was on 80%, started charging it to 100%

After that I even tried setting it to:

enable_thresholds = true
start_threshold = 80
stop_threshold = 85

While battery was on 82% and it still kept charging.

Then I let it discharge to 75% and set:

enable_thresholds = true
start_threshold = 20
stop_threshold = 80

and it was charging, but then I let it discharge to 70% with same settings, and I had same problem as above where it wouldn't charge.

This is where I figured what the issue was, in background it was still set to use GNOME Battery charging and "Preserve Battery Health" was last selected:

Which if clicked (with or without auto-cpufreq daemon running), it'll set values to:

cat /sys/class/power_supply/BAT0/charge_control_end_threshold
80
cat /sys/class/power_supply/BAT0/charge_control_start_threshold
75

So, although with auto-cpufreq.conf these values were overwritten to 80/20 respectively, it seems like it still had above values cached somewhere and they were overriding what was defined with auto-cpufreq.

So once I switch to "Maximize Charge" ...

it started charging because it also reset:

cat /sys/class/power_supply/BAT0/charge_control_end_threshold
0
cat /sys/class/power_supply/BAT0/charge_control_start_threshold
100

although, these were still set to:

enable_thresholds = true
start_threshold = 20
stop_threshold = 80

in auto-cpufreq.conf

Switching back to "Preserve Battery Health" also reset them again to 75/80:

Running sudo systemctl restart auto-cpufreq.service set to pick up values defined in auto-cpufreq.conf and everything was working as it's supposed to:

If battery was charged above defined threshold, it would not be further charged which is good:

So, in a nutshell code changes in this PR are working as expected, only (major) problem is that GNOME Battery charging is interfering, and should be disabled completely if battery threshold values from auto-cpufreq.conf are about to picked up properly.

Same happened after GNOME Power profiles were introduced, they had to be disabled by auto-cpufreq otherwise they would conflict with each other.

[be-west]

Ah great, you found the issue.
Do you want an automatic solution for the GNOME battery charging interference?
Is this a standalone systemd service that can be disabled like the power profiles daemon?

[AdnanHodzic]

Do you want an automatic solution for the GNOME battery charging interference?

What exactly do you by "automatic solution for the GNOME battery charging interference?"

Is this a standalone systemd service that can be disabled like the power profiles daemon?

Not sure, quick search is not as conclusive, but ultimately you/we would have to look into a way to have it disabled.

[be-west]

Yeah, that's what I meant, disabling it like it is done with the power profiles daemon.
But I'm wondering why someone would try to use both features at once.
I don't have a GNOME environment, so I wouldn't mind someone posting the steps to disable it.

[AdnanHodzic]

But I'm wondering why someone would try to use both features at once.

I see value in someone wanting to use GNOME Battery Charging feature along with auto-cpufreq (in case you're not using battery thresholds). So ideally this feature would be disabled only if auto-cpufreq battery threshold is used, but ...

I don't have a GNOME environment, so I wouldn't mind someone posting the steps to disable it.

There's no way to disable it like e.g GNOME Power Profiles, it's not a systemd service or anything like that. It works on setting and picking up the same values in:

/sys/class/power_supply/BAT0/charge_control_start_threshold
/sys/class/power_supply/BAT0/charge_control_end_threshold

There could be an aggressive way to disable it with:

gsettings set org.gnome.settings-daemon.plugins.power active false

But then, this would This prevents GNOME from:

• showing Battery Charging UI
• writing thresholds
• interfering

with side effects we don't want:

• No lid handling
• No brightness key handling
• No low battery notifications

Hence, what I propose it that we simple leave it there, and add to README something like "Please note: if you're using auto-cpufreq battery threshold features, ignore settings defined in "GNOME Battery Charging" as they will be overwritten with values defined in auto-cpufreq.conf".

In addition to that, I would add additional check, once start/stop thresholds are set by auto-cpufreq, to periodically check (once set, and every few hours?) if they match values defined in auto-cpufreq.conf file, if not overwrite them. This way we would avoid what happened to me before and what's the source of our problem.

[be-west]

@AdnanHodzic
hey, I now made the daemon regularly apply (1 hour) the threshold settings. Is that fine?
I also added the note you suggested to the README and removed the remaining debug statements.
Maybe one last test, and it should be good to go.