Refactor forecast metrics into dedicated module#374
Open
MaStr wants to merge 6 commits into
Open
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
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
Extract battery forecast metrics computation from
Batcontrolinto a new pure-function moduleForecastMetrics, replacing night-surplus logic with two more granular metrics:pv_start_battery(battery level at next net-charging point) andforecast_min_battery(minimum battery level over forecast horizon).Key Changes
New module:
src/batcontrol/forecast_metrics.pyForecastMetrics.solar_active_and_surplus()— moved fromBatcontrol._compute_solar_active_and_surplus()ForecastMetrics.pv_start_battery()— new metric: battery level when PV first exceeds consumptionForecastMetrics.forecast_min_battery()— new metric: minimum battery level over entire forecast horizonRemoved from
Batcontrol:_compute_solar_active_and_surplus()method_compute_night_surplus()method and all associated test file (tests/batcontrol/test_night_surplus.py)Updated
Batcontrol.run():ForecastMetrics.solar_active_and_surplus()instead of instance method_compute_night_surplus()with two new metrics:pv_start_battery()→ publish viamqtt_api.publish_pv_start_battery()forecast_min_battery()→ publish viamqtt_api.publish_forecast_min_battery()MQTT API changes:
/night_surplus_whtopic/pv_start_battery_wh— battery level (Wh above MIN_SOC) at next net-charging point/forecast_min_battery_wh— minimum battery level (Wh above MIN_SOC) over forecast horizonnight_surplus_whdiscovery configTest refactoring:
tests/batcontrol/test_night_surplus.py(137 lines)tests/batcontrol/test_solar_surplus.py— now testsForecastMetricsdirectly instead of mockingBatcontroltests/batcontrol/test_pv_battery_metrics.py— comprehensive tests for both new metrics (103 lines)Implementation Details
ForecastMetricsmethods are pure functions with no side effects, acceptingnet_consumptionarrays and battery state parameterspv_start_battery()simulates discharge slot-by-slot until the first net-charging slot (production > consumption)forecast_min_battery()tracks the deepest trough during slot-by-slot simulation with proper floor/ceiling clampingsolar_active_and_surplus()behavior while improving testability and separation of concernshttps://claude.ai/code/session_01AbZuZ73iEQMiwTkwy3Mt1h