Peak Shaving: review findings and improvements

[MaStr]

Summary

Code review of the peak shaving feature revealed several areas for improvement. None are critical bugs, but they affect robustness, user experience, and code quality.

---

1. combined mode silently disabled when price_limit is not set

File: src/batcontrol/logic/next.py (lines ~237-239)

When mode: combined is configured but price_limit is omitted (defaults to None), _apply_peak_shaving() returns early and skips both the time-based and price-based components. This is surprising because the time component does not need a price_limit at all.

Suggestion: Fall back to time-only behaviour when price_limit is None in combined mode, or at least log a warning so users know why peak shaving is not active.

---

2. No validation in PeakShavingConfig dataclass

File: src/batcontrol/core.py (lines ~57-74)

PeakShavingConfig has no __post_init__ validation. Invalid values (e.g. mode: 'invalid', allow_full_battery_after: 99) are only caught later when CalculationParameters.__post_init__ fires. The resulting error message references CalculationParameters, not the config key, making debugging harder.

Suggestion: Add a __post_init__ validator to PeakShavingConfig that checks:

• mode in ('time', 'price', 'combined')
• 0 <= allow_full_battery_after <= 23
• price_limit is numeric or None

---

3. Incomplete MQTT runtime control (undocumented limitation)

enabled and allow_full_battery_after are settable via MQTT at runtime, but mode and price_limit are not. This asymmetry is not documented. Users may expect all peak shaving parameters to be changeable without restart.

Suggestion: Either add MQTT set-callbacks for mode and price_limit, or document this as a known limitation. An MQTT discovery entity for price_limit (as a number) would be a useful addition for HA users.

---

4. Incorrect test comment

File: tests/batcontrol/logic/test_peak_shaving.py (around line 341)

The comment in test_currently_in_cheap_slot_surplus_overflow states:

"Time-based (mode=combined): 6 slots to target, 6*3000=18000>5000 -> 5000/6=833 W"

This describes flat division, but the actual algorithm uses a counter-linear ramp: 2*5000/(6*7) = 238 W, raised to 500 W by the minimum charge rate. The test assertion (500) is correct -- only the comment is wrong.

Suggestion: Fix the comment to reflect the counter-linear formula.

---

5. Minor: MODE 8 reference in logic layer

File: src/batcontrol/logic/next.py (line ~293)

A comment says "MODE 8" which is an inverter implementation detail. The logic layer should ideally not reference mode numbers.

Suggestion: Replace with a descriptive comment like "limit_battery_charge_rate mode".