Refactor forecast metrics into dedicated ForecastMetrics module#374
Open
MaStr wants to merge 7 commits into
Open
Refactor forecast metrics into dedicated ForecastMetrics module#374MaStr wants to merge 7 commits into
MaStr wants to merge 7 commits into
Conversation
…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
Contributor
There was a problem hiding this comment.
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
ForecastMetricspure-style helpers for solar surplus plus two new battery forecast metrics (pv_start_battery,forecast_min_battery). - Updated
Batcontrol.run()to useForecastMetricsand publish the new MQTT metrics instead ofnight_surplus_wh. - Refactored tests to target
ForecastMetricsdirectly; removed the legacynight_surplustests 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
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
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.
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:
solar_surplus_whpv_start_battery_whforecast_min_battery_whThese three values form a 2-D decision matrix for flexible load control (heat
pump, EV charging). See
docs/WIKI_forecast_metrics.mdfor the full decisionguide.
Key Changes
New module:
src/batcontrol/forecast_metrics.pyAll three metrics extracted into a dedicated
ForecastMetricsclass withstatic methods. Pure computation, no inverter or MQTT dependencies.
solar_active_and_surplus()— moved fromBatcontrol._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 clampingBoth 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
/night_surplus_wh/pv_start_battery_wh/forecast_min_battery_wh/solar_surplus_wh,/solar_activeHome Assistant auto-discovery updated accordingly. A tombstone (empty retained
payload) is published to the legacy
batcontrol_night_surplus_whdiscoverytopic 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
net_consumptionincore.py:net_consumptionwas computedbefore
production[0]/consumption[0]were factorized for elapsed time(lines 606-607). The array was not updated after the in-place mutation.
pv_start_batteryandforecast_min_batterynow receive the correcttime-adjusted slot-0 value.
Tests
tests/batcontrol/test_night_surplus.pytests/batcontrol/test_solar_surplus.py— callsForecastMetricsdirectly, noBatcontrolmock neededtests/batcontrol/test_pv_battery_metrics.py— 13 tests forpv_start_batteryandforecast_min_batterycovering edge cases (depletion before sunrise, multi-day troughs, 15-min resolution, floor clamp)Documentation
docs/WIKI_forecast_metrics.md— explains each metric, the 2-D decisionmatrix, MQTT topics, HA auto-discovery entities, and implementation notes
Test plan
solar_surplus_wh,pv_start_battery_wh,forecast_min_battery_whappear in HA after upgradenight_surplus_whHA entity disappears automatically (tombstone cleanup)pv_start_battery_wh(check near interval boundary)https://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h