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

Update README with new battery thresholds feature #644

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

shadeyg56
Copy link
Collaborator

Apologies for the mess in the last attempt I made lol. The rebase went horribly wrong.

Added a new section for the battery thresholds as discussed in #637

@PurpleWazard
Copy link
Contributor

PurpleWazard commented Feb 12, 2024

Some thing you will want to add is the stop charge threshold can not be below 65 ( at least with thinkpad_acpi ) I've had errors having it lower than 65

Edit: I'll try this test again just to make sure

@shadeyg56
Copy link
Collaborator Author

the stop charge threshold can not be below 65 ( at least with thinkpad_acpi )

Hm, I can't find any documentation on this, so maybe this needs to be explored further?
Would you mind sharing the errors you get?

@PurpleWazard
Copy link
Contributor

I started building a test however I won't be able to finish it until tonight I'll reply back with results

@PurpleWazard
Copy link
Contributor

So it seems my test is working without issue so I'm going to do some investigating

@AdnanHodzic
Copy link
Owner

Maybe it would also be a good idea to first merge #645 ?

@shadeyg56
Copy link
Collaborator Author

Agreed.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@AdnanHodzic
Copy link
Owner

AdnanHodzic commented Feb 13, 2024

#645 merged, added a few comments, nothing major other then that LGTM!

Let's wait for @PurpleWazard to finish with:

So it seems my test is working without issue so I'm going to do some investigating

and if he doesn't have any additional comments we're ready to merge.

@PurpleWazard
Copy link
Contributor

It all looks good to me @AdnanHodzic suggested that we clarify that --live doesn't call the battery functions.
Is was because I had having issues when I ran --live with the battery functions however since I've rewritten the battery scripts I tested it with --live and I haven't had an issue.

I can add that change before or after the new release if needed

@AdnanHodzic
Copy link
Owner

@PurpleWazard I would rather have it before and hence part of 2.2.0 release.

@PurpleWazard
Copy link
Contributor

It is done

@AdnanHodzic
Copy link
Owner

Removed this comment because this is now addressed with merge of #646

Would also be good to include #645 (comment). Maybe as a last line for this section to say something like: "Please note: only --monitor and --stats have battery threshold function"

Or maybe it could be omitted, my only concern is that folks will ask it as a question. Alternatively it could be added as a comment for config file as part of battery threshold?

@shadeyg56 then let's add those 2 links I suggested and it's ready to merge.

@AdnanHodzic
Copy link
Owner

LGTM, thank you for your contribution will make sure to make this PR as part of release notes for 2.2.0.

@AdnanHodzic AdnanHodzic merged commit 9b8e873 into AdnanHodzic:master Feb 14, 2024
rootCircle pushed a commit to rootCircle/auto-cpufreq that referenced this pull request Feb 16, 2024
* README: add battery threshold sections

* README: link to version
@shadeyg56 shadeyg56 deleted the readme-update branch February 23, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants