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

Battery calibration #169

Merged
merged 16 commits into from
Jun 28, 2024
Merged

Battery calibration #169

merged 16 commits into from
Jun 28, 2024

Conversation

KyrosWeb
Copy link
Contributor

Allow battery tool to discharge your battery until it reaches 15%. When the 15% is reached, it will fully recharge the battery and keep it at 100% for 1 hour.Finally the battery will be discharged until it reaches 80% and will be kept on this percentage.
From what I understand it would be advisable to calibrate the battery about every 3 months of continuous use at 80%

@LorenzoRogai
Copy link

Looking good for me 👍

@Manoplay
Copy link

Great job, seems good! 👍🏻

Copy link
Owner

@actuallymentor actuallymentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this command would be more useful it it runs in the background without needing to be foreground in a terminal. In addition to the comments here, it would be very useful if you could:

  1. Rename your function from calibrate to calibrate_syncronous
  2. Add a function calibrate and model it after the maintain function
  3. This function should use a new $calibrate_pidfile
  4. battery maintain and battery maintain_synchronous should check for $calibrate_pidfile and if it exists exit with Calibration is running, please run "battery calibrate stop" or wait for it to finish

I understand this increases the scope of this PR a lot, but it would take it to a great level of user-experience.

battery.sh Outdated Show resolved Hide resolved
battery.sh Outdated
# Discharge battery to 15%
battery discharge 15

while true; do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, please use the built in battery charge command. When battery charge 100 is done, the command exits and lets your script continue

Copy link
Contributor Author

@KyrosWeb KyrosWeb Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I use "battery charge 100" won't it start battery discharging after the execution? I need to wait 1 hour before discharging process. @actuallymentor

battery.sh Outdated Show resolved Hide resolved
@actuallymentor actuallymentor added the feature New feature or request label Aug 24, 2023
@KyrosWeb
Copy link
Contributor Author

I've done the calibration checking, i wait for others.
Thank you @actuallymentor

@KyrosWeb
Copy link
Contributor Author

What do you think if during the calibration the magsafe led will blinking in orange? @actuallymentor

@actuallymentor
Copy link
Owner

What do you think if during the calibration the magsafe led will blinking in orange? @actuallymentor

I think that's a very cool idea!

Do make sure to:

  1. Tell the user when they run the command
  2. Add the orange blink to the visudo list
  3. Add it to the visudo check (see visudo command usage of Sudo -n)

@KyrosWeb
Copy link
Contributor Author

What do you think if during the calibration the magsafe led will blinking in orange? @actuallymentor

I think that's a very cool idea!

Do make sure to:

  1. Tell the user when they run the command
  2. Add the orange blink to the visudo list
  3. Add it to the visudo check (see visudo command usage of Sudo -n)

I was checking about it but the led is continuously overwritten by other commands. Maybe something for the future!

@odpay
Copy link

odpay commented Oct 14, 2023

looks good to me

@hexclann
Copy link

Tested on Macbook Air M2, looks good!

Copy link
Owner

@actuallymentor actuallymentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you ver much for this PR @KyrosWeb!

My apologies for only getting to it so late.

I've requested some clarifications/edits, but they are quite minor. Let me know what you think and add edits to the extent that you think they make sense.

I have some bandwidth this week so will respond quicker than I have over the past months :)

battery.sh Outdated Show resolved Hide resolved
battery.sh Outdated Show resolved Hide resolved
battery.sh Outdated Show resolved Hide resolved
battery.sh Show resolved Hide resolved
battery.sh Show resolved Hide resolved
@KyrosWeb
Copy link
Contributor Author

@actuallymentor Check for the commit.

@seamusdemora
Copy link

Apologies for "butting in" at this late date, but...

Just out of curiosity, did the authors consult this article: Testing and Calibrating Smart Batteries in the development of this PR? I ask because I've not seen any mention of "Impedance Tracking"... I wonder if Apple actually uses this technology? It's been available in hardware for a while, but no idea how - or if - Apple has implemented it.

And please don't misunderstand; I think the PR is a sound idea, but I couldn't help but wonder if Apple might have implemented impedance tracking - and if anyone has given thought as to how to learn if they have - and how to take advantage of it if they have.

@KyrosWeb
Copy link
Contributor Author

@seamusdemora Hi , Yes i read it all and i think the process of discharging and recharging the battery when it stay lots of time stuck in a percentage is important to preserve the battery life and in order to avoid these situations : #191
This kind of method is also used by other battery maintaining app and I think mine is the faster and easier way to implement it

@seamusdemora
Copy link

@KyrosWeb :
OK - thank you... I've read through all of the above & think I understand the approach. Is this included the ver 1.2.1 Release??

P.S. I liked the approach except for the "blinking light" :) IMHO that might be distracting?

@KyrosWeb
Copy link
Contributor Author

@seamusdemora Hi , it will be included soon. I haven’t insert anymore the blinking light for your happiness 😂

@seamusdemora
Copy link

@KyrosWeb That is good to hear. I'm currently evaluating bclm, and so I will wait for the next release of battery before installing.

P.S. Yes, you have made me very happy :)

@djbob2000
Copy link

I'm looking forward to the release of this version with calibration:)

@KyrosWeb
Copy link
Contributor Author

I'm looking forward to the release of this version with calibration:)

We need to wait the repo owner.

@odpay
Copy link

odpay commented Feb 11, 2024

can we get a merge pls

@KyrosWeb
Copy link
Contributor Author

@actuallymentor Hey, no news for PR?

@actuallymentor actuallymentor merged commit 8d531ff into actuallymentor:main Jun 28, 2024
@actuallymentor
Copy link
Owner

Apologies on the delay, merged!

Thank you for your work @KyrosWeb!

Please keep an eye on the issues tab the coming weeks to make sure to catch any issues :)

@seamusdemora
Copy link

It's only been 4 ½ months - there's no rush; you need to take it easy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants