Cleanup non-existing climate and humidifier devices for Comelit (#144624)

* Cleanup non-existing climate and humidifier devices for Comelit

* skip removing main hub device

* add tests

* complete tests

* improve logging

* fix post rebase

* apply  review comments

* typos

* fix identifiers

* fix ruff post merge

* clean post merge
This commit is contained in:
Simone Chemelli 2025-05-26 15:59:01 +03:00 committed by GitHub
parent ba0f6c3ba2
commit 1c1f5a779b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 244 additions and 41 deletions

View File

@ -9,6 +9,7 @@ from aiocomelit import ComelitSerialBridgeObject
from aiocomelit.const import CLIMATE
from homeassistant.components.climate import (
DOMAIN as CLIMATE_DOMAIN,
ClimateEntity,
ClimateEntityFeature,
HVACAction,
@ -17,18 +18,12 @@ from homeassistant.components.climate import (
)
from homeassistant.const import ATTR_TEMPERATURE, PRECISION_TENTHS
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
from .const import (
DOMAIN,
PRESET_MODE_AUTO,
PRESET_MODE_AUTO_TARGET_TEMP,
PRESET_MODE_MANUAL,
)
from .const import PRESET_MODE_AUTO, PRESET_MODE_AUTO_TARGET_TEMP, PRESET_MODE_MANUAL
from .coordinator import ComelitConfigEntry, ComelitSerialBridge
from .entity import ComelitBridgeBaseEntity
from .utils import bridge_api_call
from .utils import bridge_api_call, cleanup_stale_entity, load_api_data
# Coordinator is used to centralize the data updates
PARALLEL_UPDATES = 0
@ -95,10 +90,23 @@ async def async_setup_entry(
coordinator = cast(ComelitSerialBridge, config_entry.runtime_data)
async_add_entities(
ComelitClimateEntity(coordinator, device, config_entry.entry_id)
for device in coordinator.data[CLIMATE].values()
)
entities: list[ClimateEntity] = []
for device in coordinator.data[CLIMATE].values():
values = load_api_data(device, CLIMATE_DOMAIN)
if values[0] == 0 and values[4] == 0:
# No climate data, device is only a humidifier/dehumidifier
await cleanup_stale_entity(
hass, config_entry, f"{config_entry.entry_id}-{device.index}", device
)
continue
entities.append(
ComelitClimateEntity(coordinator, device, config_entry.entry_id)
)
async_add_entities(entities)
class ComelitClimateEntity(ComelitBridgeBaseEntity, ClimateEntity):
@ -132,15 +140,7 @@ class ComelitClimateEntity(ComelitBridgeBaseEntity, ClimateEntity):
def _update_attributes(self) -> None:
"""Update class attributes."""
device = self.coordinator.data[CLIMATE][self._device.index]
if not isinstance(device.val, list):
raise HomeAssistantError(
translation_domain=DOMAIN, translation_key="invalid_clima_data"
)
# CLIMATE has a 2 item tuple:
# - first for Clima
# - second for Humidifier
values = device.val[0]
values = load_api_data(device, CLIMATE_DOMAIN)
_active = values[1]
_mode = values[2] # Values from API: "O", "L", "U"

View File

@ -9,6 +9,7 @@ from aiocomelit import ComelitSerialBridgeObject
from aiocomelit.const import CLIMATE
from homeassistant.components.humidifier import (
DOMAIN as HUMIDIFIER_DOMAIN,
MODE_AUTO,
MODE_NORMAL,
HumidifierAction,
@ -17,13 +18,13 @@ from homeassistant.components.humidifier import (
HumidifierEntityFeature,
)
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError, ServiceValidationError
from homeassistant.exceptions import ServiceValidationError
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
from .const import DOMAIN
from .coordinator import ComelitConfigEntry, ComelitSerialBridge
from .entity import ComelitBridgeBaseEntity
from .utils import bridge_api_call
from .utils import bridge_api_call, cleanup_stale_entity, load_api_data
# Coordinator is used to centralize the data updates
PARALLEL_UPDATES = 0
@ -67,6 +68,23 @@ async def async_setup_entry(
entities: list[ComelitHumidifierEntity] = []
for device in coordinator.data[CLIMATE].values():
values = load_api_data(device, HUMIDIFIER_DOMAIN)
if values[0] == 0 and values[4] == 0:
# No humidity data, device is only a climate
for device_class in (
HumidifierDeviceClass.HUMIDIFIER,
HumidifierDeviceClass.DEHUMIDIFIER,
):
await cleanup_stale_entity(
hass,
config_entry,
f"{config_entry.entry_id}-{device.index}-{device_class}",
device,
)
continue
entities.append(
ComelitHumidifierEntity(
coordinator,
@ -124,15 +142,7 @@ class ComelitHumidifierEntity(ComelitBridgeBaseEntity, HumidifierEntity):
def _update_attributes(self) -> None:
"""Update class attributes."""
device = self.coordinator.data[CLIMATE][self._device.index]
if not isinstance(device.val, list):
raise HomeAssistantError(
translation_domain=DOMAIN, translation_key="invalid_clima_data"
)
# CLIMATE has a 2 item tuple:
# - first for Clima
# - second for Humidifier
values = device.val[1]
values = load_api_data(device, HUMIDIFIER_DOMAIN)
_active = values[1]
_mode = values[2] # Values from API: "O", "L", "U"

View File

@ -4,14 +4,21 @@ from collections.abc import Awaitable, Callable, Coroutine
from functools import wraps
from typing import Any, Concatenate
from aiocomelit import ComelitSerialBridgeObject
from aiocomelit.exceptions import CannotAuthenticate, CannotConnect, CannotRetrieveData
from aiohttp import ClientSession, CookieJar
from homeassistant.components.climate import DOMAIN as CLIMATE_DOMAIN
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import aiohttp_client
from homeassistant.helpers import (
aiohttp_client,
device_registry as dr,
entity_registry as er,
)
from .const import DOMAIN
from .const import _LOGGER, DOMAIN
from .entity import ComelitBridgeBaseEntity
@ -22,6 +29,61 @@ async def async_client_session(hass: HomeAssistant) -> ClientSession:
)
def load_api_data(device: ComelitSerialBridgeObject, domain: str) -> list[Any]:
"""Load data from the API."""
# This function is called when the data is loaded from the API
if not isinstance(device.val, list):
raise HomeAssistantError(
translation_domain=domain, translation_key="invalid_clima_data"
)
# CLIMATE has a 2 item tuple:
# - first for Clima
# - second for Humidifier
return device.val[0] if domain == CLIMATE_DOMAIN else device.val[1]
async def cleanup_stale_entity(
hass: HomeAssistant,
config_entry: ConfigEntry,
entry_unique_id: str,
device: ComelitSerialBridgeObject,
) -> None:
"""Cleanup stale entity."""
entity_reg: er.EntityRegistry = er.async_get(hass)
identifiers: list[str] = []
for entry in er.async_entries_for_config_entry(entity_reg, config_entry.entry_id):
if entry.unique_id == entry_unique_id:
entry_name = entry.name or entry.original_name
_LOGGER.info("Removing entity: %s [%s]", entry.entity_id, entry_name)
entity_reg.async_remove(entry.entity_id)
identifiers.append(f"{config_entry.entry_id}-{device.type}-{device.index}")
if len(identifiers) > 0:
_async_remove_state_config_entry_from_devices(hass, identifiers, config_entry)
def _async_remove_state_config_entry_from_devices(
hass: HomeAssistant, identifiers: list[str], config_entry: ConfigEntry
) -> None:
"""Remove config entry from device."""
device_registry = dr.async_get(hass)
for identifier in identifiers:
device = device_registry.async_get_device(identifiers={(DOMAIN, identifier)})
if device:
_LOGGER.info(
"Removing config entry %s from device %s",
config_entry.title,
device.name,
)
device_registry.async_update_device(
device_id=device.id,
remove_config_entry_id=config_entry.entry_id,
)
def bridge_api_call[_T: ComelitBridgeBaseEntity, **_P](
func: Callable[Concatenate[_T, _P], Awaitable[None]],
) -> Callable[Concatenate[_T, _P], Coroutine[Any, Any, None]]:

View File

@ -352,3 +352,41 @@ async def test_climate_preset_mode_when_off(
assert (state := hass.states.get(ENTITY_ID))
assert state.state == HVACMode.OFF
async def test_climate_remove_stale(
hass: HomeAssistant,
mock_serial_bridge: AsyncMock,
mock_serial_bridge_config_entry: MockConfigEntry,
) -> None:
"""Test removal of stale climate entities."""
await setup_integration(hass, mock_serial_bridge_config_entry)
assert (state := hass.states.get(ENTITY_ID))
assert state.state == HVACMode.HEAT
assert state.attributes[ATTR_TEMPERATURE] == 5.0
mock_serial_bridge.get_all_devices.return_value[CLIMATE] = {
0: ComelitSerialBridgeObject(
index=0,
name="Climate0",
status=0,
human_status="off",
type="climate",
val=[
[0, 0, "O", "A", 0, 0, 0, "N"],
[650, 0, "U", "M", 500, 0, 0, "U"],
[0, 0],
],
protected=0,
zone="Living room",
power=0.0,
power_unit=WATT,
),
}
await hass.config_entries.async_reload(mock_serial_bridge_config_entry.entry_id)
await hass.async_block_till_done()
assert (state := hass.states.get(ENTITY_ID)) is None

View File

@ -290,3 +290,41 @@ async def test_humidifier_set_status(
assert (state := hass.states.get(ENTITY_ID))
assert state.state == STATE_ON
async def test_humidifier_dehumidifier_remove_stale(
hass: HomeAssistant,
mock_serial_bridge: AsyncMock,
mock_serial_bridge_config_entry: MockConfigEntry,
) -> None:
"""Test removal of stale humidifier/dehumidifier entities."""
await setup_integration(hass, mock_serial_bridge_config_entry)
assert (state := hass.states.get(ENTITY_ID))
assert state.state == STATE_ON
assert state.attributes[ATTR_HUMIDITY] == 50.0
mock_serial_bridge.get_all_devices.return_value[CLIMATE] = {
0: ComelitSerialBridgeObject(
index=0,
name="Climate0",
status=0,
human_status="off",
type="climate",
val=[
[221, 0, "U", "M", 50, 0, 0, "U"],
[0, 0, "O", "A", 0, 0, 0, "N"],
[0, 0],
],
protected=0,
zone="Living room",
power=0.0,
power_unit=WATT,
),
}
await hass.config_entries.async_reload(mock_serial_bridge_config_entry.entry_id)
await hass.async_block_till_done()
assert (state := hass.states.get(ENTITY_ID)) is None

View File

@ -1,14 +1,18 @@
"""Tests for Comelit SimpleHome switch platform."""
"""Tests for Comelit SimpleHome utils."""
from unittest.mock import AsyncMock
from aiocomelit.api import ComelitSerialBridgeObject
from aiocomelit.const import CLIMATE, WATT
from aiocomelit.exceptions import CannotAuthenticate, CannotConnect, CannotRetrieveData
import pytest
from homeassistant.components.climate import HVACMode
from homeassistant.components.comelit.const import DOMAIN
from homeassistant.components.humidifier import ATTR_HUMIDITY
from homeassistant.components.switch import DOMAIN as SWITCH_DOMAIN, SERVICE_TURN_ON
from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState
from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF
from homeassistant.const import ATTR_ENTITY_ID, ATTR_TEMPERATURE, STATE_OFF, STATE_ON
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
@ -16,7 +20,58 @@ from . import setup_integration
from tests.common import MockConfigEntry
ENTITY_ID = "switch.switch0"
ENTITY_ID_0 = "switch.switch0"
ENTITY_ID_1 = "climate.climate0"
ENTITY_ID_2 = "humidifier.climate0_dehumidifier"
ENTITY_ID_3 = "humidifier.climate0_humidifier"
async def test_device_remove_stale(
hass: HomeAssistant,
mock_serial_bridge: AsyncMock,
mock_serial_bridge_config_entry: MockConfigEntry,
) -> None:
"""Test removal of stale devices with no entities."""
await setup_integration(hass, mock_serial_bridge_config_entry)
assert (state := hass.states.get(ENTITY_ID_1))
assert state.state == HVACMode.HEAT
assert state.attributes[ATTR_TEMPERATURE] == 5.0
assert (state := hass.states.get(ENTITY_ID_2))
assert state.state == STATE_OFF
assert state.attributes[ATTR_HUMIDITY] == 50.0
assert (state := hass.states.get(ENTITY_ID_3))
assert state.state == STATE_ON
assert state.attributes[ATTR_HUMIDITY] == 50.0
mock_serial_bridge.get_all_devices.return_value[CLIMATE] = {
0: ComelitSerialBridgeObject(
index=0,
name="Climate0",
status=0,
human_status="off",
type="climate",
val=[
[0, 0, "O", "A", 0, 0, 0, "N"],
[0, 0, "O", "A", 0, 0, 0, "N"],
[0, 0],
],
protected=0,
zone="Living room",
power=0.0,
power_unit=WATT,
),
}
await hass.config_entries.async_reload(mock_serial_bridge_config_entry.entry_id)
await hass.async_block_till_done()
assert (state := hass.states.get(ENTITY_ID_1)) is None
assert (state := hass.states.get(ENTITY_ID_2)) is None
assert (state := hass.states.get(ENTITY_ID_3)) is None
@pytest.mark.parametrize(
@ -38,7 +93,7 @@ async def test_bridge_api_call_exceptions(
await setup_integration(hass, mock_serial_bridge_config_entry)
assert (state := hass.states.get(ENTITY_ID))
assert (state := hass.states.get(ENTITY_ID_0))
assert state.state == STATE_OFF
mock_serial_bridge.set_device_status.side_effect = side_effect
@ -48,7 +103,7 @@ async def test_bridge_api_call_exceptions(
await hass.services.async_call(
SWITCH_DOMAIN,
SERVICE_TURN_ON,
{ATTR_ENTITY_ID: ENTITY_ID},
{ATTR_ENTITY_ID: ENTITY_ID_0},
blocking=True,
)
@ -66,7 +121,7 @@ async def test_bridge_api_call_reauth(
await setup_integration(hass, mock_serial_bridge_config_entry)
assert (state := hass.states.get(ENTITY_ID))
assert (state := hass.states.get(ENTITY_ID_0))
assert state.state == STATE_OFF
mock_serial_bridge.set_device_status.side_effect = CannotAuthenticate
@ -75,7 +130,7 @@ async def test_bridge_api_call_reauth(
await hass.services.async_call(
SWITCH_DOMAIN,
SERVICE_TURN_ON,
{ATTR_ENTITY_ID: ENTITY_ID},
{ATTR_ENTITY_ID: ENTITY_ID_0},
blocking=True,
)