From ef42682250b76ad1d5eb6a26607780750861424c Mon Sep 17 00:00:00 2001 From: James Mackay Date: Mon, 12 Oct 2026 08:04:06 +0100 Subject: [PATCH] fix(media-player): ON means no power or playback fact reported (t58 UNIT 0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The t59 F2 ruling, implemented. The ladder said ON was for "no source reported anything at all" but tested for something looser — no transport data — so a source that ANSWERED "not playing" still fell through to ON. A source that answers is not a source that is silent, and `on` is the "we cannot know" state. The ladder now tracks an explicit `reported` flag, set by every source that gave us a power or playback fact, and the last tier is `IDLE if reported else ON`. Each case the ruling names now lands on IDLE: * a configured Jellyfin entity in ANY state — idle, off, standby, unavailable, or even absent from the state machine. It reports "not playing" without an early return, so a silent Jellyfin still cannot mask a live media session below it (both directions pinned); * a live media session that carries no playback flag — the app publishes a session but is not playing, paused or buffering; * a bare foreground app: either transport's answer counts (the protocol's current_app push or the ADB mResumedActivity parse); * a configured power entity, whatever it currently says; * protocol is_on / ADB awake, plugged or not. `on` survives only for an entry where none of those exists. The t55 rules are untouched: an off power entity still outranks a playing session, and no source may mask playing/paused/buffering. The module docstring's tier 6, the `state` docstring and the README's ladder (moving "no data" to the precise wording) all say the same thing now, and the README's one-line state table was aligned with it. RED PROOF (the new tests against the parent tree): five of the reporting cases read `on` where they must read `idle` — `assert 'on' == 'idle'` — plus the "ON is unreachable with any source configured" shape test. SIMULATED vs EXECUTED: no real Shield was available, so these are the documented source shapes, not captures; the state=6 → buffering mapping stays marked inferred. No browser exists either, so the README pins are textual and the production behaviour is exercised against real HA's state machine in tests/test_ha_api_drift.py. --- README.md | 24 +++- .../nvidia_shield/media_player.py | 94 ++++++++++--- tests/test_command_routing.py | 131 +++++++++++++++++- tests/test_readme.py | 9 ++ 4 files changed, 228 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 993f149..609361a 100644 --- a/README.md +++ b/README.md @@ -75,11 +75,23 @@ remote protocol is the fast push/command path: 6. **Idle** — the device is powered (protocol `is_on` true, ADB awake, or the power plug on) and nothing is playing, or ADB reports it asleep (standby). A powered Shield with an app open and nothing playing reads `idle`. -7. **On** — **no source reported anything at all**: no transport connected, no - power entity, no Jellyfin, no media session. `on` is the "we cannot know" - state and the only situation it describes — it never appears while any - source reported a power or media state, and never masks a - `playing`/`paused`/`buffering` source. +7. **On** — **no source reported a power or playback fact** (t58 UNIT 0; the + earlier wording said "no data", which was too loose). `on` is the "we cannot + know" state and the only situation it describes, so every source that + *answers* — including an answer of "not playing" — stops the ladder at + `idle` at worst, never here: + - a configured **Jellyfin** entity in **any** state: `idle`, `off`, + `standby` and `unavailable` all mean "its session is not playing", so they + report — and because the ladder keeps going, a silent Jellyfin still + cannot mask a live media session below it; + - a **live media session with no playback flag** — the app publishes a + session but is not playing, paused or buffering; + - a **bare foreground app** — the protocol's `current_app` push or the ADB + `mResumedActivity` parse answered; + - a configured **power entity**, whatever it currently says; + - protocol `is_on` / ADB awake, plugged or not. + `on` survives only for an entry where none of those exists, and it never + masks a `playing`/`paused`/`buffering` source. The foreground tier is what keeps an app's identity visible while the state is `idle`: the `current_app` protocol push (then the ADB `mResumedActivity` @@ -185,7 +197,7 @@ What each retired piece used to provide, and what covers it now: | Retired piece | Covered by | |---------------|------------| -| `androidtv` playback state | the ADB media-session state ladder (`playing` / `paused` / `buffering` / `idle` / `off`, plus `on` only when no source reported anything) | +| `androidtv` playback state | the ADB media-session state ladder (`playing` / `paused` / `buffering` / `idle` / `off`, plus `on` only when no source reported a power or playback fact) | | `androidtv` app name / `app_id` lookups | the manager's store + the app-name map + the manifest label of the app that is currently on screen (fetched on demand; a failed fetch is retried on the next poll instead of being cached as a permanent miss) | | `androidtv_remote` power, volume and key commands | the protocol transport: the `remote` entity, `media_player` volume, `KEYCODE_*` commands | | `androidtv_remote` availability | the availability contract (protocol `is_on`, ADB awake, power switch) | diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index a5eb7ce..a7fb844 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -31,13 +31,29 @@ source that has something to say, and the earlier steps always win: powered on (protocol is_on True / ADB awake / the plug is on) and NOTHING is playing. "Powered on, app open, nothing playing" is IDLE for exactly this reason. - 6. ON — NOTHING reported anything at all: no transport connected, no - power entity, no Jellyfin, no media session, no intent. ON is + 6. ON — NOTHING reported a POWER OR PLAYBACK FACT — not "no data" (t58 + UNIT 0, the t59 F2 ruling, which this wording follows). ON is the "we cannot know" state and the only situation it may - describe. It must NEVER appear while any source reported a - power or media state, and it must never mask a - playing/paused/buffering source — the rule that fixes the live - "On overrides my playing states" report. + describe, so every source that ANSWERS — even to say "not + playing" — takes the ladder to IDLE at worst, never here: + * a configured Jellyfin entity in ANY state, including idle / + off / standby / unavailable (and it reports "not playing" + without ever masking a live media session, because the + playing/paused tiers above still win); + * a live media session that carries no playback flag, i.e. the + app publishes a session but is not playing, paused or + buffering; + * a bare foreground app — the protocol's current-app push or + the ADB mResumedActivity parse answered; + * a configured power entity, whatever it currently says; + * protocol is_on / ADB awake, plugged or not. + ON survives for an entry where NONE of those exists: no + transport data, no power entity, no Jellyfin entity, no session, + no foreground app. It must never mask a playing/paused/buffering + source — the rule that fixes the live "On overrides my playing + states" report. + + Intent tracking bridges the gap between instant commands and slow Jellyfin polling: 1. User presses pause → intent recorded → UI shows paused immediately @@ -676,17 +692,28 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): Exactly ``{playing, paused, buffering, idle, off}`` are reported for a device we have data about; ``ON`` is reserved for the case where NO - source reported anything at all (no transport connected, no power and - no media data) and must never mask a playing/paused/buffering source. - A powered-on Shield with an app open but nothing playing reads IDLE, - not ON (t55 — the live defect users read as "On overrides my playing - states"). + source reported a POWER OR PLAYBACK FACT — not merely "no transport is + connected" (t58 UNIT 0, the t59 F2 ruling). A source that exists and + answers "not playing" (Jellyfin idle/off/standby/unavailable, a live + media session with no playback flag, a bare foreground app) HAS reported, + so the ladder stops at IDLE, never ON. ON must also never mask a + playing/paused/buffering source. A powered-on Shield with an app open but + nothing playing reads IDLE, not ON (t55 — the live defect users read as + "On overrides my playing states"). """ + # A source that reported a power or playback fact, even a negative one. + # ON is the "we cannot know" state, so it survives only when nothing at + # all answered — this flag IS the distinction the ruling turns on. + reported = False + # 1. Explicit OFF. The plug being off is definitive; the protocol's # is_on push is the device reporting its own power state. Either # one OUTRANKS every media source — a stale Jellyfin session must # never resurrect playback on a device that is off. if self._power_entity: + # a CONFIGURED source counts as reporting whatever it says now, + # which is why an unavailable plug yields IDLE rather than ON + reported = True power_state = self._read(self._power_entity) if power_state and power_state.state == "off": return MediaPlayerState.OFF @@ -698,7 +725,15 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): if self._intent is not None: return MediaPlayerState(self._intent.target_state) - # 3. Jellyfin (slow, but authoritative for its own sessions). + # 3. Jellyfin (slow, but authoritative for its own sessions). A + # configured entity reports in EVERY state it can be in: playing and + # paused are truth, and idle / off / standby / unavailable / missing + # all mean "its session is not playing" — reported, just not + # playable, so the ladder continues WITHOUT an early return (a silent + # Jellyfin must never mask a live media session) and IDLE is where it + # ends (t58 UNIT 0). + if self._jellyfin_entity: + reported = True jf = self._read(self._jellyfin_entity) if jf: if jf.state == STATE_PLAYING: @@ -706,11 +741,16 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): if jf.state == STATE_PAUSED: return MediaPlayerState.PAUSED - # 4. Live media session (non-Jellyfin apps), buffering included. + # 4. Live media session (non-Jellyfin apps), buffering included. The + # session EXISTING is a report even when it carries no playback flag + # (an idle or paused app still publishes one): that is IDLE, and it is + # the t58 UNIT 0 case that used to fall through to ON. session = manager.best_session(self.app_id) if manager is not None else None - session_state = self._session_state(session) - if session_state is not None: - return session_state + if session is not None: + reported = True + session_state = self._session_state(session) + if session_state is not None: + return session_state # 5. ADB standby: powered but asleep — still data (the device # answered), so IDLE, never ON. media_player.STANDBY is deprecated @@ -721,14 +761,22 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity): # 6. The device is on (protocol is_on True, ADB awake, or the power # plug is on — every one of those is a source that reported # something). An open app or an awake device reads IDLE, not ON. - power = self._is_power_on() - if power is not None or self._power_entity: - return MediaPlayerState.IDLE + # A bare foreground app is a report in its own right: the protocol's + # current-app push or the ADB mResumedActivity parse answered. + if self._is_power_on() is not None: + reported = True + if manager is not None and manager.awake is not None: + reported = True + # EITHER transport's foreground answer counts: the protocol's + # current_app push and the ADB mResumedActivity parse are both + # "something reported", even when neither is playing anything. + if manager is not None and manager.foreground_app is not None: + reported = True - # 7. Nothing anywhere reported anything: no transport connected, no - # power entity, no media source. ON is the "we cannot know" state, - # and the only situation it is allowed to describe. - return MediaPlayerState.ON + # 7. Nothing anywhere reported a power or playback fact. ON is the "we + # cannot know" state, and the only situation it is allowed to + # describe. + return MediaPlayerState.IDLE if reported else MediaPlayerState.ON @property def supported_features(self) -> MediaPlayerEntityFeature: diff --git a/tests/test_command_routing.py b/tests/test_command_routing.py index 757d02f..00407ee 100644 --- a/tests/test_command_routing.py +++ b/tests/test_command_routing.py @@ -535,11 +535,14 @@ def _state_player( protocol_on=None, power_entity_state=None, jellyfin_state=None, + jellyfin_configured=False, ): """A player whose power/media sources are exactly what the test sets. ``None`` means "this source has not reported" — the distinction the t55 ladder turns on, so every test spells out which sources spoke. + ``jellyfin_configured`` covers the entity being configured while ``_read`` + still answers None (the entity is gone). """ player, _served, _intents = make_player(modules) manager = player._manager @@ -556,7 +559,7 @@ def _state_player( entry.options = dict(entry.options or {}) if power_entity_state is not None: entry.options["power_entity"] = "switch.shield_plug" - if jellyfin_state is not None: + if jellyfin_state is not None or jellyfin_configured: entry.options["jellyfin_entity"] = "media_player.jellyfin" def _read(entity_id): @@ -1978,3 +1981,129 @@ class TestAvailabilityLadder: assert transport["entity_available"] is True assert transport["adb"] == {"configured": True, "connected": False, "awake": None} assert transport["protocol"]["active"] is True + + +class TestReportingFactsT58: + """t58 UNIT 0 — the t59 F2 ruling: ON = "no power or playback fact reported". + + The live report was "On overrides my playing states"; t55 fixed the loud half + (a powered-on Shield with an app open). This is the quiet half: a source that + ANSWERS "not playing" was still falling through to ON. Every case below is a + source that answered, so the ladder must stop at IDLE. + """ + + # ── Jellyfin, in every state it can report ── + + async def test_jellyfin_idle_is_idle_not_on(self, modules): + player, _m = _state_player(modules, jellyfin_state="idle") + + assert player.state.value == "idle" + + async def test_jellyfin_off_is_idle_not_on(self, modules): + """`off` is a REPORT about the session, not the Shield's own power: + only the configured power entity and the protocol push can say OFF.""" + player, _m = _state_player(modules, jellyfin_state="off") + + assert player.state.value == "idle" + + async def test_jellyfin_standby_is_idle_not_on(self, modules): + player, _m = _state_player(modules, jellyfin_state="standby") + + assert player.state.value == "idle" + + async def test_jellyfin_unavailable_is_idle_not_on(self, modules): + """The one the ruling names explicitly: unavailable still answered.""" + player, _m = _state_player(modules, jellyfin_state="unavailable") + + assert player.state.value == "idle" + + async def test_a_configured_but_missing_jellyfin_entity_is_idle(self, modules): + """Configured IS the report: we are watching a source that exists. + + `_read` returns None here (the entity is not in the state machine), and + the old ladder fell straight through to ON — for an entry whose owner + had explicitly configured a media source. + """ + player, _m = _state_player(modules, jellyfin_configured=True) + + assert player.state.value == "idle" + + # ── a live session with no playback flag, and a bare foreground app ── + + async def test_a_session_with_no_playback_flag_is_idle_not_on(self, modules): + """The app publishes a session; that it is NOT playing is the report.""" + from nsh.ha_adb_common import MediaSessionInfo + + player, _m = _state_player( + modules, sessions=[MediaSessionInfo(package="com.netflix.ninja")] + ) + + assert player.state.value == "idle" + + async def test_a_bare_foreground_app_is_idle_not_on(self, modules): + """The protocol's current-app push answered — nothing else did.""" + player, _m = _state_player(modules, foreground="ar.tvplayer.tv") + + assert player.state.value == "idle" + + # ── the negative control: ON is still reachable, just narrowly ── + + async def test_on_survives_when_truly_nothing_reported(self, modules): + player, _m = _state_player(modules) + + assert player.state.value == "on" + + async def test_on_is_unreachable_with_any_source_configured(self, modules): + """The rule as a shape: each single source takes ON off the table.""" + for kwargs in ( + {"jellyfin_state": "idle"}, + {"jellyfin_state": "off"}, + {"jellyfin_state": "standby"}, + {"jellyfin_state": "unavailable"}, + {"jellyfin_configured": True}, + {"foreground": "ar.tvplayer.tv"}, + {"protocol_on": True}, + {"awake": True}, + {"power_entity_state": "on"}, + ): + player, _m = _state_player(modules, **kwargs) + assert player.state.value != "on", kwargs + + # ── a silent source must not MASK a live one (the ladder keeps going) ── + + async def test_idle_jellyfin_does_not_mask_a_playing_session(self, modules): + from nsh.ha_adb_common import MediaSessionInfo + + player, _m = _state_player( + modules, + jellyfin_state="idle", + sessions=[MediaSessionInfo(package="com.netflix.ninja", playing=True)], + foreground="com.netflix.ninja", + ) + + assert player.state.value == "playing" + + async def test_unavailable_jellyfin_does_not_mask_a_paused_session(self, modules): + from nsh.ha_adb_common import MediaSessionInfo + + player, _m = _state_player( + modules, + jellyfin_state="unavailable", + sessions=[MediaSessionInfo(package="com.netflix.ninja", paused=True)], + foreground="com.netflix.ninja", + ) + + assert player.state.value == "paused" + + async def test_an_off_power_entity_still_outranks_a_playing_session(self, modules): + """The t55 rule is untouched by the F2 correction.""" + from nsh.ha_adb_common import MediaSessionInfo + + player, _m = _state_player( + modules, + power_entity_state="off", + sessions=[MediaSessionInfo(package="com.netflix.ninja", playing=True)], + ) + + assert player.state.value == "off" + diff --git a/tests/test_readme.py b/tests/test_readme.py index 6685558..331440e 100644 --- a/tests/test_readme.py +++ b/tests/test_readme.py @@ -61,3 +61,12 @@ def test_the_replacement_guide_retires_the_old_stack(readme_text): assert needle in readme_text, needle # and it says the UI is the primary path assert "primary path" in readme_text + + +def test_the_readme_states_the_on_rule_precisely(readme_text): + """t58 UNIT 0: "no data" was too loose; the README must not still say it.""" + assert "no source reported a power or playback fact" in readme_text + # the four reporting-but-not-playing shapes, named + for shape in ("`idle`", "`standby`", "`unavailable`", "foreground app"): + assert shape in readme_text, shape + assert "no source reported anything at all" not in readme_text