InfiniTime
InfiniTime copied to clipboard
InfiniTimeOrg
Approximate discharge curve using linear line segments
This PR builds on the work of PR 443 by approximating the discharge curve using multiple linear line segments.
Things to note:
-
As suggested by @Riksu9000, 3.5v (instead of 2.98v) is now defined as 0% battery charge. This is to encourage users to charge their device before the battery fully drains, to prevent excessive strain on the battery.
-
Also noted by @Riksu9000, the implementation of PR 443 can use at most two decimal digits of precision of the supplied voltage value (that is, decivolt precision), causing jumps in the resulting battery percentage of almost 5 percentage points around the 50%-35% battery charge range. Increasing the precision of the voltage (to e.g. millivolts), combined with approximating the discharge curve using multiple linear line segments, should reduce those jumps.
-
This implementation uses 7 linear line segments to approximate the observed discharge curve. Figure "Approximation using linear line segments" below illustrates this by plotting the 7 line segments (in red) against the observed discharge values (in blue). In this figure, the error of the approximated curve with respect to the observed curve is, on average, about 0.6 percentage points of battery charge. The maximum error is about 1.6 percentage points.
Approximation using linear line segments
- The figure "Battery percentages rounded to integers" displays the resulting battery percentages when rounded to integers. This is how the percentages would be presented to the user. As you can see, there are no big drops in battery percentage. Do note however, that the voltage value supplied as input to this implementation needs to have sufficient precision, otherwise jumps in the battery percentage along the curve will still occur.
Battery percentages rounded to integers
- The implementation uses a table of pre-calculated line segment slopes (percentageSlopes), to save a floating point division during calculation of the battery percentage. This table could be removed in favor of calculating the slopes on the fly, if we would feel that the additional memory costs do not outweigh the saved floating point division. The percentageOffsets table would need to hold an extra value, though, to be able to calculate the slope of the last line segment.
Did you redo the test because #444 and #483 may have changed the results?
Did you redo the test because #444 and #483 may have changed the results?
No, I didn't, I was hoping to avoid redoing the test. Do you suspect #444 and/or #483 alter the shape of the curve, or shift the voltage values along the y-axis? If it's just added precision, then I would not expect the shape of the curve to change significantly and I don't think it is much of a problem. However, if #444 or #483 may significantly alter the voltage values, then I can see that makes this implementation inaccurate.
The changes were not just about increasing precision, but fixing errors as well.
#483 fixes an issue with the ADC configuration. It couldn't get a proper sample before. This change shifts the voltages quite a bit. I don't think we can predict how much it shifts across the range unfortunately.
In #444 the conversion from ADC value to voltage was changed. Previously for some reason the voltage divider was being compensated by 2.04 and not 2, so the multplier in the end was 5.9765625, and now it is 5.859375.
The changes were not just about increasing precision, but fixing errors as well.
#483 fixes an issue with the ADC configuration. It couldn't get a proper sample before. This change shifts the voltages quite a bit. I don't think we can predict how much it shifts across the range unfortunately.
In #444 the conversion from ADC value to voltage was changed. Previously for some reason the voltage divider was being compensated by 2.04 and not 2, so the multplier in the end was 5.9765625, and now it is 5.859375.
Ok, thanks for clarifying. I guess that means we cannot safely assume the current data to still be accurate and I should redo the measurements. It'll be some work, but it's probably the best way to make sure that we're using accurate values.
I redid the measurements after updating the firmware to include the changes from #444 and #483. The new observed discharge curve is shown here in full:
The observed discharge curve from full battery to system turning off
Approximating the newly observed curve using 7 line segments and defining 3.5v to be 0% battery charge gives the result shown below. The average error with respect to the observed curve is slightly more than 0.5 percentage points and the maximum error is slightly less than 1.7 percentage points.
Approximation using linear line segments
Lastly, when the battery percentages are rounded to integers, that graph will look like this:
I updated the tables in the implementation to use the values for these new line segments.
I think this PR is kind of broken right now. It has the previous values instead of the new ones and a lot of other changes. I would fix this by jumping to a previous working commit and start fixing conflicts from there.
Also it's interesting that the maximum voltage dropped to 4157. It can reach higher than that while charging at least, but if anything it would just show 100% for a bit longer, which is fine.
Hi there, I'm solely a InfiniTime user but I'd like to contribute two ideas.
-
The line segment approach was what came first to my mind, too. And it might very well the best choice for the job (precise enough, not consuming too much flash). I was wondering though, if Bezier curves could be an alternative that occupies even less flash space, while being quite precise, too. I'd assume 3 points on the curve plus two control points should be enough.
-
A higher battery voltage reading while charging is to be expected. If I'm not mistaken, a charge indication is available. So I'm wondering if one could compensate the voltage rise with an offset? Maybe one could simply measure/calculate the voltage rise/offset on detection of the charge indication.
I have a branch with this feature fixed up here https://github.com/Riksu9000/InfiniTime/tree/discharge_curve
The left slope is with the current code and the right one is with the new code. It looks like it has some dips, but I'm not sure if that just depends on the usage of the watch. It's also significantly better at the high and low percentages. I'll keep testing this a bit more still.
One idea that I'm experimenting with is to only decrease the percentage by one every read. This would minimize the effect of noise, because now in certain ranges noise could make the percentage vary significantly. This would require the percentage conversion to be somewhat accurate so the displayed percentage doesn't lag behind the "real" percentage.
Hello, Since 1.7.0 my PineTime last only a week where before i can be with the same charge for 11 days. The watch goes around 48% then you have a quick peak to 0%.
Show large images
Today another one week only of battery. This morning i was at 42% tonight i am at 17% doing nothing just wearing the watch.
I´ve applied your changes on top of the 1.9.0 tag (see: https://github.com/arjanvlek/InfiniTime/tree/feature/1.9.0-accurate-battery-percentage).
I'd have to say it is a great improvement over the previous implementation and I vote for this change to be merged in the develop branch.
Some more details:
Discharging
I now have an almost linear discharge graph (counting in usage, since screen-on will obviously drain the battery faster), so i can much better count on how long the watch will still last at a given battery percentage. I think this version works perfectly fine.
Charging
The battery percentage still increases very fast to around 50%, to continue charging to 100% at a much slower rate. This probably comes due to the voltage being higher during charging than during discharging. Maybe the algorithm can see some improvements by differentiating charging and discharging. With the charging process being relatively constant, a new set of segments for charging will probably be enough to get this right.
Noteworthy: Since charging is only needed for a few hours per week, I think fixing the charge curve is a lower priority than getting the discharge curve right.
I think that charging does generally happen faster at lower percentages (but don't quote me on that). I like this idea, already looked into the (currently in dev) code to find out that the curve was linear and hence the battery reading was acting so strangely. In general I support a discharge curve, since it should give a much better idea of battery percentage.
Barring the single justified quibble above, I would love for this to get merged. More reliable info on battery state would be exceedingly helpful.
To me it seems like hassless won't be working on this any time soon. Apologies to hassless if this is not the case but it seems the last activity from you was in February.
This is a shame in my opinion because this is something that a lot of people want and it's probably just one commit away from being merged. I'm willing to help move this along if possible and wanted.
Since this implementation is the only sensible way to do this it doesn't really make sense for me - or anyone - to make their own implementation since it would look pretty much exactly like this.
What is blocking this? I merged this branch locally two months ago and have been testing it since. I have not watched the charge curve too closely, but the battery percentage shown when discharging is much much better than with current develop.
The watch dying quickly after 40% is apparently a very frequent point of confusion and disappointment for new users. This is probably the PR that improves UX most, of all open ones, since it's such a basic functionality relevant for everybody, and the current behavior is so off.
@ght Well yes, the only thing that I can see that is blocking this from getting merged is an issue of code quality. There's a number that is hard coded which doesn't have any impact on the functionality but could maybe be annoying in the future, tho very unlikely in my opinion.
A commit fixing the one remaining issue that was mentioned is available at https://github.com/ght/InfiniTime/tree/pr/585-line-segment-discharge-curve (commit: https://github.com/ght/InfiniTime/commit/e12b266cfbdd35d2e0cf602e44db321aec228ac0), and below as a patch, to allow any maintainer who can push to this PR to fix that up with the least possible amount of extra work and get this merged.
diff --git a/src/components/battery/BatteryController.cpp b/src/components/battery/BatteryController.cpp
index 206b26ed..5fe5414d 100644
--- a/src/components/battery/BatteryController.cpp
+++ b/src/components/battery/BatteryController.cpp
@@ -115,7 +115,7 @@ uint8_t Battery::GetBatteryPercentageFromVoltage(uint16_t voltage) {
return 100;
}
- if (voltage <= voltageOffsets[7]) {
+ if (voltage <= voltageOffsets[LINE_SEGMENT_COUNT]) {
return 0;
}
"Issue of code quality" is putting it a bit harshly for that one little nit-pick, the overall code in this PR is well written, and fairly small: ~20 lines of code for such a big improvement, plus ~10 lines of comments documenting the implementation so it is easy to understand.
I think we should be grateful that @hassless has given up their time to write such a piece of elegant code with huge positive ramifications for the InfiniTime firmware. Thank you! 👍🏻 Let's hope this gets merged soon.
What is blocking this?
My and other core developers' time ;-) Reviewing PRs takes time and energy, and we are mostly busy working on other things in InfiniTime and/or busy in our respective lives. Sorry about that!
My and other core developers' time ;-) Reviewing PRs takes time and energy, and we are mostly busy working on other things in InfiniTime and/or busy in our respective lives. Sorry about that!
No worries, nothing to be sorry about! Thank you and the others for all the work you put into InfiniTime! :) I wasn't asking to stress you, but because I'm using this PR for a while and it's great and I wanted to help get it into everybody's hands and help fix the remaining issues, but was unsure what they were.
So nothing to be sorry about, it's done when it's done :)
@hassless are you alive and well? i'ts more than 9 months since the last commit could you give any news on this pr ?
