fix(inverter): average battery power across each SoC step in find_charge_curve

[springfall2008]

Problem

find_charge_curve() walks target_minute over the minutes the battery spends at each 1% of SoC, but read battery_power at the fixed trigger minute instead:

total_power += abs(battery_power.get(minute, 0))   # minute, not target_minute
total_count += 1

Because the accumulator adds the same value every iteration, average_power = total_power / total_count reduces algebraically to abs(battery_power[minute]) for any period length. The curve was built from a single instantaneous sample, not the mean the surrounding code and naming (total_power, average_power, "the power for this data point average has been stored") describe.

The loop's two break guards indexed the trigger minute for the same reason, so charge_rate and battery_power were only ever evaluated on the period's first minute — the walk could continue past the point where full-rate charging had actually stopped.

Fix

Index target_minute in all three places, so the curve reflects the mean power over each genuinely sustained full-rate step.

Both parts are the same bug and are fixed together deliberately: correcting the accumulator alone would average over minutes the stale guards should have excluded, which is wrong in a new way.

Why it wasn't caught

Every existing fixture holds battery power constant for a whole charging session, where the mean equals any single sample, so all four variants agree.

The new test test_find_charge_curve_averages_power_across_period varies power within each step: steps below 93% draw a steady 2600W, while steps at 93%+ average 1040W but read 2600W at either end — so a curve sampling one minute of the step reads 2600W whichever end it triggers on and comes out flat. Only an averaging curve shows the taper.

• before the fix: {85..100: 1.0} — flat
• after the fix: 1.0 below 93%, 0.4 above (= 1040/2600) ✅

Both ends carry the high reading on purpose, so the test doesn't depend on which end of the step triggers.

Impact

Measured against 21 days of real history from a GivEnergy system, this moves 6 of 14 curve points, by up to 0.08, in both directions — so it is not a uniform bias, it was genuinely sampling the wrong minute.

Note the returned curve is normalised by its own maximum, so a uniformly-scaled change lands in the suggested battery_rate_max_scaling rather than in the curve values; only shape changes show up in the curve itself.

Testing

• ./run_all --test find_charge_curve — all 6 tests pass; verified the new test fails on unfixed code and passes with the fix
• ./run_all --quick — all tests pass, 20/20 random scenarios match the baseline across 320 fields (they don't exercise find_charge_curve)
• ./run_pre_commit — all 25 hooks pass

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes Inverter.find_charge_curve() so it computes the mean battery power across the entire minute-span of each 1% SoC step, instead of repeatedly sampling the trigger minute. This improves the learned charge/discharge curve shape when power tapers within a step (common at high SoC), which impacts downstream planning decisions.

Changes:

• Update find_charge_curve() to index target_minute for full-rate guards and power accumulation, so the computed curve reflects the sustained period.
• Add a new test fixture with within-step power variation and a regression test asserting the curve tapers when averaged.

Reviewed changes

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

File	Description
apps/predbat/inverter.py	Fixes charge/discharge period scanning and averaging to use target_minute, producing a true per-step mean.
apps/predbat/tests/test_find_charge_curve.py	Adds tapering power history fixture and a regression test to detect single-sample vs averaged behavior.

Suppressed comments (1)

apps/predbat/tests/test_find_charge_curve.py:475

• Same cleanup issue as above: remove_test_history_data should run before returning early when the curve is missing required SoC keys, otherwise the history mock can leak.

    if not low or not high:
        print("ERROR: Curve missing the SoC range needed to check the taper: {}".format(sorted(charge_curve)))
        return True

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.