Skip to content

Commit 9c0e39d

Browse files
author
belinea4071
committed
refactor: tariff-pause warning-flap suppression (v1.6.3 review)
Pre-deploy review for v1.6.3 (#301 PR series) flagged duplicated ``_tariff_pause_warned = False`` calls in the daytime tariff-pause path — once inside the EXPENSIVE branch, once after it. The review's read was that the EXPENSIVE-branch clear was a bug; closer reading shows both clears are inside the ``try``, so both fire only on a successful ``get_price_level()``. The actual invariant is ``successful read ⇒ flag cleared``, regardless of which price level came back. Refactor pulls the clear above the price-level branch so the invariant is local to one line. Behaviour-identical to the prior factoring but no longer reads like a price-level bug to future reviewers. New ``TestTariffPauseWarningFlap`` covers the contract: - one warning per provider outage, even across many ticks - successful read with EXPENSIVE price clears the flag - successful read with CHEAP price clears the flag - a recovery + fresh outage re-arms and re-fires Follow-up cleanups deferred to #304 (select.py orphan-removal gap for previously-removed chargers) and #305 (dead _auto_mode_strategy + min_pv mapper branch). 2140 / 2140 tests passing (4 new).
1 parent b90001d commit 9c0e39d

2 files changed

Lines changed: 135 additions & 4 deletions

File tree

coordinator/coordinator.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2464,18 +2464,24 @@ def _determine_charging_strategy(self, power: PowerReadings, energy: Any,
24642464
if mode == "solar_plus_cheap":
24652465
try:
24662466
level = self._tariff_provider.get_price_level()
2467+
# A successful call means the provider is healthy — drop
2468+
# the one-shot ``provider error'' warning flag regardless
2469+
# of the returned price level. The next exception will
2470+
# re-arm it. Cleared *before* the branch on ``level`` so
2471+
# the EXPENSIVE and CHEAP paths share identical recovery
2472+
# semantics (#301 review pushed back on a duplicated
2473+
# clear that read like a price-level bug; the canonical
2474+
# invariant is ``successful read ⇒ no pending warning'').
2475+
if getattr(self, "_tariff_pause_warned", False):
2476+
self._tariff_pause_warned = False
24672477
if level in (PriceLevel.EXPENSIVE, PriceLevel.VERY_EXPENSIVE):
24682478
_LOGGER.debug(
24692479
"Tariff pause: %s price — solar_plus_cheap "
24702480
"falling through to pure surplus", level.value,
24712481
)
24722482
# Falls through to self_consumption to enforce
24732483
# surplus-only during expensive windows.
2474-
if getattr(self, "_tariff_pause_warned", False):
2475-
self._tariff_pause_warned = False
24762484
return self._self_consumption_strategy(power, energy)
2477-
if getattr(self, "_tariff_pause_warned", False):
2478-
self._tariff_pause_warned = False # provider recovered
24792485
except Exception as e:
24802486
# Surface once (#274/L1) — a persistently broken provider
24812487
# would otherwise silently keep the surplus-from-grid

tests/test_ev_deadline_tariff.py

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- the daytime tariff pause in _determine_charging_strategy
77
- the deadline current-floor applied in _execute_ev_control's night branch
88
"""
9+
import logging
910
from datetime import timedelta
1011
from unittest.mock import AsyncMock, MagicMock, patch
1112

@@ -358,6 +359,130 @@ def test_min_plus_solar_ignores_tariff_signal(self):
358359
assert strategy == "solar_only"
359360

360361

362+
class TestTariffPauseWarningFlap:
363+
"""``_tariff_pause_warned`` rate-limits the one-shot ``provider error''
364+
warning when the tariff provider raises while ``solar_plus_cheap`` is
365+
active. v1.6.3 review surfaced an unguarded code path: the original
366+
factoring cleared the flag in two places (the EXPENSIVE branch and the
367+
cheap-or-normal branch). The duplicated clear looked like a bug
368+
(reviewer flagged it), but the actual invariant is
369+
``successful get_price_level() ⇒ flag cleared`` regardless of price.
370+
The refactor pulled the clear above the price-level branch so the
371+
invariant is local to one line. These tests pin that contract so a
372+
future split-by-branch doesn't silently restore the multi-warn loop.
373+
"""
374+
375+
def _power(self, battery_soc=50):
376+
return PowerReadings(
377+
solar_power=2000.0, grid_power=-500.0, home_consumption_power=1500.0,
378+
battery_power=0.0, battery_soc=battery_soc, ev_power=0.0,
379+
ev_connected=True, ev_charging=False,
380+
)
381+
382+
def _drive(self, coord, n=1):
383+
"""Run the daytime strategy ``n`` times for ``solar_plus_cheap``."""
384+
cfg = {"id": "keba", "charge_mode": "solar_plus_cheap"}
385+
coord.time_manager.is_night_mode = MagicMock(return_value=False)
386+
for _ in range(n):
387+
coord._determine_charging_strategy(
388+
self._power(battery_soc=50), MagicMock(daily_ev=0.0), cfg,
389+
)
390+
391+
def test_warning_emitted_once_per_provider_outage(self, caplog):
392+
"""A first provider error logs the warning; subsequent ticks
393+
during the same outage do not re-fire it."""
394+
coord = _build_coordinator(tariff_on=True)
395+
coord._tariff_provider.get_price_level = MagicMock(
396+
side_effect=RuntimeError("provider down"),
397+
)
398+
399+
with caplog.at_level(logging.WARNING,
400+
logger="custom_components.solar_energy_management.coordinator.coordinator"):
401+
self._drive(coord, n=5)
402+
403+
msgs = [r.message for r in caplog.records
404+
if "Tariff-optimized daytime pause disabled" in r.message]
405+
assert len(msgs) == 1, (
406+
f"expected one warning across 5 ticks of a single outage; got {len(msgs)}"
407+
)
408+
assert coord._tariff_pause_warned is True
409+
410+
def test_flag_cleared_on_successful_call_even_when_price_expensive(self):
411+
"""Successful ``get_price_level()`` is the recovery signal — the
412+
flag must clear regardless of whether the returned price is
413+
CHEAP or EXPENSIVE. Pinning the v1.6.3 invariant: a provider
414+
recovery during an expensive window still re-arms the next
415+
warning rather than letting it silently re-fire."""
416+
coord = _build_coordinator(tariff_on=True)
417+
# Tick 1: provider errors → flag set
418+
coord._tariff_provider.get_price_level = MagicMock(
419+
side_effect=RuntimeError("provider down"),
420+
)
421+
self._drive(coord, n=1)
422+
assert coord._tariff_pause_warned is True
423+
424+
# Tick 2: provider recovers but happens to return EXPENSIVE
425+
# → flag MUST clear (EXPENSIVE return is still a successful call).
426+
coord._tariff_provider.get_price_level = MagicMock(
427+
return_value=PriceLevel.EXPENSIVE,
428+
)
429+
self._drive(coord, n=1)
430+
assert coord._tariff_pause_warned is False, (
431+
"successful read with EXPENSIVE price must clear the flag; "
432+
"otherwise the next outage silently fails the rate-limit "
433+
"because the flag is already True"
434+
)
435+
436+
def test_flag_cleared_on_successful_call_with_cheap_price(self):
437+
"""Same invariant via the other branch — successful CHEAP read
438+
also clears the flag."""
439+
coord = _build_coordinator(tariff_on=True)
440+
coord._tariff_provider.get_price_level = MagicMock(
441+
side_effect=RuntimeError("provider down"),
442+
)
443+
self._drive(coord, n=1)
444+
assert coord._tariff_pause_warned is True
445+
446+
coord._tariff_provider.get_price_level = MagicMock(
447+
return_value=PriceLevel.CHEAP,
448+
)
449+
self._drive(coord, n=1)
450+
assert coord._tariff_pause_warned is False
451+
452+
def test_warning_re_fires_after_recovery_then_new_outage(self, caplog):
453+
"""Recovery resets the rate-limit, so a fresh outage warns
454+
again. This is the user-visible behaviour the suppression
455+
wants: notify on transitions, not every tick of a continuous
456+
outage."""
457+
coord = _build_coordinator(tariff_on=True)
458+
459+
# Outage 1
460+
coord._tariff_provider.get_price_level = MagicMock(
461+
side_effect=RuntimeError("provider down"),
462+
)
463+
with caplog.at_level(logging.WARNING,
464+
logger="custom_components.solar_energy_management.coordinator.coordinator"):
465+
self._drive(coord, n=3)
466+
467+
# Recovery
468+
coord._tariff_provider.get_price_level = MagicMock(
469+
return_value=PriceLevel.CHEAP,
470+
)
471+
self._drive(coord, n=2)
472+
473+
# Outage 2
474+
coord._tariff_provider.get_price_level = MagicMock(
475+
side_effect=RuntimeError("provider down again"),
476+
)
477+
self._drive(coord, n=3)
478+
479+
msgs = [r.message for r in caplog.records
480+
if "Tariff-optimized daytime pause disabled" in r.message]
481+
assert len(msgs) == 2, (
482+
f"expected one warning per outage across 2 outages; got {len(msgs)}"
483+
)
484+
485+
361486
# ---------------------------------------------------------------------------
362487
# Deadline floor in _execute_ev_control night branch (#246)
363488
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)