diff --git a/apps/predbat/component_base.py b/apps/predbat/component_base.py index d710ddc25..31b8bf22a 100644 --- a/apps/predbat/component_base.py +++ b/apps/predbat/component_base.py @@ -313,6 +313,16 @@ def get_state_wrapper(self, entity_id=None, default=None, attribute=None, refres def set_state_wrapper(self, entity_id, state, attributes={}, required_unit=None): return self.base.set_state_wrapper(entity_id, state, attributes=attributes, required_unit=required_unit) + async def set_state_external(self, entity_id, state, attributes={}): + """Change one of Predbat's OWN entities as if a user had, updating its CONFIG_ITEMS value. + + Distinct from set_state_wrapper, which only writes the entity state: components use this when + auto-discovery has to change a Predbat setting (e.g. teslemetry turning inverter_hybrid off + for an AC-coupled Powerwall), where writing the state alone would move the displayed entity + without changing the value the planner reads. + """ + return await self.base.ha_interface.set_state_external(entity_id, state, attributes=attributes) + def call_notify(self, message): return self.base.call_notify(message) diff --git a/apps/predbat/gecloud.py b/apps/predbat/gecloud.py index 0c0acd369..93afe72eb 100644 --- a/apps/predbat/gecloud.py +++ b/apps/predbat/gecloud.py @@ -1165,7 +1165,7 @@ def build_entities(domain, candidates): break entity_id = "switch.{}_inverter_hybrid".format(self.prefix) self.log("GECloud: Detected inverter model {} indicates ac_coupled={}, setting {} to {}".format(model_name, ac_coupled, entity_id, "off" if ac_coupled else "on")) - await self.base.ha_interface.set_state_external(entity_id, not ac_coupled) + await self.set_state_external(entity_id, not ac_coupled) self.log("GECloud: Automatic configuration complete") @@ -2157,23 +2157,12 @@ def get_data(self): return self.mdata, self.oldest_data_time -class MockHAInterface: # pragma: no cover - """Mock HA interface for testing""" - - def __init__(self): - pass - - async def set_state_external(self, entity_id, state): - print(f"Set state external {entity_id} = {state}") - - class MockBase(SharedMockBase): # pragma: no cover - """Mock base for the GE Cloud command-line harness, with its own cache root and HA interface.""" + """Mock base for the GE Cloud command-line harness, with its own cache root.""" def __init__(self): - """Initialise the shared mock with the GE Cloud cache root and a mock HA interface.""" + """Initialise the shared mock with the GE Cloud cache root.""" super().__init__(config_root="./temp_gecloud") - self.ha_interface = MockHAInterface() def find_registers_by_name(gecloud_direct, register_name, device=None): # pragma: no cover diff --git a/apps/predbat/mock_base.py b/apps/predbat/mock_base.py index 5639cbacc..45a1dbfd8 100644 --- a/apps/predbat/mock_base.py +++ b/apps/predbat/mock_base.py @@ -26,6 +26,20 @@ import json +class MockHAInterface: + """Minimal stand-in for the HA interface, used by the standalone CLI harnesses. + + ComponentBase.set_state_external forwards here so components can change Predbat's OWN config + entities during auto-discovery (e.g. teslemetry turning inverter_hybrid off) - that is the only + write path that also updates the matching CONFIG_ITEMS value. Without this the harnesses would + crash on any component that auto-configures a Predbat setting. + """ + + async def set_state_external(self, entity_id, state, attributes={}): + """Print an external state write instead of applying it.""" + print(f"SET EXTERNAL: {entity_id} = {state}") + + class MockBase: """Minimal stand-in for the PredBat base object, used by the standalone CLI harnesses.""" @@ -51,6 +65,7 @@ def __init__(self, config_root="./temp_predbat", local_tz=None, **kwargs): self.currency_symbols = "£p" self.arg_errors = {} self.args = {key: value for key, value in kwargs.items() if value is not None} + self.ha_interface = MockHAInterface() def log(self, message, quiet=True): """Print a timestamped log line. diff --git a/apps/predbat/teslemetry.py b/apps/predbat/teslemetry.py index 44ba376f7..dd6029d77 100644 --- a/apps/predbat/teslemetry.py +++ b/apps/predbat/teslemetry.py @@ -57,6 +57,9 @@ OPERATION_MODES = ["self_consumption", "autonomous", "backup"] EXPORT_RULES = ["never", "pv_only", "battery_ok"] +# Large nested tariff structures that are never worth republishing - hidden from both the debug log +# summary and the site_info review entity. +TARIFF_BLOB_KEYS = ("tariff_content", "tariff_content_v2") # tou_settings.optimization_strategy: the OTHER dial the Fleet API exposes alongside the tariff itself, # deciding whether Time-Based Control actually acts on price. "balanced" (the device default, and what # is left in place if this is never sent) only discharges to offset house load and never exports stored @@ -240,7 +243,7 @@ def _summarize_for_log(data): for key in ("time_series", "SmartBreakerEnergyLogs"): if isinstance(response.get(key), list): response[key] = "[{} entries hidden]".format(len(response[key])) - for key in ("tariff_content", "tariff_content_v2"): + for key in TARIFF_BLOB_KEYS: if isinstance(response.get(key), dict): response[key] = "[hidden, code={}]".format(response[key].get("code")) return {"response": response} @@ -252,6 +255,21 @@ def publish_sensor(self, suffix, state, unit=None, state_class="measurement", fr attributes["unit_of_measurement"] = unit self.dashboard_item(self.entity(suffix), state, attributes, app="teslemetry") + def publish_site_info(self, response): + """Publish the site_info response as one entity so the device's own view of the site is reviewable. + + Capacity, AC rating, battery coupling and the export rule all come from here and all change how + Predbat models the site, but none of it was visible outside a debug log line. The whole response + is published rather than a hand-picked subset, so fields added by future firmware appear without + a code change - and so nothing is lost to a wrong guess about where a field is nested (batteries, + customer_preferred_export_rule and net_meter_mode all live under components, not at the top). + Only the tariff blobs are dropped, the same large nested structures _summarize_for_log hides. + """ + attributes = {key: value for key, value in response.items() if key not in TARIFF_BLOB_KEYS} + attributes["friendly_name"] = "Powerwall Site Info" + attributes["state_class"] = None + self.dashboard_item(self.entity("site_info"), response.get("site_name") or "unknown", attributes, app="teslemetry") + def publish_soc_max(self, kwh, estimate=False): """Publish the battery capacity (soc_max) in kWh, preferring a real device value over an estimate. @@ -322,6 +340,7 @@ async def fetch_site_info(self): if not data: return False response = data.get("response", {}) + self.publish_site_info(response) nameplate_wh = response.get("nameplate_energy", 0) battery_count = response.get("battery_count") if nameplate_wh: @@ -634,6 +653,9 @@ async def automatic_config(self): max_site_meter_power_ac, so wiring them unconditionally would point Predbat at entities that never exist on a site missing those fields. Predbat falls back to its own defaults for absent args, so skipping the wiring here is safe. + + Unlike the args above, inverter_hybrid is one of Predbat's OWN config switches rather than a + component entity, so it is written through set_state_external (see below). """ self.log("Info: Teslemetry automatic configuration - wiring Predbat to the TESLA inverter type") self.set_arg("inverter_type", ["TESLA"]) @@ -664,6 +686,16 @@ async def automatic_config(self): self.set_arg("discharge_target_soc", [self.entity("schedule_discharge_soc", domain="number")]) self.set_arg("scheduled_discharge_enable", [self.entity("schedule_discharge_enable", domain="switch")]) self.set_arg("schedule_write_button", [self.entity("schedule_write", domain="switch")]) + # Every Powerwall is an AC-coupled battery, so Predbat must not model it as a hybrid. Left at + # Predbat's default (on), get_total_inverted() folds PV into the inverter_limit budget, so the + # Powerwall's own AC rating is applied as a cap on battery + PV combined - modelling a + # separately inverted solar array as clipping against a limit it never passes through, which + # invents both the clipping and the export windows that "recover" it. + # set_state_external is the write path that updates the matching CONFIG_ITEMS value; a plain + # state write would move the entity without changing the setting Predbat plans with. + hybrid_entity = "switch.{}_inverter_hybrid".format(self.prefix) + self.log("Info: Teslemetry setting {} off - Tesla Powerwall batteries are AC coupled".format(hybrid_entity)) + await self.set_state_external(hybrid_entity, False) async def schedule_event(self, entity_id, value): """Stage a schedule entity write into pending_schedule; the write switch commits it. diff --git a/apps/predbat/tests/test_component_base.py b/apps/predbat/tests/test_component_base.py index 3573caa0d..b1c549770 100644 --- a/apps/predbat/tests/test_component_base.py +++ b/apps/predbat/tests/test_component_base.py @@ -13,6 +13,7 @@ """ import asyncio +from types import SimpleNamespace from datetime import timezone from unittest.mock import patch @@ -405,6 +406,38 @@ def test_component_base_set_arg_auto(my_predbat): return False +def test_component_base_set_state_external(my_predbat): + """ + Test ComponentBase.set_state_external() forwards to the HA interface with the attributes intact. + + Components use this (rather than set_state_wrapper) when auto-discovery has to change one of + Predbat's own settings - only this path updates the matching CONFIG_ITEMS value, so writing the + state alone would move the displayed entity without changing what the planner reads. + """ + print("\n*** Test: ComponentBase.set_state_external forwards to the HA interface ***") + + calls = [] + + async def capture(entity_id, state, attributes={}): + """Record a forwarded external state write.""" + calls.append((entity_id, state, attributes)) + return "written" + + base = MockBase() + base.ha_interface = SimpleNamespace(set_state_external=capture) + component = TestComponent(base) + + result = asyncio.run(component.set_state_external("switch.predbat_inverter_hybrid", False)) + assert calls == [("switch.predbat_inverter_hybrid", False, {})], f"Unexpected forwarded call {calls}" + assert result == "written", "The HA interface's return value should be passed back to the caller" + + asyncio.run(component.set_state_external("sensor.predbat_test", 42, {"unit_of_measurement": "W"})) + assert calls[1] == ("sensor.predbat_test", 42, {"unit_of_measurement": "W"}), f"Attributes not forwarded: {calls[1]}" + + print("PASS: set_state_external forwards entity, state and attributes and returns the result") + return False + + def test_component_base_all(my_predbat): """Run all component_base tests""" tests = [ @@ -417,6 +450,7 @@ def test_component_base_all(my_predbat): ("run_timeout", test_component_base_run_timeout, "Hung run() triggers timeout, stack trace, and error count"), ("first_cleared_preset", test_component_base_first_cleared_when_run_presets_api_started, "first flag clears even when run() pre-sets api_started"), ("set_arg_auto", test_component_base_set_arg_auto, "set_arg_auto warns once on an apps.yaml override, silent otherwise"), + ("set_state_external", test_component_base_set_state_external, "set_state_external forwards to the HA interface"), ] failed = [] diff --git a/apps/predbat/tests/test_ge_cloud.py b/apps/predbat/tests/test_ge_cloud.py index 1375f215f..6a2b33d2d 100644 --- a/apps/predbat/tests/test_ge_cloud.py +++ b/apps/predbat/tests/test_ge_cloud.py @@ -72,7 +72,7 @@ class MockHAInterface: def __init__(self): self.external_states = {} - async def set_state_external(self, entity_id, state): + async def set_state_external(self, entity_id, state, attributes={}): self.external_states[entity_id] = state class MockBase: diff --git a/apps/predbat/tests/test_teslemetry.py b/apps/predbat/tests/test_teslemetry.py index 4ca4b9cd5..0110fb70b 100644 --- a/apps/predbat/tests/test_teslemetry.py +++ b/apps/predbat/tests/test_teslemetry.py @@ -6,6 +6,7 @@ """Unit tests for the TeslemetryAPI component (Tesla Powerwall via Teslemetry).""" import copy +from types import SimpleNamespace from unittest.mock import MagicMock, patch, AsyncMock from tests.test_infra import create_aiohttp_mock_response, create_aiohttp_mock_session, run_async @@ -67,6 +68,20 @@ def __init__(self): self.oauth_failed = False self._refresh_in_progress = False self.token_hash = "" + # automatic_config drives Predbat's own config switches (e.g. inverter_hybrid) through + # ComponentBase.set_state_external, which is the only path that updates the matching + # CONFIG_ITEMS value. Capture those writes rather than plain entity states so tests can + # tell a real config change from a display-only publish. + self.external_states = {} + # get_arg mirrors _rate_base's: the keyword-only "d" never matches the "default=" keyword + # ComponentBase.get_arg forwards, so it returns None - i.e. not read-only. That preserves + # the behaviour tests had before this double existed, when _is_read_only()'s missing-base + # guard returned False. + self.base = SimpleNamespace(ha_interface=SimpleNamespace(set_state_external=self._capture_external), get_arg=lambda a, d=None, **k: d) + + async def _capture_external(self, entity_id, state, attributes={}): + """Capture set_state_external calls made against Predbat's own config entities.""" + self.external_states[entity_id] = state @property def storage(self): @@ -153,6 +168,42 @@ def _rate_base(import_p, export_p): } } +# A real-world Powerwall 2 site_info (identifiers replaced). Note the nesting: batteries, +# customer_preferred_export_rule and net_meter_mode live under components, NOT at the top level - +# publish_site_info republishes the response wholesale partly so that distinction cannot be got wrong. +# max_site_meter_power_ac carries the 1e9 "unlimited" sentinel. +SITE_INFO_AC_POWERWALL = { + "response": { + "site_name": "Home", + "default_real_mode": "self_consumption", + "backup_reserve_percent": 4, + "installation_date": "2023-04-27T14:12:19+01:00", + "version": "26.18.3 184289b9", + "battery_count": 1, + "nameplate_power": 5000, + "nameplate_energy": 13500, + "max_site_meter_power_ac": 1000000000, + "min_site_meter_power_ac": -1000000000, + "installation_time_zone": "Europe/London", + "components": { + "solar": True, + "solar_type": "pv_panel", + "battery": True, + "grid": True, + "backup": True, + "gateway": "teg", + "battery_type": "ac_powerwall", + "grid_services_enabled": False, + "customer_preferred_export_rule": "pv_only", + "net_meter_mode": "battery_ok", + "gateways": [{"part_name": "Tesla Backup Gateway 2", "serial_number": "CN322313G3J054"}], + "batteries": [{"part_name": "Powerwall 2", "serial_number": "TG123022000237", "nameplate_max_charge_power": 5000, "nameplate_max_discharge_power": 5000, "nameplate_energy": 13500}], + }, + "tariff_content": {"code": "PREDBAT-NORMAL"}, + "tariff_content_v2": {"code": "PREDBAT-NORMAL"}, + } +} + TARIFF_RATE_NORMAL = {"response": {"tariff_content_v2": {"version": 1, "utility": "Predbat", "code": "PREDBAT-NORMAL", "name": "Predbat (normal)"}}} ENERGY_HISTORY = { @@ -1371,6 +1422,81 @@ def test_teslemetry_run_skips_assert_without_soc(): assert [req for req in api.requests_made if req[0] == "POST"] == [] +def test_teslemetry_automatic_config_disables_inverter_hybrid(): + """Tesla Powerwall batteries are AC coupled, so automatic_config must turn inverter_hybrid off. + + Left at Predbat's default (True), inverter_limit is modelled as a cap on battery + PV combined + (see get_total_inverted in prediction.py), so the Powerwall's own 5 kW rating clips a separately + inverted PV array that it has no bearing on - inventing solar clipping and, with it, phantom + export windows. Writing through set_state_external (not set_state_wrapper) is what actually + updates the matching CONFIG_ITEMS entry rather than just the displayed entity state. + """ + api = MockTeslemetryAPI() + api.mock_responses["/api/1/energy_sites/123456/site_info"] = SITE_INFO_AC_POWERWALL + assert run_async(api.fetch_site_info()) is True + run_async(api.automatic_config()) + assert api.external_states["switch.predbat_inverter_hybrid"] is False + + +def test_teslemetry_automatic_config_disables_hybrid_without_site_info(): + """The hybrid switch is a property of the hardware family, not of any site_info field, so it is + turned off even when site_info never ran or carried none of the optional fields.""" + api = MockTeslemetryAPI() + run_async(api.automatic_config()) + assert api.external_states["switch.predbat_inverter_hybrid"] is False + + +def test_teslemetry_site_info_publishes_site_info_entity(): + """site_info is republished wholesale as a review entity, nesting intact.""" + api = MockTeslemetryAPI() + api.mock_responses["/api/1/energy_sites/123456/site_info"] = SITE_INFO_AC_POWERWALL + assert run_async(api.fetch_site_info()) is True + item = api.dashboard_items["sensor.predbat_teslemetry_site_info"] + assert item["state"] == "Home" + attributes = item["attributes"] + assert attributes["friendly_name"] == "Powerwall Site Info" + assert attributes["state_class"] is None + assert attributes["nameplate_power"] == 5000 + assert attributes["nameplate_energy"] == 13500 + assert attributes["max_site_meter_power_ac"] == 1000000000 + # The fields that decide how Predbat models the site are all nested under components. + assert attributes["components"]["battery_type"] == "ac_powerwall" + assert attributes["components"]["solar_type"] == "pv_panel" + assert attributes["components"]["customer_preferred_export_rule"] == "pv_only" + assert attributes["components"]["batteries"][0]["part_name"] == "Powerwall 2" + assert attributes["components"]["batteries"][0]["nameplate_max_discharge_power"] == 5000 + + +def test_teslemetry_site_info_entity_omits_tariff_blobs(): + """The tariff structures are dropped - large, nested, and already hidden from the debug log.""" + api = MockTeslemetryAPI() + api.mock_responses["/api/1/energy_sites/123456/site_info"] = SITE_INFO_AC_POWERWALL + assert run_async(api.fetch_site_info()) is True + attributes = api.dashboard_items["sensor.predbat_teslemetry_site_info"]["attributes"] + assert "tariff_content" not in attributes + assert "tariff_content_v2" not in attributes + + +def test_teslemetry_site_info_entity_does_not_mutate_response(): + """Filtering builds a new dict, so the response the rest of fetch_site_info reads is untouched.""" + api = MockTeslemetryAPI() + site_info = copy.deepcopy(SITE_INFO_AC_POWERWALL) + api.mock_responses["/api/1/energy_sites/123456/site_info"] = site_info + assert run_async(api.fetch_site_info()) is True + assert site_info["response"]["tariff_content_v2"] == {"code": "PREDBAT-NORMAL"} + assert api.entity_states["sensor.predbat_teslemetry_soc_max"] == 13.5 + + +def test_teslemetry_site_info_entity_survives_minimal_response(): + """A site_info carrying almost nothing still publishes the entity rather than raising.""" + api = MockTeslemetryAPI() + api.mock_responses["/api/1/energy_sites/123456/site_info"] = {"response": {"nameplate_energy": 13500}} + assert run_async(api.fetch_site_info()) is True + item = api.dashboard_items["sensor.predbat_teslemetry_site_info"] + assert item["state"] == "unknown" + assert item["attributes"]["nameplate_energy"] == 13500 + + def test_teslemetry_automatic_config_sets_args(): """automatic_config wires every inverter arg to this component's published entities. @@ -2200,6 +2326,12 @@ def test_teslemetry(my_predbat=None): test_teslemetry_run_skips_assert_when_read_only() test_teslemetry_run_skips_assert_without_soc() test_teslemetry_automatic_config_sets_args() + test_teslemetry_automatic_config_disables_inverter_hybrid() + test_teslemetry_automatic_config_disables_hybrid_without_site_info() + test_teslemetry_site_info_publishes_site_info_entity() + test_teslemetry_site_info_entity_omits_tariff_blobs() + test_teslemetry_site_info_entity_does_not_mutate_response() + test_teslemetry_site_info_entity_survives_minimal_response() test_teslemetry_automatic_config_references_published_entities() test_teslemetry_run_triggers_automatic_config_once_after_site_info() test_teslemetry_emulator_failure_does_not_fail_run() diff --git a/apps/predbat/web.py b/apps/predbat/web.py index 8f698b16c..6d6166ac0 100644 --- a/apps/predbat/web.py +++ b/apps/predbat/web.py @@ -1495,7 +1495,7 @@ async def html_entity_post(self, request): pass # Set the entity state - await self.base.ha_interface.set_state_external(entity_id, new_value, attributes=attributes) + await self.set_state_external(entity_id, new_value, attributes=attributes) self.log(f"Entity {entity_id} updated to {new_value} via web interface") except Exception as e: @@ -2653,7 +2653,7 @@ async def html_config_post(self, request): new_value = float(new_value) self.log("Web interface setting {} to {}".format(pitem, new_value)) - await self.base.ha_interface.set_state_external(pitem, new_value) + await self.set_state_external(pitem, new_value) raise web.HTTPFound("./config") @@ -2898,12 +2898,12 @@ async def html_dash_post(self, request): if key == "mode": # Update mode - it's a select type entity_id = f"select.{self.prefix}_{key}" - await self.base.ha_interface.set_state_external(entity_id, value) + await self.set_state_external(entity_id, value) elif key in ["debug_enable", "set_read_only", "active"]: # Update switches - convert to boolean entity_id = f"switch.{self.prefix}_{key}" bool_value = value == "on" - await self.base.ha_interface.set_state_external(entity_id, bool_value) + await self.set_state_external(entity_id, bool_value) # Log the update self.log(f"Dashboard status updated: {dict(data)}") @@ -4287,7 +4287,7 @@ async def html_rate_override(self, request): await self.base.async_manual_select("manual_import_rates", clear_option) elif action == "Set Import": item = self.base.config_index.get("manual_import_value", {}) - await self.base.ha_interface.set_state_external(item.get("entity", None), rate) + await self.set_state_external(item.get("entity", None), rate) await self.base.async_manual_select("manual_import_rates", selection_option) elif action == "Clear Export": manual_export_rates = self.base.manual_rates("manual_export_rates") @@ -4296,11 +4296,11 @@ async def html_rate_override(self, request): await self.base.async_manual_select("manual_export_rates", clear_option) elif action == "Set Export": item = self.base.config_index.get("manual_export_value", {}) - await self.base.ha_interface.set_state_external(item.get("entity", None), rate) + await self.set_state_external(item.get("entity", None), rate) await self.base.async_manual_select("manual_export_rates", selection_option) elif action == "Set Load": item = self.base.config_index.get("manual_load_value", {}) - await self.base.ha_interface.set_state_external(item.get("entity", None), rate) + await self.set_state_external(item.get("entity", None), rate) await self.base.async_manual_select("manual_load_adjust", selection_option) elif action == "Clear Load": manual_load_adjust = self.base.manual_rates("manual_load_adjust") @@ -4309,7 +4309,7 @@ async def html_rate_override(self, request): await self.base.async_manual_select("manual_load_adjust", clear_option) elif action == "Set SOC": item = self.base.config_index.get("manual_soc_value", {}) - await self.base.ha_interface.set_state_external(item.get("entity", None), rate) + await self.set_state_external(item.get("entity", None), rate) await self.base.async_manual_select("manual_soc", selection_option) elif action == "Clear SOC": manual_soc = self.base.manual_rates("manual_soc") diff --git a/apps/predbat/web_mcp.py b/apps/predbat/web_mcp.py index 28c9b10e6..28b474049 100644 --- a/apps/predbat/web_mcp.py +++ b/apps/predbat/web_mcp.py @@ -1134,7 +1134,7 @@ async def _execute_set_config(self, arguments: Dict[str, Any]) -> Dict[str, Any] return {"success": False, "error": "Both 'entity_id' and 'value' must be provided", "data": None} # Update the configuration setting - await self.base.ha_interface.set_state_external(entity_id, value) + await self.set_state_external(entity_id, value) return {"success": True, "error": None, "data": {"entity_id": entity_id, "new_value": value}, "timestamp": datetime.now().isoformat(), "description": f"Configuration setting '{entity_id}' updated successfully"}