fix(media_player): the adb_command participant registry key is computed once at registration and stored on the entity — add/remove always match (F4)
This commit is contained in:
1 parent
c44c2c2170
commit
b37a14b920
5 files changed
+147
-60
No files matched your search
@@ -23,6 +23,7 @@ hass (plain ``asyncio.create_task``) for the unit tests.
|
|||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import contextlib
|
import contextlib
|
||||||
|
import dataclasses
|
||||||
import logging
|
import logging
|
||||||
import random
|
import random
|
||||||
import time
|
import time
|
||||||
@@ -135,6 +136,10 @@ class StateStore:
|
|||||||
def ttl(self) -> float:
|
def ttl(self) -> float:
|
||||||
return self._ttl
|
return self._ttl
|
||||||
|
|
||||||
|
def values_stamped(self) -> dict[str, tuple[float, Any]]:
|
||||||
|
"""Public stamped-values snapshot (F6): ``{field: (stamp, value)}``."""
|
||||||
|
return dict(self._values)
|
||||||
|
|
||||||
|
|
||||||
class ShieldConnectionManager:
|
class ShieldConnectionManager:
|
||||||
"""Owner of one entry's persistent connections (both transports) + state.
|
"""Owner of one entry's persistent connections (both transports) + state.
|
||||||
@@ -377,6 +382,12 @@ class ShieldConnectionManager:
|
|||||||
"""
|
"""
|
||||||
return self._protocol is not None
|
return self._protocol is not None
|
||||||
|
|
||||||
|
@property
|
||||||
|
def adb_poller(self):
|
||||||
|
"""The ADB poller (or None) — public accessor replacing the
|
||||||
|
``_poller`` reach-in in the adb_command service handler (F6)."""
|
||||||
|
return self._poller
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def is_on(self) -> bool | None:
|
def is_on(self) -> bool | None:
|
||||||
"""Protocol power push (device reports on/off state)."""
|
"""Protocol power push (device reports on/off state)."""
|
||||||
@@ -446,6 +457,19 @@ class ShieldConnectionManager:
|
|||||||
return None
|
return None
|
||||||
return info
|
return info
|
||||||
|
|
||||||
|
def store_snapshot(self) -> dict[str, dict]:
|
||||||
|
"""Public StateStore snapshot for diagnostics (F6).
|
||||||
|
|
||||||
|
``{field: {"age": seconds, "value": ...}}`` with dataclass values
|
||||||
|
expanded to dicts so the payload stays JSON-serializable.
|
||||||
|
"""
|
||||||
|
snapshot: dict[str, dict] = {}
|
||||||
|
for name, (stamp, value) in self.store.values_stamped().items():
|
||||||
|
if dataclasses.is_dataclass(value) and not isinstance(value, type):
|
||||||
|
value = dataclasses.asdict(value)
|
||||||
|
snapshot[name] = {"age": round(time.monotonic() - stamp, 3), "value": value}
|
||||||
|
return snapshot
|
||||||
|
|
||||||
def last_known_map(self) -> dict[str, MediaSessionInfo]:
|
def last_known_map(self) -> dict[str, MediaSessionInfo]:
|
||||||
"""TTL-filtered snapshot of the per-package last-known cache."""
|
"""TTL-filtered snapshot of the per-package last-known cache."""
|
||||||
result: dict[str, MediaSessionInfo] = {}
|
result: dict[str, MediaSessionInfo] = {}
|
||||||
|
|||||||
@@ -0,0 +1,78 @@
|
|||||||
|
"""Entry diagnostics for Nvidia Shield (P4-2b, t21 F1 platform-module move).
|
||||||
|
|
||||||
|
HA discovers ``async_get_config_entry_diagnostics`` through the integration's
|
||||||
|
``diagnostics`` platform module — a placement inside media_player.py never
|
||||||
|
serves in a real install (t20 finding F1).
|
||||||
|
|
||||||
|
The payload: redacted entry config (data + options), the manager's StateStore
|
||||||
|
snapshot via the public ``store_snapshot`` accessor, and both transports'
|
||||||
|
health. Cert paths are kept readable; key material / pairing secrets never
|
||||||
|
exist in stored state, and any key carrying a secret-shaped name is redacted
|
||||||
|
defensively.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import time
|
||||||
|
from dataclasses import asdict, is_dataclass
|
||||||
|
|
||||||
|
from homeassistant.config_entries import ConfigEntry
|
||||||
|
from homeassistant.core import HomeAssistant
|
||||||
|
|
||||||
|
from .connection import ShieldConnectionManager
|
||||||
|
|
||||||
|
_REDACT_KEY_MARKERS = ("pairing", "pin", "secret", "password", "token", "key_material")
|
||||||
|
_REDACTED = "**REDACTED**"
|
||||||
|
|
||||||
|
|
||||||
|
def _redact(obj):
|
||||||
|
"""Redact sensitive keys from a JSON-ish diagnostics payload."""
|
||||||
|
if isinstance(obj, dict):
|
||||||
|
return {
|
||||||
|
key: (
|
||||||
|
_REDACTED
|
||||||
|
if any(m in str(key).lower() for m in _REDACT_KEY_MARKERS)
|
||||||
|
else _redact(value)
|
||||||
|
)
|
||||||
|
for key, value in obj.items()
|
||||||
|
}
|
||||||
|
if isinstance(obj, list):
|
||||||
|
return [_redact(item) for item in obj]
|
||||||
|
return obj
|
||||||
|
|
||||||
|
|
||||||
|
def _store_snapshot(manager) -> dict[str, dict]:
|
||||||
|
"""TTL-stamped store snapshot; dataclass values expand to dicts."""
|
||||||
|
snapshot = {}
|
||||||
|
for name, (stamp, value) in manager.store.values_stamped().items():
|
||||||
|
if is_dataclass(value) and not isinstance(value, type):
|
||||||
|
value = asdict(value)
|
||||||
|
snapshot[name] = {"age": round(time.monotonic() - stamp, 3), "value": value}
|
||||||
|
return snapshot
|
||||||
|
|
||||||
|
|
||||||
|
async def async_get_config_entry_diagnostics(
|
||||||
|
hass: HomeAssistant, entry: ConfigEntry
|
||||||
|
) -> dict:
|
||||||
|
"""Entry diagnostics: redacted config + store snapshot + transport health."""
|
||||||
|
manager = getattr(entry, "runtime_data", None)
|
||||||
|
payload: dict = {
|
||||||
|
"entry": {
|
||||||
|
"data": dict(getattr(entry, "data", {}) or {}),
|
||||||
|
"options": dict(getattr(entry, "options", {}) or {}),
|
||||||
|
},
|
||||||
|
"store": {},
|
||||||
|
"transport": {},
|
||||||
|
}
|
||||||
|
if isinstance(manager, ShieldConnectionManager):
|
||||||
|
payload["store"] = manager.store_snapshot()
|
||||||
|
payload["transport"] = {
|
||||||
|
"adb": {"connected": manager.connected, "awake": manager.awake},
|
||||||
|
"protocol": {
|
||||||
|
"active": manager.protocol_active,
|
||||||
|
"available": manager.is_available,
|
||||||
|
"is_on": manager.is_on,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
return _redact(payload)
|
||||||
|
|
||||||
|
|
||||||
|
__all__ = ["async_get_config_entry_diagnostics"]
|
||||||
@@ -1039,60 +1039,3 @@ def _adb_command_service_maybe_remove(hass) -> None:
|
|||||||
DOMAIN, SERVICE_ADB_COMMAND
|
DOMAIN, SERVICE_ADB_COMMAND
|
||||||
):
|
):
|
||||||
hass.services.async_remove(DOMAIN, SERVICE_ADB_COMMAND)
|
hass.services.async_remove(DOMAIN, SERVICE_ADB_COMMAND)
|
||||||
|
|
||||||
|
|
||||||
# ── P4-2b: diagnostics ──
|
|
||||||
|
|
||||||
_REDACT_KEY_MARKERS = ("pairing", "pin", "secret", "password", "token", "key_material")
|
|
||||||
_REDACTED = "**REDACTED**"
|
|
||||||
|
|
||||||
|
|
||||||
def _redact(obj):
|
|
||||||
"""Redact sensitive keys from a JSON-ish diagnostics payload."""
|
|
||||||
if isinstance(obj, dict):
|
|
||||||
return {
|
|
||||||
key: (
|
|
||||||
_REDACTED
|
|
||||||
if any(m in str(key).lower() for m in _REDACT_KEY_MARKERS)
|
|
||||||
else _redact(value)
|
|
||||||
)
|
|
||||||
for key, value in obj.items()
|
|
||||||
}
|
|
||||||
if isinstance(obj, list):
|
|
||||||
return [_redact(item) for item in obj]
|
|
||||||
return obj
|
|
||||||
|
|
||||||
|
|
||||||
async def async_get_config_entry_diagnostics(hass, entry):
|
|
||||||
"""Entry diagnostics (P4-2b): redacted config + store + transport health.
|
|
||||||
|
|
||||||
HA resolves diagnostics from a ``diagnostics`` module; the function is
|
|
||||||
import-ready there — kept in this module until a pure-diagnostics file
|
|
||||||
is added (Phase 4 trim).
|
|
||||||
"""
|
|
||||||
import dataclasses
|
|
||||||
|
|
||||||
manager = getattr(entry, "runtime_data", None)
|
|
||||||
payload: dict = {
|
|
||||||
"entry": {
|
|
||||||
"data": dict(getattr(entry, "data", {}) or {}),
|
|
||||||
"options": dict(getattr(entry, "options", {}) or {}),
|
|
||||||
},
|
|
||||||
"store": {},
|
|
||||||
"transport": {},
|
|
||||||
}
|
|
||||||
if manager is not None:
|
|
||||||
snapshot = getattr(manager.store, "_values", {})
|
|
||||||
for name, (stamp, value) in snapshot.items():
|
|
||||||
if dataclasses.is_dataclass(value) and not isinstance(value, type):
|
|
||||||
value = dataclasses.asdict(value)
|
|
||||||
payload["store"][name] = {"age": round(time.monotonic() - stamp, 3), "value": value}
|
|
||||||
payload["transport"] = {
|
|
||||||
"adb": {"connected": manager.connected, "awake": manager.awake},
|
|
||||||
"protocol": {
|
|
||||||
"active": manager.protocol_active,
|
|
||||||
"available": manager.is_available,
|
|
||||||
"is_on": manager.is_on,
|
|
||||||
},
|
|
||||||
}
|
|
||||||
return _redact(payload)
|
|
||||||
@@ -31,6 +31,7 @@ ENTITIES_STUB_NAMES = (
|
|||||||
"nsh.power",
|
"nsh.power",
|
||||||
"nsh.jellyfin",
|
"nsh.jellyfin",
|
||||||
"nsh.media_player",
|
"nsh.media_player",
|
||||||
|
"nsh.diagnostics",
|
||||||
"nsh.remote",
|
"nsh.remote",
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -206,12 +207,12 @@ def load_entity_modules() -> dict:
|
|||||||
install()
|
install()
|
||||||
from conftest import _SHIELD_DIR, _load_module
|
from conftest import _SHIELD_DIR, _load_module
|
||||||
|
|
||||||
for name in ("helpers", "power", "jellyfin", "media_player", "remote"):
|
for name in ("helpers", "power", "jellyfin", "media_player", "remote", "diagnostics"):
|
||||||
if f"nsh.{name}" not in sys.modules:
|
if f"nsh.{name}" not in sys.modules:
|
||||||
_load_module(f"nsh.{name}", _SHIELD_DIR / f"{name}.py")
|
_load_module(f"nsh.{name}", _SHIELD_DIR / f"{name}.py")
|
||||||
return {
|
return {
|
||||||
name: sys.modules[f"nsh.{name}"]
|
name: sys.modules[f"nsh.{name}"]
|
||||||
for name in ("power", "jellyfin", "media_player", "remote")
|
for name in ("power", "jellyfin", "media_player", "remote", "diagnostics")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -724,6 +724,47 @@ class TestAdbCommandService:
|
|||||||
assert not player.hass.services.has_service("nvidia_shield", "adb_command")
|
assert not player.hass.services.has_service("nvidia_shield", "adb_command")
|
||||||
|
|
||||||
|
|
||||||
|
class TestServiceRegistryKey:
|
||||||
|
"""F4: the participant key is computed once — add/remove always match."""
|
||||||
|
|
||||||
|
async def test_late_entity_id_assignment_still_removes(self, modules):
|
||||||
|
import entity_stublib
|
||||||
|
from nsh.media_player import (
|
||||||
|
_ADB_COMMAND_TARGETS,
|
||||||
|
_adb_command_service_ensure,
|
||||||
|
_adb_command_service_maybe_remove,
|
||||||
|
)
|
||||||
|
|
||||||
|
mods = load_entity_modules()
|
||||||
|
player, _s, _i = make_player(mods)
|
||||||
|
player._entry.options["adb_command_enabled"] = True
|
||||||
|
player._attr_unique_id = "uid_late"
|
||||||
|
# no entity_id yet at registration (HA core assigns it right after)
|
||||||
|
player.hass = entity_stublib.FakeHass()
|
||||||
|
player.hass.services = FakeServices()
|
||||||
|
if hasattr(player, "_adb_command_key"):
|
||||||
|
del player._adb_command_key # start clean
|
||||||
|
|
||||||
|
# add path — the key is computed once from unique_id
|
||||||
|
key = (
|
||||||
|
getattr(player, "entity_id", None) or player._attr_unique_id
|
||||||
|
)
|
||||||
|
player._adb_command_key = key
|
||||||
|
_ADB_COMMAND_TARGETS[key] = player._manager
|
||||||
|
_adb_command_service_ensure(player.hass)
|
||||||
|
assert "uid_late" in _ADB_COMMAND_TARGETS
|
||||||
|
|
||||||
|
# HA core assigned the entity_id afterwards — removal must still match
|
||||||
|
player.entity_id = "media_player.shield_test"
|
||||||
|
_adb_command_service_maybe_remove(player.hass)
|
||||||
|
assert "uid_late" in _ADB_COMMAND_TARGETS # service stays until unload
|
||||||
|
|
||||||
|
# removal uses the stored key (does not consult the new entity_id)
|
||||||
|
_ADB_COMMAND_TARGETS.pop(player._adb_command_key)
|
||||||
|
_adb_command_service_maybe_remove(player.hass)
|
||||||
|
assert not player.hass.services.has_service("nvidia_shield", "adb_command")
|
||||||
|
|
||||||
|
|
||||||
class TestEntryDiagnostics:
|
class TestEntryDiagnostics:
|
||||||
"""P4-2b: redacted entry diagnostics with store + transport health."""
|
"""P4-2b: redacted entry diagnostics with store + transport health."""
|
||||||
|
|
||||||
@@ -746,7 +787,7 @@ class TestEntryDiagnostics:
|
|||||||
manager.store.set("awake", True)
|
manager.store.set("awake", True)
|
||||||
entry.runtime_data = manager
|
entry.runtime_data = manager
|
||||||
|
|
||||||
from nsh.media_player import async_get_config_entry_diagnostics
|
from nsh.diagnostics import async_get_config_entry_diagnostics
|
||||||
|
|
||||||
payload = await async_get_config_entry_diagnostics(None, entry)
|
payload = await async_get_config_entry_diagnostics(None, entry)
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user