Skip to content

Commit

Permalink
Merge pull request #405 from canton7/bugfix/entity_id_prefix
Browse files Browse the repository at this point in the history
Fix issues around the entity id prefix introduced in 1.7.1
  • Loading branch information
canton7 authored Sep 2, 2023
2 parents 8aa04a1 + 225229e commit 98474e8
Show file tree
Hide file tree
Showing 22 changed files with 122 additions and 54 deletions.
2 changes: 1 addition & 1 deletion custom_components/foxess_modbus/binary_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry, async_add_d
inverters = hass.data[DOMAIN][entry.entry_id][INVERTERS]

for inverter, controller in inverters:
async_add_devices(create_entities(BinarySensorEntity, controller, entry, inverter))
async_add_devices(create_entities(BinarySensorEntity, hass, controller, entry, inverter))
2 changes: 2 additions & 0 deletions custom_components/foxess_modbus/entities/entity_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing import Sequence

from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity

from ..common.entity_controller import EntityController
Expand All @@ -23,6 +24,7 @@ def entity_type(self) -> type[Entity]:
@abstractmethod
def create_entity_if_supported(
self,
hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Any

from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity

from ..common.entity_controller import EntityController
Expand All @@ -23,6 +24,7 @@ class ModbusBatterySensorDescription(ModbusSensorDescription):

def create_entity_if_supported(
self,
_hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from homeassistant.components.binary_sensor import BinarySensorEntity
from homeassistant.components.binary_sensor import BinarySensorEntityDescription
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity

from ..common.entity_controller import EntityController
Expand All @@ -35,6 +37,7 @@ def entity_type(self) -> type[Entity]:

def create_entity_if_supported(
self,
_hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand Down Expand Up @@ -63,7 +66,7 @@ def __init__(
self._address = address
self._entry = entry
self._inv_details = inv_details
self.entity_id = "binary_sensor." + self._get_unique_id()
self.entity_id = self._get_entity_id(Platform.BINARY_SENSOR)

@property
def is_on(self) -> bool | None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
from typing import Any

from homeassistant.components.binary_sensor import BinarySensorDeviceClass
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant

from ..common.register_type import RegisterType
from .inverter_model_spec import InverterModelSpec
from .inverter_model_spec import ModbusAddressSpecBase
from .modbus_binary_sensor import ModbusBinarySensorDescription
from .modbus_charge_period_sensors import ModbusChargePeriodStartEndSensorDescription
from .modbus_charge_period_sensors import ModbusEnableForceChargeSensorDescription
from .modbus_entity_mixin import add_entity_id_prefix
from .modbus_entity_mixin import get_entity_id
from .validation import Time

_LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -52,7 +54,7 @@ class ChargePeriodAddressSpec:
def __init__(
self,
models: list[str],
input: ModbusChargePeriodAddressConfig | None = None, # noqa
input: ModbusChargePeriodAddressConfig | None = None, # noqa: A002
holding: ModbusChargePeriodAddressConfig | None = None,
) -> None:
self.models = models
Expand Down Expand Up @@ -162,7 +164,7 @@ def __init__(
]

def create_charge_period_config_if_supported(
self, inverter_model: str, register_type: RegisterType, inv_details: dict[str, Any]
self, hass: HomeAssistant, inverter_model: str, register_type: RegisterType, inv_details: dict[str, Any]
) -> ModbusChargePeriodInfo | None:
"""
If the inverter model / connection type supports a charge period, fetches a ModbusChargePeriodAddressConfig
Expand All @@ -178,14 +180,13 @@ def create_charge_period_config_if_supported(
result is None
), f"{self}: multiple charge periods defined for ({inverter_model}, {register_type})"

# TODO: It isn't great that this logic is duplicated between this and the entities
start_id = f"sensor.{add_entity_id_prefix(self._period_start_key, inv_details)}"
end_id = f"sensor.{add_entity_id_prefix(self._period_end_key, inv_details)}"
enable_force_charge_id = (
f"binary_sensor.{add_entity_id_prefix(self._enable_force_charge_key, inv_details)}"
start_id = get_entity_id(hass, Platform.SENSOR, self._period_start_key, inv_details)
end_id = get_entity_id(hass, Platform.SENSOR, self._period_end_key, inv_details)
enable_force_charge_id = get_entity_id(
hass, Platform.BINARY_SENSOR, self._enable_force_charge_key, inv_details
)
enable_charge_from_grid_id = (
f"binary_sensor.{add_entity_id_prefix(self._enable_charge_from_grid_key, inv_details)}"
enable_charge_from_grid_id = get_entity_id(
hass, Platform.BINARY_SENSOR, self._enable_charge_from_grid_key, inv_details
)
result = ModbusChargePeriodInfo(
addresses=address_config,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from homeassistant.components.sensor import SensorEntity
from homeassistant.components.sensor import SensorEntityDescription
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.restore_state import ExtraStoredData
from homeassistant.helpers.restore_state import RestoredExtraData
Expand Down Expand Up @@ -66,6 +68,7 @@ def entity_type(self) -> type[Entity]:

def create_entity_if_supported(
self,
_hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand Down Expand Up @@ -105,7 +108,7 @@ def __init__(
self._other_address = other_address
self._entry = entry
self._inv_details = inv_details
self.entity_id = "sensor." + self._get_unique_id()
self.entity_id = self._get_entity_id(Platform.SENSOR)
# The last value this sensor had when force-charge was enabled
self._last_enabled_value: int | None = None

Expand Down Expand Up @@ -190,6 +193,7 @@ def entity_type(self) -> type[Entity]:

def create_entity_if_supported(
self,
_hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand Down Expand Up @@ -239,7 +243,7 @@ def __init__(
self._period_end_address = period_end_address
self._entry = entry
self._inv_details = inv_details
self.entity_id = "binary_sensor." + self._get_unique_id()
self.entity_id = self._get_entity_id(Platform.BINARY_SENSOR)
self._attr_device_class = BinarySensorDeviceClass.POWER

@property
Expand Down
55 changes: 37 additions & 18 deletions custom_components/foxess_modbus/entities/modbus_entity_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
from typing import Protocol
from typing import cast

from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry
from homeassistant.helpers.entity import DeviceInfo
from homeassistant.helpers.entity import Entity

Expand All @@ -21,14 +24,41 @@
_LOGGER = logging.getLogger(__name__)


def add_entity_id_prefix(entity_id: str, inv_details: dict[str, Any]) -> str:
def get_entity_id(hass: HomeAssistant, platform: Platform, key: str, inv_details: dict[str, Any]) -> str:
"""Gets the entity ID for the entity with the given platform and key"""

unique_id = _create_unique_id(key, inv_details)

er = entity_registry.async_get(hass)

# Type annotation missing in the annotations package maybe?
entity_id = cast(str | None, er.async_get_entity_id(platform, DOMAIN, unique_id))

if entity_id is None:
# This can happen when first setting up, as the target entity hasn't been created yet.
# In this case, assume that it's going to be correctly named
entity_id = _add_entity_id_prefix(key, inv_details)

return entity_id


def _add_entity_id_prefix(key: str, inv_details: dict[str, Any]) -> str:
"""Add the entity ID prefix to the beginning of the given input string"""
entity_id_prefix = inv_details[ENTITY_ID_PREFIX]

if entity_id_prefix:
entity_id = f"{entity_id_prefix}_{entity_id}"
key = f"{entity_id_prefix}_{key}"

return entity_id
return key


def _create_unique_id(key: str, inv_details: dict[str, Any]) -> str:
unique_id_prefix = inv_details[UNIQUE_ID_PREFIX]
if unique_id_prefix:
key = f"{unique_id_prefix}_{key}"

# We don't need to prefix unique ids with foxess_modbus, but we do for some reason.
return "foxess_modbus_" + key


class ModbusEntityProtocol(Protocol):
Expand All @@ -54,7 +84,7 @@ class ModbusEntityMixin(ModbusControllerEntity, ModbusEntityProtocol, _ModbusEnt
@property
def unique_id(self) -> str:
"""Return a unique ID."""
return "foxess_modbus_" + self._get_unique_id()
return _create_unique_id(self.entity_description.key, self._inv_details)

@property
def device_info(self) -> DeviceInfo:
Expand Down Expand Up @@ -109,20 +139,9 @@ def _address_updated(self) -> None:
"""Called when the controller reads an updated to any of the addresses in self.addresses"""
self.schedule_update_ha_state()

def _get_unique_id(self) -> str:
"""Get unique ID"""

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"""
return add_entity_id_prefix(entity_id, self._inv_details)
def _get_entity_id(self, platform: Platform) -> str:
"""Gets the entity ID"""
return f"{platform}.{_add_entity_id_prefix(self.entity_description.key, self._inv_details)}"

def _validate(
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
from homeassistant.components.sensor import SensorEntity
from homeassistant.components.sensor import SensorEntityDescription
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity

from ..common.entity_controller import EntityController
Expand Down Expand Up @@ -167,6 +169,7 @@ def entity_type(self) -> type[Entity]:

def create_entity_if_supported(
self,
_hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand All @@ -193,7 +196,7 @@ def __init__(
self.entity_description = entity_description
self._addresses = addresses
self._inv_details = inv_details
self.entity_id = "sensor." + self._get_unique_id()
self.entity_id = self._get_entity_id(Platform.SENSOR)

@property
def native_value(self) -> str | None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@
from homeassistant.components.sensor import SensorEntity
from homeassistant.components.sensor import SensorEntityDescription
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import Platform
from homeassistant.const import UnitOfTime
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity

from ..common.entity_controller import EntityController
from ..common.register_type import RegisterType
from .entity_factory import EntityFactory
from .inverter_model_spec import EntitySpec
from .modbus_entity_mixin import ModbusEntityMixin
from .modbus_entity_mixin import get_entity_id

_LOGGER = logging.getLogger(__name__)

Expand All @@ -36,6 +39,7 @@ def entity_type(self) -> type[Entity]:

def create_entity_if_supported(
self,
hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand All @@ -45,6 +49,8 @@ def create_entity_if_supported(
if not self._supports_inverter_model(self.models, inverter_model, register_type):
return None

source_entity = get_entity_id(hass, Platform.SENSOR, self.source_entity, inv_details)

# this piggybacks on the existing factory to create IntegrationSensors
return ModbusIntegrationSensor(
controller=controller,
Expand All @@ -53,7 +59,7 @@ def create_entity_if_supported(
inv_details=inv_details,
integration_method=self.integration_method,
round_digits=self.round_digits,
source_entity=self.source_entity,
source_entity=source_entity,
unit_time=self.unit_time,
)

Expand Down Expand Up @@ -81,9 +87,7 @@ def __init__(
self._entry = entry
self._inv_details = inv_details
self.entity_description = entity_description
self.entity_id = "sensor." + self._get_unique_id()

source_entity = f"sensor.{self._add_entity_id_prefix(source_entity)}"
self.entity_id = self._get_entity_id(Platform.SENSOR)

IntegrationSensor.__init__(
self=self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from homeassistant.components.sensor import SensorEntity
from homeassistant.components.sensor import SensorEntityDescription
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity import Entity

from ..common.entity_controller import EntityController
Expand Down Expand Up @@ -48,6 +50,7 @@ def entity_type(self) -> type[Entity]:

def create_entity_if_supported(
self,
_hass: HomeAssistant,
controller: EntityController,
inverter_model: str,
register_type: RegisterType,
Expand Down Expand Up @@ -75,7 +78,7 @@ def __init__(
self.entity_description = entity_description
self._address = address
self._inv_details = inv_details
self.entity_id = "sensor." + self._get_unique_id()
self.entity_id = self._get_entity_id(Platform.SENSOR)

@property
def native_value(self) -> str | None:
Expand Down
Loading

0 comments on commit 98474e8

Please sign in to comment.