Code cleanup: Separate fibaro scene logic (#91895)

* Code cleanup fibaro scene

* Remove duplicated code

* Fixes

homeassistant/components/fibaro/__init__.py

@@ -8,7 +8,6 @@ from typing import Any
 
 from pyfibaro.fibaro_client import FibaroClient
 from pyfibaro.fibaro_device import DeviceModel
-from pyfibaro.fibaro_scene import SceneModel
 from requests.exceptions import HTTPError
 
 from homeassistant.config_entries import ConfigEntry
@@ -278,24 +277,18 @@ class FibaroController:
             return self._device_infos[device.parent_fibaro_id]
         return DeviceInfo(identifiers={(DOMAIN, self.hub_serial)})
 
+    def get_room_name(self, room_id: int) -> str | None:
+        """Get the room name by room id."""
+        assert self._room_map
+        room = self._room_map.get(room_id)
+        return room.name if room else None
+
     def _read_scenes(self):
         scenes = self._client.read_scenes()
         for device in scenes:
             device.fibaro_controller = self
-            if device.room_id == 0:
-                room_name = "Unknown"
-            else:
-                room_name = self._room_map[device.room_id].name
-            device.room_name = room_name
-            device.friendly_name = f"{room_name} {device.name}"
-            device.ha_id = (
-                f"scene_{slugify(room_name)}_{slugify(device.name)}_{device.fibaro_id}"
-            )
-            device.unique_id_str = (
-                f"{slugify(self.hub_serial)}.scene.{device.fibaro_id}"
-            )
             self.fibaro_devices[Platform.SCENE].append(device)
-            _LOGGER.debug("%s scene -> %s", device.ha_id, device)
+            _LOGGER.debug("Scene -> %s", device)
 
     def _read_devices(self):
         """Read and process the device list."""
@@ -425,7 +418,7 @@ class FibaroDevice(Entity):
 
     _attr_should_poll = False
 
-    def __init__(self, fibaro_device: DeviceModel | SceneModel) -> None:
+    def __init__(self, fibaro_device: DeviceModel) -> None:
         """Initialize the device."""
         self.fibaro_device = fibaro_device
         self.controller = fibaro_device.fibaro_controller
@@ -433,7 +426,6 @@ class FibaroDevice(Entity):
         self._attr_name = fibaro_device.friendly_name
         self._attr_unique_id = fibaro_device.unique_id_str
 
-        if isinstance(fibaro_device, DeviceModel):
         self._attr_device_info = self.controller.get_device_info(fibaro_device)
         # propagate hidden attribute set in fibaro home center to HA
         if not fibaro_device.visible:
@@ -519,7 +511,6 @@ class FibaroDevice(Entity):
         """Return the state attributes of the device."""
         attr = {"fibaro_id": self.fibaro_device.fibaro_id}
 
-        if isinstance(self.fibaro_device, DeviceModel):
         if self.fibaro_device.has_battery_level:
             attr[ATTR_BATTERY_LEVEL] = self.fibaro_device.battery_level
         if self.fibaro_device.has_armed:

homeassistant/components/fibaro/scene.py

@@ -11,8 +11,9 @@ from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
+from homeassistant.util import slugify
 
-from . import FIBARO_DEVICES, FibaroDevice
+from . import FIBARO_DEVICES, FibaroController
 from .const import DOMAIN
 
 
@@ -33,18 +34,30 @@ async def async_setup_entry(
     )
 
 
-class FibaroScene(FibaroDevice, Scene):
+class FibaroScene(Scene):
     """Representation of a Fibaro scene entity."""
 
-    def __init__(self, fibaro_device: SceneModel) -> None:
+    def __init__(self, fibaro_scene: SceneModel) -> None:
         """Initialize the Fibaro scene."""
-        super().__init__(fibaro_device)
+        self._fibaro_scene = fibaro_scene
 
+        controller: FibaroController = fibaro_scene.fibaro_controller
+        room_name = controller.get_room_name(fibaro_scene.room_id)
+        if not room_name:
+            room_name = "Unknown"
+
+        self._attr_name = f"{room_name} {fibaro_scene.name}"
+        self._attr_unique_id = (
+            f"{slugify(controller.hub_serial)}.scene.{fibaro_scene.fibaro_id}"
+        )
+        self._attr_extra_state_attributes = {"fibaro_id": fibaro_scene.fibaro_id}
+        # propagate hidden attribute set in fibaro home center to HA
+        self._attr_entity_registry_visible_default = fibaro_scene.visible
         # All scenes are shown on hub device
         self._attr_device_info = DeviceInfo(
-            identifiers={(DOMAIN, self.controller.hub_serial)}
+            identifiers={(DOMAIN, controller.hub_serial)}
         )
 
     def activate(self, **kwargs: Any) -> None:
         """Activate the scene."""
-        self.fibaro_device.start()
+        self._fibaro_scene.start()

tests/components/fibaro/conftest.py

@@ -1,9 +1,15 @@
 """Test helpers."""
 from collections.abc import Generator
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, Mock, patch
 
+from pyfibaro.fibaro_scene import SceneModel
 import pytest
 
+from homeassistant.components.fibaro import DOMAIN, FIBARO_CONTROLLER, FIBARO_DEVICES
+from homeassistant.config_entries import SOURCE_USER, ConfigEntry
+from homeassistant.const import Platform
+from homeassistant.core import HomeAssistant
+
 
 @pytest.fixture
 def mock_setup_entry() -> Generator[AsyncMock, None, None]:
@@ -12,3 +18,48 @@ def mock_setup_entry() -> Generator[AsyncMock, None, None]:
         "homeassistant.components.fibaro.async_setup_entry", return_value=True
     ) as mock_setup_entry:
         yield mock_setup_entry
+
+
+@pytest.fixture(name="fibaro_scene")
+def mock_scene() -> SceneModel:
+    """Fixture for an individual scene."""
+    scene = Mock(SceneModel)
+    scene.fibaro_id = 1
+    scene.name = "Test scene"
+    scene.room_id = 1
+    scene.visible = True
+    return scene
+
+
+async def setup_platform(
+    hass: HomeAssistant,
+    platform: Platform,
+    room_name: str | None,
+    scenes: list[SceneModel],
+) -> ConfigEntry:
+    """Set up the fibaro platform and prerequisites."""
+    hass.config.components.add(DOMAIN)
+    config_entry = ConfigEntry(
+        1,
+        DOMAIN,
+        "Test",
+        {},
+        SOURCE_USER,
+    )
+
+    controller_mock = Mock()
+    controller_mock.hub_serial = "HC2-111111"
+    controller_mock.get_room_name.return_value = room_name
+
+    for scene in scenes:
+        scene.fibaro_controller = controller_mock
+
+    hass.data[DOMAIN] = {
+        config_entry.entry_id: {
+            FIBARO_CONTROLLER: controller_mock,
+            FIBARO_DEVICES: {Platform.SCENE: scenes},
+        }
+    }
+    await hass.config_entries.async_forward_entry_setup(config_entry, platform)
+    await hass.async_block_till_done()
+    return config_entry

tests/components/fibaro/test_scene.py

@@ -0,0 +1,54 @@
+"""Test the Fibaro scene platform."""
+
+from pyfibaro.fibaro_scene import SceneModel
+
+from homeassistant.components.scene import DOMAIN as SCENE_DOMAIN
+from homeassistant.const import ATTR_ENTITY_ID, SERVICE_TURN_ON, Platform
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import entity_registry as er
+
+from .conftest import setup_platform
+
+
+async def test_entity_attributes(hass: HomeAssistant, fibaro_scene: SceneModel) -> None:
+    """Test that the attributes of the entity are correct."""
+    # Arrange
+    entity_registry = er.async_get(hass)
+    # Act
+    await setup_platform(hass, Platform.SCENE, "Room 1", [fibaro_scene])
+    # Assert
+    entry = entity_registry.async_get("scene.room_1_test_scene")
+
+    assert entry
+    assert entry.unique_id == "hc2_111111.scene.1"
+    assert entry.original_name == "Room 1 Test scene"
+
+
+async def test_entity_attributes_without_room(
+    hass: HomeAssistant, fibaro_scene: SceneModel
+) -> None:
+    """Test that the attributes of the entity are correct."""
+    # Arrange
+    entity_registry = er.async_get(hass)
+    # Act
+    await setup_platform(hass, Platform.SCENE, None, [fibaro_scene])
+    # Assert
+    entry = entity_registry.async_get("scene.unknown_test_scene")
+
+    assert entry
+    assert entry.unique_id == "hc2_111111.scene.1"
+
+
+async def test_activate_scene(hass: HomeAssistant, fibaro_scene: SceneModel) -> None:
+    """Test activate scene is called."""
+    # Arrange
+    await setup_platform(hass, Platform.SCENE, "Room 1", [fibaro_scene])
+    # Act
+    await hass.services.async_call(
+        SCENE_DOMAIN,
+        SERVICE_TURN_ON,
+        {ATTR_ENTITY_ID: "scene.room_1_test_scene"},
+        blocking=True,
+    )
+    # Assert
+    assert fibaro_scene.start.call_count == 1