fix(media_player): dedupe source_list case-insensitively (t53 F1)
t52 F1: the dropdown collapsed exact duplicates but not names that differ only by case.
With a map of {com.a: 'Dup', com.b: 'dup'} the pre-fix tree offered ['Dup', 'dup']
(two rows) while the reverse lookup — case-insensitive by design since t47 — resolved
BOTH spellings to the first entry. So row two was a row that LIED: picking it launched
com.a, the app row one already names. Editor-reachable (two valid lines), unlike the
earlier hand-edit-only edges, which is why it is fixed rather than documented.
source_list now dedupes on name.casefold(), keeping the first spelling in the user's
typed map order. 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 package id
(the t51 rule — both entries kept, each reachable by id) and select_source accepts that
id, pinned by test. The t51 policy is otherwise untouched: names stripped once, blank
name/package entries dropped, exact duplicates already collapsed, well-formed maps
byte-identical to the Browse titles with inner spaces preserved.
README: a short 'Known limits (the Apps map)' note records the two accepted boundaries
so they are known rather than latent — names differing only by case offer one row (the
shadowed app stays reachable by id), and a hand-padded map value is shown cleaned in
the dropdown and the Browse tree while app_name/source/Current App sensor display it
verbatim, so `source` can be a string that is not in source_list (a blank value falls
through to the raw package).
RED PROOF (previous tree swapped in, then restored): 4 failed / 5 passed pre-fix for
the case tests, all green after. The literal pre-fix symptom:
`assert ['Dup', 'dup'] == ['Dup']` — "Left contains one more item: 'dup'"; the
wrong-app half is pinned by `_source_package("dup") == "com.a"`, which holds on BOTH
trees and is exactly the chain that made row two launch row one's app. The new
no-duplicate invariant fails pre-fix with len(['dup', 'dup']) == 2 versus 1.
Verified: 471 passed / 2 skipped, 472 / 1 with androidtvremote2, ruff clean, drift
engine exit 0 under real HA (23 modules, 58 imports, 164 relative imports, 36 call
arguments, 7 awaited HA calls); README YAML blocks still parse (5) and the t47/t48/t51
rig is green.
This commit is contained in:
1 parent
dc3cfbbe3e
commit
0357f47284
3 files changed
+143
-8
No files matched your search
@@ -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
|
switch. That is what the **Select source script** above is for: with one configured it
|
||||||
replaces the native launch entirely.
|
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 command script
|
||||||
|
|
||||||
`remote.send_command` sends protocol keys: repeats, long presses (`START_LONG`/
|
`remote.send_command` sends protocol keys: repeats, long presses (`START_LONG`/
|
||||||
|
|||||||
@@ -734,7 +734,10 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity):
|
|||||||
tree reaches each by its own package id, so neither app is hidden — while
|
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
|
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
|
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
|
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
|
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
|
the page does not shuffle it. Blanks are already gone and names already
|
||||||
stripped by ``_app_directory`` (t49/t51) — the normalization lives in that
|
stripped by ``_app_directory`` (t49/t51) — the normalization lives in that
|
||||||
one accessor so the dropdown, the browse tree and the reverse lookup cannot
|
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
|
disagree.
|
||||||
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).
|
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
|
@property
|
||||||
def source(self) -> str | None:
|
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
|
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
|
names were stripped when the directory was built (t51), so a padded entry
|
||||||
matches its trimmed spelling. Names are compared case-insensitively, and
|
matches its trimmed spelling. Names are compared case-insensitively — which
|
||||||
when two packages share a name the first in map order wins — the same one
|
is why ``source_list`` collapses names that differ only by case (t53), so
|
||||||
``source_list`` offers (see ``_app_directory``).
|
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
|
directory = self._app_directory
|
||||||
wanted = str(source or "").strip()
|
wanted = str(source or "").strip()
|
||||||
|
|||||||
@@ -371,3 +371,104 @@ class TestNormalizedDirectories:
|
|||||||
assert player.source_list == ["TiviMate Pro", "Netflix"]
|
assert player.source_list == ["TiviMate Pro", "Netflix"]
|
||||||
assert [child.title for child in root.children] == ["TiviMate Pro", "Netflix"]
|
assert [child.title for child in root.children] == ["TiviMate Pro", "Netflix"]
|
||||||
assert player._source_package("TiviMate Pro") == TIVIMATE
|
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
|
||||||
Reference in new issue
Block a user