From 87c640e02a6fbf19fe5de8cd9afca27b51077f18 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:07:52 -0700 Subject: [PATCH 1/4] fix(naming): Recreate entity IDs proposes what the panel says now Renaming a circuit in the SPAN app left "Recreate entity IDs" offering the entity its own ID, so the button appeared to do nothing (#252). The registry generates an ID from the user's `name` override, else `suggested_object_id`, else `object_id_base`. Friendly-names mode writes no registry `name` -- the panel name arrives as `original_name` -- so the suggestion decides, and `construct_single_circuit_entity_id` was handing back the entity's stored ID on every reload. The suggestion was therefore frozen at whatever the circuit was called the day it was added. It now computes the ID from current panel data and the naming flags, for existing entities as much as new ones. That cannot move a live entity ID: an entity_id set before add is only a suggestion, and for a unique_id already on file `async_get_or_create` routes to `_async_update_entity` with no `new_entity_id` before `entity.entity_id` is reassigned from the stored entry. Unique IDs are untouched -- they derive from the description key and never read a name. Circuit-numbers mode is deliberately unchanged, guarded by a test. There the registry `name` phase 2 sync writes is both what shows the panel's name in the UI and what outranks the suggestion, so Recreate keeps composing from that name. Correcting it would mean rerouting phase 2 sync, which is a product decision, not part of this fix. The registry lookup was the only use of the `unique_id` argument, and `existing_entity_id` the only use of that parameter on `_construct_entity_id`; both are gone rather than left as parameters that no longer decide anything. Every test reloads before asserting -- asserting straight after creation exercises the first-add path, where the suggestion is trivially current and the bug cannot appear. The three that demonstrate the bug fail on the previous code; the five guards pass on both. --- .../span_panel/entity_resolver.py | 40 +-- custom_components/span_panel/select.py | 7 +- custom_components/span_panel/sensor_base.py | 8 +- .../span_panel/sensor_circuit.py | 4 - custom_components/span_panel/switch.py | 7 +- tests/test_recreate_entity_ids.py | 306 ++++++++++++++++++ 6 files changed, 334 insertions(+), 38 deletions(-) create mode 100644 tests/test_recreate_entity_ids.py diff --git a/custom_components/span_panel/entity_resolver.py b/custom_components/span_panel/entity_resolver.py index bf637cc9..3443f5a7 100644 --- a/custom_components/span_panel/entity_resolver.py +++ b/custom_components/span_panel/entity_resolver.py @@ -284,10 +284,25 @@ def construct_single_circuit_entity_id( platform: str, suffix: str, circuit_data: SpanCircuitSnapshot, - unique_id: str | None = None, device_name: str | None = None, ) -> str | None: - """Construct entity ID for single-circuit sensors. + """Construct the entity ID the current panel data and naming flags produce. + + Always computed, never read back from the registry -- including for a circuit + that already has entities. Home Assistant treats an entity_id set before the + entity is added as a *suggestion*: `EntityPlatform` splits it into + `suggested_object_id`, hands that to `async_get_or_create`, and for a + unique_id already on file that call routes to `_async_update_entity` with no + `new_entity_id`, then reassigns `entity.entity_id` from the stored entry. A + live entity ID therefore cannot move from here; only the stored suggestion + changes. + + That suggestion is the field "Recreate entity IDs" regenerates from when the + registry holds no user `name` override, which in friendly-names mode is + always the case -- the panel name reaches the UI as `original_name`. Handing + back the stored entity ID froze the suggestion at whatever the circuit was + called on the day it was added, so a circuit renamed in the SPAN app was + offered its own ID and Recreate looked broken (issue #252). Args: coordinator: The coordinator instance @@ -295,33 +310,12 @@ def construct_single_circuit_entity_id( platform: Platform name ("sensor", "switch", "select") suffix: Entity-specific suffix ("power", "energy_produced", etc.) circuit_data: Circuit data object - unique_id: The unique ID for this entity (None to skip registry lookup) device_name: Device name for entity ID construction (None to use from config entry) Returns: Constructed entity ID string or None if device info unavailable """ - # Check registry first only if unique_id is provided - if unique_id is not None: - entity_registry = er.async_get(coordinator.hass) - existing_entity_id = entity_registry.async_get_entity_id(platform, DOMAIN, unique_id) - - _LOGGER.debug( - "Circuit helper registry lookup - unique_id=%s, found_entity_id=%s", - unique_id, - existing_entity_id, - ) - - if existing_entity_id: - return existing_entity_id - # FATAL ERROR: Expected unique_id not found in registry - raise ValueError( - f"REGISTRY LOOKUP ERROR: Expected unique_id '{unique_id}' not found in registry. " - f"This indicates a migration or configuration mismatch." - ) - _LOGGER.debug("Circuit helper - no unique_id provided, skipping registry lookup") - # Get device info device_info = snapshot_to_device_info(snapshot, device_name) if not device_info or not device_info.get("name"): diff --git a/custom_components/span_panel/select.py b/custom_components/span_panel/select.py index ebf87070..b08176c3 100644 --- a/custom_components/span_panel/select.py +++ b/custom_components/span_panel/select.py @@ -166,16 +166,15 @@ def __init__( self._attr_name = None # Explicitly set entity_id using construct_single_circuit_entity_id - # which correctly handles 240V two-tab circuits. - # Only pass unique_id for existing entities (registry lookup); - # for new entities pass None to get the constructed default. + # which correctly handles 240V two-tab circuits. For an entity already + # in the registry this is a suggestion HA records and does not act on -- + # the stored entity_id stands. See the helper's docstring. constructed_id = construct_single_circuit_entity_id( coordinator, snapshot, "select", description.entity_description.key, circuit, - unique_id=self._attr_unique_id if existing_entity_id else None, ) if constructed_id: self.entity_id = constructed_id diff --git a/custom_components/span_panel/sensor_base.py b/custom_components/span_panel/sensor_base.py index 88dbf41f..d40152ae 100644 --- a/custom_components/span_panel/sensor_base.py +++ b/custom_components/span_panel/sensor_base.py @@ -123,7 +123,7 @@ def __init__( ) # Wire explicit entity_id via subclass helper - entity_id = self._construct_entity_id(snapshot, description, existing_entity_id) + entity_id = self._construct_entity_id(snapshot, description) if entity_id: self.entity_id = entity_id else: @@ -228,17 +228,19 @@ def _construct_entity_id( self, snapshot: SpanPanelSnapshot, description: T, - existing_entity_id: str | None = None, ) -> str | None: """Construct explicit entity_id for the sensor. Subclasses may override to use entity_id helpers from helpers.py. Returns None to let HA auto-generate from _attr_name. + Whether the entity is already in the registry is deliberately not an + input: the value is what current panel data and the naming flags + produce, and HA keeps an existing entity's stored ID regardless. + Args: snapshot: The panel snapshot data description: The sensor description - existing_entity_id: The existing entity_id from registry, or None for new entities """ return None diff --git a/custom_components/span_panel/sensor_circuit.py b/custom_components/span_panel/sensor_circuit.py index 83cba192..c9802957 100644 --- a/custom_components/span_panel/sensor_circuit.py +++ b/custom_components/span_panel/sensor_circuit.py @@ -204,7 +204,6 @@ def _construct_entity_id( self, snapshot: SpanPanelSnapshot, description: SpanPanelCircuitsSensorEntityDescription, - existing_entity_id: str | None = None, ) -> str | None: """Construct explicit entity_id for circuit power sensors.""" circuit = snapshot.circuits.get(self.circuit_id) @@ -219,7 +218,6 @@ def _construct_entity_id( "sensor", suffix, circuit, - unique_id=self._attr_unique_id if existing_entity_id else None, ) def get_data_source(self, snapshot: SpanPanelSnapshot) -> SpanCircuitSnapshot: @@ -389,7 +387,6 @@ def _construct_entity_id( self, snapshot: SpanPanelSnapshot, description: SpanPanelCircuitsSensorEntityDescription, - existing_entity_id: str | None = None, ) -> str | None: """Construct explicit entity_id for circuit energy sensors.""" circuit = snapshot.circuits.get(self.circuit_id) @@ -408,7 +405,6 @@ def _construct_entity_id( "sensor", suffix, circuit, - unique_id=self._attr_unique_id if existing_entity_id else None, ) # Map original_key to the energy type used for coordinator dip offset tracking diff --git a/custom_components/span_panel/switch.py b/custom_components/span_panel/switch.py index a4dba851..35923d95 100644 --- a/custom_components/span_panel/switch.py +++ b/custom_components/span_panel/switch.py @@ -120,16 +120,15 @@ def __init__( super().__init__(coordinator) # Explicitly set entity_id using construct_single_circuit_entity_id - # which correctly handles 240V two-tab circuits. - # Only pass unique_id for existing entities (registry lookup); - # for new entities pass None to get the constructed default. + # which correctly handles 240V two-tab circuits. For an entity already + # in the registry this is a suggestion HA records and does not act on -- + # the stored entity_id stands. See the helper's docstring. constructed_id = construct_single_circuit_entity_id( coordinator, snapshot, "switch", "breaker", circuit, - unique_id=self._attr_unique_id if existing_entity_id else None, ) if constructed_id: self.entity_id = constructed_id diff --git a/tests/test_recreate_entity_ids.py b/tests/test_recreate_entity_ids.py new file mode 100644 index 00000000..4d664a39 --- /dev/null +++ b/tests/test_recreate_entity_ids.py @@ -0,0 +1,306 @@ +"""Recreate entity IDs proposes the ID the current panel data would produce. + +Issue #252: renaming a circuit in the SPAN app left "Recreate entity IDs" (the +HA registry's `async_regenerate_entity_id`) proposing the entity's own ID, so +the button appeared to do nothing. + +The registry generates an ID from three fields in priority order: the user's +`name` override, then `suggested_object_id`, then `object_id_base`. In +friendly-names mode this integration never writes a registry `name` -- the +panel name reaches the UI through `original_name` -- so `name` is None, the +generator short-circuits to `suggested_object_id`, and that field is whatever +was suggested when the entity was first added. It was frozen because the +integration preset the entity's *stored* ID on every reload, suggesting the +value already in place. + +Every case here reloads before asserting. Asserting straight after creation +tests the first-add path, where the suggestion is trivially current and the bug +cannot appear. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +import pytest +from homeassistant.const import CONF_HOST +from homeassistant.core import HomeAssistant +from homeassistant.helpers import entity_registry as er +from pytest_homeassistant_custom_component.common import ( + MockConfigEntry, + MockEntityPlatform, +) + +from custom_components.span_panel import SpanPanelRuntimeData +from custom_components.span_panel.const import ( + DOMAIN, + USE_CIRCUIT_NUMBERS, + USE_DEVICE_PREFIX, +) +from custom_components.span_panel.sensor_circuit import SpanCircuitPowerSensor +from custom_components.span_panel.sensor_definitions import CIRCUIT_SENSORS +from custom_components.span_panel.switch import SpanPanelCircuitsSwitch + +from .factories import SpanCircuitSnapshotFactory, SpanPanelSnapshotFactory + +CIRCUIT_ID = "15" +SERIAL = "sp3-recreate-001" + +ORIGINAL_NAME = "Refrigerator" +RENAMED = "Beer Fridge" + +ORIGINAL_ENTITY_ID = "sensor.span_panel_refrigerator_power" +RENAMED_ENTITY_ID = "sensor.span_panel_beer_fridge_power" +CIRCUIT_NUMBERS_ENTITY_ID = "sensor.span_panel_circuit_15_power" + +FRIENDLY_NAMES = {USE_DEVICE_PREFIX: True, USE_CIRCUIT_NUMBERS: False} +CIRCUIT_NUMBERS = {USE_DEVICE_PREFIX: True, USE_CIRCUIT_NUMBERS: True} + +POWER_DESCRIPTION = next(desc for desc in CIRCUIT_SENSORS if desc.key == "circuit_power") + + +def _snapshot(circuit_name: str): + """Build a one-circuit panel snapshot with the circuit named as given.""" + circuit = SpanCircuitSnapshotFactory.create( + circuit_id=CIRCUIT_ID, name=circuit_name, tabs=[15] + ) + return SpanPanelSnapshotFactory.create(serial_number=SERIAL, circuits={CIRCUIT_ID: circuit}) + + +def _coordinator(hass: HomeAssistant, snapshot, entry: MockConfigEntry) -> MagicMock: + """Build a coordinator standing in for a live one, bound to the real hass.""" + coordinator = MagicMock() + coordinator.hass = hass + coordinator.data = snapshot + coordinator.panel_offline = False + coordinator.config_entry = entry + coordinator.request_reload = MagicMock() + coordinator.register_circuit_energy_sensor = MagicMock() + coordinator.get_circuit_dip_offset = MagicMock(return_value=0.0) + return coordinator + + +class _Install: + """One install of the sensor platform, reloadable. + + `load` a second time is what a reload is: the entry's entities are torn down + and rebuilt from the current snapshot, against the entity registry that + survived. Nothing here is faked -- `async_add_entities` is the real + `EntityPlatform` path, so a preset entity_id travels the same route into + `async_get_or_create` that it does in a running install, and the teardown is + the same `async_reset` an entry unload performs. + """ + + def __init__(self, hass: HomeAssistant, entry: MockConfigEntry) -> None: + self._hass = hass + self._entry = entry + self._platform: MockEntityPlatform | None = None + + async def load(self, circuit_name: str) -> SpanCircuitPowerSensor: + """Tear down any previous platform, then set one up from fresh panel data.""" + if self._platform is not None: + await self._platform.async_reset() + + snapshot = _snapshot(circuit_name) + coordinator = _coordinator(self._hass, snapshot, self._entry) + self._entry.runtime_data = SpanPanelRuntimeData( + coordinator=coordinator, panel_device_id="panel-device-id" + ) + + self._platform = MockEntityPlatform(self._hass, domain="sensor", platform_name=DOMAIN) + self._platform.config_entry = self._entry + + sensor = SpanCircuitPowerSensor(coordinator, POWER_DESCRIPTION, snapshot, CIRCUIT_ID) + await self._platform.async_add_entities([sensor]) + await self._hass.async_block_till_done() + + assert sensor.hass is not None, "entity was rejected before it reached the registry" + return sensor + + +@pytest.fixture +def entry(hass: HomeAssistant) -> MockConfigEntry: + """A config entry in friendly-names mode.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + data={CONF_HOST: "192.168.1.50", "device_name": "SPAN Panel"}, + options=dict(FRIENDLY_NAMES), + title="SPAN Panel", + unique_id=SERIAL, + entry_id="entry-recreate", + ) + config_entry.add_to_hass(hass) + return config_entry + + +async def test_the_first_install_takes_its_entity_id_from_the_circuit_name( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Baseline for every case below: the ID before any rename.""" + sensor = await _Install(hass, entry).load(ORIGINAL_NAME) + + assert sensor.entity_id == ORIGINAL_ENTITY_ID + + +async def test_renaming_a_circuit_does_not_move_an_existing_entity_id( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """The non-negotiable one: a rename must never move a live entity_id. + + Dashboards, automations, and recorder history all key off the entity_id. + Recreate is an offer the user accepts; a rename is not. + """ + install = _Install(hass, entry) + await install.load(ORIGINAL_NAME) + sensor = await install.load(RENAMED) + + assert sensor.entity_id == ORIGINAL_ENTITY_ID + + registry = er.async_get(hass) + assert registry.async_get(ORIGINAL_ENTITY_ID) is not None + assert registry.async_get(RENAMED_ENTITY_ID) is None + + +async def test_renaming_a_circuit_does_not_move_the_unique_id( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Unique IDs are derived from the description key and never from a name. + + A moved unique_id orphans the entity and drops its long-term statistics, and + it would break any future migration that has to predict what a unique_id + looks like. + """ + install = _Install(hass, entry) + before = await install.load(ORIGINAL_NAME) + unique_id_before = before.unique_id + + after = await install.load(RENAMED) + + assert after.unique_id == unique_id_before + + registry = er.async_get(hass) + entry_after = registry.async_get(ORIGINAL_ENTITY_ID) + assert entry_after is not None + assert entry_after.unique_id == unique_id_before + + +async def test_renaming_a_circuit_refreshes_the_registrys_entity_id_suggestion( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """The stored suggestion has to track the panel, not the install date. + + This is the field `async_regenerate_entity_id` reads when there is no user + `name` override, which in friendly-names mode is always. + """ + install = _Install(hass, entry) + await install.load(ORIGINAL_NAME) + await install.load(RENAMED) + + registry = er.async_get(hass) + registry_entry = registry.async_get(ORIGINAL_ENTITY_ID) + assert registry_entry is not None + assert registry_entry.suggested_object_id == "span_panel_beer_fridge_power" + + +async def test_recreate_entity_ids_proposes_the_renamed_id( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Issue #252 itself, at the API the button calls.""" + install = _Install(hass, entry) + await install.load(ORIGINAL_NAME) + await install.load(RENAMED) + + registry = er.async_get(hass) + registry_entry = registry.async_get(ORIGINAL_ENTITY_ID) + assert registry_entry is not None + + proposed = registry.async_regenerate_entity_id(registry_entry) + + assert proposed == RENAMED_ENTITY_ID + assert proposed != registry_entry.entity_id + + +async def test_an_unrenamed_circuit_is_offered_its_own_entity_id( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Recreate must be a no-op when nothing changed. + + Without this the previous test passes for the wrong reason -- a suggestion + that moves on every reload would satisfy it while offering every user a + pointless rename. + """ + install = _Install(hass, entry) + await install.load(ORIGINAL_NAME) + await install.load(ORIGINAL_NAME) + + registry = er.async_get(hass) + registry_entry = registry.async_get(ORIGINAL_ENTITY_ID) + assert registry_entry is not None + + assert registry.async_regenerate_entity_id(registry_entry) == ORIGINAL_ENTITY_ID + + +async def test_circuit_numbers_mode_keeps_its_id_its_display_name_and_its_sync( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Regression guard: nothing in circuit-numbers mode may change. + + There the registry `name` written by phase 2 name sync is both what puts the + panel's name in the UI and what outranks `suggested_object_id` during + regeneration. That second effect means Recreate in this mode proposes a + friendly-name ID for a circuit-numbered entity -- a known limitation, and + the assertion below pins it deliberately: it is what the mode did before + this fix, and this fix must not disturb it. + + The two are the same write, so correcting Recreate here would mean dropping + or rerouting phase 2 sync. That is a product decision, recorded in the design + doc, not something to change while fixing friendly-names mode. + """ + hass.config_entries.async_update_entry(entry, options=dict(CIRCUIT_NUMBERS)) + + install = _Install(hass, entry) + await install.load(ORIGINAL_NAME) + sensor = await install.load(RENAMED) + + assert sensor.entity_id == CIRCUIT_NUMBERS_ENTITY_ID + + registry = er.async_get(hass) + registry_entry = registry.async_get(CIRCUIT_NUMBERS_ENTITY_ID) + assert registry_entry is not None + + # Phase 2 sync still writes the panel's name as the display name. + assert registry_entry.name == f"{RENAMED} Power" + + # And that name still outranks the suggestion, so the offer is composed + # from it -- unchanged, limitation included. + assert registry.async_regenerate_entity_id(registry_entry) == RENAMED_ENTITY_ID + + +async def test_the_breaker_switch_gets_the_same_refreshed_suggestion( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Switches and selects preset their IDs through the same helper. + + They call it from their own constructors rather than through + `_construct_entity_id`, so a fix that only reached the sensor path would + leave a renamed circuit's breaker switch still offering its old ID. + """ + platform = MockEntityPlatform(hass, domain="switch", platform_name=DOMAIN) + platform.config_entry = entry + + for circuit_name in (ORIGINAL_NAME, RENAMED): + snapshot = _snapshot(circuit_name) + coordinator = _coordinator(hass, snapshot, entry) + switch = SpanPanelCircuitsSwitch(coordinator, CIRCUIT_ID, circuit_name, "SPAN Panel") + await platform.async_add_entities([switch]) + await hass.async_block_till_done() + await platform.async_reset() + + registry = er.async_get(hass) + registry_entry = registry.async_get("switch.span_panel_refrigerator_breaker") + assert registry_entry is not None + assert registry_entry.suggested_object_id == "span_panel_beer_fridge_breaker" + assert ( + registry.async_regenerate_entity_id(registry_entry) + == "switch.span_panel_beer_fridge_breaker" + ) From f91d3b255ad106880bd50eef6ff35d7f4c36cbb3 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:25:49 -0700 Subject: [PATCH 2/4] chore(release): 2.1.0b10 Cut from the entity-id branch so the "Recreate entity IDs" fix gets field time before it merges. Carries span-panel-api 3.0.1 and both adapters at 1.0.0. --- custom_components/span_panel/manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom_components/span_panel/manifest.json b/custom_components/span_panel/manifest.json index 775c5489..a409fab8 100644 --- a/custom_components/span_panel/manifest.json +++ b/custom_components/span_panel/manifest.json @@ -26,7 +26,7 @@ "span-panel-api-schema-0==1.0.0", "span-panel-api-schema-1==1.0.0" ], - "version": "2.1.0b9", + "version": "2.1.0b10", "zeroconf": [ { "type": "_span._tcp.local." From 28e6035419e37aace116fda20bb23f3804672a95 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:33:54 -0700 Subject: [PATCH 3/4] docs(changelog): the entity-id fix and the energy sensor rename Both are outward facing and neither was recorded. Written against 2.1.0, not a beta -- the changelog describes the public release, and the steps between betas are not what a reader is looking for. --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b313695..0a0a8380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,8 +149,18 @@ on an older Home Assistant, stay on 2.0.8 until you can update; HACS will not of `panel_size` was already in that attribute block. If you have a template reading `state_attr('sensor.span_panel_software_version', 'wifi_ssid')`, point it at the Wi-Fi Link binary sensor instead. `panel_size` is unaffected and stays where it is. +- **The three circuit energy sensors are renamed to match the entity ids they have always had.** "Produced Energy", "Consumed Energy" and "Net Energy" become + **Energy Produced**, **Energy Consumed** and **Energy Net** — the order their entity ids (`..._energy_produced`, `..._energy_consumed`, `..._energy_net`) have + used since those sensors shipped. **Entity ids, unique ids and history are unchanged**; only the name shown in the UI reorders. Left alone, the word order + would have had Recreate entity IDs offering you a rename for every circuit on your panel. + ### Fixed +- **Recreate entity IDs proposes the ids your panel would produce now.** Renaming a circuit in the SPAN app used to leave the button offering each entity the id + it already had, so it looked like it did nothing (#252). The proposal was frozen at whatever the circuit was called when the entity was first created; it now + follows the panel. **It is still an offer you accept** — a rename in the SPAN app never moves a live entity id by itself, and unique ids and statistics are + untouched. Rename nothing and Recreate proposes the ids you already have. Circuit-numbers installations are unchanged: there the display name written by name + sync is also what Home Assistant builds the proposal from, so the button behaves exactly as it did. - **Enum sensors advertise the states they can actually report.** Nine sensors declared only `unknown`, so `DSM Grid State` sitting at `On Grid` showed "Possible states: Unknown". - **The README described Battery Power's sign backwards.** The sensor reports **discharging** as positive and always has — that is what release 2.0.5 From d1c580a09af988a346d6a1817e4da6a8beb420b3 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:54:53 -0700 Subject: [PATCH 4/4] fix(naming): keep the entity-id suffix an install already has Recreate offered 74 renames on a real panel after the previous beta: every circuit energy sensor, `_consumed_energy` to `_energy_consumed`. Only one of the 74 was a circuit the owner had renamed, and that one was buried. Installs predating the point where the suffix mapping reached entity ids took their id from the descriptor name, so they carry `_consumed_energy` while their own unique id carries `_energy_consumed`. The two have disagreed all along; recomputing the suggestion is what made the disagreement visible. The descriptor rename is not the cause and reverting it would not have removed a single offer: in friendly-names mode the registry holds no `name`, so `suggested_object_id` -- our suffix-based preset -- outranks `original_name` and the descriptor name never reaches the id. Measured both ways; the proposal is `_energy_consumed` either way. So an existing entity keeps the suffix it shipped with and only the circuit-name half of its id follows the panel. `LEGACY_ENTITY_ID_SUFFIXES` records the older spellings; comparing with the suffix removed means a renamed circuit still gets the computed id, which is the whole point of #252. A trailing segment that merely looks like a suffix change -- "Kitchen Outlets" renamed to "Kitchen" -- is a rename and is still offered. `_construct_entity_id` takes `existing_entity_id` again. It no longer decides whether to compute an id, only which suffix the computed one carries. Four tests, two of which fail with preservation disabled. The earlier ones all built their entities with current code, where preset and live id are the same string by construction and this could not appear -- which is why it reached a beta. --- CHANGELOG.md | 15 +- .../span_panel/entity_resolver.py | 12 +- custom_components/span_panel/id_builder.py | 90 +++++++++-- custom_components/span_panel/manifest.json | 2 +- custom_components/span_panel/select.py | 1 + custom_components/span_panel/sensor_base.py | 10 +- .../span_panel/sensor_circuit.py | 4 + custom_components/span_panel/switch.py | 1 + tests/test_recreate_entity_ids.py | 144 +++++++++++++++++- 9 files changed, 254 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a0a8380..22001379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,18 +149,21 @@ on an older Home Assistant, stay on 2.0.8 until you can update; HACS will not of `panel_size` was already in that attribute block. If you have a template reading `state_attr('sensor.span_panel_software_version', 'wifi_ssid')`, point it at the Wi-Fi Link binary sensor instead. `panel_size` is unaffected and stays where it is. -- **The three circuit energy sensors are renamed to match the entity ids they have always had.** "Produced Energy", "Consumed Energy" and "Net Energy" become - **Energy Produced**, **Energy Consumed** and **Energy Net** — the order their entity ids (`..._energy_produced`, `..._energy_consumed`, `..._energy_net`) have - used since those sensors shipped. **Entity ids, unique ids and history are unchanged**; only the name shown in the UI reorders. Left alone, the word order - would have had Recreate entity IDs offering you a rename for every circuit on your panel. +- **The three circuit energy sensors are renamed to match the ids they are given.** "Produced Energy", "Consumed Energy" and "Net Energy" become **Energy + Produced**, **Energy Consumed** and **Energy Net**, the order used by the `energy_produced`, `energy_consumed` and `energy_net` suffixes that these sensors' + unique ids carry and that new entities are given. **Entity ids, unique ids and history are unchanged**; only the name shown in the UI reorders. ### Fixed - **Recreate entity IDs proposes the ids your panel would produce now.** Renaming a circuit in the SPAN app used to leave the button offering each entity the id it already had, so it looked like it did nothing (#252). The proposal was frozen at whatever the circuit was called when the entity was first created; it now follows the panel. **It is still an offer you accept** — a rename in the SPAN app never moves a live entity id by itself, and unique ids and statistics are - untouched. Rename nothing and Recreate proposes the ids you already have. Circuit-numbers installations are unchanged: there the display name written by name - sync is also what Home Assistant builds the proposal from, so the button behaves exactly as it did. + untouched. Circuit-numbers installations are unchanged: there the display name written by name sync is also what Home Assistant builds the proposal from, so + the button behaves exactly as it did. +- **Only circuits you actually renamed are offered.** Installations old enough to predate the current suffixes carry entity ids ending `_consumed_energy`, + `_produced_energy`, `_net_energy` or `_current_power`, where an entity created today would end `_energy_consumed`, `_energy_produced`, `_energy_net` or + `_power`. Those ids keep the suffix they have. Renormalising them would have offered a rename for **every circuit on the panel** — seventy-four on one we + measured — burying the one circuit that had actually been renamed and breaking the dashboards and automations of anyone who accepted. - **Enum sensors advertise the states they can actually report.** Nine sensors declared only `unknown`, so `DSM Grid State` sitting at `On Grid` showed "Possible states: Unknown". - **The README described Battery Power's sign backwards.** The sensor reports **discharging** as positive and always has — that is what release 2.0.5 diff --git a/custom_components/span_panel/entity_resolver.py b/custom_components/span_panel/entity_resolver.py index 3443f5a7..d006705a 100644 --- a/custom_components/span_panel/entity_resolver.py +++ b/custom_components/span_panel/entity_resolver.py @@ -25,6 +25,7 @@ build_select_unique_id, build_switch_unique_id, construct_synthetic_unique_id, + preserve_legacy_entity_id_suffix, ) from .util import snapshot_to_device_info @@ -285,6 +286,7 @@ def construct_single_circuit_entity_id( suffix: str, circuit_data: SpanCircuitSnapshot, device_name: str | None = None, + existing_entity_id: str | None = None, ) -> str | None: """Construct the entity ID the current panel data and naming flags produce. @@ -311,6 +313,8 @@ def construct_single_circuit_entity_id( suffix: Entity-specific suffix ("power", "energy_produced", etc.) circuit_data: Circuit data object device_name: Device name for entity ID construction (None to use from config entry) + existing_entity_id: This entity's id in the registry, when it has one, so an + id predating the suffix mapping keeps the suffix it shipped with Returns: Constructed entity ID string or None if device info unavailable @@ -366,7 +370,13 @@ def construct_single_circuit_entity_id( if suffix and not circuit_part.endswith(f"_{suffix}"): parts.append(suffix) - return f"{platform}.{'_'.join(parts)}" + # An entity created before the suffix mapping reached entity ids carries the + # older spelling. Renormalising it would offer a rename to every circuit on + # the panel, so the circuit-name half follows the panel and the suffix half + # stays as it shipped. + return preserve_legacy_entity_id_suffix( + f"{platform}.{'_'.join(parts)}", existing_entity_id, suffix + ) def construct_unmapped_entity_id( diff --git a/custom_components/span_panel/id_builder.py b/custom_components/span_panel/id_builder.py index 5d02bb8d..61055dca 100644 --- a/custom_components/span_panel/id_builder.py +++ b/custom_components/span_panel/id_builder.py @@ -34,10 +34,14 @@ """**Closed.** A compatibility shim for the keys that predate snake_case, not a house style. Every entry here translates a legacy camelCase description key into the suffix -its entities have carried since before 2.0.8 -- and that suffix is shared by the -`unique_id` *and* the `entity_id`, so a changed entry moves both on every -installed panel. A moved `unique_id` costs the statistics; a moved `entity_id` -breaks the templates and automations a user wrote. +its entities have carried since before 2.0.8, so a changed entry moves a live +`unique_id` on every installed panel, and a moved `unique_id` costs the +statistics. + +It governs the `entity_id` too, but only for entities created since the +integration began presetting one. Older entities took their id from the +descriptor name instead, which used the opposite word order -- that is what +`LEGACY_ENTITY_ID_SUFFIXES` records, and why it has to exist. So the rule for anything new is **verbatim**: a description key added from here on resolves to itself, exactly as the sub-device builders (`build_bess_unique_id`, @@ -50,6 +54,60 @@ """ +# Entity-id suffixes that predate the mapping above, keyed by the suffix that +# replaced them. +LEGACY_ENTITY_ID_SUFFIXES: dict[str, frozenset[str]] = { + "power": frozenset({"current_power"}), + "energy_produced": frozenset({"produced_energy"}), + "energy_consumed": frozenset({"consumed_energy"}), + "energy_net": frozenset({"net_energy"}), +} +"""Entity ids only -- never a `unique_id`, which has always used the canonical form. + +Before the integration preset an `entity_id`, Home Assistant composed one from the +descriptor name: "Consumed Energy" gave `..._consumed_energy` where the mapping +above says `energy_consumed`. Installs from that era carry an entity id whose +suffix disagrees with their own unique id; the two orders were only reconciled +going forward. + +That disagreement is not a defect to correct on a user's behalf. Renormalising it +offers a rename for every circuit on the panel -- seventy-four on a measured one -- +which buries the circuit they actually renamed and breaks every dashboard and +automation belonging to anyone who accepts. So an existing entity keeps the suffix +it has; only the circuit-name half of its id follows the panel. + +Entries are historical fact, so this table only grows by discovering another form +that shipped. `energy_imported`, `energy_exported`, `priority`, `current` and +`breaker_rating` have no entry because they were never named the other way round. +""" + + +def preserve_legacy_entity_id_suffix( + computed_entity_id: str, existing_entity_id: str | None, suffix: str +) -> str: + """Return the id to use, keeping an existing entity's legacy suffix form. + + The ids are compared with the suffix removed, so the existing id wins only + when the circuit-name half already agrees and the suffix is a known older + spelling. A circuit renamed on the panel differs in that half and gets the + computed id, which is what issue #252 is about. + """ + if not existing_entity_id or existing_entity_id == computed_entity_id: + return computed_entity_id + + legacy_forms = LEGACY_ENTITY_ID_SUFFIXES.get(suffix) + if not legacy_forms: + return computed_entity_id + + stem = computed_entity_id.removesuffix(f"_{suffix}") + if stem == computed_entity_id: + return computed_entity_id + + if any(existing_entity_id == f"{stem}_{form}" for form in legacy_forms): + return existing_entity_id + return computed_entity_id + + # Panel sensor API field mappings (used by get_user_friendly_suffix) # Includes main meter/feedthrough produced, consumed, and net energy PANEL_SUFFIX_MAPPING = { @@ -70,10 +128,14 @@ """**Closed.** A compatibility shim for the keys that predate snake_case, not a house style. Every entry here translates a legacy camelCase description key into the suffix -its entities have carried since before 2.0.8 -- and that suffix is shared by the -`unique_id` *and* the `entity_id`, so a changed entry moves both on every -installed panel. A moved `unique_id` costs the statistics; a moved `entity_id` -breaks the templates and automations a user wrote. +its entities have carried since before 2.0.8, so a changed entry moves a live +`unique_id` on every installed panel, and a moved `unique_id` costs the +statistics. + +It governs the `entity_id` too, but only for entities created since the +integration began presetting one. Older entities took their id from the +descriptor name instead, which used the opposite word order -- that is what +`LEGACY_ENTITY_ID_SUFFIXES` records, and why it has to exist. So the rule for anything new is **verbatim**: a description key added from here on resolves to itself, exactly as the sub-device builders (`build_bess_unique_id`, @@ -107,10 +169,14 @@ """**Closed.** A compatibility shim for the keys that predate snake_case, not a house style. Every entry here translates a legacy camelCase description key into the suffix -its entities have carried since before 2.0.8 -- and that suffix is shared by the -`unique_id` *and* the `entity_id`, so a changed entry moves both on every -installed panel. A moved `unique_id` costs the statistics; a moved `entity_id` -breaks the templates and automations a user wrote. +its entities have carried since before 2.0.8, so a changed entry moves a live +`unique_id` on every installed panel, and a moved `unique_id` costs the +statistics. + +It governs the `entity_id` too, but only for entities created since the +integration began presetting one. Older entities took their id from the +descriptor name instead, which used the opposite word order -- that is what +`LEGACY_ENTITY_ID_SUFFIXES` records, and why it has to exist. So the rule for anything new is **verbatim**: a description key added from here on resolves to itself, exactly as the sub-device builders (`build_bess_unique_id`, diff --git a/custom_components/span_panel/manifest.json b/custom_components/span_panel/manifest.json index a409fab8..53a8d03e 100644 --- a/custom_components/span_panel/manifest.json +++ b/custom_components/span_panel/manifest.json @@ -26,7 +26,7 @@ "span-panel-api-schema-0==1.0.0", "span-panel-api-schema-1==1.0.0" ], - "version": "2.1.0b10", + "version": "2.1.0b11", "zeroconf": [ { "type": "_span._tcp.local." diff --git a/custom_components/span_panel/select.py b/custom_components/span_panel/select.py index b08176c3..88ae38a8 100644 --- a/custom_components/span_panel/select.py +++ b/custom_components/span_panel/select.py @@ -175,6 +175,7 @@ def __init__( "select", description.entity_description.key, circuit, + existing_entity_id=existing_entity_id, ) if constructed_id: self.entity_id = constructed_id diff --git a/custom_components/span_panel/sensor_base.py b/custom_components/span_panel/sensor_base.py index d40152ae..6d85ca4a 100644 --- a/custom_components/span_panel/sensor_base.py +++ b/custom_components/span_panel/sensor_base.py @@ -123,7 +123,7 @@ def __init__( ) # Wire explicit entity_id via subclass helper - entity_id = self._construct_entity_id(snapshot, description) + entity_id = self._construct_entity_id(snapshot, description, existing_entity_id) if entity_id: self.entity_id = entity_id else: @@ -228,19 +228,21 @@ def _construct_entity_id( self, snapshot: SpanPanelSnapshot, description: T, + existing_entity_id: str | None = None, ) -> str | None: """Construct explicit entity_id for the sensor. Subclasses may override to use entity_id helpers from helpers.py. Returns None to let HA auto-generate from _attr_name. - Whether the entity is already in the registry is deliberately not an - input: the value is what current panel data and the naming flags - produce, and HA keeps an existing entity's stored ID regardless. + The value is what current panel data and the naming flags produce; an + existing id is not consulted to decide *whether* to compute one, only so + that an id predating the suffix mapping keeps the suffix it shipped with. Args: snapshot: The panel snapshot data description: The sensor description + existing_entity_id: This entity's id in the registry, or None if new """ return None diff --git a/custom_components/span_panel/sensor_circuit.py b/custom_components/span_panel/sensor_circuit.py index c9802957..33da8579 100644 --- a/custom_components/span_panel/sensor_circuit.py +++ b/custom_components/span_panel/sensor_circuit.py @@ -204,6 +204,7 @@ def _construct_entity_id( self, snapshot: SpanPanelSnapshot, description: SpanPanelCircuitsSensorEntityDescription, + existing_entity_id: str | None = None, ) -> str | None: """Construct explicit entity_id for circuit power sensors.""" circuit = snapshot.circuits.get(self.circuit_id) @@ -218,6 +219,7 @@ def _construct_entity_id( "sensor", suffix, circuit, + existing_entity_id=existing_entity_id, ) def get_data_source(self, snapshot: SpanPanelSnapshot) -> SpanCircuitSnapshot: @@ -387,6 +389,7 @@ def _construct_entity_id( self, snapshot: SpanPanelSnapshot, description: SpanPanelCircuitsSensorEntityDescription, + existing_entity_id: str | None = None, ) -> str | None: """Construct explicit entity_id for circuit energy sensors.""" circuit = snapshot.circuits.get(self.circuit_id) @@ -405,6 +408,7 @@ def _construct_entity_id( "sensor", suffix, circuit, + existing_entity_id=existing_entity_id, ) # Map original_key to the energy type used for coordinator dip offset tracking diff --git a/custom_components/span_panel/switch.py b/custom_components/span_panel/switch.py index 35923d95..bf9f988b 100644 --- a/custom_components/span_panel/switch.py +++ b/custom_components/span_panel/switch.py @@ -129,6 +129,7 @@ def __init__( "switch", "breaker", circuit, + existing_entity_id=existing_entity_id, ) if constructed_id: self.entity_id = constructed_id diff --git a/tests/test_recreate_entity_ids.py b/tests/test_recreate_entity_ids.py index 4d664a39..d8298fc4 100644 --- a/tests/test_recreate_entity_ids.py +++ b/tests/test_recreate_entity_ids.py @@ -37,7 +37,14 @@ USE_CIRCUIT_NUMBERS, USE_DEVICE_PREFIX, ) -from custom_components.span_panel.sensor_circuit import SpanCircuitPowerSensor +from custom_components.span_panel.id_builder import ( + build_circuit_unique_id, + preserve_legacy_entity_id_suffix, +) +from custom_components.span_panel.sensor_circuit import ( + SpanCircuitEnergySensor, + SpanCircuitPowerSensor, +) from custom_components.span_panel.sensor_definitions import CIRCUIT_SENSORS from custom_components.span_panel.switch import SpanPanelCircuitsSwitch @@ -304,3 +311,138 @@ async def test_the_breaker_switch_gets_the_same_refreshed_suggestion( registry.async_regenerate_entity_id(registry_entry) == "switch.span_panel_beer_fridge_breaker" ) + + +# --- Entities that predate the suffix mapping reaching entity ids ------------- +# +# Every case above builds its entities with the current code, so the preset and +# the live id are the same string by construction and a suffix disagreement +# cannot appear. A real install upgrading is the case that matters: those +# entities took their id from the descriptor name ("Consumed Energy" -> +# `..._consumed_energy`) where the mapping says `energy_consumed`, so their +# entity id and their own unique id have always disagreed. Recomputing the +# suggestion surfaces that, and on a measured panel it offered 74 renames. + +LEGACY_ENTITY_ID = "sensor.span_panel_refrigerator_consumed_energy" +CANONICAL_ENTITY_ID = "sensor.span_panel_refrigerator_energy_consumed" +RENAMED_LEGACY_ENTITY_ID = "sensor.span_panel_beer_fridge_energy_consumed" + +ENERGY_DESCRIPTION = next( + desc for desc in CIRCUIT_SENSORS if desc.key == "circuit_energy_consumed" +) + + +class _LegacyInstall(_Install): + """An install whose energy sensor id was composed from the descriptor name.""" + + def _seed(self) -> str: + """Register the entity the way a pre-preset install left it.""" + registry = er.async_get(self._hass) + unique_id = build_circuit_unique_id(SERIAL, CIRCUIT_ID, "consumedEnergyWh") + entry = registry.async_get_or_create( + "sensor", + DOMAIN, + unique_id, + suggested_object_id="span_panel_refrigerator_consumed_energy", + original_name=f"{ORIGINAL_NAME} Consumed Energy", + config_entry=self._entry, + ) + return entry.entity_id + + async def load(self, circuit_name: str) -> SpanCircuitEnergySensor: # type: ignore[override] + """Set the platform up for the energy sensor, tearing down any previous one.""" + if self._platform is not None: + await self._platform.async_reset() + + snapshot = _snapshot(circuit_name) + coordinator = _coordinator(self._hass, snapshot, self._entry) + self._entry.runtime_data = SpanPanelRuntimeData( + coordinator=coordinator, panel_device_id="panel-device-id" + ) + + self._platform = MockEntityPlatform(self._hass, domain="sensor", platform_name=DOMAIN) + self._platform.config_entry = self._entry + + sensor = SpanCircuitEnergySensor( + coordinator, ENERGY_DESCRIPTION, snapshot, CIRCUIT_ID + ) + await self._platform.async_add_entities([sensor]) + await self._hass.async_block_till_done() + + assert sensor.hass is not None, "entity was rejected before it reached the registry" + return sensor + + +async def test_upgrading_does_not_offer_to_renormalise_a_legacy_suffix( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """The 74-rename case. Nothing was renamed on the panel, so nothing is offered.""" + install = _LegacyInstall(hass, entry) + seeded = install._seed() + assert seeded == LEGACY_ENTITY_ID + + sensor = await install.load(ORIGINAL_NAME) + + assert sensor.entity_id == LEGACY_ENTITY_ID + + registry = er.async_get(hass) + registry_entry = registry.async_get(LEGACY_ENTITY_ID) + assert registry_entry is not None + assert registry.async_regenerate_entity_id(registry_entry) == LEGACY_ENTITY_ID + + +async def test_a_legacy_entity_still_follows_a_circuit_rename( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Preserving the suffix must not cost the fix. + + A renamed circuit differs in the name half, so the proposal is the computed + id -- carrying the canonical suffix, because there is no older spelling of + `beer_fridge` to preserve. + """ + install = _LegacyInstall(hass, entry) + install._seed() + + await install.load(ORIGINAL_NAME) + sensor = await install.load(RENAMED) + + assert sensor.entity_id == LEGACY_ENTITY_ID + + registry = er.async_get(hass) + registry_entry = registry.async_get(LEGACY_ENTITY_ID) + assert registry_entry is not None + assert registry.async_regenerate_entity_id(registry_entry) == RENAMED_LEGACY_ENTITY_ID + + +async def test_a_new_install_gets_the_canonical_suffix( + hass: HomeAssistant, entry: MockConfigEntry +) -> None: + """Preservation is for ids that already exist; nothing new inherits the old form.""" + sensor = await _LegacyInstall(hass, entry).load(ORIGINAL_NAME) + + assert sensor.entity_id == CANONICAL_ENTITY_ID + + +def test_a_suffix_with_no_older_spelling_is_left_alone() -> None: + """A name that merely looks like a suffix change is a rename, not a legacy form. + + Renaming a circuit "Kitchen Outlets" to "Kitchen" leaves an existing id whose + trailing segments differ from the computed suffix. That is exactly the case + #252 exists to offer, so it must not be mistaken for an older spelling. + """ + assert ( + preserve_legacy_entity_id_suffix( + "sensor.span_panel_kitchen_power", + "sensor.span_panel_kitchen_outlets_power", + "power", + ) + == "sensor.span_panel_kitchen_power" + ) + assert ( + preserve_legacy_entity_id_suffix( + "sensor.span_panel_kitchen_energy_consumed", + "sensor.span_panel_kitchen_consumed_energy", + "energy_consumed", + ) + == "sensor.span_panel_kitchen_consumed_energy" + )