From c3e4c7855ce42603f6aacb6b9c591bb2802a8178 Mon Sep 17 00:00:00 2001 From: James Mackay Date: Sun, 11 Oct 2026 07:07:06 +0100 Subject: [PATCH] feat(remote): optional command script for remote.send_command (t47 B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One optional 'Remote command script' on the Configure page. Configured, it replaces the whole key path for remote.send_command; unset, repeats, long presses and delays go out as protocol keys exactly as before (the existing send_command tests pass untouched). A remote command is a SEQUENCE — repeats, a START_LONG/END_LONG pair, an inter-repeat delay — so the escape hatch is one script for the call rather than per-key overrides: an IR bridge, a macro, or a second device needs the call, not individual keys. The script receives the service's own parameters as variables, named as they are on the service and documented in the README and pinned in tests: command the list of key commands as submitted (e.g. ["HOME", "KEYCODE_DPAD_CENTER"]) num_repeats the repeat count (default 1) delay_secs the inter-repeat delay (default 0.0) hold_secs the long-press duration, START_LONG/END_LONG handled by the script (default 0.0) — the script owns the key semantics, so it decides what "hold" means All four are always present, including defaults, so a script can rely on the shape. A raising script surfaces a translated error rather than a silent no-op. That made the t44 script-failure mechanism consistent in the process: async_call_script() is now the one caller for both the media player's per-command scripts and the remote's, raising ServiceValidationError(translation_domain="nvidia_shield", translation_key="command_script_failed") with {action, script, error} placeholders instead of a bare HomeAssistantError — the t44 test was updated to assert the translation contract (domain, key, placeholders) rather than a message string, which is stronger evidence and gives the user real copy. The new copy lives in BOTH strings files, as does the unknown_source key from UNIT A. Option handling reuses the t47 C work: the field is an entity selector (so a cleared value is tolerated by the schema), the stored value is trimmed, and a blank, hand-edited or non-string value means "not configured" rather than a call to nowhere. Verified: 454 passed / 2 skipped, ruff clean, drift engine exit 0 under real HA (23 modules, 58 imports, 164 relative imports, 36 call arguments, 7 awaited HA calls). --- .../nvidia_shield/config_flow.py | 8 + custom_components/nvidia_shield/const.py | 26 ++++ .../nvidia_shield/media_player.py | 42 +++-- custom_components/nvidia_shield/remote.py | 36 ++++- custom_components/nvidia_shield/strings.json | 10 +- .../nvidia_shield/translations/en.json | 10 +- tests/test_command_scripts.py | 144 +++++++++++++++++- tests/test_config_flow_ux.py | 51 +++++++ 8 files changed, 299 insertions(+), 28 deletions(-) diff --git a/custom_components/nvidia_shield/config_flow.py b/custom_components/nvidia_shield/config_flow.py index bffafbf..6c7508d 100644 --- a/custom_components/nvidia_shield/config_flow.py +++ b/custom_components/nvidia_shield/config_flow.py @@ -30,6 +30,7 @@ from .const import ( CONF_PROTOCOL_ENABLED, CONF_RECEIVER_ENTITY, CONF_RECEIVER_INPUT, + CONF_REMOTE_COMMAND_SCRIPT, CONF_SKIP_PAIRING, DEFAULT_ADB_PORT, DEFAULT_NAME, @@ -895,6 +896,11 @@ class NvidiaShieldOptionsFlow(config_entries.OptionsFlowWithReload): ).strip() ) } + # t47: one optional script for the remote entity — a plain string + # option, unlike the per-command dict above + remote_script = user_input.get(CONF_REMOTE_COMMAND_SCRIPT) + if remote_script is not None: + user_input[CONF_REMOTE_COMMAND_SCRIPT] = str(remote_script).strip() app_map_text = user_input.pop(CONF_APP_NAME_MAP, None) icons: dict[str, str] = {} if app_map_text is None: @@ -942,6 +948,8 @@ class NvidiaShieldOptionsFlow(config_entries.OptionsFlowWithReload): # t44: command script overrides — the escape hatch for hardware we do not # drive (a speaker switch owning the volume). Unset = native path as before. + schema_dict[vol.Optional(CONF_REMOTE_COMMAND_SCRIPT)] = sel["script"] + configured_scripts = command_scripts(self._options_dict) suggested_scripts = {} for command in SCRIPT_COMMANDS: diff --git a/custom_components/nvidia_shield/const.py b/custom_components/nvidia_shield/const.py index 38b3341..119d719 100644 --- a/custom_components/nvidia_shield/const.py +++ b/custom_components/nvidia_shield/const.py @@ -203,3 +203,29 @@ def command_scripts(options) -> dict[str, str]: if isinstance(script, str) and script.strip(): scripts[command] = script.strip() return scripts + + +# t47: ONE optional script for the remote entity's send_command. It replaces the +# whole key path (repeats, hold, delays included) rather than per-key overrides, +# because a remote command is a sequence: an IR bridge, a macro or a second device +# needs the call, not individual keys. +CONF_REMOTE_COMMAND_SCRIPT = "remote_command_script" +#: Variables remote.send_command hands its script — the names are the service's own +#: parameter names, and they are documented in the README and pinned in tests. +SCRIPT_VAR_COMMAND = "command" +SCRIPT_VAR_NUM_REPEATS = "num_repeats" +SCRIPT_VAR_DELAY_SECS = "delay_secs" +SCRIPT_VAR_HOLD_SECS = "hold_secs" + + +def remote_command_script(options) -> str | None: + """The script configured for remote.send_command, or None (t47). + + Same defensive read as ``command_scripts``: options round-trip through HA + storage and can be hand-edited, so anything that is not a non-empty string is + "not configured" rather than a service call to nowhere. + """ + script = (options or {}).get(CONF_REMOTE_COMMAND_SCRIPT) + if isinstance(script, str) and script.strip(): + return script.strip() + return None diff --git a/custom_components/nvidia_shield/media_player.py b/custom_components/nvidia_shield/media_player.py index f1a539b..d1a0a85 100644 --- a/custom_components/nvidia_shield/media_player.py +++ b/custom_components/nvidia_shield/media_player.py @@ -38,7 +38,7 @@ from homeassistant.components.media_player import ( from homeassistant.config_entries import ConfigEntry from homeassistant.const import STATE_PAUSED, STATE_PLAYING from homeassistant.core import CALLBACK_TYPE, Event, HomeAssistant, callback -from homeassistant.exceptions import HomeAssistantError, ServiceValidationError +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.event import async_call_later, async_track_state_change_event @@ -106,29 +106,41 @@ _PLAYBACK_FEATURES = ( ) -async def async_run_command_script( - hass: HomeAssistant, options: dict, command: str, variables: dict | None = None -) -> bool: - """Run the script configured for ``command``; True when one handled it (t44). +async def async_call_script( + hass: HomeAssistant, script: str, action: str, variables: dict | None = None +) -> None: + """Call a configured script with variables, surfacing failures (t44/t47). - ``blocking=True`` so a script that fails surfaces HERE: a mapped command that - cannot run must be a visible error, never a silent no-op. Module-level so the - remote entity shares it — one configuration governs both routes to POWER. + ``blocking=True`` so a script that fails surfaces HERE: a command that cannot + run must be a visible, translated error rather than a silent no-op. Module-level + so the media player and the remote entity share one implementation. """ - script = command_scripts(options).get(command) - if not script: - return False - data: dict = {"entity_id": script} if variables: data["variables"] = variables try: await hass.services.async_call("script", "turn_on", data, blocking=True) except Exception as err: # noqa: BLE001 - re-raised as a user-facing error - _LOGGER.error("the script for %s (%s) failed: %s", command, script, err) - raise HomeAssistantError( - f"the script configured for {command} ({script}) failed: {err}" + _LOGGER.error("the script for %s (%s) failed: %s", action, script, err) + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="command_script_failed", + translation_placeholders={ + "action": action, + "script": script, + "error": str(err), + }, ) from err + + +async def async_run_command_script( + hass: HomeAssistant, options: dict, command: str, variables: dict | None = None +) -> bool: + """Run the script configured for ``command``; True when one handled it (t44).""" + script = command_scripts(options).get(command) + if not script: + return False + await async_call_script(hass, script, command, variables) return True diff --git a/custom_components/nvidia_shield/remote.py b/custom_components/nvidia_shield/remote.py index 259a7e7..7e90941 100644 --- a/custom_components/nvidia_shield/remote.py +++ b/custom_components/nvidia_shield/remote.py @@ -24,8 +24,16 @@ from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from .connection import ShieldConnectionManager -from .const import DOMAIN, PROTOCOL_REMOTE_KEYS -from .media_player import async_run_command_script, entity_available +from .const import ( + DOMAIN, + PROTOCOL_REMOTE_KEYS, + SCRIPT_VAR_COMMAND, + SCRIPT_VAR_DELAY_SECS, + SCRIPT_VAR_HOLD_SECS, + SCRIPT_VAR_NUM_REPEATS, + remote_command_script, +) +from .media_player import async_call_script, async_run_command_script, entity_available _START_PREFIX = "START_LONG:" _END_PREFIX = "END_LONG:" @@ -162,7 +170,29 @@ class NvidiaShieldRemoteEntity(RemoteEntity): delay_secs: float = 0.0, hold_secs: float = 0.0, ) -> None: - """Send protocol keys: repeats × (key, or START_LONG/hold/END_LONG).""" + """Send protocol keys — or hand the whole command to the configured script. + + t47: with a remote command script configured, it replaces the key path + entirely and receives the service call's own parameters, named as they are + on the service: ``command`` (a list of key commands), ``num_repeats``, + ``delay_secs`` and ``hold_secs``. Unset, nothing changes: repeats, long + presses and delays go out as protocol keys exactly as before. + """ + script = remote_command_script(self._entry.options) + if script: + await async_call_script( + self.hass, + script, + "remote.send_command", + { + SCRIPT_VAR_COMMAND: command, + SCRIPT_VAR_NUM_REPEATS: num_repeats, + SCRIPT_VAR_DELAY_SECS: delay_secs, + SCRIPT_VAR_HOLD_SECS: hold_secs, + }, + ) + return + manager = self._manager_of() if not isinstance(manager, ShieldConnectionManager): return diff --git a/custom_components/nvidia_shield/strings.json b/custom_components/nvidia_shield/strings.json index a37dd14..32e2dc5 100644 --- a/custom_components/nvidia_shield/strings.json +++ b/custom_components/nvidia_shield/strings.json @@ -113,7 +113,8 @@ "script_volume_set": "Volume set script", "script_volume_mute": "Mute script", "script_select_source": "Select source script", - "logcat_events": "Instant app detection (logcat stream)" + "logcat_events": "Instant app detection (logcat stream)", + "remote_command_script": "Remote command script" }, "data_description": { "jellyfin_entity": "The Jellyfin media_player for this Shield. Stream details come from the HA Jellyfin integration \u2014 no extra setup needed.", @@ -129,7 +130,8 @@ "adb_command_enabled": "Adds the nvidia_shield.adb_command service (a raw ADB shell command over the persistent connection). Security-relevant: leave it off unless you need it \u2014 the same opt-in posture as the core androidtv integration.", "channel_digit_delay": "Pause between the channel digits entered via play_media(channel) \u2014 0.05 to 1.0 seconds (default 0.1, the tested value).", "channel_send_enter": "Presses ENTER after the last digit (protocol key first, then the ADB keyevent fallback). Off by default, matching TiviMate's tested behavior.", - "logcat_events": "Detects app changes instantly through a dedicated logcat stream (AdbDevice.streaming_shell). Off by default \u2014 polling keeps the state correct either way, and the integration falls back to polling if the stream keeps failing." + "logcat_events": "Detects app changes instantly through a dedicated logcat stream (AdbDevice.streaming_shell). Off by default \u2014 polling keeps the state correct either way, and the integration falls back to polling if the stream keeps failing.", + "remote_command_script": "One script for remote.send_command. It replaces the key path when set and receives the call's own parameters as variables: command (the list of key commands), num_repeats, delay_secs and hold_secs." } } } @@ -166,6 +168,10 @@ "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." + }, + "command_script_failed": { + "title": "Command script failed", + "message": "The script configured for {action} ({script}) failed: {error}" } }, "entity": { diff --git a/custom_components/nvidia_shield/translations/en.json b/custom_components/nvidia_shield/translations/en.json index a37dd14..32e2dc5 100644 --- a/custom_components/nvidia_shield/translations/en.json +++ b/custom_components/nvidia_shield/translations/en.json @@ -113,7 +113,8 @@ "script_volume_set": "Volume set script", "script_volume_mute": "Mute script", "script_select_source": "Select source script", - "logcat_events": "Instant app detection (logcat stream)" + "logcat_events": "Instant app detection (logcat stream)", + "remote_command_script": "Remote command script" }, "data_description": { "jellyfin_entity": "The Jellyfin media_player for this Shield. Stream details come from the HA Jellyfin integration \u2014 no extra setup needed.", @@ -129,7 +130,8 @@ "adb_command_enabled": "Adds the nvidia_shield.adb_command service (a raw ADB shell command over the persistent connection). Security-relevant: leave it off unless you need it \u2014 the same opt-in posture as the core androidtv integration.", "channel_digit_delay": "Pause between the channel digits entered via play_media(channel) \u2014 0.05 to 1.0 seconds (default 0.1, the tested value).", "channel_send_enter": "Presses ENTER after the last digit (protocol key first, then the ADB keyevent fallback). Off by default, matching TiviMate's tested behavior.", - "logcat_events": "Detects app changes instantly through a dedicated logcat stream (AdbDevice.streaming_shell). Off by default \u2014 polling keeps the state correct either way, and the integration falls back to polling if the stream keeps failing." + "logcat_events": "Detects app changes instantly through a dedicated logcat stream (AdbDevice.streaming_shell). Off by default \u2014 polling keeps the state correct either way, and the integration falls back to polling if the stream keeps failing.", + "remote_command_script": "One script for remote.send_command. It replaces the key path when set and receives the call's own parameters as variables: command (the list of key commands), num_repeats, delay_secs and hold_secs." } } } @@ -166,6 +168,10 @@ "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." + }, + "command_script_failed": { + "title": "Command script failed", + "message": "The script configured for {action} ({script}) failed: {error}" } }, "entity": { diff --git a/tests/test_command_scripts.py b/tests/test_command_scripts.py index dcbce7f..acea314 100644 --- a/tests/test_command_scripts.py +++ b/tests/test_command_scripts.py @@ -20,9 +20,14 @@ import entity_stublib import pytest from nsh.const import ( CONF_COMMAND_SCRIPTS, + CONF_REMOTE_COMMAND_SCRIPT, SCRIPT_VAR_APP_NAME, + SCRIPT_VAR_COMMAND, + SCRIPT_VAR_DELAY_SECS, + SCRIPT_VAR_HOLD_SECS, SCRIPT_VAR_LEVEL, SCRIPT_VAR_MUTED, + SCRIPT_VAR_NUM_REPEATS, ) from test_command_routing import _remote_entity, make_manager, make_player @@ -200,7 +205,7 @@ class TestScriptFailures: """A mapped command that cannot run must be visible, never a silent no-op.""" async def test_a_raising_script_surfaces_a_clean_error(self, modules): - from homeassistant.exceptions import HomeAssistantError + from homeassistant.exceptions import ServiceValidationError player, _served = _player(modules, {"volume_up": WATCH}, is_on=False) @@ -210,13 +215,17 @@ class TestScriptFailures: player.hass.services = _FailingServices() - with pytest.raises(HomeAssistantError) as raised: + with pytest.raises(ServiceValidationError) as raised: await player.async_volume_up() - message = str(raised.value) - assert "volume_up" in message - assert WATCH in message - assert "speaker switch is offline" in message + # translated (t47): the command, the script and the cause are placeholders + assert raised.value.translation_domain == "nvidia_shield" + assert raised.value.translation_key == "command_script_failed" + assert raised.value.translation_placeholders == { + "action": "volume_up", + "script": WATCH, + "error": "speaker switch is offline", + } # ...and it did NOT quietly fall back to the native path assert player._manager._protocol.key_calls == [] assert player._manager._poller.media_calls == [] @@ -309,3 +318,126 @@ def test_the_remote_entry_options_are_the_media_player_options(modules): == entity._entry.options[CONF_COMMAND_SCRIPTS] ) assert make_manager().entry is None # the harness's manager starts unbound + + +WATCH_SCRIPT = "script.shield_remote" + + +class TestRemoteCommandScript: + """t47 B: one optional script for remote.send_command, replacing the key path. + + A remote command is a SEQUENCE (repeats, long presses, delays), so the escape + hatch is one script for the whole call rather than a per-key override: an IR + bridge, a macro or a second device needs the call, not the keys. + """ + + def _entity(self, modules, script=None): + entity = _remote_entity(modules) + served = [] + entity.hass = SimpleNamespace(services=_services(served)) + if script is not None: + entity._entry.options[CONF_REMOTE_COMMAND_SCRIPT] = script + return entity, served + + async def test_a_configured_script_receives_the_exact_parameters(self, modules): + entity, served = self._entity(modules, WATCH_SCRIPT) + + await entity.async_send_command( + ["HOME", "KEYCODE_DPAD_CENTER"], + num_repeats=3, + delay_secs=0.25, + hold_secs=1.5, + ) + + assert served == [ + ( + "script", + "turn_on", + { + "entity_id": WATCH_SCRIPT, + "variables": { + SCRIPT_VAR_COMMAND: ["HOME", "KEYCODE_DPAD_CENTER"], + SCRIPT_VAR_NUM_REPEATS: 3, + SCRIPT_VAR_DELAY_SECS: 0.25, + SCRIPT_VAR_HOLD_SECS: 1.5, + }, + }, + ) + ] + assert entity._manager._protocol.key_calls == [] # key path untouched + + async def test_the_service_defaults_are_named_too(self, modules): + """The variable set is fixed: a script can rely on all four being present.""" + entity, served = self._entity(modules, WATCH_SCRIPT) + + await entity.async_send_command(["home"]) + + variables = served[0][2]["variables"] + assert variables == { + SCRIPT_VAR_COMMAND: ["home"], + SCRIPT_VAR_NUM_REPEATS: 1, + SCRIPT_VAR_DELAY_SECS: 0.0, + SCRIPT_VAR_HOLD_SECS: 0.0, + } + + async def test_unset_keeps_the_native_key_path(self, modules): + entity, served = self._entity(modules) + + await entity.async_send_command(["HOME"], num_repeats=2, hold_secs=0.0) + + assert served == [] + assert entity._manager._protocol.key_calls == [ + ("KEYCODE_HOME", "SHORT"), + ("KEYCODE_HOME", "SHORT"), + ] + + @pytest.mark.parametrize("stored", ["", " ", None, 7]) + async def test_a_blank_or_hand_edited_option_means_unset(self, modules, stored): + entity, served = self._entity(modules, stored) + + await entity.async_send_command(["HOME"]) + + assert served == [] + assert entity._manager._protocol.key_calls == [("KEYCODE_HOME", "SHORT")] + + async def test_a_raising_script_is_a_translated_error(self, modules): + from homeassistant.exceptions import ServiceValidationError + + entity, _served = self._entity(modules, WATCH_SCRIPT) + + class _FailingServices: + async def async_call(self, domain, service, data=None, **kwargs): + raise RuntimeError("IR bridge is offline") + + entity.hass.services = _FailingServices() + + with pytest.raises(ServiceValidationError) as raised: + await entity.async_send_command(["HOME"]) + + assert raised.value.translation_domain == "nvidia_shield" + assert raised.value.translation_key == "command_script_failed" + assert raised.value.translation_placeholders == { + "action": "remote.send_command", + "script": WATCH_SCRIPT, + "error": "IR bridge is offline", + } + assert entity._manager._protocol.key_calls == [] # never a silent no-op + + +def test_both_new_exception_keys_have_copy_in_both_files(): + """A raised translation_key with no copy shows the user a bare key.""" + import json + from pathlib import Path + + root = Path(__file__).resolve().parent.parent / "custom_components" / "nvidia_shield" + for name in ("strings.json", "translations/en.json"): + exceptions = json.loads((root / name).read_text(encoding="utf-8"))["exceptions"] + for key in ("unknown_source", "command_script_failed"): + assert exceptions[key]["title"], (name, key) + assert exceptions[key]["message"], (name, key) + assert "{source}" in json.loads( + (root / "strings.json").read_text(encoding="utf-8") + )["exceptions"]["unknown_source"]["message"] + assert "remote_command_script" in json.loads( + (root / "translations/en.json").read_text(encoding="utf-8") + )["options"]["step"]["init"]["data"] diff --git a/tests/test_config_flow_ux.py b/tests/test_config_flow_ux.py index 1a71dd5..9a8fecc 100644 --- a/tests/test_config_flow_ux.py +++ b/tests/test_config_flow_ux.py @@ -39,6 +39,7 @@ from nsh.const import ( CONF_PAIRING_CODE, CONF_POWER_ENTITY, CONF_PROTOCOL_ENABLED, + CONF_REMOTE_COMMAND_SCRIPT, CONF_SKIP_PAIRING, DEFAULT_ADB_PORT, SCRIPT_COMMANDS, @@ -1696,6 +1697,56 @@ class TestClearingAnEntityField: assert result["data"][CONF_HOST] == "10.0.0.9" +class TestRemoteCommandScriptOption: + """t47 B: one optional script for remote.send_command, on the same page.""" + + async def test_the_field_is_offered_and_stored_as_a_string(self, tmp_path): + flow = _options_flow(tmp_path) + form = await flow.async_step_init(None) + + assert CONF_REMOTE_COMMAND_SCRIPT in vol_mirror.declared_fields( + form["data_schema"] + ) + + result = await flow.async_step_init( + {CONF_REMOTE_COMMAND_SCRIPT: "script.shield_remote"} + ) + + assert result["data"][CONF_REMOTE_COMMAND_SCRIPT] == "script.shield_remote" + + async def test_it_prefills_from_the_entry_options(self, tmp_path): + flow = _options_flow( + tmp_path, {CONF_REMOTE_COMMAND_SCRIPT: "script.shield_remote"} + ) + form = await flow.async_step_init(None) + + assert form["data_schema"]["__suggested__"][CONF_REMOTE_COMMAND_SCRIPT] == ( + "script.shield_remote" + ) + + async def test_a_padded_value_is_trimmed(self, tmp_path): + flow = _options_flow(tmp_path) + + result = await flow.async_step_init( + {CONF_REMOTE_COMMAND_SCRIPT: " script.shield_remote "} + ) + + assert result["data"][CONF_REMOTE_COMMAND_SCRIPT] == "script.shield_remote" + + async def test_an_explicit_empty_value_clears_it(self, tmp_path): + flow = _options_flow( + tmp_path, {CONF_REMOTE_COMMAND_SCRIPT: "script.shield_remote"} + ) + form = await flow.async_step_init(None) + validated = TestClearingAnEntityField._validate( + form, {CONF_REMOTE_COMMAND_SCRIPT: ""} + ) + + result = await flow.async_step_init(validated) + + assert result["data"][CONF_REMOTE_COMMAND_SCRIPT] == "" + + class TestStringsTranslationsParity: """Both copies of the UI copy must expose the same key tree."""