InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.7k stars 923 forks source link

Proper battery percentage #1398

Closed AlexXZero closed 1 year ago

AlexXZero commented 1 year ago

This PR adds linear approximation util and use it for making battery discharge curve more linear.

These changes were done independently on https://github.com/InfiniTimeOrg/InfiniTime/pull/585, I guess my solution is more generic. The main idea of my PR is adding linear approximation which might be used in any other feature including this old PR. I used my own measurements of battery, not sure if they accurate enough, so you are welcome to update my digits:

  static const Utility::LinearApproximation<uint16_t, uint8_t, 5> aprox {{{
    {3200, 0},  // minimum voltage of battery before shutdown ( depends on the battery )
    {3600, 10}, // keen point corresponded to 10% of battery
    {3700, 25},
    {3800, 50},
    {4180, 100} // maximum voltage of battery ( max charging voltage is 4.21 )
  }}};

My measurements:

image blue - measured voltage red - measured percentage (using 1.9.0) orange - filtered voltage (I'm working on adding filter in another branch) green - expected percentage using this algorithm (actually data is filtered, but it shows trade line of approximation)

I can share my data by request.

minacode commented 1 year ago

I will cut out the hacky rescaling in my low battery warning PR for this. This looks good.

AlexXZero commented 1 year ago

I updated approximation points to get better results: image

blue - measured voltage orange - filtered voltage (filtering will be implemented as another feature) green - percentage using old algorithm black dashed - ideal percentage red - percentage using approximation (current PR version) purple - percentage using approximation + filtering (future version)

FintasticMan commented 1 year ago

Looks good! I quite like that #585 changes the minimum voltage to 3.5v, which makes it easier to remember to charge the watch on time. Maybe that could be done here as well?

AlexXZero commented 1 year ago

Changed minimum voltage to 3.5V (now red and purple drop down to 0% when voltage is 3.5V) image

AlexXZero commented 1 year ago

@JF002 I think this PR is ready to be merged into develop. Let me know if I need to run some extra tests before it. I have tested these changes on my dev kit and on a sealed during a few weeks and everything looks good. Here is diagram which only shows how battery percentage was dropped before (green line) and after (red line). It also shows measured voltage (blue line) and ideal percentage line.

image In my last commit I fixed code style to pass github pipeline, but I don't have permissions to ruin it.

alexdolzhenkovbdl commented 1 year ago

@JF002 It looks like I don't have permissions for merging it. But I believe it is ready for that. Could you provide any steps I need to do next or just merge the PR if you agree with what was done.

AlexXZero commented 1 year ago

@JF002 It looks like I don't have permissions for merging it. But I believe it is ready for that. Could you provide any steps I need to do next or just merge the PR if you agree with what was done.

Oh, it looks like I wrote my message with working account. So it was me :-)

JF002 commented 1 year ago

It looks like I don't have permissions for merging it.

Only me and the member of the @InfiniTimeOrg/core-developers team have write access to the repo :)

But I believe it is ready for that. Could you provide any steps I need to do next or just merge the PR if you agree with what was done.

Thanks for your contribution! The next step is to wait for us to review the PR, which can take some time (there are a lot of PRs in the backlog, as you can see).

There's already another PR that does more of less the same things : https://github.com/InfiniTimeOrg/InfiniTime/pull/585. How does it compare to this one?

AlexXZero commented 1 year ago

Thank you for your feedback. I saw this PR and agree that it seems to do the same thing. But I found their implementation too complex and more hardcoded to be used only for battery percentage. Also comparing with #585 it doesn't use float pointer numbers, so it should work quicker and take less memory/CPU resources. My changes adds class for approximation any data and anyone can reuse it for something else.

Actually I would appreciate merging any of this two PRs, as both of them improve user experience in terms of showing proper battery percentage.

FintasticMan commented 1 year ago

I'm currently collecting battery data from all 3 of my PineTimes, so that the data from multiple batteries can be combined. This should give slightly more accurate results.

JF002 commented 1 year ago

Actually I would appreciate merging any of this two PRs

I think this is what I'll eventually do :)

as both of them improve user experience in terms of showing proper battery percentage.

Agreed!

JF002 commented 1 year ago

We merged this PR in #1444. Thanks a lot for your work, @AlexXZero !