Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 71 additions & 24 deletions src/interfaces/pv_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ def __init__(
# Cache mechanism for fallback on API failures (similar to PriceInterface)
# When Akkudoktor is unavailable, reuse last successful forecast
self.last_successful_pv_forecast = []
self.last_successful_temp_forecast = []
self.consecutive_failures = 0
self.consecutive_temp_failures = 0
self.max_failures = 24 # Max consecutive failures before using defaults

self._update_thread = None
Expand Down Expand Up @@ -199,7 +201,9 @@ def reload_config(
}
# Reset cache when configuration changes (source switch, etc.)
self.last_successful_pv_forecast = []
self.last_successful_temp_forecast = []
self.consecutive_failures = 0
self.consecutive_temp_failures = 0

try:
self.__configure_update_interval()
Expand Down Expand Up @@ -666,7 +670,10 @@ def __update_pv_state_loop(self):
temp_result = self.__get_pv_forecast_akkudoktor_api(
tgt_value="temperature", pv_config_entry=temp_config
)
if not temp_result: # If empty array or None due to API error
# Reject empty/None results and physically implausible values
# (e.g. PV Watts leaking into the temperature array) as a
# fail-safe on top of the target-aware cache/counter below.
if not temp_result or any(v > 60 or v < -60 for v in temp_result):
logger.warning(
"[PV-IF] Temperature forecast API failed - using default"
+ " temperature forecast (15°C)"
Expand Down Expand Up @@ -1476,6 +1483,7 @@ def __get_pv_forecast_akkudoktor_api(
f"No PV config entry provided for target: {tgt_value}",
{},
"akkudoktor",
target=tgt_value,
)

# Use standard request format for both PV and temperature
Expand All @@ -1490,15 +1498,29 @@ def request_func():
day_values = response.json()
return day_values["values"]

# Tracked locally (not via self.pv_forcast_request_error, which can
# still hold a stale error from an earlier unrelated call) so we know
# whether day_values below is raw API JSON or an already-final
# fallback array from _handle_interface_error.
failure = {"occurred": False}

def error_handler(error_type, exception):
failure["occurred"] = True
return self._handle_interface_error(
error_type,
f"Akkudoktor API error for {tgt_value}: {exception}",
pv_config_entry,
"akkudoktor",
target=tgt_value,
)

day_values = self._retry_request(request_func, error_handler, 5, 3)
if failure["occurred"] and day_values:
# day_values is a non-empty cache _handle_interface_error chose as
# fallback; feeding it into the raw-JSON processing below would
# corrupt it. An empty fallback ([] - no cache available) is safe
# to fall through: the processing below pads it to tgt_duration.
return day_values

# Data processing
try:
Expand Down Expand Up @@ -1571,21 +1593,28 @@ def error_handler(error_type, exception):
)

if self.time_frame_base == 900 and tgt_value == "power":
return self._convert_hourly_to_15min(forecast_values)
# all value have to be repeated 4 times for 15min base for temperature
if self.time_frame_base == 900 and tgt_value == "temperature":
extended_values = []
result = self._convert_hourly_to_15min(forecast_values)
elif self.time_frame_base == 900 and tgt_value == "temperature":
# all values have to be repeated 4 times for 15min base for temperature
result = []
for val in forecast_values:
extended_values.extend([val] * 4)
return extended_values
return forecast_values
result.extend([val] * 4)
else:
result = forecast_values

if tgt_value == "temperature" and result:
self.last_successful_temp_forecast = list(result)
self.consecutive_temp_failures = 0

return result

except (ValueError, TypeError, AttributeError, KeyError) as e:
return self._handle_interface_error(
"processing_error",
f"Error processing {tgt_value} forecast data: {e}",
pv_config_entry,
"akkudoktor",
target=tgt_value,
)

def __get_horizon_elevation(self, sun_azimuth, horizon_for_elev):
Expand Down Expand Up @@ -2766,12 +2795,16 @@ def _retry_request(self, request_func, error_handler, max_retries=3, delay=1):
time.sleep(delay)

def _handle_interface_error(
self, error_type, message, pv_config_entry, source="unknown"
self, error_type, message, pv_config_entry, source="unknown", target="power"
):
"""
Centralized error handling for all API errors.
Uses last successful forecast as fallback if available.
Similar to PriceInterface.last_successful_prices mechanism.

`target` selects which cache/counter pair to use ("power" or
"temperature") so a failed temperature request never falls back to
the PV power cache (and vice versa).
"""
logger.error("[PV-IF] %s", message)
self.pv_forcast_request_error.update(
Expand All @@ -2783,34 +2816,42 @@ def _handle_interface_error(
"source": source,
}
)
self.consecutive_failures += 1

if target == "temperature":
self.consecutive_temp_failures += 1
failures = self.consecutive_temp_failures
last_successful = self.last_successful_temp_forecast
else:
self.consecutive_failures += 1
failures = self.consecutive_failures
last_successful = self.last_successful_pv_forecast

# Fallback strategy: Use last successful forecast if available
# and within failure threshold
if (
self.consecutive_failures <= self.max_failures
and len(self.last_successful_pv_forecast) > 0
):
if failures <= self.max_failures and len(last_successful) > 0:
logger.warning(
"[PV-IF] No forecast retrieved (failure %d/%d). Using last successful forecast.",
self.consecutive_failures,
"[PV-IF] No %s forecast retrieved (failure %d/%d)."
" Using last successful forecast.",
target,
failures,
self.max_failures,
)
return self.last_successful_pv_forecast
return last_successful

# If max failures exceeded or no cache available, return empty array
# (let caller handle default generation)
if len(self.last_successful_pv_forecast) == 0:
if len(last_successful) == 0:
logger.warning(
"[PV-IF] No forecast available and no cache - returning empty array"
"[PV-IF] No %s forecast available and no cache - returning empty array",
target,
)

# Log detailed recovery diagnostics for troubleshooting
self._log_error_diagnostics(error_type, source)
self._log_error_diagnostics(error_type, source, target)

return []

def _log_error_diagnostics(self, error_type, source):
def _log_error_diagnostics(self, error_type, source, target="power"):
"""
Log detailed error diagnostics including available sources and recovery hints.
Helps users troubleshoot and fix configuration issues faster.
Expand All @@ -2828,11 +2869,17 @@ def _log_error_diagnostics(self, error_type, source):
]
current_source = self.config_source.get("source", "unknown")

if self.consecutive_failures >= self.max_failures:
failures = (
self.consecutive_temp_failures
if target == "temperature"
else self.consecutive_failures
)
if failures >= self.max_failures:
logger.error(
"[PV-IF] Maximum failures reached (%d) - "
"[PV-IF] Maximum %s failures reached (%d) - "
"please check configuration in Settings > PV Forecast",
self.consecutive_failures,
target,
failures,
)

if source == "timeseries":
Expand Down
185 changes: 185 additions & 0 deletions tests/interfaces/test_pv_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,63 @@ def test_handle_interface_error_with_empty_config():
assert pv.pv_forcast_request_error["timestamp"] is not None


def test_handle_interface_error_temperature_target_ignores_pv_cache():
"""
Regression test for issue #276: a failed temperature request must not
fall back to the PV power cache, even when it holds unrelated Watt
values and the temperature cache is still empty.
"""
pv = PvInterface({}, [], time_frame_base, {}, timezone="UTC")
pv.last_successful_pv_forecast = [2090.0, 1500.0, 800.0]
pv.last_successful_temp_forecast = []

result = pv._handle_interface_error(
"timeout", "temp failed", {}, "akkudoktor", target="temperature"
)

assert result == []
assert pv.consecutive_temp_failures == 1
assert pv.consecutive_failures == 0


def test_handle_interface_error_temperature_target_uses_own_cache():
"""
Test that a failed temperature request falls back to its own cache when
available, not the PV power cache.
"""
pv = PvInterface({}, [], time_frame_base, {}, timezone="UTC")
pv.last_successful_pv_forecast = [2090.0, 1500.0, 800.0]
pv.last_successful_temp_forecast = [15.0, 16.0, 17.0]

result = pv._handle_interface_error(
"timeout", "temp failed", {}, "akkudoktor", target="temperature"
)

assert result == [15.0, 16.0, 17.0]


def test_consecutive_temp_failures_independent_of_pv_success():
"""
Temperature failures must accumulate toward max_failures even while PV
power keeps succeeding and resetting its own counter every cycle -
reproduces the 0/1 oscillation from issue #276 that kept the temperature
failure count from ever reaching the threshold.
"""
pv = PvInterface({}, [], time_frame_base, {}, timezone="UTC")
for _ in range(30):
# PV power fetch succeeds and resets only the power counter.
pv.last_successful_pv_forecast = [100.0]
pv.consecutive_failures = 0
# Temperature fetch fails every cycle.
pv._handle_interface_error(
"timeout", "temp failed", {}, "akkudoktor", target="temperature"
)

assert pv.consecutive_temp_failures == 30
assert pv.consecutive_temp_failures >= pv.max_failures
assert pv.consecutive_failures == 0


def test_default_pv_forecast_length_and_values():
"""
Test that the default PV forecast returns 48 values of type int or float.
Expand Down Expand Up @@ -422,6 +479,134 @@ def test_api_error_triggers_fallback(monkeypatch):
assert pv.pv_forcast_request_error["error"] in (None, "api_error")


def test_temperature_api_error_never_returns_pv_watts(monkeypatch):
"""
Regression test for issue #276: a total temperature-fetch failure must
never return data derived from the cached PV power (Watts) array, even
though both forecasts are fetched through this same shared method.
"""
pv = PvInterface({}, [], time_frame_base, {}, timezone="UTC")
pv.last_successful_pv_forecast = [2090.0, 1500.0, 800.0]
pv._retry_request = lambda req, err, *args, **kwargs: err(
"timeout", Exception("fail")
)

result = pv._PvInterface__get_pv_forecast_akkudoktor_api(
tgt_value="temperature",
pv_config_entry={
"lat": 50,
"lon": 8,
"azimuth": 180,
"tilt": 30,
"power": 100,
"powerInverter": 800,
"inverterEfficiency": 0.95,
"horizon": "0",
},
)

# No temperature cache exists yet, so this falls through to the same
# zero-padding default the power path uses (see test_api_error_triggers_fallback).
assert result == [0] * 48
assert 2090.0 not in result


def test_temperature_api_error_falls_back_to_temp_cache(monkeypatch):
"""
Once a temperature forecast has succeeded at least once, a later
failure must reuse the temperature cache, not the PV power cache.
"""
pv = PvInterface({}, [], time_frame_base, {}, timezone="UTC")
pv.last_successful_pv_forecast = [2090.0, 1500.0, 800.0]
pv.last_successful_temp_forecast = [15.0, 16.0, 17.0]
pv._retry_request = lambda req, err, *args, **kwargs: err(
"timeout", Exception("fail")
)

result = pv._PvInterface__get_pv_forecast_akkudoktor_api(
tgt_value="temperature",
pv_config_entry={
"lat": 50,
"lon": 8,
"azimuth": 180,
"tilt": 30,
"power": 100,
"powerInverter": 800,
"inverterEfficiency": 0.95,
"horizon": "0",
},
)

assert result == [15.0, 16.0, 17.0]


def _run_one_update_loop_iteration(pv):
"""
Helper to run exactly one iteration of the background update loop.
threading.Thread is stubbed by the autouse patch_thread fixture, so the
loop was never actually started - it is safe to invoke directly here.
"""
calls = {"n": 0}

def is_set_once():
calls["n"] += 1
return calls["n"] > 1

pv._stop_event.is_set = is_set_once
pv._PvInterface__update_pv_state_loop()


def test_update_loop_temperature_failure_never_uses_pv_cache(monkeypatch):
"""
End-to-end regression test for issue #276: with PV power succeeding and
caching Watt values, and the temperature fetch exhausting its own retries
with no temperature cache yet, the update loop must fall back to the
default 15C forecast - never to the cached PV power array.
"""
config = [{"name": "roof", "lat": 50, "lon": 8, "power": 5000}]
pv = PvInterface(
{}, config, time_frame_base, {}, temperature_forecast_enabled=True,
timezone="UTC",
)
pv.last_successful_pv_forecast = [2090.0, 1500.0, 800.0]
monkeypatch.setattr(pv, "get_summarized_pv_forecast", lambda scale=False: [100.0])
monkeypatch.setattr(pv, "apply_autoscaling", lambda values: values)
monkeypatch.setattr(
pv,
"_PvInterface__get_pv_forecast_akkudoktor_api",
lambda tgt_value, pv_config_entry: [],
)

_run_one_update_loop_iteration(pv)

assert pv.temp_forecast_array == pv._PvInterface__get_default_temperature_forecast()
assert pv.temp_forecast_array != pv.last_successful_pv_forecast


def test_update_loop_rejects_implausible_temperature_values(monkeypatch):
"""
Defense-in-depth: even if a mislabeled PV-Watts array slipped past the
target-aware cache fix, physically implausible values must still be
rejected in favor of the default 15C forecast.
"""
config = [{"name": "roof", "lat": 50, "lon": 8, "power": 5000}]
pv = PvInterface(
{}, config, time_frame_base, {}, temperature_forecast_enabled=True,
timezone="UTC",
)
monkeypatch.setattr(pv, "get_summarized_pv_forecast", lambda scale=False: [100.0])
monkeypatch.setattr(pv, "apply_autoscaling", lambda values: values)
monkeypatch.setattr(
pv,
"_PvInterface__get_pv_forecast_akkudoktor_api",
lambda tgt_value, pv_config_entry: [2090.0, 1500.0, 800.0],
)

_run_one_update_loop_iteration(pv)

assert pv.temp_forecast_array == pv._PvInterface__get_default_temperature_forecast()


def test_get_current_pv_forecast_returns_array():
"""
Test that get_current_pv_forecast returns the correct array.
Expand Down
Loading