Add state multiplexer in fibaro integration (#139649)

* Add state multiplexer in fibaro integration

* Add unload test

* Adjust code comments

* Add event entity test

* .

homeassistant/components/fibaro/__init__.py

@@ -14,9 +14,10 @@ from pyfibaro.fibaro_client import (
 )
 from pyfibaro.fibaro_data_helper import read_rooms
 from pyfibaro.fibaro_device import DeviceModel
+from pyfibaro.fibaro_device_manager import FibaroDeviceManager
 from pyfibaro.fibaro_info import InfoModel
 from pyfibaro.fibaro_scene import SceneModel
-from pyfibaro.fibaro_state_resolver import FibaroEvent, FibaroStateResolver
+from pyfibaro.fibaro_state_resolver import FibaroEvent
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME, Platform
@@ -81,8 +82,8 @@ class FibaroController:
         self._client = fibaro_client
         self._fibaro_info = info
 
-        # Whether to import devices from plugins
+        # The fibaro device manager exposes higher level API to access fibaro devices
-        self._import_plugins = import_plugins
+        self._fibaro_device_manager = FibaroDeviceManager(fibaro_client, import_plugins)
         # Mapping roomId to room object
         self._room_map = read_rooms(fibaro_client)
         self._device_map: dict[int, DeviceModel]  # Mapping deviceId to device object
@@ -91,79 +92,30 @@ class FibaroController:
         )  # List of devices by entity platform
         # All scenes
         self._scenes = self._client.read_scenes()
-        self._callbacks: dict[int, list[Any]] = {}  # Update value callbacks by deviceId
-        # Event callbacks by device id
-        self._event_callbacks: dict[int, list[Callable[[FibaroEvent], None]]] = {}
         # Unique serial number of the hub
         self.hub_serial = info.serial_number
         # Device infos by fibaro device id
         self._device_infos: dict[int, DeviceInfo] = {}
         self._read_devices()
 
-    def enable_state_handler(self) -> None:
+    def disconnect(self) -> None:
-        """Start StateHandler thread for monitoring updates."""
+        """Close push channel."""
-        self._client.register_update_handler(self._on_state_change)
+        self._fibaro_device_manager.close()
 
-    def disable_state_handler(self) -> None:
+    def register(
-        """Stop StateHandler thread used for monitoring updates."""
+        self, device_id: int, callback: Callable[[DeviceModel], None]
-        self._client.unregister_update_handler()
+    ) -> Callable[[], None]:
-
-    def _on_state_change(self, state: Any) -> None:
-        """Handle change report received from the HomeCenter."""
-        callback_set = set()
-        for change in state.get("changes", []):
-            try:
-                dev_id = change.pop("id")
-                if dev_id not in self._device_map:
-                    continue
-                device = self._device_map[dev_id]
-                for property_name, value in change.items():
-                    if property_name == "log":
-                        if value and value != "transfer OK":
-                            _LOGGER.debug("LOG %s: %s", device.friendly_name, value)
-                        continue
-                    if property_name == "logTemp":
-                        continue
-                    if property_name in device.properties:
-                        device.properties[property_name] = value
-                        _LOGGER.debug(
-                            "<- %s.%s = %s", device.ha_id, property_name, str(value)
-                        )
-                    else:
-                        _LOGGER.warning("%s.%s not found", device.ha_id, property_name)
-                    if dev_id in self._callbacks:
-                        callback_set.add(dev_id)
-            except (ValueError, KeyError):
-                pass
-        for item in callback_set:
-            for callback in self._callbacks[item]:
-                callback()
-
-        resolver = FibaroStateResolver(state)
-        for event in resolver.get_events():
-            # event does not always have a fibaro id, therefore it is
-            # essential that we first check for relevant event type
-            if (
-                event.event_type.lower() == "centralsceneevent"
-                and event.fibaro_id in self._event_callbacks
-            ):
-                for callback in self._event_callbacks[event.fibaro_id]:
-                    callback(event)
-
-    def register(self, device_id: int, callback: Any) -> None:
         """Register device with a callback for updates."""
-        device_callbacks = self._callbacks.setdefault(device_id, [])
+        return self._fibaro_device_manager.add_change_listener(device_id, callback)
-        device_callbacks.append(callback)
 
     def register_event(
         self, device_id: int, callback: Callable[[FibaroEvent], None]
-    ) -> None:
+    ) -> Callable[[], None]:
         """Register device with a callback for central scene events.
 
         The callback receives one parameter with the event.
         """
-        device_callbacks = self._event_callbacks.setdefault(device_id, [])
+        return self._fibaro_device_manager.add_event_listener(device_id, callback)
-        device_callbacks.append(callback)
 
     def get_children(self, device_id: int) -> list[DeviceModel]:
         """Get a list of child devices."""
@@ -286,7 +238,7 @@ class FibaroController:
 
     def _read_devices(self) -> None:
         """Read and process the device list."""
-        devices = self._client.read_devices()
+        devices = self._fibaro_device_manager.get_devices()
         self._device_map = {}
         last_climate_parent = None
         last_endpoint = None
@@ -301,9 +253,8 @@ class FibaroController:
                 device.ha_id = (
                     f"{slugify(room_name)}_{slugify(device.name)}_{device.fibaro_id}"
                 )
-                platform = None
+
-                if device.enabled and (not device.is_plugin or self._import_plugins):
+                platform = self._map_device_to_platform(device)
-                    platform = self._map_device_to_platform(device)
                 if platform is None:
                     continue
                 device.unique_id_str = f"{slugify(self.hub_serial)}.{device.fibaro_id}"
@@ -393,8 +344,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: FibaroConfigEntry) -> bo
 
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
-    controller.enable_state_handler()
-
     return True
 
 
@@ -403,8 +352,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: FibaroConfigEntry) -> b
     _LOGGER.debug("Shutting down Fibaro connection")
     unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
 
-    entry.runtime_data.disable_state_handler()
+    entry.runtime_data.disconnect()
-
     return unload_ok
 
 

homeassistant/components/fibaro/entity.py

@@ -36,9 +36,13 @@ class FibaroEntity(Entity):
 
     async def async_added_to_hass(self) -> None:
         """Call when entity is added to hass."""
-        self.controller.register(self.fibaro_device.fibaro_id, self._update_callback)
+        self.async_on_remove(
+            self.controller.register(
+                self.fibaro_device.fibaro_id, self._update_callback
+            )
+        )
 
-    def _update_callback(self) -> None:
+    def _update_callback(self, fibaro_device: DeviceModel) -> None:
         """Update the state."""
         self.schedule_update_ha_state(True)
 

homeassistant/components/fibaro/event.py

@@ -60,11 +60,16 @@ class FibaroEventEntity(FibaroEntity, EventEntity):
         await super().async_added_to_hass()
 
         # Register event callback
-        self.controller.register_event(
+        self.async_on_remove(
-            self.fibaro_device.fibaro_id, self._event_callback
+            self.controller.register_event(
+                self.fibaro_device.fibaro_id, self._event_callback
+            )
         )
 
     def _event_callback(self, event: FibaroEvent) -> None:
-        if event.key_id == self._button:
+        if (
+            event.event_type.lower() == "centralsceneevent"
+            and event.key_id == self._button
+        ):
             self._trigger_event(event.key_event_type)
             self.schedule_update_ha_state()

tests/components/fibaro/conftest.py

@@ -3,6 +3,7 @@
 from collections.abc import Generator
 from unittest.mock import AsyncMock, Mock, patch
 
+from pyfibaro.fibaro_device import SceneEvent
 import pytest
 
 from homeassistant.components.fibaro import CONF_IMPORT_PLUGINS, DOMAIN
@@ -231,6 +232,26 @@ def mock_fan_device() -> Mock:
     return climate
 
 
+@pytest.fixture
+def mock_button_device() -> Mock:
+    """Fixture for a button device."""
+    climate = Mock()
+    climate.fibaro_id = 8
+    climate.parent_fibaro_id = 0
+    climate.name = "Test button"
+    climate.room_id = 1
+    climate.dead = False
+    climate.visible = True
+    climate.enabled = True
+    climate.type = "com.fibaro.remoteController"
+    climate.base_type = "com.fibaro.actor"
+    climate.properties = {"manufacturer": ""}
+    climate.central_scene_event = [SceneEvent(1, "Pressed")]
+    climate.actions = {}
+    climate.interfaces = ["zwaveCentralScene"]
+    return climate
+
+
 @pytest.fixture
 def mock_config_entry(hass: HomeAssistant) -> MockConfigEntry:
     """Return the default mocked config entry."""

tests/components/fibaro/test_event.py

@@ -0,0 +1,35 @@
+"""Test the Fibaro event platform."""
+
+from unittest.mock import Mock, patch
+
+from homeassistant.const import Platform
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import entity_registry as er
+
+from .conftest import init_integration
+
+from tests.common import MockConfigEntry
+
+
+async def test_entity_setup(
+    hass: HomeAssistant,
+    entity_registry: er.EntityRegistry,
+    mock_fibaro_client: Mock,
+    mock_config_entry: MockConfigEntry,
+    mock_button_device: Mock,
+    mock_room: Mock,
+) -> None:
+    """Test that the button device creates an entity."""
+
+    # Arrange
+    mock_fibaro_client.read_rooms.return_value = [mock_room]
+    mock_fibaro_client.read_devices.return_value = [mock_button_device]
+
+    with patch("homeassistant.components.fibaro.PLATFORMS", [Platform.EVENT]):
+        # Act
+        await init_integration(hass, mock_config_entry)
+        # Assert
+        entry = entity_registry.async_get("event.room_1_test_button_8_button_1")
+        assert entry
+        assert entry.unique_id == "hc2_111111.8.1"
+        assert entry.original_name == "Room 1 Test button Button 1"

tests/components/fibaro/test_init.py

@@ -0,0 +1,31 @@
+"""Test init methods."""
+
+from unittest.mock import Mock, patch
+
+from homeassistant.const import Platform
+from homeassistant.core import HomeAssistant
+
+from .conftest import init_integration
+
+from tests.common import MockConfigEntry
+
+
+async def test_unload_integration(
+    hass: HomeAssistant,
+    mock_fibaro_client: Mock,
+    mock_config_entry: MockConfigEntry,
+    mock_light: Mock,
+    mock_room: Mock,
+) -> None:
+    """Test unload integration stops state listener."""
+    # Arrange
+    mock_fibaro_client.read_rooms.return_value = [mock_room]
+    mock_fibaro_client.read_devices.return_value = [mock_light]
+
+    with patch("homeassistant.components.fibaro.PLATFORMS", [Platform.LIGHT]):
+        await init_integration(hass, mock_config_entry)
+        # Act
+        await hass.config_entries.async_unload(mock_config_entry.entry_id)
+        await hass.async_block_till_done()
+        # Assert
+        assert mock_fibaro_client.unregister_update_handler.call_count == 1