diff --git a/README.md b/README.md index ae8ccbb..9c16db2 100644 --- a/README.md +++ b/README.md @@ -352,8 +352,15 @@ list). A malformed row — no `=`, or an empty name — is rejected with an erro page rather than being half-saved. A receiver may be a **`switch.*`** as well as a `media_player.*`: a soundbar or -speaker that only knows on and off is a valid receiver. (Driving that switch during -playback is a separate, later change.) +speaker that only knows on and off is a valid receiver. Playback then **ensures +that switch is on**: `turn_on`, `play_media` and `select_source` call +`switch.turn_on` when — and only when — it reports `off`, and touch nothing else +about it (never volume, never a toggle). An already-on switch gets no call at all, +a switch whose state cannot be read (`unavailable`/`unknown`) is left alone rather +than guessed at, and a receiver that is off while the Shield is idle stays off. +That is what makes receiver **volume scripts unnecessary** for this shape: there is +no volume to script, only an on/off state to ensure. A `media_player.*` receiver +keeps its own behaviour — input selection and attribute enrichment. - **Jellyfin Media Player** — auto-detected or manually selected - **Receiver + Input** — for media-zone coordination; `media_player` or `switch` @@ -497,13 +504,22 @@ sweep could not run), while the Configure row forces one. ### Sources (the app list in a dropdown) -The media player's source list is your **Apps** map — the same map the Browse tree and -app-name resolution use. With no map configured it falls back to a small built-in list +The media player's source list is your **Apps** map — the same map app-name resolution +uses. With no map configured it falls back to a small built-in list (TiviMate, Wholphin, Netflix, YouTube), so a universal-media-player card always has something to show. Discovery fills the **Configure** table, not this fallback: the dropdown stays exactly the map you saved, so what a card offers is always what you chose. An app that discovery found and you want in the dropdown is one row to add. +**Apps are sources, not browser media.** This integration no longer publishes the app +directory through HA's media **Browser**: the Browse dialog shows an **empty tree** for +this entity (the device name, no rows) instead of a second, inconsistent copy of the +same list — the user report was *"Media Browser shows Apps. To me it should probably be +integrated with the home assistant media display, keep select sources for apps"*. +Everything an app row could do lives in `source_list` and `media_player.select_source` +below, and nothing else on this entity is browsable, so there is no dead entry to click +and nothing else the browser could show. + ```yaml # a media-player card, or anything that reads source_list type: media-control @@ -516,7 +532,7 @@ entity: media_player.living_room_shield sensor: your map first, then the cached device label, then the raw package. - **`media_player.select_source`** — picks one **by name** and launches it over the protocol, the same path `play_media` uses for apps. A raw package id works too (that - is what the Browse tree publishes), names match case-insensitively, and a name that + holds the package id), names match case-insensitively, and a name that is not in the list is an error rather than a silent no-op: ```yaml @@ -537,10 +553,9 @@ The map is your own override, so two shapes are accepted rather than papered ove - **Names differing only by case** (`com.a=Dup`, `com.b=dup`) offer **one** dropdown row — the first spelling — because name matching is case-insensitive. The shadowed - app is not lost: the Browse tree still lists it under its own package id, and - `media_player.select_source` accepts that id. + app is not lost: `media_player.select_source` accepts its own package id. - A **padded value** typed by hand (`com.x= Netflix `) is shown cleaned in the - dropdown and the Browse tree, but `app_name`, `source` and the Current App sensor + dropdown, but `app_name`, `source` and the Current App sensor display the map value **verbatim** — so `source` can be a string that is not in `source_list`. A blank value falls through to the raw package instead. diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index a7fb844..3effd49 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -78,9 +78,9 @@ from homeassistant.components.media_player import ( MediaType, ) from homeassistant.config_entries import ConfigEntry -from homeassistant.const import STATE_PAUSED, STATE_PLAYING +from homeassistant.const import STATE_OFF, STATE_ON, STATE_PAUSED, STATE_PLAYING from homeassistant.core import CALLBACK_TYPE, Event, HomeAssistant, callback -from homeassistant.exceptions import ServiceValidationError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError 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 @@ -130,6 +130,11 @@ SERVICE_TO_STATE = { "media_stop": MediaPlayerState.IDLE, } +#: t61: the domain a receiver must be in for playback to ENSURE it is on. A +#: receiver in any other domain (an AVR media_player) keeps its own semantics — +#: input selection and attribute enrichment — and is never switched here. +RECEIVER_SWITCH_DOMAIN = "switch" + _BASE_FEATURES = ( MediaPlayerEntityFeature.TURN_ON | MediaPlayerEntityFeature.TURN_OFF @@ -828,8 +833,9 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): def _app_directory(self) -> dict: """Launchable apps: the user's map, else the curated default (P4-1). - ONE source for the browse tree, the source list and the reverse lookup, so - the dropdown can never offer a name the launch path cannot resolve. + ONE source for the source list and the reverse lookup (t61 removed the + browse tree that used to be the third consumer), so the dropdown can + never offer a name the launch path cannot resolve. The map is hand-editable, so it is NORMALIZED here — once, so that every consumer sees the same thing (t51): @@ -839,9 +845,10 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): "TiviMate", instead of being shown padded and missing every lookup; * an entry whose name — or whose package — is blank/whitespace-only is dropped, so no surface can offer a child with no package behind it or a - name no launch can reach (the t48 invariant, extended to the tree); - * two packages normalizing to the same name are BOTH KEPT — the browse - tree reaches each by its own package id, so neither app is hidden — while + name no launch can reach (the t48 invariant, extended to the table); + * two packages normalizing to the same name are BOTH KEPT — each stays + reachable by its own package id, which ``select_source`` accepts (t61: + the browse tree that used to publish those ids is gone) — while the FIRST in map order wins the name: that is the single entry source_list offers and the package select_source resolves. Map order is the user's order, so the winner does not move between rebuilds. "The same @@ -870,8 +877,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): Directory order (the user's map order, or the curated list), so rebuilding the page does not shuffle it. Blanks are already gone and names already stripped by ``_app_directory`` (t49/t51) — the normalization lives in that - one accessor so the dropdown, the browse tree and the reverse lookup cannot - disagree. + one accessor so the dropdown and the reverse lookup cannot disagree. Each name is offered ONCE, compared CASE-INSENSITIVELY (t53), keeping the first spelling the user typed. Exact duplicates were already collapsed, but @@ -879,8 +885,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): reverse lookup — which is case-insensitive by design (t47) — resolved BOTH of them to the first entry, so the second row launched the wrong app. One row per name means a row can only launch the app it shows. The shadowed - package is not lost: the browse tree still lists it under its own id, and - select_source accepts that id. + package is not lost: ``select_source`` accepts its own package id. """ seen: set[str] = set() offered: list[str] = [] @@ -1212,8 +1217,67 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): self.hass, self._entry.options, command, variables ) + async def _ensure_receiver_on(self) -> None: + """Turn a SWITCH receiver on before playback — only when it is off (t61). + + The user's request, verbatim: "it would be nice if a receiver could just + be a switch, I don't need my volume scripts then because the receiver + portion could just make sure the receiver is on if it's a switch". So a + ``switch.*`` receiver is ENSURED ON by the playback entry points + (:meth:`async_turn_on`, :meth:`async_play_media`, + :meth:`async_select_source`) and nothing else happens to it: no volume, + no toggle, no other switch semantics. + + Deliberately narrow, in four ways: + + * only a ``switch.*`` receiver is touched — a ``media_player.*`` + receiver keeps its behaviour exactly; + * only when the switch reports ``off``. An ``on`` switch issues no + service call at all, so this is idempotent — a state write, a poll or + a second entry point in the same action never re-calls it. A state we + cannot read (missing entity, ``unavailable``, ``unknown``) is left + alone rather than guessed at, with a debug line saying so; + * never called from a state read, a poll, a listener or attribute + rendering, so a receiver that is off while the Shield is idle stays + off — this is a playback action, not a synchronisation; + * a service failure (the entity vanished between the read and the call) + is logged, not raised: a receiver that cannot be switched on must not + stop the Shield from playing. + """ + entity_id = self._receiver_entity + if not entity_id or not entity_id.startswith(f"{RECEIVER_SWITCH_DOMAIN}."): + return + state = self._read(entity_id) + if state is None: + _LOGGER.debug( + "receiver %s is not in the state machine — not switching it", + entity_id, + ) + return + if state.state != STATE_OFF: + if state.state != STATE_ON: + _LOGGER.debug( + "receiver %s reports %r — not switching it", entity_id, state.state + ) + return + try: + await self.hass.services.async_call( + RECEIVER_SWITCH_DOMAIN, + "turn_on", + {"entity_id": entity_id}, + blocking=True, + ) + except HomeAssistantError as err: + _LOGGER.debug("receiver %s could not be turned on: %s", entity_id, err) + async def async_turn_on(self): - """Wake the Shield (only when it reports OFF) — or run the turn_on script.""" + """Wake the Shield (only when it reports OFF) — or run the turn_on script. + + A ``switch.*`` receiver is ensured on first (t61): the receiver is where + the Shield's sound comes out, so a playback entry point that wakes the + Shield wakes it too — and only when it is actually off. + """ + await self._ensure_receiver_on() if await self._script_served("turn_on"): return await self._power_key(send_when_on=False) @@ -1267,8 +1331,9 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): if not wanted: return None if wanted in directory: - # an automation may hand us the package directly; that is what the - # browse tree publishes as media_content_id + # an automation may hand us the package directly; that is the id + # this table is keyed by (t61: the browse tree that used to publish + # it is gone, the id itself is unchanged) return wanted folded = wanted.casefold() for package, name in directory.items(): @@ -1285,7 +1350,12 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): directory the source list came from and launched over the shared ``play_media("app", …)`` path, so a universal-media-player replacement gets the native behaviour. An unknown name is an error, never a silent no-op. + + A ``switch.*`` receiver is ensured on first (t61), before the script: a + source selection is the start of playback, and it dispatches internally + so the switch is looked at exactly once per user action. """ + await self._ensure_receiver_on() if await self._script_served("select_source", {SCRIPT_VAR_APP_NAME: source}): return package = self._source_package(source) @@ -1295,7 +1365,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): translation_key="unknown_source", translation_placeholders={"source": str(source)}, ) - await self.async_play_media(MediaType.APP, package) + await self._dispatch_play_media(MediaType.APP, package) async def _remote_key_cmd(self, remote_key: str, adb_key: str) -> None: """Simple key command without intent bookkeeping: protocol → ADB keyevent.""" @@ -1374,45 +1444,48 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): # ── BrowseMedia app directory (P4-2c) ── async def async_browse_media(self, media_content_type=None, media_content_id=None): - """Root lists the apps map (or the curated default) → play_media(app). + """The media browser is EMPTY on purpose — apps live in the sources (t61). - t37 D1: the standard media_player tree, exactly as core's own - androidtv_remote/media_player.py builds it — ``BrowseMedia`` from - homeassistant.components.media_player, every node carrying an explicit - media_content_id/media_content_type, and the directory naming the - class of its children. ``BrowseMediaSource`` only ever existed in - homeassistant.components.media_source.models (t30 G18's domain - parameter is ITS contract: it builds ``media_source://`` ids), - so an app-directory node must never claim one — and the import above - removed a name media_player has not exported for a long time, which - made every real browse raise. + The user's report, verbatim: "Media Browser shows Apps. To me it should + probably be integrated with the home assistant media display, keep + select sources for apps". They are right about the surface: HA's media + browser is for BROWSABLE media, while those rows are SOURCES. They stay + where a source belongs — ``source_list``/``source``, the source + dropdown, ``select_source`` and ``play_media("app", …)`` — and that + offer is complete. + + So this returns a valid, standard ``media_player.BrowseMedia`` root with + 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. """ - app_map = self._app_directory - children = [ - BrowseMedia( - title=name, - media_class=MediaClass.APP, - media_content_type=MediaType.APP, - media_content_id=pkg, - can_play=True, - can_expand=False, - ) - for pkg, name in app_map.items() - ] return BrowseMedia( - title="Apps", + title=self._entry.title or "NVIDIA Shield", media_class=MediaClass.DIRECTORY, media_content_type=MediaType.APPS, - media_content_id="apps", - children_media_class=MediaClass.APP, + media_content_id=None, can_play=False, - can_expand=True, - children=children, + can_expand=False, + children=[], ) # ── play_media (§7): channel digits, app/url launch ── async def async_play_media(self, media_type: str, media_id: str) -> None: + """HA's ``play_media`` entry point — ensure the receiver, then dispatch. + + The receiver ensure (t61) lives on the PUBLIC path only, so an internal + caller (:meth:`async_select_source`) dispatches without a second look at + the same switch inside one user action. + """ + await self._ensure_receiver_on() + await self._dispatch_play_media(media_type, media_id) + + async def _dispatch_play_media(self, media_type: str, media_id: str) -> None: """Channel selection by digit number; app/url launches (C-LAUNCH). Channel path replicates HA core androidtv_remote exactly: digits only, diff --git a/custom_components/nvidia_shield/strings.json b/custom_components/nvidia_shield/strings.json index dcf0f8b..adb182e 100644 --- a/custom_components/nvidia_shield/strings.json +++ b/custom_components/nvidia_shield/strings.json @@ -53,7 +53,7 @@ }, "receiver": { "title": "Receiver", - "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch.", + "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch. Playback turns a switch receiver on when it is off.", "data": { "receiver_entity": "Receiver", "receiver_input": "Receiver input" @@ -169,7 +169,7 @@ }, "data_description": { "jellyfin_entity": "The Jellyfin media_player for this Shield. Stream details come from the HA Jellyfin integration — no extra setup needed.", - "receiver_entity": "The receiver or soundbar this Shield plays through.", + "receiver_entity": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch that playback turns on when it is off.", "receiver_input": "The receiver input the Shield uses, for example 'Shield'.", "power_entity": "A smart plug switch, if you want reliable on/off detection.", "adb_enabled": "Reads now-playing information over ADB from any app. Requires Developer options > Network debugging on the Shield.", @@ -221,7 +221,7 @@ }, "receiver": { "title": "Receiver", - "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch for a soundbar that only turns on and off.", + "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch for a soundbar that only turns on and off. Playback turns a switch receiver on when it is off, so no receiver volume scripts are needed for that.", "data": { "receiver_entity": "Receiver", "receiver_input": "Receiver input" diff --git a/custom_components/nvidia_shield/translations/en.json b/custom_components/nvidia_shield/translations/en.json index dcf0f8b..adb182e 100644 --- a/custom_components/nvidia_shield/translations/en.json +++ b/custom_components/nvidia_shield/translations/en.json @@ -53,7 +53,7 @@ }, "receiver": { "title": "Receiver", - "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch.", + "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch. Playback turns a switch receiver on when it is off.", "data": { "receiver_entity": "Receiver", "receiver_input": "Receiver input" @@ -169,7 +169,7 @@ }, "data_description": { "jellyfin_entity": "The Jellyfin media_player for this Shield. Stream details come from the HA Jellyfin integration — no extra setup needed.", - "receiver_entity": "The receiver or soundbar this Shield plays through.", + "receiver_entity": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch that playback turns on when it is off.", "receiver_input": "The receiver input the Shield uses, for example 'Shield'.", "power_entity": "A smart plug switch, if you want reliable on/off detection.", "adb_enabled": "Reads now-playing information over ADB from any app. Requires Developer options > Network debugging on the Shield.", @@ -221,7 +221,7 @@ }, "receiver": { "title": "Receiver", - "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch for a soundbar that only turns on and off.", + "description": "The receiver or soundbar this Shield plays through — a media_player, or a plain switch for a soundbar that only turns on and off. Playback turns a switch receiver on when it is off, so no receiver volume scripts are needed for that.", "data": { "receiver_entity": "Receiver", "receiver_input": "Receiver input" diff --git a/tests/entity_stublib.py b/tests/entity_stublib.py index 879e507..5bd9a40 100644 --- a/tests/entity_stublib.py +++ b/tests/entity_stublib.py @@ -398,6 +398,10 @@ def install() -> None: ha_const.EVENT_HOMEASSISTANT_STOP = "homeassistant_stop" ha_const.STATE_PLAYING = "playing" ha_const.STATE_PAUSED = "paused" + # t61: a receiver switch is read and switched by the media player, so the + # two switch states have to exist with their real values. + ha_const.STATE_ON = "on" + ha_const.STATE_OFF = "off" ha_const.Platform = types.SimpleNamespace( MEDIA_PLAYER="media_player", SENSOR="sensor", @@ -609,8 +613,43 @@ def load_entity_modules() -> dict: } +class FakeState: + """The fields entity code actually reads off an HA ``State`` (t61).""" + + def __init__(self, entity_id, state, attributes=None): + self.entity_id = entity_id + self.state = state + self.attributes = dict(attributes or {}) + + +class FakeStates: + """A dict-backed stand-in for HA's StateMachine (t61). + + Only the read shape entities use: ``get(entity_id)`` returns the state + object, or None for an entity that does not exist — exactly what the real + machine returns. ``set`` is a test's way of giving an entity a state. + """ + + def __init__(self): + self._states: dict = {} + + def set(self, entity_id, state, attributes=None): + self._states[entity_id] = FakeState(entity_id, state, attributes) + + def get(self, entity_id): + return self._states.get(entity_id) + + class FakeHass: - """Sized to entity unit tests: background tasks + no-op call_later.""" + """Sized to entity unit tests: background tasks + no-op call_later. + + ``states`` is the real HomeAssistant shape (every hass has one), so entity + code that reads a linked entity works unmodified; it is empty until a test + sets something. + """ + + def __init__(self): + self.states = FakeStates() def async_create_task(self, coro): return asyncio.create_task(coro) diff --git a/tests/test_command_routing.py b/tests/test_command_routing.py index 00407ee..d124264 100644 --- a/tests/test_command_routing.py +++ b/tests/test_command_routing.py @@ -118,7 +118,11 @@ def make_player(modules, manager=None, jellyfin_entity=None, options=None): player._entity_tasks = set() player._channel_task = None player.async_write_ha_state = lambda: None - player._read = lambda eid: None + # t61: FakeHass carries the real `states` shape, so the entity's own `_read` + # is the one under test — an entity no test set still reads as None. + # t61: `__init__` gives the entity an (empty) power-sensor map; the harness + # mirrors it so the attribute payload can be read here too. + player._power_sensors = {} # t41: nothing may leave this integration, so an HA service call is now the # failure signal rather than a supported tier. `served` stays empty unless a @@ -1455,9 +1459,10 @@ class TestFaithfulHaStubs: class TestBrowseMediaDirectory: - """P4-2c: the app directory root and its launch path.""" + """t61: the HA media browser publishes NO media — apps are sources.""" - async def test_root_lists_the_options_map(self, modules): + async def test_the_browser_is_an_empty_root(self, modules): + """The user's report: apps must not appear in the media browser.""" player, _s, _i = make_player(modules) player._entry.options["app_name_map"] = { "ar.tvplayer.tv": "TiviMate", @@ -1466,53 +1471,56 @@ class TestBrowseMediaDirectory: root = await player.async_browse_media() - assert root.title == "Apps" - assert root.can_play is False and root.can_expand is True - ids = [(c.media_content_id, c.title) for c in root.children] - assert ids == [ - ("ar.tvplayer.tv", "TiviMate"), - ("com.netflix.ninja", "Netflix"), - ] - assert all(c.media_content_type == "app" for c in root.children) - # t37 D1: a standard media_player.BrowseMedia tree — explicit - # media_content_id/-type at every level, the directory naming the class - # of its children, and NO media_source `domain` (that parameter belongs - # to media_source.models.BrowseMediaSource, which also builds the - # media_source:// ids). + assert root.children == [] # not one app row + 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 + # 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_id == "apps" assert root.media_content_type == "apps" - assert root.children_media_class == "app" + assert root.media_content_id is None # no node to select + assert root.children_media_class is None assert not hasattr(root, "domain") - assert [c.media_class for c in root.children] == ["app", "app"] # ...and it serialises into the payload the frontend consumes payload = root.as_dict() - assert payload["media_content_id"] == "apps" - assert payload["children_media_class"] == "app" - assert payload["children"][0]["media_content_id"] == "ar.tvplayer.tv" - assert payload["children"][0]["media_content_type"] == "app" - assert payload["children"][0]["can_play"] is True + assert payload["children"] == [] + assert payload["can_expand"] is False - async def test_root_falls_back_to_the_curated_default(self, modules): - player, _s, _i = make_player(modules) # no apps map configured - - root = await player.async_browse_media() - - ids = [c.media_content_id for c in root.children] - assert "ar.tvplayer.tv" in ids # TiviMate - assert "com.github.damontecres.wholphin" in ids # Wholphin - assert "com.netflix.ninja" in ids # Netflix - assert "com.google.android.youtube.tv" in ids # YouTube - - async def test_selection_launches_via_play_media_app(self, modules): + async def test_apps_stay_reachable_through_the_sources(self, modules): + """What the browser no longer offers, source_list/select_source still do.""" player, _s, _i = make_player(modules) - protocol = player._manager._protocol + player._entry.options["app_name_map"] = { + "ar.tvplayer.tv": "TiviMate", + "com.netflix.ninja": "Netflix", + } + root = await player.async_browse_media() - selected = root.children[0] - await player.async_play_media("app", selected.media_content_id) + assert root.children == [] + assert player.source_list == ["TiviMate", "Netflix"] - assert protocol.launch_calls == [selected.media_content_id] # C-LAUNCH + protocol = player._manager._protocol + await player.async_select_source("Netflix") + + assert protocol.launch_calls == ["com.netflix.ninja"] # still launchable + + 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.""" + player, _s, _i = make_player(modules) + + for content_type, content_id in ( + (None, None), + ("apps", "apps"), + ("app", "ar.tvplayer.tv"), + ("channel", "12"), + ): + root = await player.async_browse_media(content_type, content_id) + + assert root.children == [] + assert root.media_content_id is None class DownAdbPoller(FakeAdbPoller): diff --git a/tests/test_media_sources.py b/tests/test_media_sources.py index 2414681..329d438 100644 --- a/tests/test_media_sources.py +++ b/tests/test_media_sources.py @@ -78,17 +78,20 @@ class TestSourceList: assert player.source_list == ["TiviMate", "Netflix"] assert "" not in player.source_list - async def test_the_list_matches_the_browse_titles_exactly(self, modules): - """Same accessor, same order, same bytes — a blank entry excepted (t49).""" + def test_the_list_is_the_normalized_directory_exactly(self, modules): + """Same accessor, same order, same bytes — a blank entry excepted (t49). + + t61 removed the browse tree that used to be the second consumer of this + one accessor; the dropdown and the reverse lookup are still fed from it, + so a row can only launch the app it shows. + """ for app_map in ( None, # the curated default {"ar.tvplayer.tv": "TiviMate", "com.netflix.ninja": "Netflix"}, ): player, _served = _player(modules, app_map=app_map) - root = await player.async_browse_media() - - assert player.source_list == [child.title for child in root.children] + assert player.source_list == list(player._app_directory.values()) async def test_the_survivors_still_resolve_and_launch(self, modules): """The filter must not cost a resolvable name (t49).""" @@ -279,6 +282,11 @@ class TestNormalizedDirectories: accessor (strip names and keys, drop blanks, first-in-map-order wins a shared name) and is pinned here, together with the promise that a well-formed map is untouched. + + t61 left two surfaces instead of three: the Browse tree that used to be the + second view is gone (apps are sources), so the pins below anchor the same + policy on `_app_directory` — the shared accessor itself — and on + `source_list`/`_source_package`, which are what a user can still reach. """ BLANKED = { @@ -288,25 +296,21 @@ class TestNormalizedDirectories: "": "Ghost", } - async def test_a_blank_entry_never_reaches_the_browse_tree(self, modules): - """The Browse tree and the dropdown are the same, resolved offer.""" + def test_a_blank_entry_never_reaches_the_dropdown(self, modules): + """Blanks are dropped once, in the accessor every offer reads (t49).""" player, _served = _player(modules, app_map=dict(self.BLANKED)) - root = await player.async_browse_media() - - assert [child.title for child in root.children] == ["TiviMate"] - assert [child.media_content_id for child in root.children] == [TIVIMATE] + assert list(player._app_directory) == [TIVIMATE] + assert list(player._app_directory.values()) == ["TiviMate"] assert player.source_list == ["TiviMate"] - async def test_a_blank_package_is_dropped_too(self, modules): - """An entry with no package is a child that cannot be launched.""" + def test_a_blank_package_is_dropped_too(self, modules): + """An entry with no package is a row that cannot be launched.""" player, _served = _player( modules, app_map={"": "Ghost", " ": "Ghost 2", TIVIMATE: "TiviMate"} ) - root = await player.async_browse_media() - - assert [child.media_content_id for child in root.children] == [TIVIMATE] + assert list(player._app_directory) == [TIVIMATE] assert player.source_list == ["TiviMate"] async def test_a_padded_name_is_stripped_once_and_then_resolves(self, modules): @@ -315,11 +319,9 @@ class TestNormalizedDirectories: app_map={TIVIMATE: " TiviMate ", "com.netflix.ninja": "Netflix"}, ) - root = await player.async_browse_media() - # display and match key are one string, everywhere assert player.source_list == ["TiviMate", "Netflix"] - assert [child.title for child in root.children] == ["TiviMate", "Netflix"] + assert list(player._app_directory.values()) == ["TiviMate", "Netflix"] # ...so every spelling the user may type finds the package assert player._source_package("TiviMate") == TIVIMATE assert player._source_package(" TiviMate ") == TIVIMATE @@ -335,13 +337,11 @@ class TestNormalizedDirectories: modules, app_map={"com.a": "Netflix", "com.b": " Netflix "} ) - root = await player.async_browse_media() - - # both are still offered in the tree, each by its own package id - assert [(c.media_content_id, c.title) for c in root.children] == [ - ("com.a", "Netflix"), - ("com.b", "Netflix"), - ] + # t61: the directory keeps BOTH entries, each under its own package id; + # the removal of the browse tree does not remove the reachability it + # used to expose — select_source-by-id is that surface now. + assert player._app_directory == {"com.a": "Netflix", "com.b": "Netflix"} + assert player._source_package("com.b") == "com.b" # the shadowed package # the dropdown offers the name once, and resolves to the first entry assert player.source_list == ["Netflix"] assert player._source_package("Netflix") == "com.a" @@ -363,10 +363,11 @@ class TestNormalizedDirectories: modules, app_map={TIVIMATE: "TiviMate Pro", "com.netflix.ninja": "Netflix"} ) - root = await player.async_browse_media() - + assert player._app_directory == { + TIVIMATE: "TiviMate Pro", + "com.netflix.ninja": "Netflix", + } assert player.source_list == ["TiviMate Pro", "Netflix"] - assert [child.title for child in root.children] == ["TiviMate Pro", "Netflix"] assert player._source_package("TiviMate Pro") == TIVIMATE @@ -386,12 +387,11 @@ class TestCaseInsensitiveNames: # one row, keeping the first spelling the user typed assert player.source_list == ["Dup"] - # ...and the Browse tree still holds both apps, each under its own id - root = await player.async_browse_media() - assert [(c.media_content_id, c.title) for c in root.children] == [ - ("com.a", "Dup"), - ("com.b", "dup"), - ] + # ...and the directory still holds both apps, each under its own id, so + # the shadowed one stays reachable by its package id (t61: that is what + # the removed browse tree used to publish) + assert player._app_directory == {"com.a": "Dup", "com.b": "dup"} + assert player._source_package("com.b") == "com.b" async def test_the_offered_row_launches_the_app_it_shows(self, modules): player, _served = _player(modules, app_map={"com.a": "Dup", "com.b": "dup"}) diff --git a/tests/test_receiver_switch.py b/tests/test_receiver_switch.py new file mode 100644 index 0000000..a045d0a --- /dev/null +++ b/tests/test_receiver_switch.py @@ -0,0 +1,180 @@ +"""tests/test_receiver_switch.py — a plain switch as the Receiver (t61). + +The user's request, verbatim: "it would be nice if a receiver could just be a +switch, I don't need my volume scripts then because the receiver portion could +just make sure the receiver is on if it's a switch". So the playback entry +points — ``turn_on``, ``play_media`` and ``select_source`` — ensure a +``switch.*`` receiver is ON, and only when it is actually off. + +The four states the contract names are all pinned here: + +* switch off + playback starts → exactly one ``switch.turn_on``; +* switch already on → NO call (idempotent, not a toggle); +* switch unavailable/unknown/missing/failing → playback continues, nothing raises; +* a ``media_player.*`` receiver → never switched; its behaviour is unchanged. + +No real Shield, no real switch and no browser take part: the entity comes from +the same stub harness every other entity test uses, the receiver's state is a +value in ``FakeStates`` and the service call is recorded. +""" + +import entity_stublib +import pytest +from nsh.const import CONF_APP_NAME_MAP, CONF_RECEIVER_ENTITY +from test_command_routing import make_player + +TIVIMATE = "ar.tvplayer.tv" +SOUNDBAR = "switch.soundbar" +AVR = "media_player.avr" + + +@pytest.fixture(name="modules", autouse=True) +def scoped_modules(): + """Stub scope + loaded entity modules for one test (t7 discipline).""" + snapshot = entity_stublib.snapshot_modules() + entity_stublib.install() + try: + yield entity_stublib.load_entity_modules() + finally: + entity_stublib.restore_modules(snapshot) + + +def _player(modules, *, receiver=SOUNDBAR, state=None, app_map=None): + """A player with a Receiver configured (a switch unless told otherwise).""" + player, served, _intents = make_player(modules) + player._entry.options[CONF_RECEIVER_ENTITY] = receiver + if app_map is not None: + player._entry.options[CONF_APP_NAME_MAP] = app_map + if state is not None: + player.hass.states.set(receiver, state) + return player, served + + +class TestTheSwitchReceiverIsEnsuredOn: + """Every playback entry point turns an off switch receiver on — once.""" + + async def test_play_media_turns_an_off_receiver_on(self, modules): + player, served = _player(modules, state="off") + + await player.async_play_media("app", TIVIMATE) + + # the ensure is the ONLY service call: no volume, no toggle, and the + # app still launches (which is why the volume scripts are unnecessary) + assert served == [("switch", "turn_on", {"entity_id": SOUNDBAR})] + assert player._manager._protocol.launch_calls == [TIVIMATE] + + async def test_turn_on_turns_an_off_receiver_on(self, modules): + player, served = _player(modules, state="off") + + await player.async_turn_on() + + assert served == [("switch", "turn_on", {"entity_id": SOUNDBAR})] + + async def test_select_source_turns_an_off_receiver_on_once(self, modules): + """select_source dispatches internally, so the switch is read once.""" + player, served = _player( + modules, state="off", app_map={TIVIMATE: "TiviMate"} + ) + + await player.async_select_source("TiviMate") + + assert served == [("switch", "turn_on", {"entity_id": SOUNDBAR})] + assert player._manager._protocol.launch_calls == [TIVIMATE] + + async def test_an_on_receiver_is_never_called(self, modules): + """Idempotent: every entry point may run, and NOT ONE call goes out.""" + player, served = _player(modules, state="on") + + await player.async_turn_on() + await player.async_play_media("app", TIVIMATE) + await player.async_select_source("TiviMate") + + assert served == [] + + async def test_no_volume_service_is_ever_aimed_at_the_receiver(self, modules): + """The user's point: a switch receiver means no volume scripts.""" + player, served = _player(modules, state="off") + + await player.async_play_media("app", TIVIMATE) + + assert [(domain, service) for domain, service, _data in served] == [ + ("switch", "turn_on") + ] + + +class TestAnUnusableReceiverNeverBreaksPlayback: + """A receiver we cannot read, or that fails, is logged — never raised.""" + + async def test_an_unavailable_switch_is_left_alone(self, modules): + player, served = _player(modules, state="unavailable") + + await player.async_play_media("app", TIVIMATE) + + assert served == [] # never guessed at + assert player._manager._protocol.launch_calls == [TIVIMATE] # still plays + + async def test_an_unknown_state_is_not_guessed_at(self, modules): + player, served = _player(modules, state="unknown") + + await player.async_turn_on() + + assert served == [] + + async def test_a_missing_entity_is_left_alone(self, modules): + player, served = _player(modules) # the receiver is not in the machine + + await player.async_play_media("app", TIVIMATE) + + assert served == [] + assert player._manager._protocol.launch_calls == [TIVIMATE] + + async def test_a_failing_service_call_does_not_stop_playback(self, modules): + from homeassistant.exceptions import HomeAssistantError + + player, _served = _player(modules, state="off") + + class _BrokenServices: + async def async_call(self, domain, service, data=None, **kwargs): + raise HomeAssistantError("the switch entity is gone") + + player.hass.services = _BrokenServices() + + await player.async_play_media("app", TIVIMATE) # must not raise + + assert player._manager._protocol.launch_calls == [TIVIMATE] + + +class TestAMediaPlayerReceiverIsUntouched: + """An AVR stays an AVR: input selection and enrichment, never a switch call.""" + + async def test_a_media_player_receiver_is_never_switched(self, modules): + player, served = _player(modules, receiver=AVR, state="off") + player.hass.states.set(AVR, "idle") + + await player.async_turn_on() + await player.async_play_media("app", TIVIMATE) + + assert served == [] + assert player._manager._protocol.launch_calls == [TIVIMATE] + + async def test_the_receiver_option_still_reaches_the_attributes(self, modules): + from nsh.const import ATTR_MEDIA_DAG_RECEIVER_ENTITY + + player, _served = _player(modules, receiver=AVR) + + assert ( + player.extra_state_attributes[ATTR_MEDIA_DAG_RECEIVER_ENTITY] == AVR + ) + + +class TestAnIdleShieldLeavesTheReceiverAlone: + """Switching the receiver on is a PLAYBACK action, not a synchronisation.""" + + async def test_reading_state_and_attributes_calls_nothing(self, modules): + player, served = _player(modules, state="off") + + assert player.state is not None # a state read + assert isinstance(player.extra_state_attributes, dict) # what the frontend reads + player._on_manager_update() # the connection manager pushed an update + + assert served == []