fix(car-charging): show car charging limit as % (with kWh) in debug logs, not mislabelled kWh-as-% - #4420
Merged
Merged
Conversation
…d as % car_charging_limit and car_charging_soc are converted from the user's configured percent into kWh internally, but three debug log lines still labelled the (by-then kWh) limit value with a "%" suffix - a cosmetic mislabel, not a functional bug (spotted while investigating #4416). Now logs the limit as both a percent (recomputed from the kWh value and battery size, matching how users actually think about a charge target) and the underlying kWh figure, e.g. "limit 80% (61.6kWh)". Related to #4416
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Predbat’s car-charging debug logs to avoid mislabelling internally kWh-based car_charging_limit values as percentages, by recomputing and displaying the percentage alongside the kWh value (e.g., 80% (61.6kWh)). This aligns debug output with how users configure targets (percent) while preserving the internal kWh arithmetic visibility for troubleshooting (as discussed in #4416).
Changes:
- Recompute charge-limit percent from the internal kWh limit + battery size for logging.
- Update three debug log lines to display
limit {percent}% ({kWh}kWh)instead of a misleading%suffix on kWh values. - Keep log output consistent across car-planning and car-status debug messages.
Comment on lines
1126
to
1128
| self.log( | ||
| "Car {} plan charging from {} to {}, with slots {} from SoC {}% to {}%, ready by {}".format( | ||
| "Car {} plan charging from {} to {}, with slots {} from SoC {}kWh to {}% ({}kWh), ready by {}".format( | ||
| car_n, |
Collaborator
Author
There was a problem hiding this comment.
Fixed - collapsed to a single SoC → limit expression with explicit units (kWh, plus recomputed %), slot list and ready-by time kept as-is. Pushed in d7e3d9c.
…plan log Per Copilot review on #4420: the log repeated SoC/limit twice - once with no units (ambiguous, could read as times or percentages) and once with units. Collapsed to a single SoC -> limit expression with explicit units.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
car_charging_limit/car_charging_socare converted from the user's configured percent into kWh internally for the SoC-gap arithmetic, but three debug log lines still labelled the (by-then kWh) limit value with a%suffix - spotted while investigating car_charging_plan_smart produces two opposite failure modes depending on max_price — blocks all planning at 0p, wildly over-plans (including peak-rate slots) at 30p #4416, where it looked like a scaling bug but turned out to be a pure log-label mismatch.limit 80% (61.6kWh)- percent is how users actually think about a charge target, kWh is still there for anyone debugging the underlying arithmetic.Related to #4416
Test plan
./run_all --quickpassesrun_pre_commitcleanfetch.pyall updated for consistency