diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index 9699433..f1a539b 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -672,13 +672,13 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): else: features = _BASE_FEATURES - # t44: a command that exists ONLY as a script still has to be reachable, and - # the feature flag is what makes HA offer it. volume_set and select_source - # have no native path at all, so they appear exactly when configured — - # without a script the feature set is what it always was. + # t44/t47: a command that exists ONLY as a script still has to be + # reachable, and the feature flag is what makes HA offer it. volume_set has + # no native path at all; select_source appears when EITHER a script is + # configured OR there is a source list to offer (t47 gave it a native path). if self._script_for("volume_set"): features |= MediaPlayerEntityFeature.VOLUME_SET - if self._script_for("select_source"): + if self._script_for("select_source") or self.source_list: features |= MediaPlayerEntityFeature.SELECT_SOURCE return features @@ -702,6 +702,34 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): """The user's apps name/icon map (P4-1 options editor).""" return self._entry.options.get(CONF_APP_NAME_MAP) or {} + @property + 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. + """ + return self._entry.options.get(CONF_APP_NAME_MAP) or DEFAULT_APP_DIRECTORY + + @property + def source_list(self) -> list[str]: + """The apps as friendly names — the dropdown a UMP replacement reads (t47). + + Directory order (the user's map order, or the curated list), so rebuilding + the page does not shuffle it. + """ + return [str(name) for name in self._app_directory.values()] + + @property + def source(self) -> str | None: + """The foreground app's friendly name — the same chain as app_name (t47). + + ``resolve_app_name``: the user's map, then the cached device-manifest label, + then the raw package. Same answer as the Current App sensor, so a + universal-media-player replacement cannot see two different truths. + """ + return self.app_name + @property def app_name(self): """P5-1 resolution order — explicit override first: @@ -1029,14 +1057,42 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): """ await self._script_served("volume_set", {SCRIPT_VAR_LEVEL: volume}) - async def async_select_source(self, source): - """Select a source — reachable only with a script (t44). + def _source_package(self, source) -> str | None: + """The package behind a source name, or a package id passed through (t47).""" + directory = self._app_directory + wanted = str(source or "").strip() + 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 + return wanted + folded = wanted.casefold() + for package, name in directory.items(): + if str(name).casefold() == folded: + return package + return None - The integration has no native source selection (app launching goes through - play_media with the app map). A script for this command is the hook for an - AVR input or anything else that has to move with the Shield. + async def async_select_source(self, source): + """Select an app by its friendly name (t47) — script first, else launch it. + + A configured ``select_source`` script keeps precedence (t44: selecting a + source can be more than launching an app — an AVR input, a speaker switch). + Without one the name is resolved back to its package through the SAME + 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. """ - await self._script_served("select_source", {SCRIPT_VAR_APP_NAME: source}) + if await self._script_served("select_source", {SCRIPT_VAR_APP_NAME: source}): + return + package = self._source_package(source) + if package is None: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="unknown_source", + translation_placeholders={"source": str(source)}, + ) + await self.async_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.""" @@ -1128,7 +1184,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): removed a name media_player has not exported for a long time, which made every real browse raise. """ - app_map = self._entry.options.get(CONF_APP_NAME_MAP) or DEFAULT_APP_DIRECTORY + app_map = self._app_directory children = [ BrowseMedia( title=name, diff --git a/custom_components/nvidia_shield/strings.json b/custom_components/nvidia_shield/strings.json index 260bbed..a37dd14 100644 --- a/custom_components/nvidia_shield/strings.json +++ b/custom_components/nvidia_shield/strings.json @@ -107,13 +107,13 @@ "channel_digit_delay": "Channel digit delay (seconds)", "channel_send_enter": "Send ENTER after channel digits", "script_turn_on": "Turn on script", - "script_turn_off": "Turn off script", - "script_volume_up": "Volume up script", - "script_volume_down": "Volume down script", - "script_volume_set": "Volume set script", - "script_volume_mute": "Mute script", - "script_select_source": "Select source script", - "logcat_events": "Instant app detection (logcat stream)" + "script_turn_off": "Turn off script", + "script_volume_up": "Volume up script", + "script_volume_down": "Volume down script", + "script_volume_set": "Volume set script", + "script_volume_mute": "Mute script", + "script_select_source": "Select source script", + "logcat_events": "Instant app detection (logcat stream)" }, "data_description": { "jellyfin_entity": "The Jellyfin media_player for this Shield. Stream details come from the HA Jellyfin integration \u2014 no extra setup needed.", @@ -162,6 +162,10 @@ "adb_unavailable": { "title": "ADB unavailable", "message": "The ADB connection of {entity_id} is down \u2014 command not sent." + }, + "unknown_source": { + "title": "Unknown source", + "message": "'{source}' is not one of this Shield's apps. Use a name from the source list \u2014 the app names from the Apps map, or the built-in list when no map is set." } }, "entity": { diff --git a/custom_components/nvidia_shield/translations/en.json b/custom_components/nvidia_shield/translations/en.json index 260bbed..a37dd14 100644 --- a/custom_components/nvidia_shield/translations/en.json +++ b/custom_components/nvidia_shield/translations/en.json @@ -107,13 +107,13 @@ "channel_digit_delay": "Channel digit delay (seconds)", "channel_send_enter": "Send ENTER after channel digits", "script_turn_on": "Turn on script", - "script_turn_off": "Turn off script", - "script_volume_up": "Volume up script", - "script_volume_down": "Volume down script", - "script_volume_set": "Volume set script", - "script_volume_mute": "Mute script", - "script_select_source": "Select source script", - "logcat_events": "Instant app detection (logcat stream)" + "script_turn_off": "Turn off script", + "script_volume_up": "Volume up script", + "script_volume_down": "Volume down script", + "script_volume_set": "Volume set script", + "script_volume_mute": "Mute script", + "script_select_source": "Select source script", + "logcat_events": "Instant app detection (logcat stream)" }, "data_description": { "jellyfin_entity": "The Jellyfin media_player for this Shield. Stream details come from the HA Jellyfin integration \u2014 no extra setup needed.", @@ -162,6 +162,10 @@ "adb_unavailable": { "title": "ADB unavailable", "message": "The ADB connection of {entity_id} is down \u2014 command not sent." + }, + "unknown_source": { + "title": "Unknown source", + "message": "'{source}' is not one of this Shield's apps. Use a name from the source list \u2014 the app names from the Apps map, or the built-in list when no map is set." } }, "entity": { diff --git a/tests/entity_stublib.py b/tests/entity_stublib.py index 5d59a4b..fd08e17 100644 --- a/tests/entity_stublib.py +++ b/tests/entity_stublib.py @@ -365,6 +365,7 @@ def install() -> None: super().__init__(*args or (kwargs.get("translation_key"),)) self.translation_domain = kwargs.get("translation_domain") self.translation_key = kwargs.get("translation_key") + self.translation_placeholders = kwargs.get("translation_placeholders") class ServiceValidationErrorStubError(HomeAssistantErrorStubError): """Real HA: ServiceValidationError derives from HomeAssistantError (t44).""" diff --git a/tests/test_command_scripts.py b/tests/test_command_scripts.py index 329ac49..dcbce7f 100644 --- a/tests/test_command_scripts.py +++ b/tests/test_command_scripts.py @@ -172,14 +172,16 @@ class TestUnmappedCommands: assert player._manager._poller.media_calls == ["volume_up"] - async def test_a_command_with_no_native_path_stays_a_no_op(self, modules): - """volume_set/select_source exist ONLY through a script — never a crash.""" + async def test_volume_set_without_a_script_stays_a_no_op(self, modules): + """Absolute volume exists ONLY through a script (the protocol has no + VOLUME_SET key), so unconfigured it sends nothing — never a crash.""" player, served = _player(modules, {}) await player.async_set_volume_level(0.4) - await player.async_select_source("TiviMate") assert served == [] + assert player._manager._protocol.key_calls == [] + assert player._manager._poller.media_calls == [] async def test_other_commands_are_unaffected_by_a_mapped_one(self, modules): from nsh.const import PROTOCOL_REMOTE_KEYS @@ -257,30 +259,35 @@ class TestFeatureSet: def _features(self, player): return int(player.supported_features) - def test_volume_set_and_select_source_appear_only_when_configured(self, modules): + def test_volume_set_appears_only_when_configured(self, modules): + """volume_set has no native path at all, so the script is the only reach.""" from homeassistant.components.media_player import MediaPlayerEntityFeature player, _served = _player(modules, {}) baseline = self._features(player) + assert baseline & int(MediaPlayerEntityFeature.VOLUME_SET) == 0 player._entry.options[CONF_COMMAND_SCRIPTS] = {"volume_set": WATCH} with_volume = self._features(player) - player._entry.options[CONF_COMMAND_SCRIPTS] = { - "volume_set": WATCH, - "select_source": WATCH, - } - with_both = self._features(player) - - extra = ( - MediaPlayerEntityFeature.VOLUME_SET | MediaPlayerEntityFeature.SELECT_SOURCE - ) assert with_volume & int(MediaPlayerEntityFeature.VOLUME_SET) - assert not with_volume & int(MediaPlayerEntityFeature.SELECT_SOURCE) - assert with_both & int(extra) == int(extra) - # nothing else moved, and unset means the feature set it always had - assert with_both & ~int(extra) == baseline - assert baseline & int(extra) == 0 + # ...and nothing else moved + assert with_volume & ~int(MediaPlayerEntityFeature.VOLUME_SET) == baseline + + def test_select_source_is_advertised_once_there_is_something_to_offer( + self, modules + ): + """t47: either a script OR a source list is enough — the native path exists + now, so an unconfigured integration still has sources to offer.""" + from homeassistant.components.media_player import MediaPlayerEntityFeature + + player, _served = _player(modules, {}) + flagged = self._features(player) + + assert flagged & int(MediaPlayerEntityFeature.SELECT_SOURCE) + # a configured script changes nothing about the flag + player._entry.options[CONF_COMMAND_SCRIPTS] = {"select_source": WATCH} + assert self._features(player) & int(MediaPlayerEntityFeature.SELECT_SOURCE) def test_a_script_for_an_already_advertised_command_changes_nothing(self, modules): player, _served = _player(modules, {}) diff --git a/tests/test_media_sources.py b/tests/test_media_sources.py new file mode 100644 index 0000000..176cffd --- /dev/null +++ b/tests/test_media_sources.py @@ -0,0 +1,210 @@ +"""tests/test_media_sources.py — source_list, source and native select_source (t47). + +The last piece a universal-media-player replacement needs from this integration: +a dropdown of the apps (``source_list``), an answer to "what is on screen" +(``source``), and a way to pick one (``media_player.select_source``). All three read +the SAME app directory — the user's Apps map, else the curated default — so the +dropdown cannot offer a name the launch path cannot resolve. + +The t44 rule still holds: a configured ``select_source`` script takes precedence, +because selecting a source can be more than launching an app. Without one, the name +resolves back to its package and goes out over the shared ``play_media("app", …)`` +path; an unknown name is a translated error, never a silent no-op. +""" + +import entity_stublib +import pytest +from nsh.const import CONF_APP_NAME_MAP, CONF_COMMAND_SCRIPTS, SCRIPT_VAR_APP_NAME +from test_command_routing import make_player + +TIVIMATE = "ar.tvplayer.tv" +SCRIPT = "script.shield_source" + + +@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, *, app_map=None, scripts=None, foreground=None): + player, served, _intents = make_player(modules) + if app_map is not None: + player._entry.options[CONF_APP_NAME_MAP] = app_map + if scripts is not None: + player._entry.options[CONF_COMMAND_SCRIPTS] = scripts + if foreground is not None: + player._manager._on_current_app(foreground) + return player, served + + +class TestSourceList: + """The dropdown: friendly names, in a stable order, from the one directory.""" + + def test_it_comes_from_the_apps_map(self, modules): + player, _served = _player( + modules, + app_map={"com.custom.one": "One", "com.custom.two": "Two"}, + ) + + assert player.source_list == ["One", "Two"] # map order, not shuffled + + def test_it_falls_back_to_the_curated_directory(self, modules): + player, _served = _player(modules) # no map configured + + assert player.source_list == ["TiviMate", "Wholphin", "Netflix", "YouTube"] + + def test_every_offered_name_resolves_to_a_package(self, modules): + """The invariant the shared directory buys: no name without a destination.""" + for app_map in (None, {"com.custom.one": "One"}): + player, _served = _player(modules, app_map=app_map) + + for name in player.source_list: + assert player._source_package(name) is not None, name + + +class TestSource: + """`source` is the foreground app's resolved name — the app_name chain.""" + + def test_it_resolves_through_the_apps_map(self, modules): + player, _served = _player( + modules, app_map={TIVIMATE: "TiviMate"}, foreground=TIVIMATE + ) + + assert player.source == "TiviMate" + assert player.source == player.app_name # one truth, not two + + def test_it_uses_the_cached_device_label_when_unmapped(self, modules): + player, _served = _player(modules, foreground="com.netflix.ninja") + player._manager._app_labels["com.netflix.ninja"] = ( + "Netflix", + player._manager._clock(), + ) + + assert player.source == "Netflix" + + def test_it_falls_back_to_the_raw_package(self, modules): + player, _served = _player(modules, foreground="com.unknown.app") + + assert player.source == "com.unknown.app" + + def test_it_is_unknown_when_nothing_has_reported(self, modules): + player, _served = _player(modules) + + assert player.source is None + + +class TestSelectSource: + """Picking a source: script first, else the native launch, else an error.""" + + async def test_a_name_launches_its_package(self, modules): + player, served = _player( + modules, app_map={TIVIMATE: "TiviMate", "com.netflix.ninja": "Netflix"} + ) + + await player.async_select_source("TiviMate") + + assert player._manager._protocol.launch_calls == [TIVIMATE] + assert served == [] # no script was configured, and nothing else left + + async def test_the_curated_names_launch_too(self, modules): + player, _served = _player(modules) + + await player.async_select_source("Netflix") + + assert player._manager._protocol.launch_calls == ["com.netflix.ninja"] + + async def test_a_raw_package_and_any_case_are_accepted(self, modules): + player, _served = _player( + modules, app_map={TIVIMATE: "TiviMate", "com.netflix.ninja": "Netflix"} + ) + + await player.async_select_source(TIVIMATE) + await player.async_select_source("tivimate") + await player.async_select_source(" NETFLIX ") + + assert player._manager._protocol.launch_calls == [ + TIVIMATE, + TIVIMATE, + "com.netflix.ninja", + ] + + async def test_a_configured_script_still_wins(self, modules): + player, served = _player( + modules, + app_map={TIVIMATE: "TiviMate"}, + scripts={"select_source": SCRIPT}, + ) + + await player.async_select_source("TiviMate") + + assert served == [ + ( + "script", + "turn_on", + {"entity_id": SCRIPT, "variables": {SCRIPT_VAR_APP_NAME: "TiviMate"}}, + ) + ] + assert player._manager._protocol.launch_calls == [] # native path untouched + + async def test_an_unknown_name_raises_a_translated_error(self, modules): + from homeassistant.exceptions import ServiceValidationError + + player, _served = _player(modules, app_map={TIVIMATE: "TiviMate"}) + + with pytest.raises(ServiceValidationError) as raised: + await player.async_select_source("Not An App") + + assert raised.value.translation_domain == "nvidia_shield" + assert raised.value.translation_key == "unknown_source" + assert raised.value.translation_placeholders == {"source": "Not An App"} + assert player._manager._protocol.launch_calls == [] # nothing was sent + + async def test_an_empty_name_is_an_error_too(self, modules): + from homeassistant.exceptions import ServiceValidationError + + player, _served = _player(modules) + + with pytest.raises(ServiceValidationError): + await player.async_select_source(" ") + + +class TestAdvertisedFeatures: + """HA routes a service by its feature flag, so the flag has to be honest.""" + + def test_a_source_list_advertises_select_source(self, modules): + from homeassistant.components.media_player import MediaPlayerEntityFeature + + player, _served = _player(modules) + + assert player.supported_features & MediaPlayerEntityFeature.SELECT_SOURCE + + def test_a_script_advertises_it_even_with_no_directory(self, modules, monkeypatch): + from homeassistant.components.media_player import MediaPlayerEntityFeature + + player, _served = _player(modules, scripts={"select_source": SCRIPT}) + monkeypatch.setattr(type(player), "_app_directory", property(lambda self: {})) + + assert player.supported_features & MediaPlayerEntityFeature.SELECT_SOURCE + + def test_neither_means_it_is_not_offered(self, modules, monkeypatch): + from homeassistant.components.media_player import MediaPlayerEntityFeature + + player, _served = _player(modules) + monkeypatch.setattr(type(player), "_app_directory", property(lambda self: {})) + + assert not player.supported_features & MediaPlayerEntityFeature.SELECT_SOURCE + + def test_select_source_is_not_gated_on_the_volume_set_script(self, modules): + """The two script-only commands stay independent.""" + from homeassistant.components.media_player import MediaPlayerEntityFeature + + player, _served = _player(modules, scripts={"volume_set": "script.volume"}) + + assert player.supported_features & MediaPlayerEntityFeature.VOLUME_SET + assert player.supported_features & MediaPlayerEntityFeature.SELECT_SOURCE