Skip to content

Conversation

@springfall2008
Copy link
Owner

Issue

Fixes #3107

When charge limit was set at reserve level (freeze charge), the status would sometimes show "Hold charging" instead of "Freeze charging", while the HTML plan correctly showed "FrzChrg". This inconsistency confused users.

Root Cause

The issue was caused by floating point rounding differences:

  • charge_limit_best[0] was rounded to 0.5 kWh (via dp2() in plan.py)
  • reserve was 0.51 kWh (5% of 10.149 kWh battery, via dp3() in execute.py)
  • Direct equality comparison 0.5 == 0.51 failed, preventing freeze charge detection

Solution

Created is_freeze_charge() helper function that compares percentages instead of raw kWh values:

  • 0.5 kWh / 10.149 kWh = 4.93% → rounds to 5%
  • 0.51 kWh / 10.149 kWh = 5.02% → rounds to 5%
  • 5% == 5% returns True

This avoids floating point precision issues while maintaining logical correctness.

Changes

  • Added is_freeze_charge() method in execute.py that converts kWh to percentage for comparison
  • Replaced all charge_limit == reserve comparisons with is_freeze_charge(charge_limit) calls:
    • execute.py: 8 locations
    • output.py: 2 locations
    • plan.py: 1 location
  • Added unit test charge_freeze_rounding_issue_3107 to verify fix with real-world values

Testing

  • All existing unit tests pass
  • New test specifically validates the reported scenario (0.5 vs 0.51 kWh)
  • Verified freeze charge detection now works consistently

…3107)

The issue occurred when charge_limit_best was rounded to 0.5 kWh (via dp2())
but reserve was 0.51 kWh (5% of 10.149 kWh battery, via dp3()). The direct
kWh comparison failed, causing status to show "Hold charging" instead of
"Freeze charging", and the HTML plan to show "FrzChrg" inconsistently.

Changes:
- Added is_freeze_charge() helper function that compares percentages instead
  of raw kWh values to avoid floating point precision issues
- Replaced all charge_limit vs reserve comparisons throughout execute.py,
  output.py, and plan.py with calls to is_freeze_charge()
- Added unit test to verify freeze charge detection works with rounding differences

This ensures 0.5 kWh and 0.51 kWh both correctly equal 5% when compared,
making freeze charge detection robust against rounding inconsistencies.
Copilot AI review requested due to automatic review settings December 23, 2025 18:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a floating point rounding issue that prevented proper freeze charge detection. When the charge limit and reserve values were very close but not exactly equal due to rounding at different precisions (dp2 vs dp3), the system would incorrectly show "Hold charging" instead of "Freeze charging".

Key Changes:

  • Introduced is_freeze_charge() helper method that compares percentages instead of raw kWh values to avoid floating point precision issues
  • Updated rounding precision from dp2 to dp3 for charge_limit_best and reserve calculations
  • Added comprehensive unit test to validate the fix with real-world values

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/predbat/execute.py Added is_freeze_charge() method and replaced 8 direct reserve comparisons with calls to the new helper
apps/predbat/output.py Updated 2 freeze charge detection points to use is_freeze_charge() helper
apps/predbat/plan.py Updated 1 freeze charge comparison and changed rounding precision from dp2 to dp3 for charge/export limits
apps/predbat/inverter.py Changed reserve calculation from dp2 to dp3 for consistency
apps/predbat/tests/test_execute.py Added new unit test charge_freeze_rounding_issue_3107 to verify the fix

save="best" if publish else None,
end_record=self.end_record,
)
# round charge_limit_best (kWh) to 2 decimal places and export_limits_best (percentage) to nearest whole number
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 1123 needs to be updated. It currently states "round charge_limit_best (kWh) to 2 decimal places" but the code now uses dp3() which rounds to 3 decimal places. The comment should be updated to reflect this change.

Suggested change
# round charge_limit_best (kWh) to 2 decimal places and export_limits_best (percentage) to nearest whole number
# round charge_limit_best (kWh) and export_limits_best (percentage) to 3 decimal places

Copilot uses AI. Check for mistakes.
self.charge_limit_best = [dp2(elem) for elem in self.charge_limit_best]
self.export_limits_best = [dp2(elem) for elem in self.export_limits_best]
self.charge_limit_best = [dp3(elem) for elem in self.charge_limit_best]
self.export_limits_best = [dp3(elem) for elem in self.export_limits_best]
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from dp2 to dp3 for export_limits_best appears unrelated to the freeze charge detection issue being fixed. The export_limits_best values are percentages (0-100), not kWh values like charge_limit_best. The original comment states these should be rounded to "nearest whole number", which suggests dp0() would be more appropriate than dp3(). If the extra precision is intentional for export limits, the comment should be updated to explain why. Otherwise, this change should be reverted to maintain the original rounding behavior for export limits.

Copilot uses AI. Check for mistakes.
@springfall2008 springfall2008 merged commit c1c80e7 into main Dec 23, 2025
1 check passed
@springfall2008 springfall2008 deleted the fixes27 branch December 23, 2025 18:51
iangregory pushed a commit to iangregory/batpred that referenced this pull request Dec 24, 2025
…pringfall2008#3107) (springfall2008#3115)

* Fix freeze charge detection failing due to floating point rounding (springfall2008#3107)

The issue occurred when charge_limit_best was rounded to 0.5 kWh (via dp2())
but reserve was 0.51 kWh (5% of 10.149 kWh battery, via dp3()). The direct
kWh comparison failed, causing status to show "Hold charging" instead of
"Freeze charging", and the HTML plan to show "FrzChrg" inconsistently.

Changes:
- Added is_freeze_charge() helper function that compares percentages instead
  of raw kWh values to avoid floating point precision issues
- Replaced all charge_limit vs reserve comparisons throughout execute.py,
  output.py, and plan.py with calls to is_freeze_charge()
- Added unit test to verify freeze charge detection works with rounding differences

This ensures 0.5 kWh and 0.51 kWh both correctly equal 5% when compared,
making freeze charge detection robust against rounding inconsistencies.

* Update apps/predbat/tests/test_execute.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove dead code

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Predbat status doesn't match plan

2 participants