feat(battery): add option to smooth power for more stable time_remaining estimates#4761
feat(battery): add option to smooth power for more stable time_remaining estimates#4761Benji-clm wants to merge 1 commit intoAlexays:masterfrom
Conversation
|
@Alexays, this PR is getting stale :D |
There was a problem hiding this comment.
Pull request overview
This PR adds an optional power smoothing feature to the battery module to stabilize time remaining estimates by using exponential smoothing over current and historical power readings. The feature addresses issue #4755 where battery time estimates fluctuate wildly due to instantaneous power variations.
Changes:
- Added configurable power smoothing with exponential moving average algorithm
- Introduced
smooth-powerboolean config option (default: false) andsmooth-power-time-constantnumeric option (default: 260.0 seconds) - Modified time remaining calculation to use smoothed power values for discharging state only
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| include/modules/battery.hpp | Added member variables for smooth power tracking: enable flag, time constant, smoothed power value, timestamp, and status tracking |
| src/modules/battery.cpp | Implemented configuration parsing, exponential smoothing algorithm, and integrated smoothed power into time remaining calculations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <cstdio> | ||
| #include <string> | ||
|
|
There was a problem hiding this comment.
These includes appear to be unused. The header file already includes <string> (line 14 of include/modules/battery.hpp), and there are no uses of cstdio functions like printf, sprintf, or fprintf in the added code. Consider removing these unnecessary includes.
| #include <cstdio> | |
| #include <string> |
| if (total_energy_exists && total_power_exists && total_power != 0) { | ||
| if (!smoothPowerEnable_) { | ||
| smooth_power_ = total_power; | ||
| } else { | ||
| if (status != old_status_raw_) { | ||
| smooth_power_ = total_power; | ||
| last_t_ = std::chrono::steady_clock::now(); | ||
| } else { | ||
| std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); | ||
| double dt_s = | ||
| std::chrono::duration_cast<std::chrono::duration<double> >(now - last_t_).count(); | ||
| smooth_power_ = smooth_power_ + ((1 - std::exp(-dt_s / time_constant_s_)) * | ||
| (total_power - smooth_power_)); | ||
| last_t_ = now; | ||
| } | ||
|
|
||
| old_status_raw_ = status; | ||
| } | ||
| } |
There was a problem hiding this comment.
The smoothing algorithm updates smooth_power_ regardless of battery status, but according to the PR description, smoothing is only intended for discharging. While smooth_power_ is correctly only used when discharging (line 597) and gets reset when status changes (line 578), updating it during charging is unnecessary computation since the value is discarded on status change. Consider wrapping the smoothing logic with a condition like if (status == "Discharging") to avoid updating smooth_power_ when it won't be used.
| if (time_to_empty_now != 0) time_remaining = (float)time_to_empty_now / 3600.0f; | ||
| } else if (status == "Discharging" && total_power_exists && total_energy_exists) { | ||
| if (total_power != 0) time_remaining = (float)total_energy / total_power; | ||
| if (smooth_power_ != 0) time_remaining = (float)total_energy / smooth_power_; |
There was a problem hiding this comment.
When total_power is 0, the smoothing logic at line 573 won't execute, leaving smooth_power_ at its previous value. This means line 597 could calculate time_remaining using a stale smooth_power_ value when total_power is 0, which differs from the original behavior where time_remaining wouldn't be calculated if total_power was 0. Consider either resetting smooth_power_ to 0 when total_power is 0, or checking total_power in addition to smooth_power_ on line 597.
| if (smooth_power_ != 0) time_remaining = (float)total_energy / smooth_power_; | |
| if (total_power != 0 && smooth_power_ != 0) | |
| time_remaining = (float)total_energy / smooth_power_; |
2 participants
This PR adds support for an option to have power based on current and past power readings to stabilize the estimation of the time remaining on the battery.
How aggressive the smoothing/responsive to changes/noise can be changed by changing
smooth-power-time-constantwhich is the half life, in seconds, of each measurement.I've decided to not use the smooth power for charging, as battery charging curves tend to be much less noisy and more predictable/smoother.
Fixes #4755