diff --git a/README.md b/README.md index 15350ff..59b2e33 100644 --- a/README.md +++ b/README.md @@ -392,6 +392,19 @@ Selecting a source is sometimes more than launching an app — an AVR input, a s switch. That is what the **Select source script** above is for: with one configured it replaces the native launch entirely. +#### Known limits (the Apps map) + +The map is your own override, so two shapes are accepted rather than papered over: + +- **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. +- 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 + 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. + ### Remote command script `remote.send_command` sends protocol keys: repeats, long presses (`START_LONG`/ diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index bdbb7da..9b4b39e 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -734,7 +734,10 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): tree reaches each by its own package id, so neither app is hidden — 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 user's order, so the winner does not move between rebuilds. "The same + name" is judged case-insensitively (t53), because the reverse lookup is: + names differing only by case would otherwise give the dropdown a second + row that launches the FIRST entry's app. A well-formed map (what the editor and the curated default produce) passes through byte-identical: stripping only touches the edges, so a name like @@ -758,11 +761,26 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): 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. Each name is offered ONCE: if two packages normalize to the same - name, the first in map order is the one this list resolves to (and the - second stays reachable through the browse tree, by its package id). + disagree. + + Each name is offered ONCE, compared CASE-INSENSITIVELY (t53), keeping the + first spelling the user typed. Exact duplicates were already collapsed, but + ``Dup``/``dup`` were not: the dropdown then showed two rows while the + 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. """ - return list(dict.fromkeys(self._app_directory.values())) + seen: set[str] = set() + offered: list[str] = [] + for name in self._app_directory.values(): + folded = name.casefold() + if folded in seen: + continue + seen.add(folded) + offered.append(name) + return offered @property def source(self) -> str | None: @@ -1106,9 +1124,12 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): Both sides are normalized the same way: the query is stripped here and the names were stripped when the directory was built (t51), so a padded entry - matches its trimmed spelling. Names are compared case-insensitively, and - when two packages share a name the first in map order wins — the same one - ``source_list`` offers (see ``_app_directory``). + matches its trimmed spelling. Names are compared case-insensitively — which + is why ``source_list`` collapses names that differ only by case (t53), so + that the row it offers resolves back to the same app it displays. When two + packages share a name the first in map order wins, the same one + ``source_list`` offers (see ``_app_directory``); the shadowed package stays + reachable by passing its id. """ directory = self._app_directory wanted = str(source or "").strip() diff --git a/tests/test_media_sources.py b/tests/test_media_sources.py index 1babdde..675d6cf 100644 --- a/tests/test_media_sources.py +++ b/tests/test_media_sources.py @@ -371,3 +371,104 @@ class TestNormalizedDirectories: 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 + + +class TestCaseInsensitiveNames: + """t53: one row per NAME, compared case-insensitively — as the lookup does. + + The dropdown and the reverse lookup disagreed about case: two entries called + `Dup` and `dup` survived an exact-match dedupe as two rows, while + `_source_package` — case-insensitive by design (t47) — resolved BOTH to the + first entry. So the second row launched the first entry's app: a row that lies + about what it does, and the one shape here the editor can produce by itself. + """ + + async def test_names_differing_only_by_case_are_offered_once(self, modules): + player, _served = _player(modules, app_map={"com.a": "Dup", "com.b": "dup"}) + + # 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"), + ] + + 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"}) + + await player.async_select_source(player.source_list[0]) + + assert player._manager._protocol.launch_calls == ["com.a"] # the row it shows + + async def test_the_shadowed_app_stays_reachable_by_id(self, modules): + player, _served = _player(modules, app_map={"com.a": "Dup", "com.b": "dup"}) + + assert player._source_package("com.b") == "com.b" + + await player.async_select_source("com.b") + + assert player._manager._protocol.launch_calls == ["com.b"] + + # the shadowed NAME still resolves to the one the list offers (first wins) + assert player._source_package("dup") == "com.a" + + async def test_the_first_spelling_wins_in_map_order(self, modules): + player, _served = _player(modules, app_map={"com.b": "dup", "com.a": "Dup"}) + + assert player.source_list == ["dup"] + + await player.async_select_source("dup") + + assert player._manager._protocol.launch_calls == ["com.b"] + + async def test_a_mixed_map_offers_each_name_once(self, modules): + player, _served = _player( + modules, + app_map={ + "com.a": "Dup", + "com.b": "dup", + "com.netflix.ninja": "Netflix", + "com.google.android.youtube.tv": "netflix", + TIVIMATE: "TiviMate Pro", + }, + ) + + assert player.source_list == ["Dup", "Netflix", "TiviMate Pro"] + assert player._source_package("Netflix") == "com.netflix.ninja" + + def test_the_offered_list_never_repeats_a_name(self, modules): + """The invariant behind the fix, over every map shape in this file.""" + for app_map in ( + None, + {"com.a": "Dup", "com.b": "dup"}, + {"com.a": "Dup", "com.b": "DUP", "com.c": "dup"}, + {"com.blank": "", TIVIMATE: " TiviMate ", "com.a": "Netflix", "com.b": "netflix"}, + ): + player, _served = _player(modules, app_map=app_map) + folded = [name.casefold() for name in player.source_list] + + assert len(folded) == len(set(folded)), app_map + assert len(folded) == len(player.source_list) + + def test_a_padded_map_value_is_displayed_verbatim_by_app_name(self, modules): + """The README's boundary, pinned: the list cleans it, app_name does not. + + The name map is the user's own override, so it is used as typed — which is + why `source` can be a string that is not in source_list. + """ + player, _served = _player( + modules, app_map={TIVIMATE: " TiviMate "}, foreground=TIVIMATE + ) + + assert player.source_list == ["TiviMate"] # the dropdown cleans it + assert player.source == " TiviMate " # this surface shows it as typed + assert player.source not in player.source_list + + def test_a_blank_map_value_falls_through_to_the_raw_package(self, modules): + """Blanks are not displayed as nothing — the chain continues past them.""" + player, _served = _player(modules, app_map={TIVIMATE: ""}, foreground=TIVIMATE) + + assert player.source == TIVIMATE