Merge pull request #383 from canton7/bugfix/invalid-entity-id-prefix

Migrate old invalid entity ID prefixes

custom_components/foxess_modbus/__init__.py

@@ -15,6 +15,7 @@
 from homeassistant.core import Config
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.typing import UNDEFINED
+from slugify import slugify
 
 from .const import ADAPTER_ID
 from .const import ADAPTER_WAS_MIGRATED
@@ -36,6 +37,7 @@
 from .const import STARTUP_MESSAGE
 from .const import TCP
 from .const import UDP
+from .const import UNIQUE_ID_PREFIX
 from .inverter_adapters import ADAPTERS
 from .inverter_profiles import inverter_connection_type_profile_from_config
 from .modbus_client import ModbusClient
@@ -227,6 +229,15 @@ async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
                 inverter[MODBUS_TYPE] = inverter[MODBUS_TYPE].lower()
         config_entry.version = 6
 
+    if config_entry.version == 6:
+        # We still have users with entity ID prefixes which aren't valid in entity IDs. HA will automatically convert
+        # them, but this causes the charge period card to complain. Fix them once and for all.
+        for inverter in config_entry.data.get(INVERTERS, {}).values():
+            inverter[UNIQUE_ID_PREFIX] = inverter[ENTITY_ID_PREFIX]
+            if inverter[ENTITY_ID_PREFIX]:
+                inverter[ENTITY_ID_PREFIX] = slugify(inverter[ENTITY_ID_PREFIX], separator="_").rstrip("_")
+        config_entry.version = 7
+
     _LOGGER.info("Migration to version %s successful", config_entry.version)
     return True
 

custom_components/foxess_modbus/const.py

@@ -31,7 +31,19 @@
 ATTR_ENTRY_TYPE = "entry_type"
 
 # Modbus Options
+# Once upon a time, we just had the friendly name, and we allowed any string. We added this to the start of all entity
+# IDs and unique IDs. HA converted the resulting entity IDs to be valid (replacing spaces with _, etc), but this caused
+# problems with e.g. the energy dashboard.
+# Then we moved to a separate friendly name (shown in the entity name) and entity ID prefix (added to the start of
+# entity and unique IDs). This was always a valid entity ID for new configs, but could still be invalid for old migrated
+# configs. This fixed the problem with the energy dashboard. However, the charge period card still got tripped up by
+# entity ID prefixes which were invalid entity IDs.
+# We then migrated all entity ID prefix to be valid entity IDs. However, this broke the fact that we were also using the
+# entity ID prefix as the unique ID prefix. Therefore we added the UNIQUE_ID_PREFIX. For old configs with an old invalid
+# entity ID prefix, the unique ID prefix retains the old invalid value while the entity ID prefix is fixed. For new
+# configs, it should be the same as the entity ID prefix.
 ENTITY_ID_PREFIX = "entity_id_prefix"
+UNIQUE_ID_PREFIX = "unique_id_prefix"
 FRIENDLY_NAME = "friendly_name"
 MODBUS_SLAVE = "modbus_slave"
 MODBUS_DEVICE = "modbus_device"

custom_components/foxess_modbus/entities/modbus_entity_mixin.py

@@ -15,6 +15,7 @@
 from ..const import FRIENDLY_NAME
 from ..const import INVERTER_CONN
 from ..const import INVERTER_MODEL
+from ..const import UNIQUE_ID_PREFIX
 from .base_validator import BaseValidator
 
 _LOGGER = logging.getLogger(__name__)
@@ -110,7 +111,14 @@ def _address_updated(self) -> None:
 
     def _get_unique_id(self) -> str:
         """Get unique ID"""
-        return self._add_entity_id_prefix(self.entity_description.key)
+
+        unique_id = self.entity_description.key
+
+        unique_id_prefix = self._inv_details[UNIQUE_ID_PREFIX]
+        if unique_id_prefix:
+            unique_id = f"{unique_id_prefix}_{unique_id}"
+
+        return unique_id
 
     def _add_entity_id_prefix(self, entity_id: str) -> str:
         """Add the entity ID prefix to the beginning of the given input string"""

custom_components/foxess_modbus/flow/flow_handler.py

@@ -37,7 +37,7 @@
 class FlowHandler(FlowHandlerMixin, config_entries.ConfigFlow, domain=DOMAIN):
     """Config flow for foxess_modbus."""
 
-    VERSION = 6
+    VERSION = 7
     CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL
 
     def __init__(self) -> None:
@@ -84,7 +84,7 @@ async def async_step_friendly_name(self, user_input: dict[str, Any] | None = Non
         # This is a bit involved, so we'll avoid _with_default_form
 
         def generate_entity_id_prefix(friendly_name: str | None) -> str:
-            return slugify(friendly_name, separator="_", regex_pattern=r"\W").strip("_") if friendly_name else ""
+            return slugify(friendly_name, separator="_").strip("_") if friendly_name else ""
 
         def is_unique_entity_id_prefix(entity_id_prefix: str) -> bool:
             return not any(x for x in self._all_inverters if x.entity_id_prefix == entity_id_prefix)

custom_components/foxess_modbus/flow/flow_handler_mixin.py

@@ -16,6 +16,7 @@
 from ..const import INVERTER_MODEL
 from ..const import MODBUS_SLAVE
 from ..const import MODBUS_TYPE
+from ..const import UNIQUE_ID_PREFIX
 from ..inverter_adapters import ADAPTERS
 from .inverter_data import InverterData
 
@@ -86,6 +87,7 @@ def _inverter_data_to_dict(self, inverter: InverterData) -> dict[str, Any]:
             MODBUS_TYPE: inverter.inverter_protocol,
             HOST: inverter.host,
             ENTITY_ID_PREFIX: inverter.entity_id_prefix if inverter.entity_id_prefix else "",
+            UNIQUE_ID_PREFIX: inverter.unique_id_prefix if inverter.unique_id_prefix else "",
             FRIENDLY_NAME: inverter.friendly_name if inverter.friendly_name else "",
         }
         return inverter_config
@@ -101,6 +103,7 @@ def _dict_to_inverter_data(self, config: dict[str, Any]) -> InverterData:
             inverter_protocol=config[MODBUS_TYPE],
             host=config[HOST],
             entity_id_prefix=config[ENTITY_ID_PREFIX] if config[ENTITY_ID_PREFIX] else None,
+            unique_id_prefix=config[UNIQUE_ID_PREFIX] if config[UNIQUE_ID_PREFIX] else None,
             friendly_name=config[FRIENDLY_NAME] if config[ENTITY_ID_PREFIX] else None,
         )
         return inverter_data

custom_components/foxess_modbus/flow/inverter_data.py

@@ -16,4 +16,5 @@ class InverterData:
     inverter_protocol: str | None = None  # TCP, UDP, SERIAL, RTU_OVER_TCP
     host: str | None = None  # host:port or /dev/serial
     entity_id_prefix: str | None = None
+    unique_id_prefix: str | None = None
     friendly_name: str | None = None

tests/test_entity_descriptions.py

@@ -9,6 +9,7 @@
 from custom_components.foxess_modbus.const import ENTITY_ID_PREFIX
 from custom_components.foxess_modbus.const import INVERTER_BASE
 from custom_components.foxess_modbus.const import INVERTER_CONN
+from custom_components.foxess_modbus.const import UNIQUE_ID_PREFIX
 from custom_components.foxess_modbus.inverter_profiles import INVERTER_PROFILES
 from custom_components.foxess_modbus.inverter_profiles import create_entities
 
@@ -20,6 +21,11 @@ def test_creates_all_entities() -> None:
     for profile in INVERTER_PROFILES.values():
         for connection_type in profile.connection_types:
             for entity_type in [SensorEntity, BinarySensorEntity, SelectEntity, NumberEntity]:
-                inverter_config = {INVERTER_BASE: profile.model, INVERTER_CONN: connection_type, ENTITY_ID_PREFIX: ""}
+                inverter_config = {
+                    INVERTER_BASE: profile.model,
+                    INVERTER_CONN: connection_type,
+                    ENTITY_ID_PREFIX: "",
+                    UNIQUE_ID_PREFIX: "",
+                }
                 # Asserts if e.g. the ModbusAddressSpecs match
                 create_entities(entity_type, controller, config_entry, inverter_config)