From 5b344dda0953f3222dbec1ae8ab9675b5e52b05a Mon Sep 17 00:00:00 2001 From: Matthias Strubel Date: Wed, 14 Oct 2026 15:28:40 +0200 Subject: [PATCH] fix(config): one name per thing - api_token and url (#451) A user configured the HomeAssistant Solar Forecast ML provider, put the long-lived access token into "apikey" and the websocket URL into "url", and got no configuration error at all - just {"type": "auth", "access_token": null} RuntimeError: Authentication failed: expected str at 'access_token'. Got None Both keys are valid in a pvinstallations entry, only for other providers: batcontrol used three spellings for a credential (apikey, apitoken, api) and two for an endpoint (url, base_url), depending on the section. The value was right, the key was not, and nothing said so. Canonical from now on: api_token for a credential, url for an endpoint, in every section. The old spellings are still read and log a deprecation warning once per section and key, so existing configurations keep working. Also fixed on the way: - solar.py read the HA-ML connection details only from pvinstallations[0], so a correctly filled entry further down the list was silently ignored - which is exactly what our own dummy config invites, with the fcsolarapi entry first and the HA-ML block as a commented second entry. The entry is now selected by "type" (documented before, read by nothing) or by entity_id; with several entries and no match startup stops instead of blaming an unrelated entry. - The provider now receives only its own entry, so refresh_data no longer pulls a full get_states dump once per unrelated installation. - Missing api_token/entity_id/url is reported as a configuration error naming the key and the installation, before any WebSocket is opened. The consumption path checked presence only, so apitoken: null passed. - cache_ttl_hours was documented for the solar provider since it was introduced, but nothing read it: the cache always ran on its 12h default. It is wired through the baseclass now; the default is unchanged. Co-Authored-By: Claude Opus 5 --- config/batcontrol_config_dummy.yaml | 17 +- docs/configuration/consumption-forecast.md | 42 ++- docs/configuration/dynamic-tariff-provider.md | 13 +- docs/configuration/solar-forecast.md | 56 +++- scripts/test_homeassistant_forecast.py | 4 +- src/batcontrol/dynamictariff/dynamictariff.py | 33 +- .../forecastconsumption/consumption.py | 25 +- src/batcontrol/forecastsolar/baseclass.py | 12 +- src/batcontrol/forecastsolar/evcc_solar.py | 8 +- src/batcontrol/forecastsolar/fcsolar.py | 11 +- .../forecast_homeassistant_ml.py | 57 +++- src/batcontrol/forecastsolar/solar.py | 167 ++++++++-- src/batcontrol/forecastsolar/solcast.py | 23 +- src/batcontrol/value_utils.py | 67 +++- .../dynamictariff/test_baseclass.py | 16 +- .../test_consumption_factory.py | 83 +++++ .../forecastsolar/test_solar_factory.py | 285 ++++++++++++++++++ .../batcontrol/forecastsolar/test_solcast.py | 14 +- tests/batcontrol/test_value_utils.py | 65 ++++ tests/test_forecast_solar_homeassistant_ml.py | 68 +++++ 20 files changed, 953 insertions(+), 113 deletions(-) create mode 100644 tests/batcontrol/forecastsolar/test_solar_factory.py diff --git a/config/batcontrol_config_dummy.yaml b/config/batcontrol_config_dummy.yaml index c5df213f..fd0e412c 100644 --- a/config/batcontrol_config_dummy.yaml +++ b/config/batcontrol_config_dummy.yaml @@ -117,7 +117,7 @@ utility: vat: 0.19 # awattar and energyforecast only fees: 0.015 # awattar and energyforecast only markup: 0.03 # awattar and energyforecast only - # apikey: YOUR_API_KEY # mandatory for tibber, energyforecast and energyforecast_total_price + # api_token: YOUR_API_KEY # mandatory for tibber, energyforecast and energyforecast_total_price (old name: apikey) # market_zone: DE # both energyforecast providers: DE/LU, AT, FR, NL, BE, PL, DK1, DK2. Default: DE # # energyforecast_total_price: the API returns the full price incl. dynamic network @@ -188,18 +188,21 @@ pvinstallations: # horizon: # leave empty for the default PVGIS horizon # homeassistant-solar-forecast-ml (HACS integration) + # Keep only the entries of the provider you selected above: fcsolarapi and + # solcast read every entry, so a leftover entry of another provider fails. # - name: HA Solar ML Forecast - # base_url: ws://homeassistant.local:8123 - # api_token: eyJ... # long-lived access token from the HA profile + # type: homeassistant-solar-forecast-ml # optional, explicit provider type + # url: ws://homeassistant.local:8123 # old name: base_url + # api_token: eyJ... # long-lived access token from the HA profile (old name: apikey) # entity_id: sensor.solar_forecast_ml_evcc_solar_prognose # sensor_unit: auto # auto (detect), Wh or kWh - # cache_ttl_hours: 24.0 + # cache_ttl_hours: 24.0 # how long a fetched forecast stays usable. Default: 12.0 # solcast - free hobbyist account, 10 requests/day. Site geometry is configured # in the Solcast web toolkit, not here. # - name: Haus # resource_id: xxxx-xxxx-xxxx-xxxx - # apikey: your-solcast-api-key + # api_token: your-solcast-api-key # old name: apikey # percentile: 50 # 10 pessimistic, 50 default, 90 optimistic #-------------------------- @@ -213,8 +216,8 @@ consumption_forecast: load_profile: load_profile.csv # file inside the config folder # homeassistant_api: - # base_url: ws://homeassistant:8123 - # apitoken: YOUR_LONG_LIVED_ACCESS_TOKEN + # url: ws://homeassistant:8123 # old name: base_url + # api_token: YOUR_LONG_LIVED_ACCESS_TOKEN # old name: apitoken # entity_id: sensor.energy_consumption # consumption data in Wh or kWh # sensor_unit: auto # auto (default), Wh or kWh. Fixed values skip detection # history_days: [-7, -14, -21] # days to look back diff --git a/docs/configuration/consumption-forecast.md b/docs/configuration/consumption-forecast.md index b448e8ae..9a93cead 100644 --- a/docs/configuration/consumption-forecast.md +++ b/docs/configuration/consumption-forecast.md @@ -114,12 +114,19 @@ The HomeAssistant API method provides **dynamic consumption forecasting** based ### Configuration +!!! info "Key names" + The credential is `api_token` and the endpoint is `url`, consistently across all + configuration sections. The older spellings `apitoken`, `apikey` and `base_url` are still + read, but log a deprecation warning at startup. The Home Assistant add-on offers only + `url` and `api_token` from version 0.10.0 on - rename them there in the same step as the + update. + ```yaml consumption_forecast: type: homeassistant-api homeassistant_api: - base_url: ws://homeassistant.local:8123 # Your HomeAssistant URL - apitoken: YOUR_LONG_LIVED_ACCESS_TOKEN # Long-Lived Access Token + url: ws://homeassistant.local:8123 # Your HomeAssistant URL + api_token: YOUR_LONG_LIVED_ACCESS_TOKEN # Long-Lived Access Token entity_id: sensor.energy_consumption # Entity ID with consumption data sensor_unit: auto # Options: 'auto', 'Wh', or 'kWh' (since 0.5.7) history_days: "-7;-14;-21" # Days to look back (negative values) @@ -134,8 +141,8 @@ consumption_forecast: | Parameter | Description | Example | |-----------|-------------|---------| -| `base_url` | HomeAssistant URL (ws is correct) | `ws://homeassistant.local:8123` | -| `apitoken` | Long-Lived Access Token from HomeAssistant | `eyJ0eXAiOiJKV1Qi...` | +| `url` | HomeAssistant URL (ws is correct) | `ws://homeassistant.local:8123` | +| `api_token` | Long-Lived Access Token from HomeAssistant | `eyJ0eXAiOiJKV1Qi...` | | `entity_id` | Entity ID tracking consumption (must have long-term statistics) | `sensor.energy_consumption` | #### Optional Parameters @@ -182,8 +189,8 @@ The `sensor_unit` parameter controls how batcontrol detects the unit of measurem consumption_forecast: type: homeassistant-api homeassistant_api: - base_url: ws://homeassistant.local:8123 - apitoken: YOUR_TOKEN + url: ws://homeassistant.local:8123 + api_token: YOUR_TOKEN entity_id: sensor.energy_consumption sensor_unit: kWh # Explicit configuration - faster startup! ``` @@ -349,9 +356,12 @@ The script will: ### HomeAssistant API Method -**Problem:** "Authentication failed" +**Problem:** "Authentication failed" or "expected str at 'access_token'. Got None" **Solution:** +- `Got None` means no token reached the provider: the credential key is `api_token` + (`apitoken` still works but is deprecated), and it belongs in the same block as + `entity_id` and `url` - Verify your access token is correct - Check if the token has been revoked in HomeAssistant - Create a new Long-Lived Access Token @@ -366,8 +376,8 @@ This error occurs when your HomeAssistant instance has many entities (sensors, l consumption_forecast: type: homeassistant-api homeassistant_api: - base_url: ws://homeassistant.local:8123 - apitoken: YOUR_TOKEN + url: ws://homeassistant.local:8123 + api_token: YOUR_TOKEN entity_id: sensor.energy_consumption sensor_unit: kWh # or 'Wh' depending on your sensor ``` @@ -391,7 +401,7 @@ consumption_forecast: **Problem:** "Connection refused" **Solution:** -- Verify `base_url` is correct and accessible from batcontrol +- Verify `url` is correct and accessible from batcontrol - Check if HomeAssistant is running - Verify network connectivity - Check firewall rules @@ -512,8 +522,8 @@ consumption_forecast: consumption_forecast: type: homeassistant-api homeassistant_api: - base_url: ws://192.168.1.100:8123 - apitoken: eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9... + url: ws://192.168.1.100:8123 + api_token: eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9... entity_id: sensor.house_energy_total sensor_unit: auto # Auto-detect (default) history_days: "-7;-14;-21" @@ -528,8 +538,8 @@ consumption_forecast: consumption_forecast: type: homeassistant-api homeassistant_api: - base_url: ws://homeassistant.local:8123 - apitoken: your_token_here + url: ws://homeassistant.local:8123 + api_token: your_token_here entity_id: sensor.energy_consumption sensor_unit: kWh # Explicit unit - faster startup, no auto-detection needed history_days: "-7;-14;-21" @@ -544,8 +554,8 @@ consumption_forecast: consumption_forecast: type: homeassistant-api homeassistant_api: - base_url: ws://homeassistant.local:8123 - apitoken: your_token_here + url: ws://homeassistant.local:8123 + api_token: your_token_here entity_id: sensor.energy_consumption sensor_unit: Wh # Explicit unit for optimal performance history_days: "-1;-2;-3" # Only last 3 days diff --git a/docs/configuration/dynamic-tariff-provider.md b/docs/configuration/dynamic-tariff-provider.md index d77f131b..606a9a09 100644 --- a/docs/configuration/dynamic-tariff-provider.md +++ b/docs/configuration/dynamic-tariff-provider.md @@ -9,13 +9,20 @@ Currently following data providers are available: You can chose one and need to adjust the configuration. +!!! info "Key names" + The credential is `api_token` and the endpoint is `url`, consistently across all + configuration sections. The older spellings `apikey`, `apitoken` and `base_url` are still + read, but log a deprecation warning at startup. The Home Assistant add-on offers only + `url` and `api_token` from version 0.10.0 on - rename them there in the same step as the + update. + ## tibber You need to get an API Key from https://developer.tibber.com/ , which looks like `Zz-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXx`. After obtaining this key, use the following configuration: ``` utility: type: tibber - apikey: YOUR-PASSWORD + api_token: YOUR-PASSWORD ``` @@ -178,7 +185,7 @@ to obtain an API key. ```yaml utility: type: energyforecast - apikey: xxxxxxxxx + api_token: xxxxxxxxx vat: 0.19 # 19% VAT fees: 0.15 # Your network fees (Netzentgelt), EUR/kWh excl. VAT markup: 0.00 # Optional markup, e.g. supplier margin @@ -205,7 +212,7 @@ This is useful if: ```yaml utility: type: energyforecast_total_price - apikey: xxxxxxxxx + api_token: xxxxxxxxx # market_zone: DE # optional: DE/LU (default, both map to DE-LU market zone), AT, FR, NL, BE, PL, DK1, DK2 ``` diff --git a/docs/configuration/solar-forecast.md b/docs/configuration/solar-forecast.md index cf0a27da..40d5bca5 100644 --- a/docs/configuration/solar-forecast.md +++ b/docs/configuration/solar-forecast.md @@ -93,12 +93,26 @@ The minimum configuration is: solar_forecast_provider: homeassistant-solar-forecast-ml pvinstallations: - name: HA Solar ML Forecast - base_url: ws://homeassistant.local:8123 # Your HomeAssistant URL + url: ws://homeassistant.local:8123 # Your HomeAssistant URL api_token: eyJ... # Long-lived access token from HA Profile entity_id: sensor.solar_forecast_ml_evcc_solar_prognose # Forecast sensor entity ``` -If you're running batcontrol in a HomeAssistant addon, use `ws://homeassistant:8123` as the base_url. For standalone installations, use your HomeAssistant IP or hostname. +If you're running batcontrol in a HomeAssistant addon, use `ws://homeassistant:8123` as the url. For standalone installations, use your HomeAssistant IP or hostname. + +!!! note "List only the entries of the provider you selected" + This provider uses a single `pvinstallations` entry: the one with a matching `type:`, or + the one carrying `entity_id`. Entries of *other* providers have to be removed, because + `fcsolarapi` and `solcast` read every entry in the list and fail on an entry that does not + carry their keys. With several entries and no way to tell which one is meant, batcontrol + stops at startup and asks you to add `type: homeassistant-solar-forecast-ml`. + +!!! info "Key names" + The credential is `api_token` and the endpoint is `url`, consistently across all + configuration sections. The older spellings `base_url`, `apikey` and `apitoken` are still + read, but log a deprecation warning at startup. The Home Assistant add-on offers only + `url` and `api_token` from version 0.10.0 on - rename them there in the same step as the + update. ### Optional Parameters @@ -107,11 +121,11 @@ You can customize the behavior with additional parameters: ```yaml pvinstallations: - name: HA Solar ML Forecast - base_url: ws://homeassistant:8123 + url: ws://homeassistant:8123 api_token: eyJ... entity_id: sensor.solar_forecast_ml_prognose_nachste_stunde sensor_unit: auto # Options: 'auto' (default, auto-detect), 'Wh', or 'kWh' - cache_ttl_hours: 24.0 # Cache duration in hours (default: 24.0) + cache_ttl_hours: 24.0 # Cache duration in hours (default: 12.0) ``` The `sensor_unit` parameter: @@ -121,6 +135,30 @@ The `sensor_unit` parameter: Setting the explicit unit (`Wh` or `kWh`) can speed up startup by skipping auto-detection. +The `cache_ttl_hours` parameter controls how long an already fetched forecast stays usable as a +fallback when HomeAssistant cannot be reached (default: 12 hours). It does not control how often +batcontrol asks for a new forecast — that is the refresh interval of the provider. + +### Troubleshooting + +**Problem:** batcontrol stops at startup with +`Auth message incorrectly formatted: expected str at 'access_token'. Got None` + +**Solution:** the access token never reached the provider. Check that + +- the token is configured as `api_token` — `apikey` is the key name of the fcsolarapi and + Solcast providers and was a common mix-up ([issue #451](https://github.com/MaStr/batcontrol/issues/451)) +- the token sits in the same `pvinstallations` entry as `entity_id` and `url` +- the entry belongs to the selected provider (see the note above about `type:`) + +Since batcontrol 0.10.0 a missing token is reported as a configuration error naming the key, +before any connection to HomeAssistant is attempted. + +**Problem:** `No "url" configured for installation , falling back to ws://homeassistant.local:8123` + +**Solution:** your URL is not in the entry batcontrol selected. Use the key `url` (or the +deprecated `base_url`) in the entry that also holds `entity_id`. + ## Solcast [Solcast](https://solcast.com/) provides satellite-based solar forecasts in native 30-minute resolution. batcontrol interpolates the values to 15-minute intervals (linear power interpolation) or aggregates them to hourly values, depending on your `time_resolution_minutes` setting. @@ -134,7 +172,7 @@ Solcast offers a free "Home User" (hobbyist) account which is sufficient for bat 1. Register at [solcast.com](https://solcast.com/free-rooftop-solar-forecasting) and choose the free **Home User** (hobbyist) plan. 2. In the Solcast toolkit, create a **Rooftop Site**. Enter your location (latitude/longitude), panel tilt, azimuth and capacity (kWp) there — the site geometry lives in your Solcast account, **not** in the batcontrol configuration. 3. Copy the site's **resource id** (shown in the site details, format `xxxx-xxxx-xxxx-xxxx`). -4. Copy your **API key** from the Solcast account settings. +4. Copy your **API key** from the Solcast account settings (configured as `api_token`). The free account is limited to **one location with up to two rooftop sites** (e.g. an east/west split) and **10 API requests per day**. @@ -147,7 +185,7 @@ solar_forecast_provider: solcast pvinstallations: - name: Haus #name resource_id: xxxx-xxxx-xxxx-xxxx # from your Solcast rooftop site - apikey: your-solcast-api-key # from your Solcast account settings + api_token: your-solcast-api-key # from your Solcast account settings ``` For two rooftop sites (e.g. east/west arrays) use one entry per site. The API key is the same for both, each site has its own resource id: @@ -157,10 +195,10 @@ solar_forecast_provider: solcast pvinstallations: - name: Dach Ost resource_id: xxxx-xxxx-xxxx-aaaa - apikey: your-solcast-api-key + api_token: your-solcast-api-key - name: Dach West resource_id: xxxx-xxxx-xxxx-bbbb - apikey: your-solcast-api-key + api_token: your-solcast-api-key ``` ### Optional percentile parameter @@ -171,7 +209,7 @@ You can select which estimate batcontrol uses per installation: pvinstallations: - name: Haus resource_id: xxxx-xxxx-xxxx-xxxx - apikey: your-solcast-api-key + api_token: your-solcast-api-key percentile: 10 # 10 (pessimistic), 50 (default, most likely), 90 (optimistic) ``` diff --git a/scripts/test_homeassistant_forecast.py b/scripts/test_homeassistant_forecast.py index 1b7c8050..7c6ed19c 100755 --- a/scripts/test_homeassistant_forecast.py +++ b/scripts/test_homeassistant_forecast.py @@ -133,8 +133,8 @@ def main(): # Build configuration dict for factory config = { 'type': 'homeassistant-api', - 'base_url': HOMEASSISTANT_URL, - 'apitoken': HOMEASSISTANT_TOKEN, + 'url': HOMEASSISTANT_URL, + 'api_token': HOMEASSISTANT_TOKEN, 'entity_id': ENTITY_ID, 'history_days': HISTORY_DAYS, 'history_weights': HISTORY_WEIGHTS, diff --git a/src/batcontrol/dynamictariff/dynamictariff.py b/src/batcontrol/dynamictariff/dynamictariff.py index 5eea1ad3..876d176d 100644 --- a/src/batcontrol/dynamictariff/dynamictariff.py +++ b/src/batcontrol/dynamictariff/dynamictariff.py @@ -17,6 +17,7 @@ """ import logging +from ..value_utils import get_config_value from .awattar import Awattar from .tibber import Tibber from .evcc import Evcc @@ -64,8 +65,10 @@ def create_tarif_provider(config: dict, timezone, timezone=timezone, country=nf_cfg['country'], operator=nf_cfg['operator'], - url=nf_cfg.get( - 'url', 'https://dyn-net.batcontrol.software/api/'), + url=get_config_value( + nf_cfg, 'url', + 'https://dyn-net.batcontrol.software/api/', + 'dynamic_network_fees'), delay_evaluation_by_seconds=delay_evaluation_by_seconds, ) @@ -110,12 +113,12 @@ def create_tarif_provider(config: dict, timezone, selected_tariff.set_network_fees_fetcher(network_fees_fetcher) elif provider.lower() == 'tibber': - if 'apikey' not in config.keys(): + token = get_config_value(config, 'api_token', None, 'utility') + if not token: raise RuntimeError( '[Dynamic Tariff] Tibber requires an API token. ' - 'Please provide "apikey :YOURKEY" in your configuration file' + 'Please provide "api_token: YOURKEY" in your configuration file' ) - token = config.get('apikey') selected_tariff = Tibber( timezone, token, @@ -125,7 +128,8 @@ def create_tarif_provider(config: dict, timezone, ) elif provider.lower() == 'evcc': - if 'url' not in config.keys(): + url = get_config_value(config, 'url', None, 'utility') + if not url: raise RuntimeError( '[Dynamic Tariff] evcc requires an URL. ' 'Please provide "url" in your configuration file, ' @@ -133,14 +137,18 @@ def create_tarif_provider(config: dict, timezone, ) selected_tariff = Evcc( timezone, - config.get('url'), + url, min_time_between_api_calls, target_resolution=target_resolution ) elif provider.lower() == 'energyforecast' or provider.lower() == 'energyforecast_96': - required_fields = ['vat', 'markup', 'fees', 'apikey'] - for field in required_fields: + token = get_config_value(config, 'api_token', None, 'utility') + if not token: + raise RuntimeError( + '[DynTariff] Please include api_token in your configuration file' + ) + for field in ('vat', 'markup', 'fees'): if field not in config.keys(): raise RuntimeError( f'[DynTariff] Please include {field} in your configuration file' @@ -154,7 +162,6 @@ def create_tarif_provider(config: dict, timezone, vat = float(config.get('vat', 0)) markup = float(config.get('markup', 0)) fees = float(config.get('fees', 0)) - token = config.get('apikey') market_zone = config.get('market_zone', 'DE') selected_tariff = Energyforecast( timezone, @@ -169,9 +176,10 @@ def create_tarif_provider(config: dict, timezone, selected_tariff.set_network_fees_fetcher(network_fees_fetcher) elif provider.lower() == 'energyforecast_total_price': - if 'apikey' not in config: + token = get_config_value(config, 'api_token', None, 'utility') + if not token: raise RuntimeError( - '[DynTariff] Please include apikey in your configuration file' + '[DynTariff] Please include api_token in your configuration file' ) for field in ('vat', 'fees', 'markup'): if field in config: @@ -185,7 +193,6 @@ def create_tarif_provider(config: dict, timezone, '[DynTariff] energyforecast_total_price: dynamic_network_fees ' 'is ignored -- the API already includes network fees in total_ct_kwh' ) - token = config.get('apikey') market_zone = config.get('market_zone', 'DE') selected_tariff = Energyforecast( timezone, diff --git a/src/batcontrol/forecastconsumption/consumption.py b/src/batcontrol/forecastconsumption/consumption.py index d1c2f104..5d1e53d9 100644 --- a/src/batcontrol/forecastconsumption/consumption.py +++ b/src/batcontrol/forecastconsumption/consumption.py @@ -1,5 +1,6 @@ """ Factory for creating consumption forecasters. """ import logging +from ..value_utils import get_config_value from .forecastconsumption_interface import ForecastConsumptionInterface from .forecast_csv import ForecastConsumptionCsv from .forecast_homeassistant import ForecastConsumptionHomeAssistant @@ -59,19 +60,25 @@ def _create_homeassistant_forecast( else: ha_config = config - # Validate required parameters - required_params = ['base_url', 'apitoken', 'entity_id'] - for param in required_params: - if param not in ha_config: + # Get configuration, accepting the deprecated 'base_url'/'apitoken' keys + context = 'consumption_forecast' + base_url = get_config_value(ha_config, 'url', None, context) + api_token = get_config_value(ha_config, 'api_token', None, context) + entity_id = get_config_value(ha_config, 'entity_id', None, context) + + # Validate required parameters. Emptiness, not presence: the add-on + # delivers a field the user cleared as '' and an unset one as null, + # both of which used to reach HomeAssistant as a broken auth message. + for param, value in ( + ('url', base_url), + ('api_token', api_token), + ('entity_id', entity_id)): + if not value: raise ValueError( f"HomeAssistant consumption forecast requires '{param}' " - f"in consumption_forecast.homeassistant_api configuration" + f"in consumption_forecast configuration" ) - # Get configuration with defaults - base_url = ha_config['base_url'] - api_token = ha_config['apitoken'] - entity_id = ha_config['entity_id'] history_days = ha_config.get('history_days', [-7, -14, -21]) history_weights = ha_config.get('history_weights', [1, 1, 1]) diff --git a/src/batcontrol/forecastsolar/baseclass.py b/src/batcontrol/forecastsolar/baseclass.py index e41c040d..f14d57c5 100644 --- a/src/batcontrol/forecastsolar/baseclass.py +++ b/src/batcontrol/forecastsolar/baseclass.py @@ -5,6 +5,7 @@ import time import random import logging +from typing import Optional from .forecastsolar_interface import ForecastSolarInterface from ..fetcher.relaxed_caching import RelaxedCaching, CacheMissError from ..scheduler import schedule_once @@ -41,8 +42,10 @@ class ForecastSolarBaseclass(ForecastSolarInterface): values in Wh per 15 minutes """ + # pylint: disable=too-many-arguments,too-many-positional-arguments def __init__(self, pvinstallations, timezone, min_time_between_API_calls, - delay_evaluation_by_seconds, target_resolution=60, native_resolution=60) -> None: + delay_evaluation_by_seconds, target_resolution=60, native_resolution=60, + cache_ttl_hours: Optional[float] = None) -> None: self.pvinstallations = pvinstallations self.next_update_ts = 0 self.min_time_between_updates = min_time_between_API_calls @@ -66,7 +69,12 @@ def __init__(self, pvinstallations, timezone, min_time_between_API_calls, try: for unit in pvinstallations: name = unit['name'] - self.cache_list[name] = RelaxedCaching() + # None keeps RelaxedCaching's own default TTL as the single + # source of truth for providers that expose no TTL setting. + if cache_ttl_hours is None: + self.cache_list[name] = RelaxedCaching() + else: + self.cache_list[name] = RelaxedCaching(ttl_hours=cache_ttl_hours) except KeyError as e: raise ValueError("Each PV installation must have a 'name' key") from e diff --git a/src/batcontrol/forecastsolar/evcc_solar.py b/src/batcontrol/forecastsolar/evcc_solar.py index d531a012..7f164ca7 100644 --- a/src/batcontrol/forecastsolar/evcc_solar.py +++ b/src/batcontrol/forecastsolar/evcc_solar.py @@ -36,6 +36,7 @@ import logging import requests +from ..value_utils import get_config_value from .baseclass import ForecastSolarBaseclass logger = logging.getLogger(__name__) @@ -75,10 +76,13 @@ def __init__(self, pvinstallations, timezone, min_time_between_api_calls, "[EvccSolar] evcc-solar provider expects exactly one installation configuration") installation = pvinstallations[0] - if 'url' not in installation: + url = get_config_value( + installation, 'url', None, + f"pvinstallations[{installation.get('name', '')}]") + if not url: raise ValueError("[EvccSolar] URL must be provided in installation configuration") - self.url = installation['url'] + self.url = url logger.info('Initialized EvccSolar with URL: %s', self.url) def get_forecast_from_raw_data(self) -> dict[int, float]: diff --git a/src/batcontrol/forecastsolar/fcsolar.py b/src/batcontrol/forecastsolar/fcsolar.py index ca7fc53a..85579930 100644 --- a/src/batcontrol/forecastsolar/fcsolar.py +++ b/src/batcontrol/forecastsolar/fcsolar.py @@ -9,6 +9,7 @@ import json import logging import requests +from ..value_utils import get_config_value from .baseclass import ForecastSolarBaseclass, ProviderError, RateLimitException @@ -98,11 +99,11 @@ def get_raw_data_from_provider(self, pvinstallation_name) -> dict: kwp = unit['kWp'] apikey_urlmod = '' - if 'apikey' in unit.keys() and unit['apikey'] is not None: - apikey_urlmod = unit['apikey'] + "/" # ForecastSolar api - # legacy naming in config file - elif 'api' in unit.keys() and unit['api'] is not None: - apikey_urlmod = unit['api'] + "/" # ForecastSolar api + # 'apikey'/'api' are the deprecated spellings of 'api_token' + apikey = get_config_value( + unit, 'api_token', None, f"pvinstallations[{unit.get('name', '')}]") + if apikey: + apikey_urlmod = apikey + "/" # ForecastSolar api horizon_querymod = '' if 'horizon' in unit.keys() and unit['horizon'] is not None: diff --git a/src/batcontrol/forecastsolar/forecast_homeassistant_ml.py b/src/batcontrol/forecastsolar/forecast_homeassistant_ml.py index f794b853..337b6ee1 100644 --- a/src/batcontrol/forecastsolar/forecast_homeassistant_ml.py +++ b/src/batcontrol/forecastsolar/forecast_homeassistant_ml.py @@ -28,6 +28,8 @@ logger_ha_communication = logging.getLogger( "batcontrol.forecastsolar.forecast_homeassistant_ml.communication") +PROVIDER_NAME = 'homeassistant-solar-forecast-ml' + # pylint: disable=too-many-instance-attributes class ForecastSolarHomeAssistantML(ForecastSolarBaseclass): @@ -57,7 +59,8 @@ def __init__( min_time_between_api_calls: int = 21600, delay_evaluation_by_seconds: int = 300, sensor_unit: Optional[str] = "auto", - target_resolution: int = 60 + target_resolution: int = 60, + cache_ttl_hours: Optional[float] = None ) -> None: """Initialize HomeAssistant solar forecast provider @@ -75,6 +78,12 @@ def __init__( If set to 'auto', queries Home Assistant to detect unit. Default: auto (auto-detect) target_resolution: Target resolution in minutes (15 or 60) + cache_ttl_hours: How long a fetched forecast stays usable as a + fallback, in hours. None keeps the cache default. + + Raises: + ValueError: If api_token, entity_id, base_url or sensor_unit is missing + or invalid """ # Initialize baseclass with native 60-minute resolution super().__init__( @@ -83,9 +92,25 @@ def __init__( min_time_between_api_calls, delay_evaluation_by_seconds, target_resolution=target_resolution, - native_resolution=60 + native_resolution=60, + cache_ttl_hours=cache_ttl_hours ) + # The factory validates these too, but the provider is also + # constructed directly (tests, embedding) and the first thing + # __init__ may do is open a WebSocket - so an empty token has to + # fail here rather than as an obscure HomeAssistant auth error (#451). + installation_name = self._installation_name() + if not base_url: + raise ValueError( + f"[HA-SolarML] No url provided for installation {installation_name}") + if not api_token: + raise ValueError( + f"[HA-SolarML] No api_token provided for installation {installation_name}") + if not entity_id: + raise ValueError( + f"[HA-SolarML] No entity_id provided for installation {installation_name}") + self.base_url = base_url.rstrip('/') self.api_token = api_token self.entity_id = entity_id @@ -126,11 +151,33 @@ def __init__( logger.info( "Initialized HomeAssistant Solar Forecast ML provider: " "entity_id=%s, sensor_unit=%s, " - "unit_conversion_factor=%0.1f", + "unit_conversion_factor=%0.1f, cache_ttl=%0.1fh", entity_id, self.sensor_unit or 'auto', - self.unit_conversion_factor + self.unit_conversion_factor, + self.cache_list[installation_name].ttl_seconds / 3600 + if installation_name in self.cache_list else 0.0 ) + def _installation_name(self) -> str: + """Name of the installation this provider was configured from + + Mirrors the selection of the factory, so log lines and error messages + name the entry the connection details were taken from rather than + whatever happens to be first in the list (#451). + """ + entries = [entry for entry in (self.pvinstallations or []) + if isinstance(entry, dict)] + if not entries: + return '' + for entry in entries: + entry_type = entry.get('type') + if isinstance(entry_type, str) and entry_type.lower() == PROVIDER_NAME: + return entry.get('name', '') + for entry in entries: + if entry.get('entity_id'): + return entry.get('name', '') + return entries[0].get('name', '') + def _check_sensor_unit(self) -> float: """Check sensor's unit_of_measurement and return conversion factor @@ -408,7 +455,7 @@ def get_forecast_from_raw_data(self) -> Dict[int, float]: RuntimeError: If no valid forecast data available """ # Get raw data from cache (stored by baseclass) - raw_data = self.get_raw_data(self.pvinstallations[0]['name']) + raw_data = self.get_raw_data(self._installation_name()) if not raw_data: logger.warning('No raw data available from cache') diff --git a/src/batcontrol/forecastsolar/solar.py b/src/batcontrol/forecastsolar/solar.py index 1d1536c2..c6e12631 100644 --- a/src/batcontrol/forecastsolar/solar.py +++ b/src/batcontrol/forecastsolar/solar.py @@ -1,11 +1,21 @@ """ Factory for solar forecast providers """ +import logging +from typing import Optional + +from ..value_utils import get_config_value, parse_positive_number from .forecastsolar_interface import ForecastSolarInterface from .fcsolar import FCSolar from .evcc_solar import EvccSolar -from .forecast_homeassistant_ml import ForecastSolarHomeAssistantML +from .forecast_homeassistant_ml import ForecastSolarHomeAssistantML, PROVIDER_NAME from .solcast import Solcast +logger = logging.getLogger(__name__) + +HA_ML_PROVIDER = PROVIDER_NAME +HA_ML_DEFAULT_URL = 'ws://homeassistant.local:8123' +DOC_URL = 'https://mastr.github.io/batcontrol/configuration/solar-forecast/' + class ForecastSolar: """ Factory for solar forecast providers """ @@ -28,7 +38,7 @@ def create_solar_provider(config: dict, target_resolution: Target resolution in minutes (15 or 60) Raises: - RuntimeError: If provider is unknown + RuntimeError: If provider is unknown or its configuration is incomplete ValueError: If configuration is invalid for the provider """ provider = None @@ -41,38 +51,145 @@ def create_solar_provider(config: dict, elif requested_provider.lower() == 'solcast': provider = Solcast(config, timezone, min_time_between_api_calls, api_delay, target_resolution) - elif requested_provider.lower() == 'homeassistant-solar-forecast-ml': - # Parse HomeAssistant Solar Forecast ML configuration from pvinstallations - # Each installation can have type='homeassistant-solar-forecast-ml' - # with connection details - provider = ForecastSolarHomeAssistantML( - pvinstallations=config, - timezone=timezone, - base_url=_get_config_value(config, 'base_url', 'ws://homeassistant.local:8123'), - api_token=_get_config_value(config, 'api_token', None), - entity_id=_get_config_value(config, 'entity_id', None), - min_time_between_api_calls=min_time_between_api_calls, - delay_evaluation_by_seconds=api_delay, - sensor_unit=_get_config_value(config, 'sensor_unit', 'auto'), - target_resolution=target_resolution + elif requested_provider.lower() == HA_ML_PROVIDER: + provider = _create_homeassistant_ml_provider( + config, + timezone, + min_time_between_api_calls, + api_delay, + target_resolution ) else: raise RuntimeError(f'[ForecastSolar] Unknown provider {requested_provider}') return provider -def _get_config_value(config: list, key: str, default=None): - """Extract configuration value from pvinstallations list +def _create_homeassistant_ml_provider( + config: list, + timezone, + min_time_between_api_calls, + api_delay, + target_resolution: int) -> ForecastSolarInterface: + """Build the HomeAssistant Solar Forecast ML provider from pvinstallations + + Picks the installation entry that belongs to this provider, reads its + connection details (accepting the deprecated 'base_url'/'apikey' + spellings) and refuses to continue when a mandatory key is missing. + """ + installation = _select_installation(config, HA_ML_PROVIDER) + if installation is None: + raise RuntimeError( + f'[ForecastSolar] No pvinstallations entry matches provider ' + f'{HA_ML_PROVIDER}. Add "type: {HA_ML_PROVIDER}" to the entry ' + f'holding url, api_token and entity_id, and remove the entries ' + f'of the other providers. See {DOC_URL}' + ) + name = installation.get('name', '') + context = f'pvinstallations[{name}]' + + url = get_config_value(installation, 'url', None, context) + if not url: + url = HA_ML_DEFAULT_URL + logger.warning( + '[ForecastSolar] No "url" configured for installation %s, ' + 'falling back to %s', + name, HA_ML_DEFAULT_URL + ) + + api_token = get_config_value(installation, 'api_token', None, context) + _require(api_token, 'api_token', name) + + entity_id = get_config_value(installation, 'entity_id', None, context) + _require(entity_id, 'entity_id', name) + + cache_ttl_hours = get_config_value(installation, 'cache_ttl_hours', None, context) + if cache_ttl_hours is not None: + cache_ttl_hours = parse_positive_number( + cache_ttl_hours, f'{context}.cache_ttl_hours') + + kwargs = { + 'pvinstallations': [installation], + 'timezone': timezone, + 'base_url': url, + 'api_token': api_token, + 'entity_id': entity_id, + 'min_time_between_api_calls': min_time_between_api_calls, + 'delay_evaluation_by_seconds': api_delay, + 'sensor_unit': get_config_value(installation, 'sensor_unit', 'auto', context), + 'target_resolution': target_resolution, + } + if cache_ttl_hours is not None: + kwargs['cache_ttl_hours'] = cache_ttl_hours + + return ForecastSolarHomeAssistantML(**kwargs) + + +def _require(value, key: str, name: str) -> None: + """Raise a RuntimeError naming the key and the installation it belongs to + + The message spells out the deprecated alternatives, because reusing the + key name of another provider is what sent users down this path (#451). + """ + if value: + return + aliases = { + 'api_token': ' Note: "apikey"/"apitoken" are the deprecated spellings ' + 'of this key - "apikey" is also the key name used by ' + 'fcsolarapi and solcast.', + 'entity_id': ' Use the sensor of the Solar Forecast ML integration, ' + 'for example sensor.solar_forecast_ml_evcc_solar_prognose.', + } + hint = aliases.get(key, '') + raise RuntimeError( + f'[ForecastSolar] {HA_ML_PROVIDER} requires "{key}" in your ' + f'pvinstallations configuration (installation "{name}").{hint} ' + f'See {DOC_URL}' + ) + + +def _select_installation(config: list, requested_provider: str) -> Optional[dict]: + """Pick the pvinstallations entry that belongs to the requested provider + + Selection order: + 1. entry whose 'type' matches the requested provider + 2. entry carrying 'entity_id', which only this provider uses + ('url' is no discriminator - evcc-solar uses the same key) + 3. the single entry, if the list holds exactly one + + Reading only the first entry used to silently ignore a correctly + configured entry further down the list (#451). With several entries and + no match there is nothing to guess: naming an unrelated entry in the + following error message would send the user to the wrong place. Args: config: List of pvinstallations dicts - key: Configuration key to find - default: Default value if key not found + requested_provider: Provider name from solar_forecast_provider Returns: - Value from first installation's config, or default if not found + The selected installation dict, or None if no entry can be identified """ - if isinstance(config, list) and len(config) > 0: - if isinstance(config[0], dict): - return config[0].get(key, default) - return default + if not isinstance(config, list): + return None + + entries = [entry for entry in config if isinstance(entry, dict)] + if not entries: + return None + + for entry in entries: + entry_type = entry.get('type') + if isinstance(entry_type, str) and entry_type.lower() == requested_provider: + return entry + + for entry in entries: + if entry.get('entity_id'): + if entry is not entries[0]: + logger.info( + '[ForecastSolar] Using pvinstallations entry "%s" for %s', + entry.get('name', ''), requested_provider + ) + return entry + + if len(entries) == 1: + return entries[0] + + return None diff --git a/src/batcontrol/forecastsolar/solcast.py b/src/batcontrol/forecastsolar/solcast.py index 8ef41c87..15d043c0 100644 --- a/src/batcontrol/forecastsolar/solcast.py +++ b/src/batcontrol/forecastsolar/solcast.py @@ -5,7 +5,7 @@ The rooftop site (location, tilt, azimuth, capacity) is configured in the Solcast web toolkit, not in batcontrol. Each installation entry only needs -the site's resource_id and the account's apikey. +the site's resource_id and the account's api_token. The free hobbyist plan allows 10 API requests per day. Each configured installation costs one request per refresh, so the minimum time between @@ -16,6 +16,7 @@ import logging import time import requests +from ..value_utils import get_config_value from .baseclass import ForecastSolarBaseclass, ProviderError, RateLimitException logger = logging.getLogger(__name__) @@ -40,7 +41,7 @@ def __init__(self, pvinstallations, timezone, min_time_between_api_calls, Args: pvinstallations: List of PV installation configurations. - Each entry needs 'name', 'resource_id' and 'apikey'; + Each entry needs 'name', 'resource_id' and 'api_token'; 'percentile' (10, 50 or 90) is optional. timezone: Timezone for forecast data min_time_between_api_calls: Minimum seconds between API calls @@ -62,9 +63,9 @@ def __init__(self, pvinstallations, timezone, min_time_between_api_calls, if not unit.get('resource_id'): raise ValueError( f'[Solcast] No resource_id provided for installation {name}') - if not unit.get('apikey'): + if not _get_api_token(unit): raise ValueError( - f'[Solcast] No API key provided for installation {name}') + f'[Solcast] No api_token provided for installation {name}') percentile = unit.get('percentile', 50) if percentile not in PERCENTILE_FIELD_MAP: raise ValueError( @@ -143,7 +144,7 @@ def get_raw_data_from_provider(self, pvinstallation_name) -> dict: name = unit['name'] resource_id = unit['resource_id'] - apikey = unit['apikey'] + api_token = _get_api_token(unit) url = f'{API_BASE_URL}/{resource_id}/forecasts' url += f'?format=json&hours={FORECAST_HOURS}' @@ -153,7 +154,7 @@ def get_raw_data_from_provider(self, pvinstallation_name) -> dict: # API key goes into the Authorization header to keep it out of # logged URLs. - headers = {'Authorization': f'Bearer {apikey}'} + headers = {'Authorization': f'Bearer {api_token}'} try: response = requests.get(url, headers=headers, timeout=60) except requests.exceptions.RequestException as e: @@ -168,11 +169,11 @@ def get_raw_data_from_provider(self, pvinstallation_name) -> dict: f'[Solcast] Invalid JSON response: {e}') from e elif response.status_code in (401, 403): logger.error( - 'API returned %s - Unauthorized, apikey correct?', + 'API returned %s - Unauthorized, api_token correct?', response.status_code) raise ProviderError( f'[Solcast] API returned {response.status_code} - ' - 'Unauthorized, apikey correct?') + 'Unauthorized, api_token correct?') elif response.status_code == 404: logger.error( 'API returned 404 - resource_id %s not found', resource_id) @@ -222,3 +223,9 @@ def __store_retry(self, response): if blackout_ts <= 0: blackout_ts = time.time() + self._provider_min self.rate_limit_blackout_window_ts = blackout_ts + + +def _get_api_token(unit: dict): + """Read the Solcast API token, accepting the deprecated 'apikey' spelling""" + return get_config_value( + unit, 'api_token', None, f"pvinstallations[{unit.get('name', '')}]") diff --git a/src/batcontrol/value_utils.py b/src/batcontrol/value_utils.py index 4552da54..93eed6a3 100644 --- a/src/batcontrol/value_utils.py +++ b/src/batcontrol/value_utils.py @@ -3,8 +3,73 @@ These helpers are independent of the Batcontrol core and raise ValueError for values that cannot be interpreted. """ +import logging import math -from typing import Optional +from typing import Any, Optional + +logger = logging.getLogger(__name__) + +# Canonical configuration key names and the deprecated spellings that are still +# accepted for them. batcontrol grew three spellings for a credential +# ('apikey', 'apitoken', 'api') and two for an endpoint ('url', 'base_url'), +# which made users put the right value into the wrong key (issue #451). +CONFIG_KEY_ALIASES = { + 'url': ('base_url',), + 'api_token': ('apikey', 'apitoken', 'api'), +} + +# Deprecation warnings are emitted once per (context, deprecated key) so a +# provider that reads the same key on every cycle does not flood the log. +_warned_aliases = set() + + +def reset_alias_warnings() -> None: + """Forget which deprecation warnings were already emitted (for tests).""" + _warned_aliases.clear() + + +def get_config_value( + config: dict, + key: str, + default=None, + context: str = '') -> Any: + """Read a config value, falling back to deprecated spellings of its key. + + An empty value (missing key, None or empty string) counts as not set. The + empty string matters for the HomeAssistant add-on, where a field the user + cleared in the UI arrives as '' instead of being absent. + + Args: + config: Configuration mapping to read from + key: Canonical key name (see CONFIG_KEY_ALIASES) + default: Value to return when neither key nor alias is set + context: Config section for the warning, e.g. 'pvinstallations[Haus]' + + Returns: + The configured value, or default if nothing is set + """ + if not isinstance(config, dict): + return default + + value = config.get(key) + if value or value == 0: + return value + + for alias in CONFIG_KEY_ALIASES.get(key, ()): + value = config.get(alias) + if value or value == 0: + warning_key = (context, alias, key) + if warning_key not in _warned_aliases: + _warned_aliases.add(warning_key) + logger.warning( + '[Config] "%s"%s is deprecated, please rename it to "%s"', + alias, + f' in {context}' if context else '', + key + ) + return value + + return default def parse_optional_ratio(value, config_key: str) -> Optional[float]: diff --git a/tests/batcontrol/dynamictariff/test_baseclass.py b/tests/batcontrol/dynamictariff/test_baseclass.py index 74bd89f5..3287ee21 100644 --- a/tests/batcontrol/dynamictariff/test_baseclass.py +++ b/tests/batcontrol/dynamictariff/test_baseclass.py @@ -409,15 +409,25 @@ def test_factory_energyforecast_total_price(self, timezone): assert provider.use_total_price is True assert provider.target_resolution == 15 - def test_factory_energyforecast_total_price_requires_apikey(self, timezone): - """Test that energyforecast_total_price raises without apikey""" + def test_factory_energyforecast_total_price_requires_api_token(self, timezone): + """Test that energyforecast_total_price raises without api_token""" from batcontrol.dynamictariff.dynamictariff import DynamicTariff config = {'type': 'energyforecast_total_price'} - with pytest.raises(RuntimeError, match='apikey'): + with pytest.raises(RuntimeError, match='api_token'): DynamicTariff.create_tarif_provider(config, timezone, 900, 0) + def test_factory_energyforecast_total_price_accepts_apikey_alias(self, timezone): + """The deprecated 'apikey' spelling still configures the token""" + from batcontrol.dynamictariff.dynamictariff import DynamicTariff + + config = {'type': 'energyforecast_total_price', 'apikey': 'legacy-key'} + + provider = DynamicTariff.create_tarif_provider(config, timezone, 900, 0) + + assert provider.token == 'legacy-key' + def test_factory_energyforecast_total_price_no_vat_required(self, timezone): """Test that energyforecast_total_price does not require vat/fees/markup""" from batcontrol.dynamictariff.dynamictariff import DynamicTariff diff --git a/tests/batcontrol/forecastconsumption/test_consumption_factory.py b/tests/batcontrol/forecastconsumption/test_consumption_factory.py index 61e82374..ca091b48 100644 --- a/tests/batcontrol/forecastconsumption/test_consumption_factory.py +++ b/tests/batcontrol/forecastconsumption/test_consumption_factory.py @@ -217,3 +217,86 @@ def test_sensor_unit_defaults_to_none(self, mock_check, timezone): # pylint: di assert forecaster.sensor_unit == 'auto' mock_check.assert_called_once() + + +class TestConfigKeyNames: + """Canonical 'url'/'api_token' keys and their deprecated spellings (#451)""" + + @patch.object(ForecastConsumptionHomeAssistant, '_check_sensor_unit', return_value=1.0) + def test_canonical_keys(self, mock_check, timezone): # pylint: disable=redefined-outer-name,unused-argument + """url/api_token are the documented key names""" + config = { + 'type': 'homeassistant-api', + 'homeassistant_api': { + 'url': 'ws://localhost:8123', + 'api_token': 'canonical_token', + 'entity_id': 'sensor.test', + 'sensor_unit': 'Wh' + } + } + + forecaster = _create_homeassistant_forecast(timezone, config['homeassistant_api']) + + assert forecaster.base_url == 'ws://localhost:8123' + assert forecaster.api_token == 'canonical_token' + + @patch.object(ForecastConsumptionHomeAssistant, '_check_sensor_unit', return_value=1.0) + def test_deprecated_keys_still_work(self, mock_check, timezone): # pylint: disable=redefined-outer-name,unused-argument + """base_url/apitoken keep working for hand-written configurations""" + config = { + 'base_url': 'ws://legacy:8123', + 'apitoken': 'legacy_token', + 'entity_id': 'sensor.test', + 'sensor_unit': 'Wh' + } + + forecaster = _create_homeassistant_forecast(timezone, config) + + assert forecaster.base_url == 'ws://legacy:8123' + assert forecaster.api_token == 'legacy_token' + + @patch.object(ForecastConsumptionHomeAssistant, '_check_sensor_unit', return_value=1.0) + def test_canonical_key_wins_over_deprecated(self, mock_check, timezone): # pylint: disable=redefined-outer-name,unused-argument + config = { + 'url': 'ws://canonical:8123', + 'base_url': 'ws://legacy:8123', + 'api_token': 'canonical_token', + 'apitoken': 'legacy_token', + 'entity_id': 'sensor.test', + 'sensor_unit': 'Wh' + } + + forecaster = _create_homeassistant_forecast(timezone, config) + + assert forecaster.base_url == 'ws://canonical:8123' + assert forecaster.api_token == 'canonical_token' + + @pytest.mark.parametrize("missing", ['url', 'api_token', 'entity_id']) + def test_missing_key_raises_naming_it(self, timezone, missing): # pylint: disable=redefined-outer-name + config = { + 'url': 'ws://localhost:8123', + 'api_token': 'test_token', + 'entity_id': 'sensor.test', + 'sensor_unit': 'Wh' + } + del config[missing] + + with pytest.raises(ValueError, match=missing): + _create_homeassistant_forecast(timezone, config) + + @pytest.mark.parametrize("empty", [None, '']) + def test_empty_token_raises(self, timezone, empty): # pylint: disable=redefined-outer-name + """The add-on delivers a cleared field as '' and an unset one as null. + + Regression for issue #451: the presence-only check let both through + and HomeAssistant answered with "expected str at 'access_token'". + """ + config = { + 'url': 'ws://localhost:8123', + 'api_token': empty, + 'entity_id': 'sensor.test', + 'sensor_unit': 'Wh' + } + + with pytest.raises(ValueError, match='api_token'): + _create_homeassistant_forecast(timezone, config) diff --git a/tests/batcontrol/forecastsolar/test_solar_factory.py b/tests/batcontrol/forecastsolar/test_solar_factory.py new file mode 100644 index 00000000..4f5999a2 --- /dev/null +++ b/tests/batcontrol/forecastsolar/test_solar_factory.py @@ -0,0 +1,285 @@ +"""Tests for the solar forecast factory, HomeAssistant Solar Forecast ML branch. + +Regression tests for issue #451: a user put the long-lived access token into +'apikey' and the websocket URL into 'url' - both valid keys of *other* solar +providers - so 'api_token' and 'base_url' stayed unset. batcontrol silently +sent '{"type": "auth", "access_token": null}' to HomeAssistant and died with +"expected str at 'access_token'. Got None" instead of naming the config key. +""" + +from unittest.mock import patch + +import pytest +import pytz + +from batcontrol.forecastsolar.solar import ForecastSolar, _select_installation +from batcontrol.forecastsolar.forecast_homeassistant_ml import ( + ForecastSolarHomeAssistantML +) +from batcontrol.value_utils import reset_alias_warnings + +PROVIDER = 'homeassistant-solar-forecast-ml' + + +@pytest.fixture +def timezone(): + """Return Berlin timezone for testing""" + return pytz.timezone('Europe/Berlin') + + +@pytest.fixture(autouse=True) +def _clear_alias_warnings(): + """Alias deprecation warnings are emitted once per process""" + reset_alias_warnings() + yield + reset_alias_warnings() + + +@pytest.fixture +def ha_ml_installation(): + """A complete HA Solar Forecast ML installation entry""" + return { + 'name': 'HA Solar ML', + 'url': 'ws://homeassistant:8123', + 'api_token': 'token-123', + 'entity_id': 'sensor.solar_forecast_ml_evcc_solar_prognose', + } + + +@pytest.fixture +def fcsolar_installation(): + """An fcsolarapi entry as shipped in the dummy config""" + return { + 'name': 'Demo PV System', + 'type': 'fcsolarapi', + 'lat': 48.4334480, + 'lon': 8.7654968, + 'declination': 30, + 'azimuth': 0, + 'kWp': 10.0, + } + + +def _create(config, timezone): + """Create the provider without touching the network + + sensor_unit defaults to 'auto', which makes __init__ open a WebSocket to + detect the unit. The factory tests only care about config handling. + """ + with patch.object(ForecastSolarHomeAssistantML, + '_check_sensor_unit', return_value=1.0): + return ForecastSolar.create_solar_provider( + config, timezone, 21600, 0, + requested_provider=PROVIDER + ) + + +class TestInstallationSelection: + """The entry belonging to the provider is used, not blindly the first one""" + + def test_single_entry(self, ha_ml_installation, timezone): + provider = _create([ha_ml_installation], timezone) + assert provider.api_token == 'token-123' + assert provider.base_url == 'ws://homeassistant:8123' + + def test_entry_behind_an_fcsolarapi_entry_is_found( + self, fcsolar_installation, ha_ml_installation, timezone): + """Regression: only pvinstallations[0] used to be read (#451). + + The shipped dummy config has the fcsolarapi entry first and the + HA-ML block as a commented second entry, so uncommenting it put + the HA-ML keys out of reach. + """ + provider = _create( + [fcsolar_installation, ha_ml_installation], timezone) + assert provider.api_token == 'token-123' + assert provider.entity_id == ( + 'sensor.solar_forecast_ml_evcc_solar_prognose') + + def test_type_key_selects_the_entry(self, timezone): + """'type' was documented but read by nothing before""" + typed = { + 'name': 'Typed HA ML', + 'type': PROVIDER, + 'url': 'ws://typed:8123', + 'api_token': 'typed-token', + 'entity_id': 'sensor.typed', + } + other = { + 'name': 'Other HA ML', + 'url': 'ws://other:8123', + 'api_token': 'other-token', + 'entity_id': 'sensor.other', + } + provider = _create([other, typed], timezone) + assert provider.api_token == 'typed-token' + + def test_type_match_is_case_insensitive(self, ha_ml_installation, timezone): + ha_ml_installation['type'] = PROVIDER.upper() + provider = _create([ha_ml_installation], timezone) + assert provider.api_token == 'token-123' + + def test_single_entry_is_used_even_without_markers( + self, fcsolar_installation): + """One entry leaves nothing to guess - the error names it correctly""" + assert _select_installation( + [fcsolar_installation], PROVIDER) is fcsolar_installation + + @pytest.mark.parametrize("config", [[], None, 'nonsense', [None]]) + def test_select_installation_without_usable_entries(self, config): + assert _select_installation(config, PROVIDER) is None + + def test_several_entries_without_a_match_are_not_guessed( + self, fcsolar_installation): + """Naming an unrelated entry would send the user to the wrong place""" + second = {'name': 'Garage', 'lat': 48.0, 'lon': 8.0, + 'declination': 30, 'azimuth': 0, 'kWp': 5.0} + + assert _select_installation( + [fcsolar_installation, second], PROVIDER) is None + + def test_several_entries_without_a_match_raise( + self, fcsolar_installation, timezone): + second = {'name': 'Garage', 'lat': 48.0, 'lon': 8.0, + 'declination': 30, 'azimuth': 0, 'kWp': 5.0} + + with pytest.raises(RuntimeError, match='No pvinstallations entry'): + _create([fcsolar_installation, second], timezone) + + def test_provider_only_gets_its_own_entry( + self, fcsolar_installation, ha_ml_installation, timezone): + """refresh_data() fetches once per entry - unrelated entries would + each trigger another full get_states dump for the same sensor""" + provider = _create( + [fcsolar_installation, ha_ml_installation], timezone) + + assert provider.pvinstallations == [ha_ml_installation] + assert list(provider.cache_list) == ['HA Solar ML'] + + +class TestDeprecatedKeySpellings: + """'apikey' and 'base_url' keep working, with a warning""" + + def test_apikey_is_accepted(self, ha_ml_installation, timezone, caplog): + del ha_ml_installation['api_token'] + ha_ml_installation['apikey'] = 'legacy-token' + + with caplog.at_level('WARNING'): + provider = _create([ha_ml_installation], timezone) + + assert provider.api_token == 'legacy-token' + assert any('deprecated' in r.getMessage() for r in caplog.records) + + def test_base_url_is_accepted(self, ha_ml_installation, timezone): + del ha_ml_installation['url'] + ha_ml_installation['base_url'] = 'ws://legacy:8123' + + provider = _create([ha_ml_installation], timezone) + + assert provider.base_url == 'ws://legacy:8123' + + def test_issue_451_configuration_works(self, timezone, caplog): + """The exact entry from issue #451 now starts up""" + installation = { + 'name': 'Haus', + 'url': 'ws://homeassistant:8123', + 'apikey': 'eyJhbGciOi-long-lived-token', + 'entity_id': 'sensor.solar_forecast_ml_evcc_solar_prognose', + } + + with caplog.at_level('WARNING'): + provider = _create([installation], timezone) + + assert provider.api_token == 'eyJhbGciOi-long-lived-token' + assert provider.base_url == 'ws://homeassistant:8123' + + +class TestMandatoryKeys: + """Missing keys fail before the first WebSocket connect""" + + @pytest.mark.parametrize("missing", ['api_token', 'entity_id']) + def test_missing_key_raises_with_key_name( + self, ha_ml_installation, timezone, missing): + del ha_ml_installation[missing] + + with pytest.raises(RuntimeError, match=missing): + _create([ha_ml_installation], timezone) + + @pytest.mark.parametrize("empty", [None, '']) + def test_empty_token_raises(self, ha_ml_installation, timezone, empty): + """The add-on delivers a cleared field as '' and an unset one as null""" + ha_ml_installation['api_token'] = empty + + with pytest.raises(RuntimeError, match='api_token'): + _create([ha_ml_installation], timezone) + + def test_error_names_the_installation_and_the_deprecated_key( + self, ha_ml_installation, timezone): + del ha_ml_installation['api_token'] + + with pytest.raises(RuntimeError) as excinfo: + _create([ha_ml_installation], timezone) + + message = str(excinfo.value) + assert 'HA Solar ML' in message + assert 'apikey' in message + + def test_no_connection_attempt_on_missing_token( + self, ha_ml_installation, timezone): + """The factory must fail before any network I/O""" + del ha_ml_installation['api_token'] + + with patch('batcontrol.forecastsolar.forecast_homeassistant_ml.connect' + ) as mock_connect: + with pytest.raises(RuntimeError): + ForecastSolar.create_solar_provider( + [ha_ml_installation], timezone, 21600, 0, + requested_provider=PROVIDER + ) + + mock_connect.assert_not_called() + + def test_missing_url_falls_back_to_default_with_warning( + self, ha_ml_installation, timezone, caplog): + del ha_ml_installation['url'] + + with caplog.at_level('WARNING'): + provider = _create([ha_ml_installation], timezone) + + assert provider.base_url == 'ws://homeassistant.local:8123' + assert any('url' in r.getMessage() for r in caplog.records) + + +class TestCacheTtlHours: + """cache_ttl_hours was documented since #257 but never read""" + + def test_configured_ttl_reaches_the_cache( + self, ha_ml_installation, timezone): + ha_ml_installation['cache_ttl_hours'] = 24.0 + + provider = _create([ha_ml_installation], timezone) + + assert provider.cache_list['HA Solar ML'].ttl_seconds == 24 * 3600 + + def test_default_ttl_is_unchanged(self, ha_ml_installation, timezone): + provider = _create([ha_ml_installation], timezone) + + assert provider.cache_list['HA Solar ML'].ttl_seconds == 12 * 3600 + + @pytest.mark.parametrize("invalid", [0, -1, 'abc']) + def test_invalid_ttl_raises(self, ha_ml_installation, timezone, invalid): + ha_ml_installation['cache_ttl_hours'] = invalid + + with pytest.raises(ValueError, match='cache_ttl_hours'): + _create([ha_ml_installation], timezone) + + +class TestUnknownProvider: + """Unchanged behaviour of the factory's else branch""" + + def test_unknown_provider_raises(self, timezone): + with pytest.raises(RuntimeError, match='Unknown provider'): + ForecastSolar.create_solar_provider( + [{'name': 'x'}], timezone, 21600, 0, + requested_provider='does-not-exist' + ) diff --git a/tests/batcontrol/forecastsolar/test_solcast.py b/tests/batcontrol/forecastsolar/test_solcast.py index a94c199e..aaf85c0e 100644 --- a/tests/batcontrol/forecastsolar/test_solcast.py +++ b/tests/batcontrol/forecastsolar/test_solcast.py @@ -110,13 +110,21 @@ def test_missing_resource_id(self, timezone): min_time_between_api_calls=900, delay_evaluation_by_seconds=0) - def test_missing_apikey(self, timezone): - """Test that a missing apikey raises ValueError.""" - with pytest.raises(ValueError, match='API key'): + def test_missing_api_token(self, timezone): + """Test that a missing api_token raises ValueError.""" + with pytest.raises(ValueError, match='api_token'): Solcast([{'name': 'default', 'resource_id': 'res-1'}], timezone, min_time_between_api_calls=900, delay_evaluation_by_seconds=0) + def test_apikey_alias_is_accepted(self, timezone): + """The deprecated 'apikey' spelling still configures the token.""" + instance = Solcast([{'name': 'default', 'resource_id': 'res-1', + 'apikey': 'legacy-key'}], timezone, + min_time_between_api_calls=900, + delay_evaluation_by_seconds=0) + assert instance is not None + def test_invalid_percentile(self, timezone): """Test that an invalid percentile raises ValueError.""" with pytest.raises(ValueError, match='percentile'): diff --git a/tests/batcontrol/test_value_utils.py b/tests/batcontrol/test_value_utils.py index 07b8dc9e..6fbb54fe 100644 --- a/tests/batcontrol/test_value_utils.py +++ b/tests/batcontrol/test_value_utils.py @@ -2,9 +2,11 @@ import pytest from batcontrol.value_utils import ( + get_config_value, parse_bool_flag, parse_optional_ratio, parse_positive_number, + reset_alias_warnings, ) @@ -91,3 +93,66 @@ def test_error_message_contains_config_key(self): with pytest.raises(ValueError, match="battery_control_expert.charge_rate_multiplier"): parse_positive_number( -1, 'battery_control_expert.charge_rate_multiplier') + + +class TestGetConfigValue: + """get_config_value() accepts deprecated spellings of a config key (issue #451). + + batcontrol grew three spellings for a credential ('apikey', 'apitoken', + 'api') and two for an endpoint ('url', 'base_url'). A user who put the + token into 'apikey' of a HomeAssistant installation got no error at all, + just a broken auth message against HomeAssistant. + """ + + @pytest.fixture(autouse=True) + def _clear_warnings(self): + reset_alias_warnings() + yield + reset_alias_warnings() + + def test_canonical_key_wins(self): + config = {'api_token': 'canonical', 'apikey': 'legacy'} + assert get_config_value(config, 'api_token') == 'canonical' + + @pytest.mark.parametrize("alias", ['apikey', 'apitoken', 'api']) + def test_token_aliases(self, alias): + assert get_config_value({alias: 'legacy'}, 'api_token') == 'legacy' + + def test_url_alias(self): + assert get_config_value( + {'base_url': 'ws://ha:8123'}, 'url') == 'ws://ha:8123' + + @pytest.mark.parametrize("empty", [None, '']) + def test_empty_canonical_falls_back_to_alias(self, empty): + """The add-on delivers a cleared field as '' and an unset one as null.""" + config = {'api_token': empty, 'apikey': 'legacy'} + assert get_config_value(config, 'api_token') == 'legacy' + + def test_zero_is_a_value(self): + assert get_config_value({'cache_ttl_hours': 0}, 'cache_ttl_hours') == 0 + + def test_default_when_nothing_is_set(self): + assert get_config_value({}, 'api_token', 'fallback') == 'fallback' + + def test_unknown_key_has_no_aliases(self): + assert get_config_value({'entity_id': 'sensor.x'}, 'entity_id') == 'sensor.x' + assert get_config_value({'other': 'x'}, 'entity_id') is None + + def test_non_dict_config_returns_default(self): + assert get_config_value(None, 'api_token', 'fallback') == 'fallback' + assert get_config_value(['a'], 'api_token', 'fallback') == 'fallback' + + def test_alias_warns_once_per_context(self, caplog): + config = {'apikey': 'legacy'} + with caplog.at_level('WARNING'): + for _ in range(3): + get_config_value(config, 'api_token', None, 'pvinstallations[Haus]') + warnings = [r for r in caplog.records if 'deprecated' in r.message] + assert len(warnings) == 1 + assert 'pvinstallations[Haus]' in warnings[0].getMessage() + assert 'api_token' in warnings[0].getMessage() + + def test_canonical_key_does_not_warn(self, caplog): + with caplog.at_level('WARNING'): + get_config_value({'api_token': 'x'}, 'api_token', None, 'utility') + assert not [r for r in caplog.records if 'deprecated' in r.message] diff --git a/tests/test_forecast_solar_homeassistant_ml.py b/tests/test_forecast_solar_homeassistant_ml.py index 560cb846..f7505507 100644 --- a/tests/test_forecast_solar_homeassistant_ml.py +++ b/tests/test_forecast_solar_homeassistant_ml.py @@ -151,6 +151,74 @@ def test_init_with_none_unit(self, pv_installations, timezone): sensor_unit=None ) + @pytest.mark.parametrize("missing_key", ["api_token", "entity_id", "base_url"]) + @pytest.mark.parametrize("empty_value", [None, ""]) + def test_init_without_credentials_raises( + self, pv_installations, timezone, missing_key, empty_value): + """Missing connection details must fail before any WebSocket connect + + Regression for issue #451: an empty api_token used to be sent to + HomeAssistant as '{"type": "auth", "access_token": null}' and surfaced + as "expected str at 'access_token'. Got None" instead of naming the + config key. + """ + kwargs = { + 'pvinstallations': pv_installations, + 'timezone': timezone, + 'base_url': "http://homeassistant.local:8123", + 'api_token': "test_token", + 'entity_id': "sensor.solar_forecast", + 'sensor_unit': "Wh", + } + kwargs[missing_key] = empty_value + + with pytest.raises(ValueError, match=missing_key.replace('base_url', 'url')): + ForecastSolarHomeAssistantML(**kwargs) + + def test_init_without_token_does_not_connect(self, pv_installations, timezone): + """sensor_unit 'auto' would open a WebSocket - validation comes first""" + with patch('src.batcontrol.forecastsolar.forecast_homeassistant_ml.connect' + ) as mock_connect: + with pytest.raises(ValueError, match="api_token"): + ForecastSolarHomeAssistantML( + pvinstallations=pv_installations, + timezone=timezone, + base_url="http://homeassistant.local:8123", + api_token=None, + entity_id="sensor.solar_forecast", + sensor_unit="auto" + ) + + mock_connect.assert_not_called() + + def test_init_error_names_the_installation(self, pv_installations, timezone): + """The message points at the installation that is misconfigured""" + with pytest.raises(ValueError) as excinfo: + ForecastSolarHomeAssistantML( + pvinstallations=pv_installations, + timezone=timezone, + base_url="http://homeassistant.local:8123", + api_token="", + entity_id="sensor.solar_forecast", + sensor_unit="Wh" + ) + + assert pv_installations[0]['name'] in str(excinfo.value) + + def test_init_with_cache_ttl_hours(self, pv_installations, timezone): + """cache_ttl_hours is forwarded to the baseclass cache""" + provider = ForecastSolarHomeAssistantML( + pvinstallations=pv_installations, + timezone=timezone, + base_url="http://homeassistant.local:8123", + api_token="test_token", + entity_id="sensor.solar_forecast", + sensor_unit="Wh", + cache_ttl_hours=24.0 + ) + name = pv_installations[0]['name'] + assert provider.cache_list[name].ttl_seconds == 24 * 3600 + def test_init_cache_configuration(self, pv_installations, timezone): """Test baseclass cache is properly configured""" provider = ForecastSolarHomeAssistantML(