diff --git a/CHANGELOG.md b/CHANGELOG.md index c3a83c80..fd21b0b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,18 @@ accepts both, but the store flags the old spelling as deprecated mid-screen and mid-Vegas included, so the route now only starts the service when it is not running. `POST /display/on-demand/stop` reads `stop_service` as a boolean, so `"false"` no longer stops the service. +- On-demand works for a disabled plugin. The display only loads enabled + plugins, so "Preview on display" on a disabled plugin's config page (which + says the plugin will be enabled for the preview) failed with + `invalid-mode`. The display now loads the plugin live for the session, + without writing `enabled` to `config.json`, and unloads it when on-demand + is stopped, expires or moves to another plugin. A plugin that fails to + load reports on-demand status `error` with `load-failed`. A session + restored after a restart unloads its disabled plugin the same way; it used + to stay loaded until the next restart. +- A stop request now clears an on-demand error. After a failed request, + `/display/on-demand/status` kept reporting `status: error` for up to two + minutes even after a stop. ## 3.7.0 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b0784e10..bf94e6c8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -84,7 +84,11 @@ then normal rotation. - **On-demand.** A request from the web interface pins one plugin (or mode) for a duration. `_activate_on_demand()` / `_clear_on_demand()`; the session is saved under `display_on_demand_config` so it survives a - restart. It also keeps the display on during scheduled off hours. + restart. It also keeps the display on during scheduled off hours. A + request for a plugin that is disabled in config loads it live + (`_load_plugin_for_on_demand()`, `load_plugin(force_enabled=True)`) + without writing `config.json`; the main loop unloads it once on-demand + moves off it (`_release_on_demand_plugins()`). - **Live priority.** `_check_live_priority()` looks for a plugin whose `has_live_priority()` and `has_live_content()` are both true and switches to it, rotating between several live games. diff --git a/src/display_controller.py b/src/display_controller.py index e6ed837c..46ff8fd0 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -29,7 +29,7 @@ import threading import types from collections import deque from contextlib import contextmanager -from typing import Dict, Any, List, Optional, Callable, Tuple +from typing import Dict, Any, List, Optional, Callable, Set, Tuple from datetime import datetime from concurrent.futures import ThreadPoolExecutor, as_completed # pylint: disable=no-name-in-module import pytz @@ -266,6 +266,10 @@ class DisplayController: self.on_demand_last_error: Optional[str] = None self.on_demand_last_event: Optional[str] = None self.on_demand_schedule_override = False + # Plugins that are disabled in config and loaded only because an + # on-demand request named them. The main loop unloads each one once + # on-demand has moved off it (_release_on_demand_plugins). + self._on_demand_loaded_plugins: Set[str] = set() self.rotation_resume_index: Optional[int] = None # Saved rotation position when a live-priority plugin preempts the # rotation, so it resumes where it left off (not after the live plugin) @@ -369,7 +373,11 @@ class DisplayController: """Load a single plugin and return result.""" plugin_load_start = time.time() try: - if self.plugin_manager.load_plugin(plugin_id): + if plugin_id in self._on_demand_loaded_plugins: + loaded = self.plugin_manager.load_plugin(plugin_id, force_enabled=True) + else: + loaded = self.plugin_manager.load_plugin(plugin_id) + if loaded: plugin_load_time = time.time() - plugin_load_start return { 'success': True, @@ -1475,8 +1483,13 @@ class DisplayController: On-demand still resumes on its saved mode; this only widens what gets loaded, so normal rotation has somewhere to return to when it ends. A plugin that is disabled in config but named by the on-demand request - is still enabled and added, since otherwise the mode being resumed - would have nothing behind it. + is still loaded, since otherwise the mode being resumed would have + nothing behind it. It is tracked as loaded for on-demand only, the + same as one loaded live by _activate_on_demand, so it is unloaded + when the session ends instead of staying loaded until the next + restart. Its config section is not touched: setting ``enabled`` in + self.config wrote into the dict config_manager caches and returns to + every later load_config() in this process. """ enabled_plugins = [p for p in discovered_plugins if self.config.get(p, {}).get('enabled', False)] @@ -1491,11 +1504,10 @@ class DisplayController: logger.warning("Falling back to normal mode (all enabled plugins)") return enabled_plugins - if not self.config.get(on_demand_plugin_id, {}).get('enabled', False): - logger.info("Temporarily enabling plugin '%s' for on-demand mode", on_demand_plugin_id) - self.config.setdefault(on_demand_plugin_id, {})['enabled'] = True - if on_demand_plugin_id not in enabled_plugins: - enabled_plugins.append(on_demand_plugin_id) + if on_demand_plugin_id not in enabled_plugins: + logger.info("Loading disabled plugin '%s' for on-demand mode only", on_demand_plugin_id) + self._on_demand_loaded_plugins.add(on_demand_plugin_id) + enabled_plugins.append(on_demand_plugin_id) # Restore on-demand state from the cached request so it resumes. self.on_demand_active = True @@ -1591,6 +1603,11 @@ class DisplayController: logger.debug("Stop request %s received but on-demand is not active", request_id) # Still update request_id to acknowledge the request self.on_demand_request_id = request_id + if self.on_demand_status == 'error': + # A failed request left status 'error' published, and + # without this the status route kept reporting it until + # the state aged out (120s) or another request came in. + self._clear_on_demand(reason='requested-stop') # Stop requests are deliberately exempt from the request_id/ # processed_id guards above, so that a second click stops a mode # that a race left running. Consuming the mailbox is therefore the @@ -1757,10 +1774,136 @@ class DisplayController: plugin_id, ordered_modes, self.on_demand_mode_index, ordered_modes[self.on_demand_mode_index] if ordered_modes else 'N/A') + def _load_plugin_for_on_demand(self, plugin_id: str) -> bool: + """Load an installed plugin that isn't running so on-demand can show it. + + This process only loads the plugins enabled in config, so a request + for a disabled one -- the config page's "Preview on display" button + offers it on every plugin -- failed with "invalid-mode" while the UI + said the plugin would be enabled for the session. Nothing did that + short of a restart, and restarts no longer happen on a request. + + Loads through the same path as a live enable (load_plugin, then + _register_loaded_plugin), with force_enabled so the instance runs + enabled while config.json keeps saying disabled. The plugin is + recorded in _on_demand_loaded_plugins, and the main loop unloads it + once on-demand moves off it (_release_on_demand_plugins). + + Returns False after publishing an error when the load fails. A + plugin that isn't installed returns True without loading anything: + the mode checks that follow report it as they always have. + """ + if self.plugin_manager is None: + return True + try: + known = self.plugin_manager.discovered_plugin_ids() + except AttributeError: + known = set(getattr(self.plugin_manager, 'plugin_manifests', ()) or ()) + if plugin_id not in known: + # Installed after this process scanned: the web process checked + # its own, fresher list before posting the request. + try: + known = set(self.plugin_manager.discover_plugins()) + except Exception: # pylint: disable=broad-except + logger.exception("On-demand: plugin discovery failed") + known = set() + if plugin_id not in known: + return True + + logger.info("On-demand: loading disabled plugin '%s' for this session only", plugin_id) + self._on_demand_loaded_plugins.add(plugin_id) + try: + loaded = self.plugin_manager.load_plugin(plugin_id, force_enabled=True) + if loaded: + modes = self._register_loaded_plugin(plugin_id) + logger.info("On-demand: loaded plugin '%s' (modes: %s)", plugin_id, modes) + except Exception: # pylint: disable=broad-except + logger.exception("On-demand: error loading plugin '%s'", plugin_id) + loaded = False + if not loaded: + # Stays in _on_demand_loaded_plugins so the main loop removes + # whatever part of it did get registered. + logger.error("On-demand: could not load plugin '%s'", plugin_id) + self._set_on_demand_error("load-failed") + return False + return True + + def _release_on_demand_plugins(self) -> None: + """Unload plugins loaded only for on-demand that it has moved off. + + Runs from the main loop, right after its own on-demand poll, not + where on-demand ends: a stop, an expiry or the next request is often + read from inside a render loop or a dwell sleep, where the plugin + being released may still be on the stack mid-display(). Unloading + goes through _unregister_plugin, as a live disable does, and nothing + is written to config.json. + + A plugin the user enabled in the meantime stays loaded and takes its + place in the rotation, which is what the reconcile that the enable + queued would have done. + """ + if self.plugin_manager is None: # plugin system failed after startup restore + self._on_demand_loaded_plugins.clear() + return + keep = self.on_demand_plugin_id if self.on_demand_active else None + releasable = [p for p in self._on_demand_loaded_plugins if p != keep] + if not releasable: + return + try: + config = self.config_service.get_config() + except Exception as e: # pylint: disable=broad-except + logger.warning("On-demand release: falling back to cached config: %s", e) + config = self.config + previous_mode = self.current_display_mode + for plugin_id in releasable: + self._on_demand_loaded_plugins.discard(plugin_id) + section = config.get(plugin_id) + if isinstance(section, dict) and section.get('enabled', False): + logger.info("On-demand: keeping plugin '%s' loaded; it was enabled " + "while on-demand showed it", plugin_id) + continue + if (plugin_id in self.plugin_display_modes + or self.plugin_manager.get_plugin(plugin_id) is not None): + logger.info("On-demand: unloading plugin '%s'; it is disabled in config", + plugin_id) + self._unregister_plugin(plugin_id) + if not self.on_demand_active: + # Only outside a session: rotation_resume_index points into + # available_modes until the session ends. + self._apply_plugin_rotation_order() + self._resync_mode_index_after_change(previous_mode) + if self.current_display_mode != previous_mode: + self.force_change = True + + def _rotation_index_outside_on_demand(self, start: int) -> Optional[int]: + """First index from `start` (wrapping) whose mode is not owned by a + plugin loaded only for on-demand, or None if every mode is. + + Ending a session must not resume the rotation onto the plugin that + is about to be unloaded. A live load appends that plugin's modes + after the saved resume index, but a session restored after a + restart has no saved index and its plugin was ordered in with the + rest -- the rotation resumed onto it, and a stop read during its own + screen changed nothing on the panel until that screen ended. + """ + if not self._on_demand_loaded_plugins: + return start + on_demand_only = {mode for plugin_id in self._on_demand_loaded_plugins + for mode in self.plugin_display_modes.get(plugin_id, [])} + count = len(self.available_modes) + for step in range(count): + index = (start + step) % count + if self.available_modes[index] not in on_demand_only: + return index + return None + def _activate_on_demand(self, request: Dict[str, Any]) -> None: """Activate on-demand mode for a specific plugin display.""" plugin_id = request.get('plugin_id') mode = request.get('mode') + if (plugin_id and plugin_id not in self.plugin_display_modes + and not self._load_plugin_for_on_demand(plugin_id)): + return resolved_mode = self._resolve_mode_for_plugin(plugin_id, mode) if not resolved_mode: @@ -1866,6 +2009,15 @@ class DisplayController: self.on_demand_last_event = 'stop-request-ignored' # Already idle self._publish_on_demand_state() return + if not self.on_demand_active and self.on_demand_status == 'error': + # _set_on_demand_error already ended any session and dropped + # rotation_resume_index; the full clear below would only move + # the rotation and force a redraw. Just drop the error. + self.on_demand_status = 'idle' + self.on_demand_last_error = None + self.on_demand_last_event = reason or 'cleared' + self._publish_on_demand_state() + return self._reset_on_demand_fields() self.on_demand_status = 'idle' @@ -1875,17 +2027,27 @@ class DisplayController: # Clear on-demand configuration from cache self.cache_manager.clear_cache('display_on_demand_config') - if self.rotation_resume_index is not None and self.available_modes: - self.current_mode_index = self.rotation_resume_index % len(self.available_modes) - self.current_display_mode = self.available_modes[self.current_mode_index] - logger.info("Resuming rotation from saved index %d: mode '%s'", - self.rotation_resume_index, self.current_display_mode) - elif self.available_modes: - # Default to first mode if no resume index - self.current_mode_index = self.current_mode_index % len(self.available_modes) - self.current_display_mode = self.available_modes[self.current_mode_index] - logger.info("Resuming rotation to mode '%s' (index %d)", - self.current_display_mode, self.current_mode_index) + if self.available_modes: + saved = self.rotation_resume_index + # Default to the current index if no resume index + start = saved if saved is not None else self.current_mode_index + index = self._rotation_index_outside_on_demand(start % len(self.available_modes)) + if index is None: + # Every mode belongs to a plugin loaded only for on-demand, + # which the main loop is about to unload; it then idles. + self.current_mode_index = 0 + self.current_display_mode = None + logger.info("No enabled mode to resume rotation to") + elif saved is not None: + self.current_mode_index = index + self.current_display_mode = self.available_modes[index] + logger.info("Resuming rotation from saved index %d: mode '%s'", + saved, self.current_display_mode) + else: + self.current_mode_index = index + self.current_display_mode = self.available_modes[index] + logger.info("Resuming rotation to mode '%s' (index %d)", + self.current_display_mode, self.current_mode_index) else: logger.warning("No available modes to resume rotation to") @@ -2082,6 +2244,14 @@ class DisplayController: # Handle on-demand commands before rendering self._poll_on_demand_requests() self._check_on_demand_expiration() + # Unload plugins loaded only to show them on-demand once it + # has moved off them. Here, where no display() is on the + # stack; one ended from inside a screen is caught here on + # the next pass. + if self._on_demand_loaded_plugins: + self._release_on_demand_plugins() + if not self.available_modes: + continue # it was all there was; idle as above self._tick_plugin_updates() # Clean up expired WiFi status messages @@ -3099,6 +3269,11 @@ class DisplayController: prepared = prepare(_pid, new_config) if callable(prepare) else None if isinstance(prepared, dict): new_config = prepared + if _pid in self._on_demand_loaded_plugins: + # Saved while on-demand shows it: config.json still + # says disabled, and on_config_change would switch + # the instance off mid-session. + new_config = {**new_config, 'enabled': True} _plugin.on_config_change(new_config) logger.debug("Plugin %s notified of config change", _pid) except Exception as e: diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 6bd798d8..92175416 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -296,7 +296,7 @@ class PluginManager: return plugin_ids - def load_plugin(self, plugin_id: str) -> bool: + def load_plugin(self, plugin_id: str, force_enabled: bool = False) -> bool: """ Load a plugin by ID. @@ -310,6 +310,10 @@ class PluginManager: Args: plugin_id: Plugin identifier + force_enabled: Run the plugin enabled even though config.json has + it disabled. On-demand uses this to show a disabled plugin + (DisplayController._load_plugin_for_on_demand). Only the + instance's config says enabled; config.json is not written. Returns: True if loaded successfully, False otherwise @@ -376,6 +380,12 @@ class PluginManager: # (prepare_plugin_config). In memory only: config.json is written # by saves, never by loading a plugin. config = self.prepare_plugin_config(plugin_id, config, schema=schema) + if force_enabled: + # A copy: prepare_plugin_config can hand back the section from + # config_manager's cached config, and setting the flag there + # would read as enabled to everything else in this process. + config = dict(config) + config['enabled'] = True # Use PluginLoader to load plugin plugin_instance, _module = self.plugin_loader.load_plugin( diff --git a/test/test_on_demand_disabled_plugin.py b/test/test_on_demand_disabled_plugin.py new file mode 100644 index 00000000..072d8805 --- /dev/null +++ b/test/test_on_demand_disabled_plugin.py @@ -0,0 +1,423 @@ +"""On-demand for a plugin that is installed but disabled in config. + +The display process only loads enabled plugins, so a request for a disabled +one -- "Preview on display" offers it on every plugin's config page, with a +note that the plugin will be enabled for the preview -- failed with +"invalid-mode". Nothing loaded it short of a restart, and the on-demand +route no longer restarts the service. + +The display now loads such a plugin live for the session (force_enabled, so +config.json keeps saying disabled) and the main loop unloads it once +on-demand moves off it: a stop, an expiry, or a request for another plugin. + +Also here: a stop sent after a failed request clears the error instead of +leaving status 'error' published until the state ages out. +""" + +import time +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system.plugin_manager import PluginManager +from src.plugin_system.plugin_state import PluginState + + +def _make_plugin(modes): + plugin = MagicMock() + plugin.modes = list(modes) + return plugin + + +@pytest.fixture +def controller(test_display_controller): + """An idle controller running 'clock', with 'preview-me' installed but disabled.""" + c = test_display_controller + clock = _make_plugin(['clock']) + preview = _make_plugin(['preview_a', 'preview_b']) + instances = {'clock': clock} + catalogue = {'clock': clock, 'preview-me': preview} + + def load_plugin(plugin_id, force_enabled=False): + instances[plugin_id] = catalogue[plugin_id] + return True + + def unload_plugin(plugin_id): + return instances.pop(plugin_id, None) is not None + + pm = c.plugin_manager + pm.discovered_plugin_ids.return_value = set(catalogue) + pm.discover_plugins.return_value = list(catalogue) + pm.plugin_manifests = {} + pm.load_plugin = MagicMock(side_effect=load_plugin) + pm.unload_plugin = MagicMock(side_effect=unload_plugin) + pm.get_plugin.side_effect = instances.get + + config = {'clock': {'enabled': True}, 'preview-me': {'enabled': False}} + c.config_service.get_config = lambda: config + c.config_manager.save_config = MagicMock() + c.cache_manager.set = MagicMock() + c.cache_manager.clear_cache = MagicMock() + + c._register_loaded_plugin('clock') + c.current_mode_index = 0 + c.current_display_mode = 'clock' + c.test_config = config + c.test_instances = instances + return c + + +def _start(c, plugin_id='preview-me', mode=None, **extra): + request = {'request_id': 'r-' + plugin_id, 'action': 'start', + 'plugin_id': plugin_id, 'mode': mode or plugin_id} + request.update(extra) + c._activate_on_demand(request) + + +class TestLoadingForOnDemand: + def test_a_disabled_plugin_is_loaded_and_shown(self, controller): + _start(controller) + + controller.plugin_manager.load_plugin.assert_called_once_with( + 'preview-me', force_enabled=True) + assert controller.on_demand_active is True + assert controller.on_demand_status == 'active' + assert controller.on_demand_plugin_id == 'preview-me' + assert controller.current_display_mode == 'preview_a' + assert controller.plugin_display_modes['preview-me'] == ['preview_a', 'preview_b'] + + def test_config_json_is_not_written(self, controller): + _start(controller) + + controller.config_manager.save_config.assert_not_called() + assert controller.test_config['preview-me'] == {'enabled': False} + + def test_a_requested_mode_is_honoured(self, controller): + _start(controller, mode='preview_b') + assert controller.current_display_mode == 'preview_b' + + def test_an_enabled_plugin_is_not_reloaded(self, controller): + _start(controller, plugin_id='clock') + + controller.plugin_manager.load_plugin.assert_not_called() + assert controller.on_demand_active is True + assert controller._on_demand_loaded_plugins == set() + + def test_a_plugin_that_is_not_installed_is_not_loaded(self, controller): + _start(controller, plugin_id='uninstalled') + + controller.plugin_manager.load_plugin.assert_not_called() + assert controller.on_demand_status == 'error' + assert controller.on_demand_last_error == 'invalid-mode' + + def test_a_plugin_installed_after_startup_is_found_by_rescanning(self, controller): + controller.plugin_manager.discovered_plugin_ids.return_value = {'clock'} + + _start(controller) + + controller.plugin_manager.discover_plugins.assert_called() + assert controller.on_demand_active is True + + +class TestLoadFailures: + def test_a_failed_load_reports_load_failed(self, controller): + controller.plugin_manager.load_plugin = MagicMock(return_value=False) + + _start(controller) + + assert controller.on_demand_active is False + assert controller.on_demand_status == 'error' + assert controller.on_demand_last_error == 'load-failed' + assert 'preview_a' not in controller.available_modes + published = controller.cache_manager.set.call_args_list[-1] + assert published.args[0] == 'display_on_demand_state' + assert published.args[1]['status'] == 'error' + assert published.args[1]['error'] == 'load-failed' + + def test_a_load_that_raises_reports_load_failed(self, controller): + controller.plugin_manager.load_plugin = MagicMock(side_effect=ImportError('no module')) + + _start(controller) + + assert controller.on_demand_status == 'error' + assert controller.on_demand_last_error == 'load-failed' + + def test_a_failed_load_leaves_the_rotation_alone(self, controller): + controller.plugin_manager.load_plugin = MagicMock(return_value=False) + + _start(controller) + controller._release_on_demand_plugins() + + assert controller.available_modes == ['clock'] + assert controller.current_display_mode == 'clock' + assert controller._on_demand_loaded_plugins == set() + controller.plugin_manager.unload_plugin.assert_not_called() + + def test_a_plugin_that_loads_but_has_no_modes_is_unloaded_again(self, controller): + """Registered, then the activation fails: the release removes it.""" + controller._on_demand_modes_for_plugin = MagicMock(return_value=[]) + + _start(controller) + assert controller.on_demand_last_error == 'no-modes' + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.available_modes == ['clock'] + + +class TestReleasingThePlugin: + def test_it_stays_loaded_while_on_demand_shows_it(self, controller): + _start(controller) + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_not_called() + assert 'preview_a' in controller.plugin_modes + + def test_a_stop_unloads_it_and_resumes_the_rotation(self, controller): + _start(controller) + controller._clear_on_demand(reason='requested-stop') + # Deferred to the main loop: the stop may be read mid-display(). + controller.plugin_manager.unload_plugin.assert_not_called() + + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.available_modes == ['clock'] + assert 'preview-me' not in controller.plugin_display_modes + assert 'preview_a' not in controller.plugin_modes + assert controller.current_display_mode == 'clock' + assert controller._on_demand_loaded_plugins == set() + assert controller.test_config['preview-me'] == {'enabled': False} + + def test_expiry_unloads_it(self, controller): + _start(controller, duration=30) + controller.on_demand_expires_at = time.time() - 1 + controller._check_on_demand_expiration() + controller._release_on_demand_plugins() + + assert controller.on_demand_last_event == 'expired' + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + + def test_a_request_for_another_plugin_unloads_it(self, controller): + _start(controller) + _start(controller, plugin_id='clock') + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.on_demand_active is True + assert controller.on_demand_plugin_id == 'clock' + assert controller.current_display_mode == 'clock' + + def test_a_failed_request_that_ends_the_session_unloads_it(self, controller): + _start(controller) + _start(controller, plugin_id='uninstalled') + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.current_display_mode == 'clock' + assert controller.force_change is True + + def test_a_plugin_enabled_during_the_session_stays_loaded(self, controller): + _start(controller) + controller.test_config['preview-me'] = {'enabled': True} + controller._clear_on_demand(reason='requested-stop') + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_not_called() + assert 'preview_a' in controller.available_modes + assert controller._on_demand_loaded_plugins == set() + + def test_the_main_loop_releases_right_after_its_own_poll(self, controller): + """A stop read by the main loop unloads before the next screen, not + one screen later. That poll runs with no display() on the stack.""" + import inspect + source = inspect.getsource(type(controller).run) + poll = source.index('self._check_on_demand_expiration()') + release = source.index('self._release_on_demand_plugins()') + render = source.index('self._tick_plugin_updates()') + assert poll < release < render + + def test_a_reconcile_that_runs_first_unloads_it_the_same_way(self, controller): + """A reconcile queued during the session runs at the top of the loop, + before the release: it removes the plugin itself (not in the enabled + set) and the release is then a no-op.""" + _start(controller) + controller._clear_on_demand(reason='requested-stop') + + controller._reconcile_enabled_plugins() + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.available_modes == ['clock'] + assert controller._on_demand_loaded_plugins == set() + + def test_a_config_save_mid_session_keeps_the_instance_enabled(self, controller): + """on_config_change would otherwise read enabled: false and switch it off.""" + controller.config_service.subscribe = MagicMock() + _start(controller) + callback = controller._plugin_config_callbacks['preview-me'] + controller.plugin_manager.prepare_plugin_config = None + + callback({}, {'enabled': False, 'color': 'red'}) + + plugin = controller.plugin_modes['preview_a'] + plugin.on_config_change.assert_called_once_with({'enabled': True, 'color': 'red'}) + + +class TestRestoredSession: + """A restart during a session for a disabled plugin restores it the same way.""" + + def test_the_plugin_is_tracked_and_config_is_left_alone(self, test_display_controller): + c = test_display_controller + c.config.update({'clock': {'enabled': True}, 'disabled-one': {'enabled': False}}) + + selected = c._select_startup_plugins( + ['clock', 'disabled-one'], {'plugin_id': 'disabled-one', 'mode': 'x'}) + + assert 'disabled-one' in selected + assert c._on_demand_loaded_plugins == {'disabled-one'} + assert c.config['disabled-one']['enabled'] is False + + +class TestResumingAfterTheSession: + """Ending a session never resumes the rotation onto the plugin that is + about to be unloaded.""" + + def _restored_session(self, c, other_modes=('clock',)): + """As after a restart: no saved resume index, and the plugin's modes + ordered in ahead of the rest (load order is not deterministic).""" + c._on_demand_loaded_plugins.add('preview-me') + c.plugin_manager.load_plugin('preview-me', force_enabled=True) + c._register_loaded_plugin('preview-me') + c.available_modes = ['preview_a', 'preview_b'] + list(other_modes) + c.on_demand_active = True + c.on_demand_status = 'active' + c.on_demand_plugin_id = 'preview-me' + c.on_demand_modes = ['preview_a', 'preview_b'] + c.rotation_resume_index = None + c.current_mode_index = 0 + c.current_display_mode = 'preview_a' + + def test_a_restored_session_resumes_on_an_enabled_mode(self, controller): + self._restored_session(controller) + + controller._clear_on_demand(reason='requested-stop') + + assert controller.current_display_mode == 'clock' + controller._release_on_demand_plugins() + assert controller.available_modes == ['clock'] + assert controller.current_display_mode == 'clock' + + def test_with_nothing_else_enabled_the_display_goes_idle(self, controller): + controller._unregister_plugin('clock') + self._restored_session(controller, other_modes=()) + + controller._clear_on_demand(reason='requested-stop') + assert controller.current_display_mode is None + + controller._release_on_demand_plugins() + assert controller.available_modes == [] + assert controller.current_display_mode is None + + def test_a_saved_resume_index_is_still_used(self, controller): + c = controller + c.available_modes = ['clock', 'other'] + c.plugin_modes['other'] = MagicMock() + c.current_mode_index = 1 + c.current_display_mode = 'other' + + _start(c) + c._clear_on_demand(reason='requested-stop') + + assert c.current_display_mode == 'other' + + +class TestStopClearsAnError: + def _post_stop(self, c): + stop = {'request_id': 'S1', 'action': 'stop'} + c._last_on_demand_poll = None + c.cache_manager.get = MagicMock( + side_effect=lambda key, *a, **kw: + stop if key == 'display_on_demand_request' else None) + c.cache_manager.delete = MagicMock() + c._poll_on_demand_requests() + + def test_a_stop_after_a_failed_request_clears_the_error(self, controller): + _start(controller, plugin_id='uninstalled') + assert controller.on_demand_status == 'error' + + self._post_stop(controller) + + assert controller.on_demand_status == 'idle' + assert controller.on_demand_last_error is None + state = controller.cache_manager.set.call_args_list[-1].args[1] + assert state['status'] == 'idle' + assert state['error'] is None + + def test_clearing_the_error_leaves_the_rotation_alone(self, controller): + _start(controller, plugin_id='uninstalled') + controller.force_change = False + + self._post_stop(controller) + + assert controller.current_display_mode == 'clock' + assert controller.force_change is False + + def test_a_stop_while_idle_is_still_just_acknowledged(self, controller): + controller._clear_on_demand = MagicMock() + + self._post_stop(controller) + + assert controller.on_demand_status == 'idle' + assert controller.on_demand_request_id == 'S1' + controller._clear_on_demand.assert_not_called() + + +class TestForceEnabledLoad: + """PluginManager.load_plugin(force_enabled=True) runs the plugin enabled + without touching the config it read.""" + + class _Plugin: + def __init__(self, config): + self.config = config + self.enabled_calls = 0 + + def on_enable(self): + self.enabled_calls += 1 + + @pytest.fixture + def pm(self, tmp_path): + plugins_dir = tmp_path / 'plugins' + (plugins_dir / 'demo').mkdir(parents=True) + manager = PluginManager(plugins_dir=str(plugins_dir)) + manager.plugin_manifests['demo'] = {'id': 'demo', 'name': 'Demo'} + manager.schema_manager = MagicMock() + manager.schema_manager.get_schema_path.return_value = None + # Hand the section back as-is, as the fallback path can: the copy in + # load_plugin is what keeps the cached config clean. + manager.schema_manager.prepare_plugin_config.side_effect = ( + lambda pid, cfg, schema=None, changed_paths=None: cfg) + manager.plugin_loader = MagicMock() + manager.plugin_loader.find_plugin_directory.return_value = plugins_dir / 'demo' + manager.plugin_loader.load_plugin.side_effect = ( + lambda **kw: (self._Plugin(kw['config']), None)) + manager.config_manager = MagicMock() + manager.cached_config = {'demo': {'enabled': False, 'color': 'red'}} + manager.config_manager.load_config.return_value = manager.cached_config + return manager + + def test_a_disabled_plugin_loads_disabled_by_default(self, pm): + assert pm.load_plugin('demo') is True + assert pm.plugins['demo'].enabled_calls == 0 + assert pm.state_manager.get_state('demo') == PluginState.DISABLED + + def test_force_enabled_runs_it_enabled(self, pm): + assert pm.load_plugin('demo', force_enabled=True) is True + plugin = pm.plugins['demo'] + assert plugin.config == {'enabled': True, 'color': 'red'} + assert plugin.enabled_calls == 1 + assert pm.state_manager.get_state('demo') == PluginState.ENABLED + + def test_force_enabled_does_not_touch_the_cached_config(self, pm): + pm.load_plugin('demo', force_enabled=True) + assert pm.cached_config['demo'] == {'enabled': False, 'color': 'red'} diff --git a/test/test_on_demand_pinning_and_restart.py b/test/test_on_demand_pinning_and_restart.py index eb33f434..a82baa9a 100644 --- a/test/test_on_demand_pinning_and_restart.py +++ b/test/test_on_demand_pinning_and_restart.py @@ -164,12 +164,14 @@ class TestRestartDoesNotStarveTheOtherPlugins: assert controller.on_demand_mode == 'app_a' assert controller.on_demand_pinned is True - def test_a_disabled_on_demand_plugin_is_enabled_and_loaded(self, controller): - """Otherwise the mode being resumed has nothing behind it.""" + def test_a_disabled_on_demand_plugin_is_still_loaded(self, controller): + """Otherwise the mode being resumed has nothing behind it. It loads + for on-demand only; its config section is left disabled.""" selected = controller._select_startup_plugins( self.DISCOVERED, {'plugin_id': 'disabled-one', 'mode': 'x'}) assert 'disabled-one' in selected - assert controller.config['disabled-one']['enabled'] is True + assert controller._on_demand_loaded_plugins == {'disabled-one'} + assert controller.config['disabled-one']['enabled'] is False def test_an_unknown_on_demand_plugin_falls_back_to_normal(self, controller): selected = controller._select_startup_plugins( diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index 6cb2ca4c..40bb83a3 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -180,9 +180,9 @@ def start_on_demand_display(): if not resolved_plugin: return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404 - # Note: On-demand can work with disabled plugins - the display controller - # will temporarily enable them during initialization if needed - # We don't block the request here, but log it for debugging + # On-demand works with disabled plugins: the running display loads one + # for the session and unloads it afterwards, leaving config.json alone + # (DisplayController._load_plugin_for_on_demand). Logged for debugging. if api_v3.config_manager and resolved_plugin: config = api_v3.config_manager.load_config() plugin_config = config.get(resolved_plugin, {})