diff --git a/.gitignore b/.gitignore index 7c97a8ea..f321166b 100644 --- a/.gitignore +++ b/.gitignore @@ -126,6 +126,10 @@ ipython_config.py .pdm-python .pdm-build/ +# uv +# uv.lock is generated locally when running run_tests.sh; not tracked. +uv.lock + # PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm __pypackages__/ diff --git a/config/batcontrol_config_dummy.yaml b/config/batcontrol_config_dummy.yaml index 0f04553a..b3631097 100644 --- a/config/batcontrol_config_dummy.yaml +++ b/config/batcontrol_config_dummy.yaml @@ -48,7 +48,13 @@ battery_control_expert: # treated as cheap PV windows. Battery capacity is reserved so the PV # surplus during those cheap slots can be fully absorbed. # Use -1 to disable the price component without changing mode. -# Required for mode 'price' and 'combined'; ignored for mode 'time'. +# Required for mode 'price'. For mode 'combined' it is optional: +# when omitted, combined mode falls back to time-only behaviour and logs +# a warning. Ignored for mode 'time'. +# +# Runtime control via MQTT is limited to 'enabled' and +# 'allow_full_battery_after'. Changes to 'mode' or 'price_limit' require +# a restart of batcontrol. #-------------------------- peak_shaving: enabled: false diff --git a/docs/WIKI_peak_shaving.md b/docs/WIKI_peak_shaving.md index 2ba86311..2c0cb030 100644 --- a/docs/WIKI_peak_shaving.md +++ b/docs/WIKI_peak_shaving.md @@ -148,6 +148,10 @@ The evcc integration derives `mode` and `connected` topics automatically from th | `{base}/peak_shaving/enabled/set` | `true`/`false` | Enable/disable peak shaving | | `{base}/peak_shaving/allow_full_battery_after/set` | int 0-23 | Set target hour | +Note: `mode` and `price_limit` are **not** settable at runtime via MQTT. +Changing either requires editing `batcontrol_config.yaml` and restarting +batcontrol. See "Known Limitations" below. + ### Home Assistant Auto-Discovery The following HA entities are automatically created: @@ -161,3 +165,7 @@ The following HA entities are automatically created: 1. **No intra-day adjustment:** If clouds reduce PV significantly, the limit stays as calculated until the next evaluation cycle (every 3 minutes). The counter-linear ramp self-corrects automatically: high free capacity at the next cycle produces a higher allowed rate. 2. **Code duplication:** `NextLogic` is a copy of `DefaultLogic` with peak shaving added. Once stable, the two could be merged or refactored. + +3. **Partial MQTT runtime control:** Only `enabled` and `allow_full_battery_after` can be changed at runtime via MQTT. `mode` and `price_limit` are read from the configuration file once at startup and require a restart to change. If you need to toggle the price component on-the-fly, set `price_limit: -1` in the config so no slots qualify as cheap; the time component continues to work (in `combined` mode) without further changes. + +4. **`combined` mode without `price_limit`:** When `mode: combined` is configured but `price_limit` is omitted (or `null`), the price component is skipped and the logic falls back to time-only behaviour. A warning is logged so the fallback is visible. To use the price component, set a numeric `price_limit`; to disable peak shaving entirely, set `enabled: false`. diff --git a/src/batcontrol/core.py b/src/batcontrol/core.py index b04cf231..096a2ab4 100644 --- a/src/batcontrol/core.py +++ b/src/batcontrol/core.py @@ -29,6 +29,7 @@ from .logic import Logic as LogicFactory from .logic import CalculationInput, CalculationParameters from .logic import CommonLogic +from .logic import PEAK_SHAVING_VALID_MODES from .dynamictariff import DynamicTariff as tariff_factory from .inverter import Inverter as inverter_factory @@ -61,16 +62,64 @@ class PeakShavingConfig: allow_full_battery_after: int = 14 price_limit: Optional[float] = None + def __post_init__(self): + """Validate configuration values and raise ValueError with a clear, + config-key-based message on invalid input. Also emit a one-time + warning for the ``combined`` + missing ``price_limit`` fallback, + so the log message fires at config load (not every evaluation).""" + if self.mode not in PEAK_SHAVING_VALID_MODES: + raise ValueError( + f"peak_shaving.mode must be one of " + f"{PEAK_SHAVING_VALID_MODES}, got '{self.mode}'" + ) + if not isinstance(self.allow_full_battery_after, int) \ + or isinstance(self.allow_full_battery_after, bool): + raise ValueError( + f"peak_shaving.allow_full_battery_after must be an integer, " + f"got {type(self.allow_full_battery_after).__name__}" + ) + if not 0 <= self.allow_full_battery_after <= 23: + raise ValueError( + f"peak_shaving.allow_full_battery_after must be between " + f"0 and 23, got {self.allow_full_battery_after}" + ) + if self.price_limit is not None and ( + isinstance(self.price_limit, bool) + or not isinstance(self.price_limit, (int, float))): + raise ValueError( + f"peak_shaving.price_limit must be numeric or None, " + f"got {type(self.price_limit).__name__}" + ) + if self.enabled and self.mode == 'combined' and self.price_limit is None: + logger.warning( + "peak_shaving.mode='combined' but no peak_shaving.price_limit " + "configured: the price component is disabled; falling back " + "to time-only behaviour. Set a numeric price_limit or change " + "mode to 'time' to silence this warning." + ) + @classmethod def from_config(cls, config: dict) -> 'PeakShavingConfig': """ Create a PeakShavingConfig instance from a configuration dict. """ ps = config.get('peak_shaving', {}) price_limit_raw = ps.get('price_limit', None) + if price_limit_raw is None or isinstance(price_limit_raw, bool): + # ``None`` stays ``None``; bool is rejected by __post_init__ with a + # key-prefixed message. Skip float() so we do not lose the type info. + price_limit = price_limit_raw + else: + try: + price_limit = float(price_limit_raw) + except (TypeError, ValueError) as exc: + raise ValueError( + f"peak_shaving.price_limit must be numeric or None, " + f"got {price_limit_raw!r}" + ) from exc return cls( enabled=ps.get('enabled', False), mode=ps.get('mode', 'combined'), allow_full_battery_after=ps.get('allow_full_battery_after', 14), - price_limit=float(price_limit_raw) if price_limit_raw is not None else None, + price_limit=price_limit, ) diff --git a/src/batcontrol/logic/__init__.py b/src/batcontrol/logic/__init__.py index 1bcf2814..e3052a3f 100644 --- a/src/batcontrol/logic/__init__.py +++ b/src/batcontrol/logic/__init__.py @@ -1,4 +1,11 @@ from .logic import Logic -from .logic_interface import LogicInterface, CalculationParameters, CalculationInput, CalculationOutput, InverterControlSettings +from .logic_interface import ( + LogicInterface, + CalculationParameters, + CalculationInput, + CalculationOutput, + InverterControlSettings, + PEAK_SHAVING_VALID_MODES, +) from .common import CommonLogic from .next import NextLogic diff --git a/src/batcontrol/logic/logic_interface.py b/src/batcontrol/logic/logic_interface.py index e9e4ef9d..db98b1b1 100644 --- a/src/batcontrol/logic/logic_interface.py +++ b/src/batcontrol/logic/logic_interface.py @@ -4,6 +4,11 @@ import datetime import numpy as np +# Shared tuple of valid peak-shaving operating modes. Defined here so that +# both CalculationParameters (this module) and PeakShavingConfig +# (batcontrol.core) can reference a single source of truth. +PEAK_SHAVING_VALID_MODES = ('time', 'price', 'combined') + @dataclass class CalculationInput: """ Input for the calculation """ @@ -40,10 +45,10 @@ def __post_init__(self): f"peak_shaving_allow_full_after must be 0-23, " f"got {self.peak_shaving_allow_full_after}" ) - valid_modes = ('time', 'price', 'combined') - if self.peak_shaving_mode not in valid_modes: + if self.peak_shaving_mode not in PEAK_SHAVING_VALID_MODES: raise ValueError( - f"peak_shaving_mode must be one of {valid_modes}, " + f"peak_shaving_mode must be one of " + f"{PEAK_SHAVING_VALID_MODES}, " f"got '{self.peak_shaving_mode}'" ) if (self.peak_shaving_price_limit is not None diff --git a/src/batcontrol/logic/next.py b/src/batcontrol/logic/next.py index 89abe945..02495434 100644 --- a/src/batcontrol/logic/next.py +++ b/src/batcontrol/logic/next.py @@ -219,23 +219,36 @@ def _apply_peak_shaving(self, settings: InverterControlSettings, 'combined' - both limits active, stricter one wins Skipped when: - - 'price'/'combined' mode and price_limit is not configured + - 'price' mode and price_limit is not configured - No PV production right now (nighttime) - Past allow_full_battery_after hour (all modes) - Battery in always_allow_discharge region (high SOC) - Force-charge from grid active (MODE -1) - Discharge not allowed (battery preserved for high-price hours) + In 'combined' mode with price_limit=None, falls back to time-only + behaviour (the time component does not require price_limit). + Note: evcc checks (charging, connected+pv mode) are handled in core.py, not here. """ mode = self.calculation_parameters.peak_shaving_mode price_limit = self.calculation_parameters.peak_shaving_price_limit - # Price component needs price_limit configured - if mode in ('price', 'combined') and price_limit is None: - logger.debug('[PeakShaving] Skipped: price_limit not configured for mode %s', mode) - return settings + # Price component needs price_limit configured. + # For 'price' mode: skip entirely (no other component to fall back to). + # For 'combined' mode: fall back to time-only behaviour. The user is + # informed once at config-load time by PeakShavingConfig, so this + # path stays at debug level to avoid per-cycle log spam. + if price_limit is None: + if mode == 'price': + logger.debug('[PeakShaving] Skipped: price_limit not ' + 'configured for mode price') + return settings + if mode == 'combined': + logger.debug('[PeakShaving] price_limit not configured; ' + 'combined mode using time-only component') + mode = 'time' # No production right now: skip if calc_input.production[0] <= 0: @@ -290,7 +303,8 @@ def _apply_peak_shaving(self, settings: InverterControlSettings, settings.limit_battery_charge_rate, charge_limit) # Note: allow_discharge is already True here (checked above). - # MODE 8 requires allow_discharge=True to work correctly. + # The limit_battery_charge_rate mode in the inverter layer requires + # allow_discharge=True to work correctly. logger.info('[PeakShaving] mode=%s, PV limit: %d W ' '(price-based=%s W, time-based=%s W, full by %d:00)', diff --git a/tests/batcontrol/logic/test_peak_shaving.py b/tests/batcontrol/logic/test_peak_shaving.py index f12da1f9..3291d425 100644 --- a/tests/batcontrol/logic/test_peak_shaving.py +++ b/tests/batcontrol/logic/test_peak_shaving.py @@ -336,8 +336,15 @@ def test_discharge_not_allowed_skips_peak_shaving(self): self.assertEqual(result.limit_battery_charge_rate, -1) self.assertFalse(result.allow_discharge) - def test_price_limit_none_disables_peak_shaving(self): - """price_limit=None with mode='combined' -> peak shaving disabled entirely.""" + def test_price_limit_none_combined_falls_back_to_time_only(self): + """price_limit=None with mode='combined' -> falls back to time-only. + + The time component does not need price_limit, so combined mode + remains active with only the time-based limiter. At 08:00 with + target hour 14 (6 slots remaining) and production 5000 W, + consumption 500 W, free_capacity 5000 Wh, the counter-linear ramp + yields 2*5000/(6*7) ~= 238 W, raised to the 500 W floor. + """ params = CalculationParameters( max_charging_from_grid_limit=0.79, min_price_difference=0.05, @@ -355,6 +362,31 @@ def test_price_limit_none_disables_peak_shaving(self): ts = datetime.datetime(2025, 6, 20, 8, 0, 0, tzinfo=datetime.timezone.utc) result = self.logic._apply_peak_shaving(settings, calc_input, ts) + self.assertEqual(result.limit_battery_charge_rate, 500) + + def test_price_limit_none_price_mode_disables_peak_shaving(self): + """price_limit=None with mode='price' -> peak shaving disabled. + + 'price' mode has no fallback: without a price_limit, there is no + component to apply, so the entire peak shaving is skipped. + """ + params = CalculationParameters( + max_charging_from_grid_limit=0.79, + min_price_difference=0.05, + min_price_difference_rel=0.2, + max_capacity=self.max_capacity, + peak_shaving_enabled=True, + peak_shaving_allow_full_after=14, + peak_shaving_mode='price', + peak_shaving_price_limit=None, + ) + self.logic.set_calculation_parameters(params) + settings = self._make_settings() + calc_input = self._make_input([5000] * 8, [500] * 8, + stored_energy=5000, free_capacity=5000) + ts = datetime.datetime(2025, 6, 20, 8, 0, 0, + tzinfo=datetime.timezone.utc) + result = self.logic._apply_peak_shaving(settings, calc_input, ts) self.assertEqual(result.limit_battery_charge_rate, -1) def test_currently_in_cheap_slot_no_limit(self): @@ -381,8 +413,10 @@ def test_currently_in_cheap_slot_surplus_overflow(self): prices all 0 (cheap), production=3000W, consumption=0, 8 slots. Total surplus = 8 * 3000 = 24000 Wh > free=5000 Wh. Price-based: spread 5000 / 8 slots = 625 W. - Time-based (mode=combined): 6 slots to target, 6*3000=18000>5000 -> 5000/6=833 W. - min(625, 833) = 625. + Time-based (mode=combined): counter-linear ramp over 6 slots to + target hour, current-slot weight 2*5000/(6*7) ~= 238 W, raised to + the 500 W min_pv_charge_rate floor. + min(625, 500) = 500. """ settings = self._make_settings() prices = np.zeros(8) diff --git a/tests/batcontrol/test_peak_shaving_config.py b/tests/batcontrol/test_peak_shaving_config.py new file mode 100644 index 00000000..cd9d3413 --- /dev/null +++ b/tests/batcontrol/test_peak_shaving_config.py @@ -0,0 +1,176 @@ +"""Tests for PeakShavingConfig dataclass validation and construction. + +These tests verify that invalid configuration values raise a clear +``ValueError`` referencing the peak_shaving.* config key, rather than +failing later in ``CalculationParameters.__post_init__``. +""" +import pytest + +from batcontrol.core import PeakShavingConfig + + +class TestPeakShavingConfigValidation: + """Test PeakShavingConfig.__post_init__ validates inputs.""" + + def test_defaults_are_valid(self): + cfg = PeakShavingConfig() + assert cfg.enabled is False + assert cfg.mode == 'combined' + assert cfg.allow_full_battery_after == 14 + assert cfg.price_limit is None + + def test_all_valid_modes_accepted(self): + for mode in ('time', 'price', 'combined'): + cfg = PeakShavingConfig(mode=mode) + assert cfg.mode == mode + + def test_invalid_mode_raises_value_error(self): + with pytest.raises(ValueError, match='peak_shaving.mode'): + PeakShavingConfig(mode='invalid') + + def test_allow_full_battery_after_out_of_range_high(self): + with pytest.raises(ValueError, + match='peak_shaving.allow_full_battery_after'): + PeakShavingConfig(allow_full_battery_after=99) + + def test_allow_full_battery_after_out_of_range_low(self): + with pytest.raises(ValueError, + match='peak_shaving.allow_full_battery_after'): + PeakShavingConfig(allow_full_battery_after=-1) + + def test_allow_full_battery_after_boundaries(self): + # boundary values (0 and 23) must be accepted + PeakShavingConfig(allow_full_battery_after=0) + PeakShavingConfig(allow_full_battery_after=23) + + def test_price_limit_none_accepted(self): + cfg = PeakShavingConfig(price_limit=None) + assert cfg.price_limit is None + + def test_price_limit_numeric_accepted(self): + cfg = PeakShavingConfig(price_limit=0.05) + assert cfg.price_limit == 0.05 + cfg_int = PeakShavingConfig(price_limit=-1) + assert cfg_int.price_limit == -1 + + def test_price_limit_string_raises_value_error(self): + with pytest.raises(ValueError, match='peak_shaving.price_limit'): + PeakShavingConfig(price_limit='cheap') + + def test_price_limit_bool_rejected(self): + # bool is a subclass of int but is almost certainly a config typo. + with pytest.raises(ValueError, match='peak_shaving.price_limit'): + PeakShavingConfig(price_limit=True) + with pytest.raises(ValueError, match='peak_shaving.price_limit'): + PeakShavingConfig(price_limit=False) + + def test_allow_full_battery_after_bool_rejected(self): + with pytest.raises(ValueError, + match='peak_shaving.allow_full_battery_after'): + PeakShavingConfig(allow_full_battery_after=True) + + def test_allow_full_battery_after_string_rejected(self): + with pytest.raises(ValueError, + match='peak_shaving.allow_full_battery_after'): + PeakShavingConfig(allow_full_battery_after='12') + + +class TestPeakShavingConfigFromConfig: + """Test PeakShavingConfig.from_config factory method.""" + + def test_missing_section_uses_defaults(self): + cfg = PeakShavingConfig.from_config({}) + assert cfg.enabled is False + assert cfg.mode == 'combined' + assert cfg.allow_full_battery_after == 14 + assert cfg.price_limit is None + + def test_full_config_applied(self): + cfg = PeakShavingConfig.from_config({ + 'peak_shaving': { + 'enabled': True, + 'mode': 'price', + 'allow_full_battery_after': 12, + 'price_limit': 0.10, + } + }) + assert cfg.enabled is True + assert cfg.mode == 'price' + assert cfg.allow_full_battery_after == 12 + assert cfg.price_limit == 0.10 + + def test_invalid_mode_from_config_raises(self): + with pytest.raises(ValueError, match='peak_shaving.mode'): + PeakShavingConfig.from_config({ + 'peak_shaving': {'mode': 'bogus'} + }) + + def test_invalid_hour_from_config_raises(self): + with pytest.raises(ValueError, + match='peak_shaving.allow_full_battery_after'): + PeakShavingConfig.from_config({ + 'peak_shaving': {'allow_full_battery_after': 99} + }) + + def test_unparseable_price_limit_string_raises_keyed_error(self): + # Reviewer comment: float() of an unparseable string should be + # re-wrapped with a peak_shaving.price_limit-prefixed message. + with pytest.raises(ValueError, match='peak_shaving.price_limit'): + PeakShavingConfig.from_config({ + 'peak_shaving': {'price_limit': 'cheap'} + }) + + def test_price_limit_list_raises_keyed_error(self): + with pytest.raises(ValueError, match='peak_shaving.price_limit'): + PeakShavingConfig.from_config({ + 'peak_shaving': {'price_limit': [0.05]} + }) + + def test_price_limit_bool_from_config_raises_keyed_error(self): + with pytest.raises(ValueError, match='peak_shaving.price_limit'): + PeakShavingConfig.from_config({ + 'peak_shaving': {'price_limit': True} + }) + + def test_price_limit_numeric_string_accepted(self): + # A YAML quoted number like "0.05" is a common HA-form-field shape; + # float() coerces it cleanly. + cfg = PeakShavingConfig.from_config({ + 'peak_shaving': {'price_limit': '0.05'} + }) + assert cfg.price_limit == 0.05 + + +class TestPeakShavingConfigFallbackWarning: + """Test the one-time warning for combined-mode + missing price_limit. + + The warning must fire at config load (not during runtime) and only + when the misconfiguration would actually be active (enabled=True and + mode='combined' with price_limit=None). + """ + + def test_combined_without_price_limit_logs_warning(self, caplog): + with caplog.at_level('WARNING', logger='batcontrol.core'): + PeakShavingConfig(enabled=True, mode='combined', price_limit=None) + messages = [r.getMessage() for r in caplog.records + if r.levelname == 'WARNING'] + assert any("combined" in m and "price_limit" in m for m in messages) + + def test_disabled_combined_without_price_limit_does_not_warn(self, caplog): + # When peak shaving is disabled there is no user-visible problem. + with caplog.at_level('WARNING', logger='batcontrol.core'): + PeakShavingConfig(enabled=False, mode='combined', price_limit=None) + warnings = [r for r in caplog.records if r.levelname == 'WARNING'] + assert warnings == [] + + def test_combined_with_price_limit_does_not_warn(self, caplog): + with caplog.at_level('WARNING', logger='batcontrol.core'): + PeakShavingConfig(enabled=True, mode='combined', price_limit=0.05) + warnings = [r for r in caplog.records if r.levelname == 'WARNING'] + assert warnings == [] + + def test_time_mode_without_price_limit_does_not_warn(self, caplog): + with caplog.at_level('WARNING', logger='batcontrol.core'): + PeakShavingConfig(enabled=True, mode='time', price_limit=None) + warnings = [r for r in caplog.records if r.levelname == 'WARNING'] + assert warnings == []