Refactor media_player and remote platforms in Xbox integration (#154986)

Co-authored-by: Josef Zweck <josef@zweck.dev>

Author: tr4nt0r

homeassistant/components/xbox/entity.py

@@ -2,12 +2,23 @@
 
 from __future__ import annotations
 
+from xbox.webapi.api.provider.smartglass.models import ConsoleType, SmartglassConsole
+
 from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
 from homeassistant.helpers.entity import EntityDescription
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
 from .const import DOMAIN
-from .coordinator import Person, XboxUpdateCoordinator
+from .coordinator import ConsoleData, Person, XboxUpdateCoordinator
+
+MAP_MODEL = {
+    ConsoleType.XboxOne: "Xbox One",
+    ConsoleType.XboxOneS: "Xbox One S",
+    ConsoleType.XboxOneSDigital: "Xbox One S All-Digital",
+    ConsoleType.XboxOneX: "Xbox One X",
+    ConsoleType.XboxSeriesS: "Xbox Series S",
+    ConsoleType.XboxSeriesX: "Xbox Series X",
+}
 
 
 class XboxBaseEntity(CoordinatorEntity[XboxUpdateCoordinator]):
@@ -21,7 +32,7 @@ def __init__(
         xuid: str,
         entity_description: EntityDescription,
     ) -> None:
-        """Initialize Xbox binary sensor."""
+        """Initialize Xbox entity."""
         super().__init__(coordinator)
         self.xuid = xuid
         self.entity_description = entity_description
@@ -40,3 +51,35 @@ def __init__(
     def data(self) -> Person:
         """Return coordinator data for this console."""
         return self.coordinator.data.presence[self.xuid]
+
+
+class XboxConsoleBaseEntity(CoordinatorEntity[XboxUpdateCoordinator]):
+    """Console base entity for the Xbox integration."""
+
+    _attr_has_entity_name = True
+
+    def __init__(
+        self,
+        console: SmartglassConsole,
+        coordinator: XboxUpdateCoordinator,
+    ) -> None:
+        """Initialize the Xbox Console entity."""
+
+        super().__init__(coordinator)
+        self.client = coordinator.client
+        self._console = console
+
+        self._attr_name = None
+        self._attr_unique_id = console.id
+
+        self._attr_device_info = DeviceInfo(
+            identifiers={(DOMAIN, console.id)},
+            manufacturer="Microsoft",
+            model=MAP_MODEL.get(self._console.console_type, "Unknown"),
+            name=console.name,
+        )
+
+    @property
+    def data(self) -> ConsoleData:
+        """Return coordinator data for this console."""
+        return self.coordinator.data.consoles[self._console.id]

homeassistant/components/xbox/media_player.py

@@ -2,31 +2,28 @@
 
 from __future__ import annotations
 
-import re
 from typing import Any
 
 from xbox.webapi.api.provider.catalog.models import Image
 from xbox.webapi.api.provider.smartglass.models import (
     PlaybackState,
     PowerState,
-    SmartglassConsole,
     VolumeDirection,
 )
 
 from homeassistant.components.media_player import (
+    BrowseMedia,
     MediaPlayerEntity,
     MediaPlayerEntityFeature,
     MediaPlayerState,
     MediaType,
 )
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
-from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
 from .browse_media import build_item_response
-from .const import DOMAIN
-from .coordinator import ConsoleData, XboxConfigEntry, XboxUpdateCoordinator
+from .coordinator import XboxConfigEntry
+from .entity import XboxConsoleBaseEntity
 
 SUPPORT_XBOX = (
     MediaPlayerEntityFeature.TURN_ON
@@ -69,33 +66,10 @@ async def async_setup_entry(
     )
 
 
-class XboxMediaPlayer(CoordinatorEntity[XboxUpdateCoordinator], MediaPlayerEntity):
+class XboxMediaPlayer(XboxConsoleBaseEntity, MediaPlayerEntity):
     """Representation of an Xbox Media Player."""
 
-    def __init__(
-        self,
-        console: SmartglassConsole,
-        coordinator: XboxUpdateCoordinator,
-    ) -> None:
-        """Initialize the Xbox Media Player."""
-        super().__init__(coordinator)
-        self.client = coordinator.client
-        self._console = console
-
-    @property
-    def name(self):
-        """Return the device name."""
-        return self._console.name
-
-    @property
-    def unique_id(self):
-        """Console device ID."""
-        return self._console.id
-
-    @property
-    def data(self) -> ConsoleData:
-        """Return coordinator data for this console."""
-        return self.coordinator.data.consoles[self._console.id]
+    _attr_media_image_remotely_accessible = True
 
     @property
     def state(self) -> MediaPlayerState | None:
@@ -117,15 +91,15 @@ def supported_features(self) -> MediaPlayerEntityFeature:
         return SUPPORT_XBOX
 
     @property
-    def media_content_type(self):
+    def media_content_type(self) -> MediaType:
         """Media content type."""
         app_details = self.data.app_details
         if app_details and app_details.product_family == "Games":
             return MediaType.GAME
         return MediaType.APP
 
     @property
-    def media_title(self):
+    def media_title(self) -> str | None:
         """Title of current playing media."""
         if not (app_details := self.data.app_details):
             return None
@@ -135,25 +109,18 @@ def media_title(self):
         )
 
     @property
-    def media_image_url(self):
+    def media_image_url(self) -> str | None:
         """Image url of current playing media."""
-        if not (app_details := self.data.app_details):
-            return None
-        image = _find_media_image(app_details.localized_properties[0].images)
-
-        if not image:
+        if not (app_details := self.data.app_details) or not (
+            image := _find_media_image(app_details.localized_properties[0].images)
+        ):
             return None
 
         url = image.uri
         if url[0] == "/":
             url = f"http:{url}"
         return url
 
-    @property
-    def media_image_remotely_accessible(self) -> bool:
-        """If the image url is remotely accessible."""
-        return True
-
     async def async_turn_on(self) -> None:
         """Turn the media player on."""
         await self.client.smartglass.wake_up(self._console.id)
@@ -193,15 +160,20 @@ async def async_media_next_track(self) -> None:
         """Send next track command."""
         await self.client.smartglass.next(self._console.id)
 
-    async def async_browse_media(self, media_content_type=None, media_content_id=None):
+    async def async_browse_media(
+        self,
+        media_content_type: MediaType | str | None = None,
+        media_content_id: str | None = None,
+    ) -> BrowseMedia:
         """Implement the websocket media browsing helper."""
+
         return await build_item_response(
             self.client,
             self._console.id,
             self.data.status.is_tv_configured,
-            media_content_type,
-            media_content_id,
-        )
+            media_content_type or "",
+            media_content_id or "",
+        )  # type: ignore[return-value]
 
     async def async_play_media(
         self, media_type: MediaType | str, media_id: str, **kwargs: Any
@@ -214,22 +186,6 @@ async def async_play_media(
         else:
             await self.client.smartglass.launch_app(self._console.id, media_id)
 
-    @property
-    def device_info(self) -> DeviceInfo:
-        """Return a device description for device registry."""
-        # Turns "XboxOneX" into "Xbox One X" for display
-        matches = re.finditer(
-            ".+?(?:(?<=[a-z])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])|$)",
-            self._console.console_type,
-        )
-
-        return DeviceInfo(
-            identifiers={(DOMAIN, self._console.id)},
-            manufacturer="Microsoft",
-            model=" ".join([m.group(0) for m in matches]),
-            name=self._console.name,
-        )
-
 
 def _find_media_image(images: list[Image]) -> Image | None:
     purpose_order = ["FeaturePromotionalSquareArt", "Tile", "Logo", "BoxArt"]

homeassistant/components/xbox/remote.py

@@ -4,14 +4,9 @@
 
 import asyncio
 from collections.abc import Iterable
-import re
 from typing import Any
 
-from xbox.webapi.api.provider.smartglass.models import (
-    InputKeyType,
-    PowerState,
-    SmartglassConsole,
-)
+from xbox.webapi.api.provider.smartglass.models import InputKeyType, PowerState
 
 from homeassistant.components.remote import (
     ATTR_DELAY_SECS,
@@ -20,12 +15,10 @@
     RemoteEntity,
 )
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
-from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
-from .const import DOMAIN
-from .coordinator import ConsoleData, XboxConfigEntry, XboxUpdateCoordinator
+from .coordinator import XboxConfigEntry
+from .entity import XboxConsoleBaseEntity
 
 
 async def async_setup_entry(
@@ -41,36 +34,11 @@ async def async_setup_entry(
     )
 
 
-class XboxRemote(CoordinatorEntity[XboxUpdateCoordinator], RemoteEntity):
+class XboxRemote(XboxConsoleBaseEntity, RemoteEntity):
     """Representation of an Xbox remote."""
 
-    def __init__(
-        self,
-        console: SmartglassConsole,
-        coordinator: XboxUpdateCoordinator,
-    ) -> None:
-        """Initialize the Xbox Media Player."""
-        super().__init__(coordinator)
-        self.client = coordinator.client
-        self._console = console
-
-    @property
-    def name(self):
-        """Return the device name."""
-        return f"{self._console.name} Remote"
-
-    @property
-    def unique_id(self):
-        """Console device ID."""
-        return self._console.id
-
     @property
-    def data(self) -> ConsoleData:
-        """Return coordinator data for this console."""
-        return self.coordinator.data.consoles[self._console.id]
-
-    @property
-    def is_on(self):
+    def is_on(self) -> bool:
         """Return True if device is on."""
         return self.data.status.power_state == PowerState.On
 
@@ -97,19 +65,3 @@ async def async_send_command(self, command: Iterable[str], **kwargs: Any) -> Non
                         self._console.id, single_command
                     )
                 await asyncio.sleep(delay)
-
-    @property
-    def device_info(self) -> DeviceInfo:
-        """Return a device description for device registry."""
-        # Turns "XboxOneX" into "Xbox One X" for display
-        matches = re.finditer(
-            ".+?(?:(?<=[a-z])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])|$)",
-            self._console.console_type,
-        )
-
-        return DeviceInfo(
-            identifiers={(DOMAIN, self._console.id)},
-            manufacturer="Microsoft",
-            model=" ".join([m.group(0) for m in matches]),
-            name=self._console.name,
-        )

tests/components/xbox/snapshots/test_media_player.ambr

@@ -14,7 +14,7 @@
     'domain': 'media_player',
     'entity_category': None,
     'entity_id': 'media_player.xone',
-    'has_entity_name': False,
+    'has_entity_name': True,
     'hidden_by': None,
     'icon': None,
     'id': <ANY>,
@@ -25,7 +25,7 @@
     }),
     'original_device_class': None,
     'original_icon': None,
-    'original_name': 'XONE',
+    'original_name': None,
     'platform': 'xbox',
     'previous_unique_id': None,
     'suggested_object_id': None,
@@ -68,7 +68,7 @@
     'domain': 'media_player',
     'entity_category': None,
     'entity_id': 'media_player.xonex',
-    'has_entity_name': False,
+    'has_entity_name': True,
     'hidden_by': None,
     'icon': None,
     'id': <ANY>,
@@ -79,7 +79,7 @@
     }),
     'original_device_class': None,
     'original_icon': None,
-    'original_name': 'XONEX',
+    'original_name': None,
     'platform': 'xbox',
     'previous_unique_id': None,
     'suggested_object_id': None,