Conversation
|
I added a rescaling of the percentage values from 35-100 to 0-100 for better battery values and lowered the threshold to 20 (which corresponds to the 50 from before the rescaling). Edit: removed this in the later commits. |
|
@NeroBurner, since the battery percentage is now fixed and you already accepted my PR for InfiniSim I would appreciate if you could also take a look at this PR. But no pressure, I know you have a lot to do. |
NeroBurner
left a comment
NeroBurner
left a comment
There was a problem hiding this comment.
looking good, have just reviewed the code, not tested on InfiniSim or PineTime, currently out of time to check that
src/systemtask/SystemTask.cpp
Outdated
| "Low Battery\0Charge your watch to prevent data loss.\0"}; | ||
| notif.message = message; |
There was a problem hiding this comment.
can the temporary variable be omitted? just curious
There was a problem hiding this comment.
The {} init syntax is needed to write the C string to the std::array. A simple assignment does not work as far as I know. My hope is that the compiler figures that out.
Cleaning up this interface would be nice though. The easiest two methods would be
- moving the default fields
idandvalidto the end of the struct to allow unnamed struct initialization - C++20 allows C style init of structs with
{ .name = value }.
There was a problem hiding this comment.
Lets leave it here as it is and I suggest that I make a PR in which I do the reordering.
|
Thank you for taking the time you did! I will keep this updated. |
|
Has anyone already had the opportunity to test this PR since we merged #1444 ? Does the low battery warning feels accurate? |
|
I tested it for stability before, but can do it again for the feeling. You are asking if 15% are a good threshold value, right? |
Yes, and also if the battery level reported by the new algorithm works good enough for this use-case. |
|
Ok, I will give it another try and report back in a few days. |
|
15% gave me about 22 hours to charge the PineTime (although I think the current watch is at the upper limit of battery life with nearly 10 days from 100% to 0%) no notification, just the linear interpolated battery curve two full discharge curves:
|
|
Sounds good. |
|
It just crashed at 15% battery and ended in a boot loop. Some of the last changes must have broken something 🤔 Good that we tested it again. |
|
My battery lasted approximately another day after reaching 15%. I saw 0%, so the new values are good on my watch. Independently I fixed the crashes with a |
Co-authored-by: JF <JF002@users.noreply.github.com>
Squashed commit of the following: commit 11bdc2c Merge: 59567a2 cb91943 Author: Riku Isokoski <riksu9000@gmail.com> Date: Tue Jan 31 22:05:04 2023 +0200 Merge branch 'develop' into low-battery-indicator commit 59567a2 Author: Riku Isokoski <riksu9000@gmail.com> Date: Sat Dec 31 10:05:38 2022 +0200 Add low battery indicator to StatusIcons, digital and analog watchfaces Define deepOrange color in InfiniTimeTheme commit 3e86ed2 Author: Riku Isokoski <riksu9000@gmail.com> Date: Sat Dec 31 09:56:03 2022 +0200 Revert "added low battery message" This reverts PR InfiniTimeOrg#1352
This reverts PR InfiniTimeOrg#1352
4 participants
This PR implements a low battery warning (issue #1342) when the battery percentage reaches 15%.
I added the threshold to the BatteryController that can be checked via
BatteryController::BatteryIsLow().The BatteryController sends a
LowBatterymessage to the SystemTask that reacts by pushing a notification.I also updated the BatteryInfo settings screen to use the threshold to color its charging bar differently.