Fix velux scenes (naming and unique ids) (home-assistant#156436)

Author: wollew

homeassistant/components/velux/quality_scale.yaml

@@ -17,9 +17,7 @@ rules:
   docs-removal-instructions: done
   entity-event-setup: done
   entity-unique-id: done
-  has-entity-name:
-    status: todo
-    comment: scenes need fixing
+  has-entity-name: done
   runtime-data: done
   test-before-configure: done
   test-before-setup:

homeassistant/components/velux/scene.py

@@ -4,11 +4,15 @@
 
 from typing import Any
 
+from pyvlx import Scene as PyVLXScene
+
 from homeassistant.components.scene import Scene
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
 
 from . import VeluxConfigEntry
+from .const import DOMAIN
 
 PARALLEL_UPDATES = 1
 
@@ -20,22 +24,32 @@ async def async_setup_entry(
 ) -> None:
     """Set up the scenes for Velux platform."""
     pyvlx = config_entry.runtime_data
-
-    entities = [VeluxScene(scene) for scene in pyvlx.scenes]
-    async_add_entities(entities)
+    async_add_entities(
+        [VeluxScene(config_entry.entry_id, scene) for scene in pyvlx.scenes]
+    )
 
 
 class VeluxScene(Scene):
     """Representation of a Velux scene."""
 
-    def __init__(self, scene):
+    _attr_has_entity_name = True
+
+    # Note: there's currently no code to update the scenes dynamically if changed in
+    # the gateway. They're only loaded on integration setup (they're probably not
+    # used heavily anyway since it's a pain to set them up in the gateway and so
+    # much easier to use HA scenes).
+
+    def __init__(self, config_entry_id: str, scene: PyVLXScene) -> None:
         """Init velux scene."""
         self.scene = scene
-
-    @property
-    def name(self):
-        """Return the name of the scene."""
-        return self.scene.name
+        # Renaming scenes in gateway keeps scene_id stable, we can use it as unique_id
+        self._attr_unique_id = f"{config_entry_id}_scene_{scene.scene_id}"
+        self._attr_name = scene.name
+
+        # Associate scenes with the gateway device (where they are stored)
+        self._attr_device_info = DeviceInfo(
+            identifiers={(DOMAIN, f"gateway_{config_entry_id}")},
+        )
 
     async def async_activate(self, **kwargs: Any) -> None:
         """Activate the scene."""

tests/components/velux/conftest.py

@@ -8,6 +8,7 @@
 from homeassistant.components.velux import DOMAIN
 from homeassistant.components.velux.binary_sensor import Window
 from homeassistant.components.velux.light import LighteningDevice
+from homeassistant.components.velux.scene import PyVLXScene as Scene
 from homeassistant.const import CONF_HOST, CONF_MAC, CONF_PASSWORD, Platform
 from homeassistant.core import HomeAssistant
 
@@ -91,10 +92,23 @@ def mock_light() -> AsyncMock:
 
 
 @pytest.fixture
-def mock_pyvlx(mock_window: MagicMock, mock_light: MagicMock) -> Generator[MagicMock]:
+def mock_scene() -> AsyncMock:
+    """Create a mock Velux scene."""
+    scene = AsyncMock(spec=Scene, autospec=True)
+    scene.name = "Test Scene"
+    scene.scene_id = "1234"
+    scene.scene = AsyncMock()
+    return scene
+
+
+@pytest.fixture
+def mock_pyvlx(
+    mock_window: MagicMock, mock_light: MagicMock, mock_scene: AsyncMock
+) -> Generator[MagicMock]:
     """Create the library mock and patch PyVLX."""
     pyvlx = MagicMock()
     pyvlx.nodes = [mock_window, mock_light]
+    pyvlx.scenes = [mock_scene]
     pyvlx.load_scenes = AsyncMock()
     pyvlx.load_nodes = AsyncMock()
     pyvlx.disconnect = AsyncMock()

tests/components/velux/snapshots/test_scene.ambr

@@ -0,0 +1,49 @@
+# serializer version: 1
+# name: test_scene_snapshot[scene.klf_200_gateway_test_scene-entry]
+  EntityRegistryEntrySnapshot({
+    'aliases': set({
+    }),
+    'area_id': None,
+    'capabilities': None,
+    'config_entry_id': <ANY>,
+    'config_subentry_id': <ANY>,
+    'device_class': None,
+    'device_id': <ANY>,
+    'disabled_by': None,
+    'domain': 'scene',
+    'entity_category': None,
+    'entity_id': 'scene.klf_200_gateway_test_scene',
+    'has_entity_name': True,
+    'hidden_by': None,
+    'icon': None,
+    'id': <ANY>,
+    'labels': set({
+    }),
+    'name': None,
+    'options': dict({
+    }),
+    'original_device_class': None,
+    'original_icon': None,
+    'original_name': 'Test Scene',
+    'platform': 'velux',
+    'previous_unique_id': None,
+    'suggested_object_id': None,
+    'supported_features': 0,
+    'translation_key': None,
+    'unique_id': 'test_entry_id_scene_1234',
+    'unit_of_measurement': None,
+  })
+# ---
+# name: test_scene_snapshot[scene.klf_200_gateway_test_scene-state]
+  StateSnapshot({
+    'attributes': ReadOnlyDict({
+      'friendly_name': 'KLF 200 Gateway Test Scene',
+    }),
+    'context': <ANY>,
+    'entity_id': 'scene.klf_200_gateway_test_scene',
+    'last_changed': <ANY>,
+    'last_reported': <ANY>,
+    'last_updated': <ANY>,
+    'state': 'unknown',
+  })
+# ---

tests/components/velux/test_scene.py

@@ -0,0 +1,68 @@
+"""Test Velux scene entities."""
+
+import pytest
+from syrupy.assertion import SnapshotAssertion
+
+from homeassistant.components.scene import DOMAIN as SCENE_DOMAIN, SERVICE_TURN_ON
+from homeassistant.components.velux import DOMAIN
+from homeassistant.const import ATTR_ENTITY_ID, Platform
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import device_registry as dr, entity_registry as er
+
+from tests.common import AsyncMock, MockConfigEntry, snapshot_platform
+
+
+@pytest.fixture
+def platform() -> Platform:
+    """Fixture to specify platform to test."""
+    return Platform.SCENE
+
+
+@pytest.mark.usefixtures("setup_integration")
+async def test_scene_snapshot(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    entity_registry: er.EntityRegistry,
+    device_registry: dr.DeviceRegistry,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Snapshot the scene entity (registry + state)."""
+    await snapshot_platform(
+        hass,
+        entity_registry,
+        snapshot,
+        mock_config_entry.entry_id,
+    )
+
+    # Get the scene entity setup and test device association
+    entity_entries = er.async_entries_for_config_entry(
+        entity_registry, mock_config_entry.entry_id
+    )
+    assert len(entity_entries) == 1
+    entry = entity_entries[0]
+
+    assert entry.device_id is not None
+    device_entry = device_registry.async_get(entry.device_id)
+    assert device_entry is not None
+    # Scenes are associated with the gateway device
+    assert (DOMAIN, f"gateway_{mock_config_entry.entry_id}") in device_entry.identifiers
+    assert device_entry.via_device_id is None
+
+
+@pytest.mark.usefixtures("setup_integration")
+async def test_scene_activation(
+    hass: HomeAssistant,
+    mock_scene: AsyncMock,
+) -> None:
+    """Test successful scene activation."""
+
+    # activate the scene via service call
+    await hass.services.async_call(
+        SCENE_DOMAIN,
+        SERVICE_TURN_ON,
+        {ATTR_ENTITY_ID: "scene.klf_200_gateway_test_scene"},
+        blocking=True,
+    )
+
+    # Verify the run method was called
+    mock_scene.run.assert_awaited_once_with(wait_for_completion=False)