Add PeakShavingConfig validation and combined-mode fallback#336
Merged
Conversation
- next.py: in 'combined' mode fall back to time-only when price_limit is None and log a warning, so the time component is no longer silently disabled. 'price' mode still skips when price_limit is missing. - next.py: replace 'MODE 8' inverter-layer reference with a descriptive comment about limit_battery_charge_rate mode. - core.py: add PeakShavingConfig.__post_init__ validation for mode, allow_full_battery_after, and price_limit. Invalid values now raise a ValueError referencing the peak_shaving.* config key instead of failing later in CalculationParameters. - docs: document that 'mode' and 'price_limit' are not settable at runtime via MQTT (restart required) and describe the combined-mode fallback. Mirror the note in batcontrol_config_dummy.yaml comments. - tests: fix the counter-linear formula comment in test_currently_in_cheap_slot_surplus_overflow; split the 'price_limit is None' case into combined (fallback) and price (skip) tests; add test_peak_shaving_config.py covering the new validation.
run_tests.sh uses `uv pip install -e .[test]` which writes a uv.lock into the working tree. The file is not intended to be tracked (no history of it in the repo), so ignore it alongside the other lock-file entries.
…fallback warning)
- logic_interface.py: introduce module-level PEAK_SHAVING_VALID_MODES
constant and reference it from CalculationParameters.__post_init__
(removes the inline ('time','price','combined') duplicate).
- logic/__init__.py: re-export PEAK_SHAVING_VALID_MODES for consumers.
- core.py: PeakShavingConfig uses the shared constant instead of its
own class-level VALID_MODES. Emit the combined+missing-price_limit
fallback warning here (once at config load) so we do not spam the log
every 3 minutes.
- next.py: demote the runtime fallback log to debug. The user-visible
warning is already produced at startup by PeakShavingConfig.
- tests: add four cases around the startup warning (fires on
enabled+combined+None, silent when disabled, when price_limit is set,
and in time mode).
Contributor
There was a problem hiding this comment.
Pull request overview
Adds early validation for peak shaving configuration and updates peak shaving runtime behavior so mode='combined' gracefully falls back to time-only limiting when price_limit is not configured.
Changes:
- Introduces
PeakShavingConfigvalidation (with warning) and wires it into core config loading. - Extracts
PEAK_SHAVING_VALID_MODESas a shared constant and updates peak shaving logic to support combined-mode fallback. - Expands tests and updates docs/config examples to reflect the new behavior and limitations.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/batcontrol/core.py |
Adds PeakShavingConfig validation + config loading + warning behavior. |
src/batcontrol/logic/next.py |
Implements combined-mode fallback to time-only when price_limit is None. |
src/batcontrol/logic/logic_interface.py |
Centralizes valid peak shaving modes in PEAK_SHAVING_VALID_MODES. |
src/batcontrol/logic/__init__.py |
Re-exports PEAK_SHAVING_VALID_MODES. |
tests/batcontrol/test_peak_shaving_config.py |
New unit tests for config validation and warning behavior. |
tests/batcontrol/logic/test_peak_shaving.py |
Updates/extends logic tests for fallback and revised combined behavior. |
docs/WIKI_peak_shaving.md |
Documents combined-mode optional price_limit and MQTT runtime limitations. |
config/batcontrol_config_dummy.yaml |
Updates example comments to match new config semantics. |
.gitignore |
Ignores uv.lock. |
…ine length)
- core.py PeakShavingConfig.__post_init__: reject bool for both
price_limit and allow_full_battery_after (bool is a subclass of int
in Python; an accidental ``price_limit: true`` would otherwise pass
validation and become 1.0). Also enforce int type for the hour value.
- core.py PeakShavingConfig.from_config: wrap float() coercion in
try/except so unparseable strings, lists, dicts and similar raise a
ValueError prefixed with peak_shaving.price_limit (matching the
key-based error policy of the other validators). bool is passed
through untouched so __post_init__ rejects it with the same prefix.
- logic/__init__.py: split the long re-export line across multiple
lines (was ~175 chars after PEAK_SHAVING_VALID_MODES was added).
- tests: cover bool rejection for both numeric fields, string and
list rejection from_config (with key-prefixed error), and the
numeric-string ('0.05') happy-path coercion.
Triage: the remaining Copilot suggestion to also strict-type
``enabled`` (e.g. reject quoted ``"false"``) is intentionally not
implemented here -- YAML parses ``false`` to a bool natively, and
HomeAssistant-form-field string coercion is a separate convention
already used elsewhere in core.py (see time_resolution_minutes).
Owner
Author
Triage of Copilot review commentsPushed
Tests: 7 new cases ( https://claude.ai/code/session_017PthEyvtN4XrYFf2MS5RP8 Generated by Claude Code |
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
Introduces a new
PeakShavingConfigdataclass with comprehensive validation and a fallback mechanism for combined-mode peak shaving whenprice_limitis not configured. This improves configuration error reporting and enables graceful degradation instead of complete disabling of peak shaving.Key Changes
New
PeakShavingConfigdataclass insrc/batcontrol/core.py:mode,allow_full_battery_after, andprice_limitat construction timeValueErrorwith clear config-key-based messages (e.g.,peak_shaving.mode) instead of failing later inCalculationParametersfrom_config()factory method for loading from configuration dictionariescombinedmode is enabled withoutprice_limitCombined-mode fallback behavior:
mode='combined'andprice_limit=None, the peak shaving now falls back to time-only behaviour instead of disabling entirelyprice_limit, so the counter-linear ramp continues to workmode='price'withprice_limit=Nonestill disables peak shaving entirely (no fallback available)Shared validation constant:
PEAK_SHAVING_VALID_MODES = ('time', 'price', 'combined')tologic_interface.pyas a single source of truthCalculationParametersandPeakShavingConfignow reference this constantUpdated peak shaving logic in
src/batcontrol/logic/next.py:price_limit=Nonein combined mode, switches to time-only internallyComprehensive test coverage in
tests/batcontrol/test_peak_shaving_config.py:from_config()factory methodDocumentation updates:
price_limitis optional for combined modemodeandprice_limitare not settable at runtime via MQTTNotable Implementation Details
PeakShavingConfig.__post_init__(), ensuring errors are caught at config load time rather than during calculationhttps://claude.ai/code/session_017PthEyvtN4XrYFf2MS5RP8