Skip to content

Refactor forecast metrics into dedicated ForecastMetrics module#374

Open
MaStr wants to merge 7 commits into
mainfrom
claude/pv-battery-metrics
Open

Refactor forecast metrics into dedicated ForecastMetrics module#374
MaStr wants to merge 7 commits into
mainfrom
claude/pv-battery-metrics

Conversation

@MaStr

@MaStr MaStr commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Motivation

The original _compute_night_surplus() method tried to answer one question
("how much battery is left at dawn?") but the answer depended on whether solar
was currently active or not, making the single value ambiguous for automation
decisions. After working through the four relevant scenarios (solar active now /
surplus or not, nighttime / surplus tomorrow or not), the metric was replaced by
a more expressive triplet:

Metric Meaning
solar_surplus_wh PV overflow that cannot be stored — safe to run flexible loads now
pv_start_battery_wh Battery level when PV first exceeds consumption — how depleted will the battery be at dawn?
forecast_min_battery_wh Minimum battery over the entire forecast horizon — will the battery ever run out?

These three values form a 2-D decision matrix for flexible load control (heat
pump, EV charging). See docs/WIKI_forecast_metrics.md for the full decision
guide.

Key Changes

New module: src/batcontrol/forecast_metrics.py

All three metrics extracted into a dedicated ForecastMetrics class with
static methods. Pure computation, no inverter or MQTT dependencies.

  • solar_active_and_surplus() — moved from Batcontrol._compute_solar_active_and_surplus()
  • pv_start_battery()new: slot-by-slot simulation until net-charging crossover (net_consumption < 0)
  • forecast_min_battery()new: deepest trough over full forecast horizon with floor/ceiling clamping

Both new metrics use proper slot-by-slot simulation instead of a net-sum, because the battery floor (MIN_SOC) and ceiling (MAX_SOC) make a naive sum incorrect across multi-day cycles.

Removed from Batcontrol (core.py)

  • _compute_solar_active_and_surplus() instance method (~30 lines)
  • _compute_night_surplus() instance method (~60 lines)

MQTT API changes

Topic Change
/night_surplus_wh Removed — replaced by the two metrics below
/pv_start_battery_wh Added — battery level (Wh above MIN_SOC) at next net-charging point
/forecast_min_battery_wh Added — minimum battery level (Wh above MIN_SOC) over forecast horizon
/solar_surplus_wh, /solar_active Unchanged

Home Assistant auto-discovery updated accordingly. A tombstone (empty retained
payload) is published to the legacy batcontrol_night_surplus_wh discovery
topic on connect so existing brokers remove the stale HA entity automatically.
The tombstone is marked # TODO(0.9.1) for removal once brokers have been cleaned up.

Bug fixes included in this PR

  • Stale net_consumption in core.py: net_consumption was computed
    before production[0] / consumption[0] were factorized for elapsed time
    (lines 606-607). The array was not updated after the in-place mutation.
    pv_start_battery and forecast_min_battery now receive the correct
    time-adjusted slot-0 value.

Tests

  • Removed: tests/batcontrol/test_night_surplus.py
  • Updated: tests/batcontrol/test_solar_surplus.py — calls ForecastMetrics directly, no Batcontrol mock needed
  • Added: tests/batcontrol/test_pv_battery_metrics.py — 13 tests for pv_start_battery and forecast_min_battery covering edge cases (depletion before sunrise, multi-day troughs, 15-min resolution, floor clamp)

Documentation

  • Added: docs/WIKI_forecast_metrics.md — explains each metric, the 2-D decision
    matrix, MQTT topics, HA auto-discovery entities, and implementation notes

Test plan

  • All CI tests green
  • solar_surplus_wh, pv_start_battery_wh, forecast_min_battery_wh appear in HA after upgrade
  • night_surplus_wh HA entity disappears automatically (tombstone cleanup)
  • Verify slot-0 time-adjustment reflected in pv_start_battery_wh (check near interval boundary)

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h

claude added 5 commits June 8, 2026 18:24
…ttery_wh

night_surplus_wh was semantically ambiguous and computed a net-sum
approximation that ignored battery floor/ceiling clamping across multiple
charge/discharge cycles.

New metrics, both using slot-by-slot simulation with proper MIN/MAX SOC:

pv_start_battery_wh:
  Battery level (above MIN_SOC) at the next net-charging point, defined
  as the first forecast slot where net_consumption < 0 (PV exceeds load).
  This is the relevant indicator for evening/night WP decisions.

forecast_min_battery_wh:
  Minimum usable battery level over the entire forecast horizon.
  Tracks the trough of the simulation, not the final value.
  0 = battery expected to hit MIN_SOC at some point -> be conservative.

Both are published via MQTT with HA auto-discovery sensors.

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h
Reflects the new metric names: pv_start_battery_wh and forecast_min_battery_wh.

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h
All three _compute_* methods were pure functions with no dependency on
Batcontrol instance state. Moved to a dedicated module:

  src/batcontrol/forecast_metrics.py  (new)
    ForecastMetrics.solar_active_and_surplus()
    ForecastMetrics.pv_start_battery()
    ForecastMetrics.forecast_min_battery()

core.py now imports and calls ForecastMetrics directly. Tests updated
to import ForecastMetrics instead of binding unbound methods via MagicMock.

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h
core.py: net_consumption was computed before production[0]/consumption[0]
were factorized for elapsed time, so pv_start_battery and
forecast_min_battery received the full-slot slot-0 value instead of the
time-adjusted one. Recompute after the factorization.

mqtt_api.py: publish an empty retained payload to the old
batcontrol_night_surplus_wh discovery topic on connect so that existing
Home Assistant brokers remove the stale entity automatically.

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h
Copilot AI review requested due to automatic review settings June 8, 2026 18:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors forecast-derived battery metrics out of Batcontrol into a dedicated ForecastMetrics module, and replaces the prior “night surplus” signal with two more granular forecast metrics that are published via MQTT (including Home Assistant discovery updates).

Changes:

  • Added ForecastMetrics pure-style helpers for solar surplus plus two new battery forecast metrics (pv_start_battery, forecast_min_battery).
  • Updated Batcontrol.run() to use ForecastMetrics and publish the new MQTT metrics instead of night_surplus_wh.
  • Refactored tests to target ForecastMetrics directly; removed the legacy night_surplus tests and added new coverage for the new metrics.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/batcontrol/forecast_metrics.py New module implementing forecast-derived metrics used by Batcontrol and tested directly.
src/batcontrol/core.py Switches runtime metric computation from instance methods to ForecastMetrics and publishes new metrics.
src/batcontrol/mqtt_api.py Replaces night_surplus_wh publishing/discovery with new topics and adds a retained tombstone cleanup for HA discovery.
tests/batcontrol/test_solar_surplus.py Updates surplus tests to call ForecastMetrics directly (no Batcontrol mocking).
tests/batcontrol/test_pv_battery_metrics.py Adds tests for pv_start_battery and forecast_min_battery.
tests/batcontrol/test_night_surplus.py Removes legacy test suite for the deleted night-surplus metric.

Comment on lines +4 to +6
ForecastMetrics computes indicators from production/consumption forecast arrays
and current battery state. All methods are pure functions with no side effects.

_make_discovery_stub lacked api.client and api.auto_discover_topic,
causing AttributeError when the tombstone block in
send_mqtt_discovery_messages called self.client.is_connected() directly.
Added both to the stub.

Also update forecast_metrics.py module docstring to not claim the
methods are side-effect-free; they emit logger.debug() calls.

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Describes solar_surplus_wh, pv_start_battery_wh and
forecast_min_battery_wh: what each value means, when it is useful,
a 2x2 decision matrix for flexible load control, MQTT topics, HA
auto-discovery entities, and implementation notes.

https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h
@MaStr MaStr changed the title Refactor forecast metrics into dedicated module Refactor forecast metrics into dedicated ForecastMetrics module Jun 9, 2026
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.

3 participants