diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index 3effd49..2afbeed 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -135,6 +135,28 @@ SERVICE_TO_STATE = { #: input selection and attribute enrichment — and is never switched here. RECEIVER_SWITCH_DOMAIN = "switch" +#: t63: the browse root's id and type — the FIX for the live report "Browse media +#: seems to spin and never finish loading". +#: +#: The id must be a real STRING. HA core never serves a node without one: 43 +#: ``BrowseMedia`` constructions across the core media_player platforms all pass a +#: string (a literal or module constant — arcam_fmj's "root", "apps", +#: ``BROWSE_ROOT``, ``TUNER_PRESETS_ROOT`` — or "" for a pure container); zero +#: pass ``None``. The media-browser frontend keys the node it is loading on that +#: id, so ``media_content_id=None`` left the dialog re-requesting the same node for +#: ever. "root" is arcam_fmj's own browse-root id, and every requested id — this +#: one, ``None``, or anything a stale frontend asks for — answers the same node, so +#: a repeat request is idempotent rather than a loop. +BROWSE_ROOT_ID = "root" + +#: The root's content type, and deliberately NOT ``MediaType.APPS``: core pairs +#: that type (with an "apps" id) with ``children=[…]`` — a folder OF apps — and this +#: node has no children at all, so the claim invites the frontend to treat an empty +#: node as an unloaded collection. No ``MediaType`` member means "a directory with +#: nothing in it", so the node carries the class it already declares, which is also +#: exactly what ``can_expand=False`` and ``children=[]`` say. +BROWSE_ROOT_TYPE = str(MediaClass.DIRECTORY) + _BASE_FEATURES = ( MediaPlayerEntityFeature.TURN_ON | MediaPlayerEntityFeature.TURN_OFF @@ -1444,7 +1466,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): # ── BrowseMedia app directory (P4-2c) ── async def async_browse_media(self, media_content_type=None, media_content_id=None): - """The media browser is EMPTY on purpose — apps live in the sources (t61). + """The media browser is EMPTY on purpose — and says so unambiguously. The user's report, verbatim: "Media Browser shows Apps. To me it should probably be integrated with the home assistant media display, keep @@ -1458,16 +1480,24 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): NO children rather than a second, inconsistent copy of the app list: ``BrowseMedia`` (never ``BrowseMediaSource``, which media_player has not exported since before t37 D1), the device as the root title, a directory - class, and no content id. There is no node to select, so no dead entry - can be offered and a click can never reach a row that cannot launch. An - integration with genuinely browsable media would publish it here; this - one has none. + class, and ``BROWSE_ROOT_ID``/``BROWSE_ROOT_TYPE``. There is no node to + select, so no dead entry can be offered and a click can never reach a row + that cannot launch. An integration with genuinely browsable media would + publish it here; this one has none. + + t63 — and the id is a real STRING, which is the fix for the next live + report, "Browse media seems to spin and never finish loading". This + method returns with no awaits, so the hang was in what it handed back: + a null ``media_content_id``. HA core never serves one (see + ``BROWSE_ROOT_ID``), because the frontend keys the node it is loading by + that id. Every requested id answers this same node, so a repeat request + is idempotent rather than a loop. """ return BrowseMedia( title=self._entry.title or "NVIDIA Shield", media_class=MediaClass.DIRECTORY, - media_content_type=MediaType.APPS, - media_content_id=None, + media_content_type=BROWSE_ROOT_TYPE, + media_content_id=BROWSE_ROOT_ID, can_play=False, can_expand=False, children=[], diff --git a/tests/test_command_routing.py b/tests/test_command_routing.py index d124264..44f65af 100644 --- a/tests/test_command_routing.py +++ b/tests/test_command_routing.py @@ -1459,10 +1459,10 @@ class TestFaithfulHaStubs: class TestBrowseMediaDirectory: - """t61: the HA media browser publishes NO media — apps are sources.""" + """t61/t63: the HA media browser publishes NO media — and says so unambiguously.""" - async def test_the_browser_is_an_empty_root(self, modules): - """The user's report: apps must not appear in the media browser.""" + async def test_the_browser_is_a_childless_root_with_a_real_id(self, modules): + """t63: the null id is what made the live browse dialog spin for ever.""" player, _s, _i = make_player(modules) player._entry.options["app_name_map"] = { "ar.tvplayer.tv": "TiviMate", @@ -1475,19 +1475,26 @@ class TestBrowseMediaDirectory: assert root.can_play is False assert root.can_expand is False # there is nothing to open assert root.title == "mdarcy" # the device, not a second app list + # t63: the id has to be a real STRING. HA core never serves a node without + # one, and the frontend keys the node it is loading on that id — a None id + # left the dialog re-requesting the same node for ever (the live report). + assert isinstance(root.media_content_id, str) and root.media_content_id + assert root.media_content_id == "root" + # ...and the type must not claim a collection this node does not have: + # core pairs MediaType.APPS with children=[...] — a folder OF apps. + assert root.media_content_type == "directory" # t37 D1: still a standard media_player.BrowseMedia root — explicit # class/type, NO media_source `domain` (that parameter belongs to # media_source.models.BrowseMediaSource, which also builds the # media_source:// ids and is not exportable from media_player at all). assert root.media_class == "directory" - assert root.media_content_type == "apps" - assert root.media_content_id is None # no node to select assert root.children_media_class is None assert not hasattr(root, "domain") # ...and it serialises into the payload the frontend consumes payload = root.as_dict() assert payload["children"] == [] assert payload["can_expand"] is False + assert payload["media_content_id"] == "root" async def test_apps_stay_reachable_through_the_sources(self, modules): """What the browser no longer offers, source_list/select_source still do.""" @@ -1506,21 +1513,27 @@ class TestBrowseMediaDirectory: await player.async_select_source("Netflix") assert protocol.launch_calls == ["com.netflix.ninja"] # still launchable + async def test_every_requested_id_answers_the_same_leaf(self, modules): + """A repeat request must be idempotent — not a loop the frontend can ride. - async def test_any_rebrowse_id_stays_a_valid_empty_root(self, modules): - """A frontend that re-browses an id cannot make this crash or lie.""" + The handler ignores both arguments, so the root id, ``None`` and a stale + child id all return the same node: the frontend cannot bounce between + "loading" and "loaded" for one id. + """ player, _s, _i = make_player(modules) + first = (await player.async_browse_media()).as_dict() + for content_type, content_id in ( (None, None), + ("apps", "root"), ("apps", "apps"), ("app", "ar.tvplayer.tv"), ("channel", "12"), ): - root = await player.async_browse_media(content_type, content_id) + repeat = await player.async_browse_media(content_type, content_id) - assert root.children == [] - assert root.media_content_id is None + assert repeat.as_dict() == first class DownAdbPoller(FakeAdbPoller): diff --git a/tests/test_ha_api_drift.py b/tests/test_ha_api_drift.py index 7b254eb..cb9163e 100644 --- a/tests/test_ha_api_drift.py +++ b/tests/test_ha_api_drift.py @@ -12,7 +12,9 @@ The sweep itself lives in ``tests/ha_api_drift.py`` (documented there). from __future__ import annotations import ast +import asyncio import importlib +import json import subprocess import sys import threading @@ -480,3 +482,137 @@ def test_removing_a_flow_display_owner_trips_the_call_argument_floor(monkeypatch assert any("call_args" in line for line in fewer.floor_violations()), ( fewer.floor_violations() ) + + +# --------------------------------------- t63: the media-browser root (real HA) +# +# The live report ("Browse media seems to spin and never finish loading") came +# from what async_browse_media HANDED BACK, not from the handler: our method +# returns with no awaits, so nothing inside it can hang. HA's handler only +# forwards the payload, which makes the defect visible on one path only: +# +# websocket_browse_media -> player.async_browse_media -> payload.as_dict() +# +# The frontend bundle is NOT installed here (no hass_frontend, and there is no +# browser to drive), so the rendered dialog itself cannot be exercised. What +# these tests prove is that backend path end to end, decorators included. The +# pre-fix payload carried ``media_content_id=None`` — a node shape no core +# integration ever serves, and the assertions below fail on it. + + +class _RecordingConnection: + """The pieces of websocket_api.ActiveConnection the browse handler calls.""" + + def __init__(self): + self.results: list = [] + self.messages: list = [] + self.exceptions: list = [] + + def send_result(self, msg_id, result): + self.results.append((msg_id, result)) + + def send_message(self, message): + self.messages.append(message) + + def async_handle_exception(self, msg, err): + self.exceptions.append(err) + + +def _wire_the_browser(entity): + """Give the entity's own hass what websocket_browse_media walks. + + ``hass.data[DATA_COMPONENT].get_entity`` is how the handler finds the player, + and ``async_create_background_task`` is how ``@websocket_api.async_response`` + schedules the real coroutine — both real shapes, so the DECORATED handler + runs unmodified. + """ + from homeassistant.components.media_player import DATA_COMPONENT + + scheduled: list = [] + + def async_create_background_task(coro, name, *, eager_start=False): + task = asyncio.create_task(coro) + scheduled.append(task) + return task + + entity.hass.data[DATA_COMPONENT] = SimpleNamespace( + get_entity=lambda entity_id: entity if entity_id == entity.entity_id else None + ) + entity.hass.async_create_background_task = async_create_background_task + return scheduled + + +async def _browse_over_the_real_handler(entity, scheduled, *, content_id=None, + content_type=None): + """Send the real websocket command and await the task it schedules.""" + from homeassistant.components.media_player import websocket_browse_media + + connection = _RecordingConnection() + msg = {"id": 7, "type": "media_player/browse_media", "entity_id": entity.entity_id} + if content_id is not None or content_type is not None: + msg["media_content_id"] = content_id + msg["media_content_type"] = content_type + + websocket_browse_media(entity.hass, connection, msg) # schedules the coroutine + await asyncio.gather(*scheduled) + + assert connection.exceptions == [], connection.exceptions + assert connection.messages == [], connection.messages + assert len(connection.results) == 1, connection.results + return connection.results[0][1] + + +async def test_the_real_websocket_handler_serves_a_string_id_root(): + """t63 regression: the browse root carries a real STRING id, not None. + + Every core media_player platform serves a string id (a literal, a module + constant or ""), because the frontend keys the node it is loading on that id; + the live dialog re-requested the same node for ever with a null one. + """ + _require_installed_homeassistant() + + entity, _states = _real_ha_entity( + "media_player", "NvidiaShieldMediaPlayer", entity_id="media_player.shield" + ) + scheduled = _wire_the_browser(entity) + + payload = await _browse_over_the_real_handler(entity, scheduled) + + assert isinstance(payload["media_content_id"], str) + assert payload["media_content_id"] == "root" + assert payload["media_content_type"] == "directory" # not MediaType.APPS + assert payload["children"] == [] + assert payload["can_expand"] is False + assert payload["can_play"] is False + assert payload["media_class"] == "directory" + # the payload the frontend receives has to survive the websocket's own encode + assert json.loads(json.dumps(payload)) == payload + + +async def test_the_real_handler_answers_every_id_with_the_same_root(): + """Idempotent: the root id, None and a stale child id give ONE payload. + + That is the property that stops a frontend bouncing between "loading" and + "loaded" for one id — browse(root_id) must be browse(None) in shape. + """ + _require_installed_homeassistant() + + entity, _states = _real_ha_entity( + "media_player", "NvidiaShieldMediaPlayer", entity_id="media_player.shield" + ) + scheduled = _wire_the_browser(entity) + + first = await _browse_over_the_real_handler(entity, scheduled) + root_id = first["media_content_id"] + + for content_type, content_id in ( + (None, None), + ("apps", root_id), + ("apps", "apps"), + ("app", "ar.tvplayer.tv"), + ): + repeat = await _browse_over_the_real_handler( + entity, scheduled, content_id=content_id, content_type=content_type + ) + + assert repeat == first