Address copilot feedbacks

homeassistant/components/overkiz/entity.py

@@ -60,11 +60,12 @@ class OverkizEntity(CoordinatorEntity[OverkizDataUpdateCoordinator]):
 
     def _get_sibling_devices(self) -> list[Device]:
         """Return sibling devices sharing the same base device URL."""
+        prefix = f"{self.base_device_url}#"
         return [
             device
             for device in self.coordinator.data.values()
             if device.device_url != self.device_url
-            and device.device_url.startswith(f"{self.base_device_url}#")
+            and device.device_url.startswith(prefix)
         ]
 
     def _has_siblings_with_different_place_oid(self) -> bool:
@@ -73,29 +74,39 @@ class OverkizEntity(CoordinatorEntity[OverkizDataUpdateCoordinator]):
         Returns True if siblings have different place_oid values, indicating
         devices should be grouped by placeOID rather than by base URL.
         """
-        if not self.device.place_oid:
+        my_place_oid = self.device.place_oid
+        if not my_place_oid:
             return False
 
         return any(
-            sibling.place_oid and sibling.place_oid != self.device.place_oid
+            sibling.place_oid and sibling.place_oid != my_place_oid
             for sibling in self._get_sibling_devices()
         )
 
+    def _get_device_index(self, device_url: str) -> int | None:
+        """Extract numeric index from device URL (e.g., 'io://gw/123#4' -> 4)."""
+        suffix = device_url.split("#")[-1]
+        return int(suffix) if suffix.isdigit() else None
+
     def _is_main_device_for_place_oid(self) -> bool:
         """Check if this device is the main device for its placeOID group.
 
         The device with the lowest URL index among siblings sharing the same
         placeOID is considered the main device and provides full device info.
         """
-        if not self.device.place_oid:
+        my_place_oid = self.device.place_oid
+        if not my_place_oid:
             return True
 
-        my_index = int(self.device_url.split("#")[-1])
+        my_index = self._get_device_index(self.device_url)
+        if my_index is None:
+            return True
 
         return not any(
-            int(sibling.device_url.split("#")[-1]) < my_index
+            (sibling_index := self._get_device_index(sibling.device_url)) is not None
+            and sibling_index < my_index
             for sibling in self._get_sibling_devices()
-            if sibling.place_oid == self.device.place_oid
+            if sibling.place_oid == my_place_oid
         )
 
     def _get_via_device_id(self, use_place_oid_grouping: bool) -> str:

tests/components/overkiz/test_entity.py

@@ -7,8 +7,10 @@ import pytest
 from homeassistant.components.overkiz.entity import OverkizEntity
 
 
-def _create_mock_device(device_url: str, place_oid: str, label: str) -> Mock:
-    """Create a mock device."""
+def _create_mock_device(
+    device_url: str, place_oid: str | None, label: str = "Device"
+) -> Mock:
+    """Create a mock device with the given properties."""
     device = Mock()
     device.device_url = device_url
     device.place_oid = place_oid
@@ -28,12 +30,16 @@ def _create_mock_entity(device: Mock, all_devices: list[Mock]) -> Mock:
     entity.base_device_url = device.device_url.split("#")[0]
     entity.coordinator = Mock()
     entity.coordinator.data = {d.device_url: d for d in all_devices}
+
+    prefix = f"{entity.base_device_url}#"
     entity._get_sibling_devices = lambda: [
         d
         for d in all_devices
-        if d.device_url != device.device_url
-        and d.device_url.startswith(f"{entity.base_device_url}#")
+        if d.device_url != device.device_url and d.device_url.startswith(prefix)
     ]
+    entity._get_device_index = lambda url: (
+        int(url.split("#")[-1]) if url.split("#")[-1].isdigit() else None
+    )
     return entity
 
 
@@ -108,3 +114,64 @@ def test_get_via_device_id_main_device_links_to_gateway() -> None:
     result = OverkizEntity._get_via_device_id(entity, use_place_oid_grouping=True)
 
     assert result == "gateway-id"
+
+
+def test_has_siblings_with_no_place_oid() -> None:
+    """Test device with no placeOID returns False."""
+    devices = [
+        _create_mock_device("io://gateway/123#1", None, "Device 1"),
+        _create_mock_device("io://gateway/123#2", "place-b", "Device 2"),
+    ]
+    entity = _create_mock_entity(devices[0], devices)
+
+    result = OverkizEntity._has_siblings_with_different_place_oid(entity)
+
+    assert result is False
+
+
+def test_is_main_device_with_no_place_oid() -> None:
+    """Test device with no placeOID is always considered main."""
+    devices = [
+        _create_mock_device("io://gateway/123#2", None, "Device 2"),
+        _create_mock_device("io://gateway/123#1", "place-a", "Device 1"),
+    ]
+    entity = _create_mock_entity(devices[0], devices)
+
+    result = OverkizEntity._is_main_device_for_place_oid(entity)
+
+    assert result is True
+
+
+def test_get_via_device_id_main_device_without_place_oid() -> None:
+    """Test fallback to gateway when #1 device has no placeOID."""
+    devices = [
+        _create_mock_device("io://gateway/123#1", None, "Actuator"),
+        _create_mock_device("io://gateway/123#2", "place-b", "Zone"),
+    ]
+    entity = _create_mock_entity(devices[1], devices)
+    entity.executor = Mock()
+    entity.executor.get_gateway_id = Mock(return_value="gateway-id")
+
+    result = OverkizEntity._get_via_device_id(entity, use_place_oid_grouping=True)
+
+    assert result == "gateway-id"
+
+
+@pytest.mark.parametrize(
+    ("device_url", "expected"),
+    [
+        ("io://gateway/123#4", 4),
+        ("io://gateway/123#10", 10),
+        ("io://gateway/123#abc", None),
+        ("io://gateway/123#", None),
+    ],
+    ids=["single_digit", "multi_digit", "non_numeric", "empty_suffix"],
+)
+def test_get_device_index(device_url: str, expected: int | None) -> None:
+    """Test extracting numeric index from device URL."""
+    device = _create_mock_device(device_url, "place-a")
+    entity = _create_mock_entity(device, [device])
+
+    result = OverkizEntity._get_device_index(entity, device_url)
+
+    assert result == expected