From 93b766ae97fbe48eafe01a8d963722b16790c2b5 Mon Sep 17 00:00:00 2001 From: James Mackay Date: Thu, 8 Oct 2026 09:03:18 +0100 Subject: [PATCH] refactor(media_player): entity becomes a thin view over the ShieldConnectionManager store; poll loop and socket removed from the entity --- .../nvidia_shield/media_player.py | 221 +++++------------- 1 file changed, 60 insertions(+), 161 deletions(-) diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index 6d4c566..e0cb8a2 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -6,6 +6,9 @@ State architecture: - Metadata: Jellyfin attributes - ALL commands: androidtv_remote (Jellyfin is read-only) +ADB connection/polling lives in the entry's ShieldConnectionManager +(connection.py); this entity is a thin callback-view over its StateStore. + Intent tracking bridges the gap between instant commands and slow Jellyfin polling: 1. User presses pause → intent recorded → UI shows paused immediately 2. Command sent to androidtv_remote → Shield pauses on screen @@ -14,7 +17,6 @@ Intent tracking bridges the gap between instant commands and slow Jellyfin polli """ import logging -import os import time from homeassistant.components.media_player import ( @@ -30,17 +32,14 @@ from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.event import async_call_later, async_track_state_change_event -from .adb_session import AdbMediaSessionPoller +from .connection import ShieldConnectionManager from .const import ( - ADB_POLL_INTERVAL, - ADB_RETRY_INTERVAL, ATTR_MEDIA_DAG_JELLYFIN_ENTITY, ATTR_MEDIA_DAG_NOW_PLAYING, ATTR_MEDIA_DAG_RECEIVER_ENTITY, ATTR_MEDIA_DAG_RECEIVER_INPUT, ATTR_MEDIA_DAG_ROLE, ATTR_MEDIA_DAG_RUNNING_APPS, - CONF_ADB_ENABLED, CONF_ADB_STORE_APP_INFO, CONF_ANDROIDTV_ENTITY, CONF_JELLYFIN_ENTITY, @@ -50,7 +49,6 @@ from .const import ( DOMAIN, ) from .ha_adb_common import PlaybackIntent, running_apps_from_tasks -from .helpers import adb_key_path, resolve_adb_config from .jellyfin import fetch_session_media_info from .power import discover_power_siblings @@ -114,15 +112,14 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): # Jellyfin media stream info (resolution, codec, etc.) self._media_info: dict | None = None - # ADB media session, wake state, and running apps - self._adb_poller: AdbMediaSessionPoller | None = None - self._adb_poll_unsub: CALLBACK_TYPE | None = None - self._adb_awake: bool | None = None - self._running_apps: list[dict] = [] - # Tracked poll tasks (cancelled on unload) and failure bookkeeping - self._adb_poll_tasks: set = set() - self._adb_failures = 0 - self._removed = False + # ADB-derived state comes from the entry's ShieldConnectionManager + # (connection.py) — this entity is a thin callback-view over its store. + self._manager: ShieldConnectionManager | None = None + self._manager_unsub: CALLBACK_TYPE | None = None + # Entity-owned background tasks (intent retries, media-info fetch); + # cancelled on unload. The manager owns the socket — never touch it + # here directly. + self._entity_tasks: set = set() @property def _jellyfin_entity(self) -> str | None: @@ -149,10 +146,6 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): def _power_entity(self) -> str | None: return self._entry.options.get(CONF_POWER_ENTITY) or None - @property - def _adb_enabled(self) -> bool: - return self._entry.options.get(CONF_ADB_ENABLED, False) - @property def _adb_store_info(self) -> bool: return self._entry.options.get(CONF_ADB_STORE_APP_INFO, True) @@ -162,9 +155,13 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): if self._power_entity: self._power_sensors = discover_power_siblings(self.hass, self._power_entity) - # Set up ADB media session poller - if self._adb_enabled: - await self._setup_adb() + # Consume the entry's ShieldConnectionManager (thin-view pattern) + manager = getattr(self._entry, "runtime_data", None) + if manager is not None: + self._manager = manager + self._manager_unsub = manager.register_listener( + self._on_manager_update + ) entities = [self._androidtv_entity] if self._jellyfin_entity: @@ -178,21 +175,18 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): ) async def async_will_remove_from_hass(self) -> None: - self._removed = True for unsub in self._unsub_listeners: unsub() self._unsub_listeners.clear() self._clear_intent() - if self._adb_poll_unsub: - self._adb_poll_unsub() - self._adb_poll_unsub = None - # Cancel in-flight poll tasks so they cannot outlive the entity and - # trigger a surprise reconnect (B2). - for task in self._adb_poll_tasks: + if self._manager_unsub: + self._manager_unsub() + self._manager_unsub = None + # Cancel in-flight entity tasks so they cannot outlive the entity. + for task in self._entity_tasks: task.cancel() - self._adb_poll_tasks.clear() - if self._adb_poller: - await self._adb_poller.disconnect() + self._entity_tasks.clear() + # ── Intent management ── @@ -254,12 +248,12 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): if ( intent.via_adb and intent.adb_command - and self._adb_poller is not None - and self._adb_poller.connected + and self._manager is not None + and self._manager.connected ): # The command was originally served by ADB — retry over ADB. self._track_task( - self._adb_poller.send_media_command(intent.adb_command) + self._manager.send_media_command(intent.adb_command) ) else: self._track_task( @@ -335,119 +329,22 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): return None return self.hass.states.get(entity_id) - async def _setup_adb(self) -> None: - """Initialize ADB connection and start polling.""" - try: - # Only auto-connect if ADB was previously paired (key exists) - key_path = adb_key_path(self.hass) - if not await self.hass.async_add_executor_job(os.path.exists, key_path): - _LOGGER.info( - "ADB key not found — skipping auto-connect. " - "Use the Pair ADB button or reconfigure the integration." - ) - return - - adb = resolve_adb_config(self.hass, self._entry) - if adb is None: - _LOGGER.warning( - "Could not determine ADB host. Set it in integration options." - ) - return - self._adb_poller = AdbMediaSessionPoller(adb.host, adb.port, adb.key_path) - - if await self._adb_poller.connect(): - # Start periodic polling - self._adb_poll_unsub = async_call_later( - self.hass, ADB_POLL_INTERVAL, self._adb_poll_callback - ) - _LOGGER.info("ADB media session polling started for %s", adb.host) - else: - _LOGGER.warning( - "ADB connection to %s failed. If this is the first connection, " - "approve the ADB debugging prompt on the device. " - "Retrying in %d seconds.", - adb.host, ADB_RETRY_INTERVAL, - ) - self._adb_poll_unsub = async_call_later( - self.hass, ADB_RETRY_INTERVAL, self._adb_poll_callback - ) - except Exception: - _LOGGER.exception("Failed to set up ADB") - @callback - def _adb_poll_callback(self, _now) -> None: - """Schedule the ADB poll as a tracked task (cancelled on unload).""" - self._track_task(self._adb_poll()) + def _on_manager_update(self) -> None: + """Connection manager's store changed — re-evaluate our state.""" + self.async_write_ha_state() def _track_task(self, coro) -> None: - """Dispatch a background task and register it for unload cancellation. + """Dispatch an entity background task; cancelled on unload. - Any fire-and-forget task that can touch the ADB poller must go through - here: a bare hass.async_create_task survives entity removal and, with - shell()'s auto-reconnect, can re-handshake the socket for a removed - entity (reviewer finding F2). Cancelation in async_will_remove_from_hass - covers poll and intent-retry tasks alike. + Entity-owned tasks (intent retries, media-info fetch) must never go + through a bare hass.async_create_task: they would survive removal + (reviewer finding F2). The connection manager owns its own tasks and + the socket — nothing here touches it directly. """ task = self.hass.async_create_task(coro) - self._adb_poll_tasks.add(task) - task.add_done_callback(self._adb_poll_tasks.discard) - - async def _adb_poll(self) -> None: - """Poll ADB for wake state and media session, then schedule next poll.""" - try: - if not self._adb_poller or self._removed: - return - - # Always check wake state - self._adb_awake = await self._adb_poller.is_awake() - - if self._adb_awake is None: - # Poll failure (connection dropped, device unreachable) — make - # it visible and clear derived state so nothing stale lingers. - self._on_adb_poll_failure() - return - - if self._adb_failures: - self._adb_failures = 0 - _LOGGER.info("ADB poll recovered") - - if self._adb_awake: - # Always poll ADB when awake — pass the foreground app - # so the parser prefers its session over stale ones - await self._adb_poller.poll(foreground_package=self.app_id) - self._running_apps = await self._adb_poller.poll_running_apps() - elif self._adb_poller.last_info: - # Clear media info when asleep - self._adb_poller.clear_info() - self._running_apps = [] - - # Always update state — any ADB-derived value may have changed - self.async_write_ha_state() - except Exception: - self._on_adb_poll_failure() - finally: - # Use slow retry if ADB is disconnected, normal interval if connected - if self._adb_enabled and not self._removed: - connected = self._adb_poller is not None and self._adb_poller.connected - interval = ADB_POLL_INTERVAL if connected else ADB_RETRY_INTERVAL - self._adb_poll_unsub = async_call_later( - self.hass, interval, self._adb_poll_callback - ) - - def _on_adb_poll_failure(self) -> None: - """Handle a failed poll: log (first failure + periodic repeats) and - clear derived state instead of clinging to the last known values.""" - self._adb_failures += 1 - if self._adb_failures == 1 or self._adb_failures % 6 == 0: - _LOGGER.warning( - "ADB poll failed (%d consecutive attempts — continuing in the background)", - self._adb_failures, - ) - self._adb_awake = None - if self._adb_poller: - self._adb_poller.clear_info() - self._running_apps = [] - self.async_write_ha_state() + self._entity_tasks.add(task) + task.add_done_callback(self._entity_tasks.discard) async def _fetch_media_info(self, entity_picture: str) -> None: """Read stream info from the HA Jellyfin integration's coordinator.""" @@ -477,9 +374,10 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): ADB is the authority — it knows if the device is awake or asleep. The plug being "on" just means the outlet has power (standby). """ - # 1. ADB wake state — most accurate - if self._adb_awake is not None: - return self._adb_awake + # 1. ADB wake state — most accurate (exact mWakefulness parse) + adb_awake = self._manager.awake if self._manager else None + if adb_awake is not None: + return adb_awake # 2. Power entity — only trust "off" (plug pulled = definitely off) if self._power_entity: @@ -518,8 +416,8 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): return MediaPlayerState.PAUSED # Tier 3: ADB media session (non-Jellyfin apps) - if self._adb_poller and self._adb_poller.last_info: - adb_info = self._adb_poller.last_info + adb_info = self._manager.best_session(self.app_id) if self._manager else None + if adb_info: if adb_info.playing: return MediaPlayerState.PLAYING if adb_info.paused: @@ -533,10 +431,9 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): if self._jf_active(): return _BASE_FEATURES | _PLAYBACK_FEATURES # ADB media session also supports playback controls - if self._adb_poller and self._adb_poller.last_info: - adb_info = self._adb_poller.last_info - if adb_info.playing or adb_info.paused: - return _BASE_FEATURES | _PLAYBACK_FEATURES + adb_info = self._manager.best_session(self.app_id) if self._manager else None + if adb_info and (adb_info.playing or adb_info.paused): + return _BASE_FEATURES | _PLAYBACK_FEATURES return _BASE_FEATURES # ── Metadata from Jellyfin ── @@ -615,11 +512,11 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): def _get_adb_now_playing(self) -> dict | None: """Build now-playing from ADB media session (non-Jellyfin apps).""" - if not self._adb_poller or not self._adb_poller.last_info: + info = self._manager.best_session(self.app_id) if self._manager else None + if not info: return None if not self._adb_store_info: # Privacy mode: show that something is playing but not what - info = self._adb_poller.last_info if info.playing or info.paused: return { "state": "playing" if info.playing else "paused", @@ -628,7 +525,6 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): } return None - info = self._adb_poller.last_info if not info.title: return None np = { @@ -660,13 +556,16 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): def _get_running_apps(self) -> list[dict] | None: """Build a list of running apps from ADB recent tasks + media sessions.""" - if not self._adb_poller or not self._running_apps: + if not self._manager: + return None + apps = self._manager.running_apps + if not apps: return None if not self._adb_store_info: return None return running_apps_from_tasks( - self._running_apps, - self._adb_poller.all_sessions, + apps, + self._manager.active_sessions, foreground_package=self.app_id, ) @@ -737,8 +636,8 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): jf = self._read(self._jellyfin_entity) jf_active = jf and jf.state in (STATE_PLAYING, STATE_PAUSED) - if self._adb_poller and self._adb_poller.connected and not jf_active: - success = await self._adb_poller.send_media_command(adb_command) + if self._manager and self._manager.connected and not jf_active: + success = await self._manager.send_media_command(adb_command) if success: # The ADB path served this command — the self-heal retry must # also go over ADB, not back to the remote. @@ -767,7 +666,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): async def async_bring_to_foreground(self, package: str) -> bool: """Bring a specific app to the foreground via ADB.""" - if not self._adb_poller: + if self._manager is None or not self._manager.connected: _LOGGER.warning("ADB not available for bring_to_foreground") return False - return await self._adb_poller.bring_to_foreground(package) + return await self._manager.bring_to_foreground(package)