mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
* feat(display-controller): hot-reload plugin enable/disable without a restart Enabling or disabling a plugin in config previously required restarting the display service: the plugin list and available_modes were built once at init and the run loop never revisited them. (Per-plugin config *values* already hot-reloaded; only the enabled set was restart-only.) Now the controller reconciles its running plugins against the config's enabled set whenever that set changes: - The ConfigService watcher thread only sets a `_pending_plugin_reconcile` flag (via a cheap enabled-set diff). It never mutates loop state. - The run loop applies the reconcile on its own thread (top of each iteration, deferred while on-demand is active), so loading/unloading and rebuilding available_modes can't race with rendering. - `_reconcile_enabled_plugins` diffs desired vs running plugins, unloads the removed ones (cleanup + on_disable + config-unsubscribe via the new `_unregister_plugin`) and loads the added ones, then clamps the rotation index so the current mode stays valid. The per-plugin registration done at startup is extracted into `_register_loaded_plugin` and reused by the live-enable path so both build identical state. Extracting it also fixes a latent late-binding bug: the per-plugin config-change callbacks were closures over the loop variable, so every plugin's callback targeted the last-loaded instance; each now binds its own id/instance. Adds test/test_display_controller_plugin_toggle.py covering live enable, live disable, index clamping, no-op when unchanged, and the enabled-set diff. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(display-controller): don't exit on empty available_modes, guard rotation modulo Hot-reload means available_modes can legitimately be empty at startup (no plugins enabled yet) and become non-empty later via the web UI, or vice versa mid-run. Fix four issues found reviewing this PR: - run() exited the process entirely when available_modes was empty at startup instead of idling, permanently defeating the point of live enable/disable for anyone who starts with zero plugins enabled. - The mode-rotation step divided by len(available_modes) unconditionally, raising ZeroDivisionError if the last enabled plugin is disabled between frames. - _reconcile_enabled_plugins() called .get('enabled', False) on a config section without checking it was a dict first, raising AttributeError on a malformed config value. - Minor: pop the config-change callback only after attempting to unsubscribe it, and log the exception in the config-read fallback instead of swallowing it silently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(display-controller): address review findings on the hot-reload PR - Idle-wait tick was a fixed 30s sleep, delaying pickup of a plugin enabled via the web UI while no modes were active. Shortened to ~1s so it's roughly as responsive as the per-frame check once modes exist. - _unregister_plugin popped the config-change callback from _plugin_config_callbacks even when config_service.unsubscribe() raised, losing the only reference to it. Now only pops on a successful unsubscribe. - _pending_plugin_reconcile was cleared before _reconcile_enabled_plugins() ran, so a retryable failure (e.g. plugin discovery erroring) silently dropped the enable/disable request. _reconcile_enabled_plugins() now returns True/False and the caller only clears the flag on True. - Added a warning log for the malformed-config case (a plugin's config section present but not a dict) so it's actually visible, and updated the existing test to assert it via caplog. Left the broad `except Exception` around config_service.unsubscribe() as Exception -- the current implementation is a simple lock+dict/list op that doesn't document or realistically raise a narrower type, so this is a defensive catch-all, not user error handling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: ChuckBuilds <charlesmynard@gmail.com>
256 lines
11 KiB
Python
256 lines
11 KiB
Python
"""Tests for live plugin enable/disable hot-reload in DisplayController.
|
|
|
|
Enabling or disabling a plugin in config used to require a full display
|
|
restart because the plugin list and available_modes were built once at init.
|
|
These tests cover the reconcile path that loads/unloads plugins and rebuilds
|
|
the dispatch maps on the main thread when the enabled set changes.
|
|
"""
|
|
|
|
from unittest.mock import MagicMock
|
|
|
|
|
|
def _make_plugin(modes):
|
|
plugin = MagicMock()
|
|
plugin.modes = list(modes)
|
|
return plugin
|
|
|
|
|
|
def _wire_plugin_manager(controller, plugins, discovered=None):
|
|
"""Point the controller's mock plugin_manager at a set of fake plugins.
|
|
|
|
`plugins` maps plugin_id -> mock instance (with a .modes list).
|
|
"""
|
|
pm = controller.plugin_manager
|
|
pm.discover_plugins.return_value = list(discovered if discovered is not None else plugins.keys())
|
|
pm.load_plugin.return_value = True
|
|
pm.unload_plugin.return_value = True
|
|
pm.plugin_manifests = {}
|
|
pm.get_plugin.side_effect = lambda pid: plugins.get(pid)
|
|
return pm
|
|
|
|
|
|
def _set_config(controller, cfg):
|
|
controller.config_service.get_config = lambda: cfg
|
|
|
|
|
|
class TestPluginEnableDisableHotReload:
|
|
def test_enable_plugin_live(self, test_display_controller):
|
|
controller = test_display_controller
|
|
assert controller.available_modes == []
|
|
|
|
plugin = _make_plugin(["foo"])
|
|
_wire_plugin_manager(controller, {"foo": plugin})
|
|
_set_config(controller, {"foo": {"enabled": True}})
|
|
|
|
controller._reconcile_enabled_plugins()
|
|
|
|
assert "foo" in controller.plugin_display_modes
|
|
assert "foo" in controller.available_modes
|
|
assert controller.plugin_modes["foo"] is plugin
|
|
assert controller.mode_to_plugin_id["foo"] == "foo"
|
|
controller.plugin_manager.load_plugin.assert_any_call("foo")
|
|
|
|
def test_disable_plugin_live(self, test_display_controller):
|
|
controller = test_display_controller
|
|
plugin = _make_plugin(["live", "recent"])
|
|
_wire_plugin_manager(controller, {"sports": plugin}, discovered=["sports"])
|
|
|
|
# Enable, then disable.
|
|
_set_config(controller, {"sports": {"enabled": True}})
|
|
controller._reconcile_enabled_plugins()
|
|
assert "sports" in controller.plugin_display_modes
|
|
assert "live" in controller.available_modes and "recent" in controller.available_modes
|
|
assert "sports" in controller._plugin_config_callbacks
|
|
|
|
_set_config(controller, {"sports": {"enabled": False}})
|
|
controller._reconcile_enabled_plugins()
|
|
|
|
assert "sports" not in controller.plugin_display_modes
|
|
assert "live" not in controller.available_modes
|
|
assert "recent" not in controller.available_modes
|
|
assert "live" not in controller.plugin_modes
|
|
assert "recent" not in controller.mode_to_plugin_id
|
|
controller.plugin_manager.unload_plugin.assert_any_call("sports")
|
|
assert "sports" not in controller._plugin_config_callbacks
|
|
|
|
def test_disable_clamps_current_mode_index(self, test_display_controller):
|
|
controller = test_display_controller
|
|
p1 = _make_plugin(["a"])
|
|
p2 = _make_plugin(["b"])
|
|
_wire_plugin_manager(controller, {"p1": p1, "p2": p2}, discovered=["p1", "p2"])
|
|
|
|
_set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": True}})
|
|
controller._reconcile_enabled_plugins()
|
|
# Add order across multiple plugins is set-driven (as at init), so
|
|
# compare membership, not order.
|
|
assert set(controller.available_modes) == {"a", "b"}
|
|
|
|
# Pretend we're currently showing p2's mode.
|
|
controller.current_mode_index = controller.available_modes.index("b")
|
|
controller.current_display_mode = "b"
|
|
|
|
_set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": False}})
|
|
controller._reconcile_enabled_plugins()
|
|
|
|
assert controller.available_modes == ["a"]
|
|
# Index must be back in range and the display mode no longer the removed one.
|
|
assert 0 <= controller.current_mode_index < len(controller.available_modes)
|
|
assert controller.current_display_mode == "a"
|
|
|
|
def test_enable_keeps_current_mode(self, test_display_controller):
|
|
controller = test_display_controller
|
|
p1 = _make_plugin(["a"])
|
|
p2 = _make_plugin(["b"])
|
|
_wire_plugin_manager(controller, {"p1": p1, "p2": p2}, discovered=["p1", "p2"])
|
|
|
|
_set_config(controller, {"p1": {"enabled": True}})
|
|
controller._reconcile_enabled_plugins()
|
|
controller.current_mode_index = 0
|
|
controller.current_display_mode = "a"
|
|
|
|
# Enabling p2 should not disturb the currently-showing mode.
|
|
_set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": True}})
|
|
controller._reconcile_enabled_plugins()
|
|
|
|
assert "b" in controller.available_modes
|
|
assert controller.current_display_mode == "a"
|
|
assert controller.available_modes[controller.current_mode_index] == "a"
|
|
|
|
def test_noop_when_enabled_set_unchanged(self, test_display_controller):
|
|
controller = test_display_controller
|
|
plugin = _make_plugin(["foo"])
|
|
_wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
|
|
_set_config(controller, {"foo": {"enabled": True}})
|
|
controller._reconcile_enabled_plugins()
|
|
|
|
load_calls = controller.plugin_manager.load_plugin.call_count
|
|
unload_calls = controller.plugin_manager.unload_plugin.call_count
|
|
|
|
# Reconcile again with no change — must not load/unload anything.
|
|
controller._reconcile_enabled_plugins()
|
|
assert controller.plugin_manager.load_plugin.call_count == load_calls
|
|
assert controller.plugin_manager.unload_plugin.call_count == unload_calls
|
|
|
|
def test_reconcile_ignores_non_dict_config_value(self, test_display_controller, caplog):
|
|
"""A malformed config value (e.g. a stray string where a plugin's
|
|
section should be a dict) must be treated as disabled, not crash
|
|
the reconcile with AttributeError, and should be logged so it's
|
|
visible to whoever has to debug the malformed config."""
|
|
controller = test_display_controller
|
|
plugin = _make_plugin(["foo"])
|
|
_wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
|
|
_set_config(controller, {"foo": "not-a-dict"})
|
|
|
|
with caplog.at_level("WARNING"):
|
|
controller._reconcile_enabled_plugins() # must not raise
|
|
|
|
assert "foo" not in controller.plugin_display_modes
|
|
assert "foo" not in controller.available_modes
|
|
assert any("foo" in r.message and "not a dict" in r.message for r in caplog.records)
|
|
|
|
def test_disable_keeps_callback_when_unsubscribe_fails(self, test_display_controller):
|
|
"""If config_service.unsubscribe() raises, _unregister_plugin must
|
|
keep the callback in _plugin_config_callbacks rather than losing the
|
|
only reference to it (it still tears down the plugin itself)."""
|
|
controller = test_display_controller
|
|
plugin = _make_plugin(["live"])
|
|
_wire_plugin_manager(controller, {"sports": plugin}, discovered=["sports"])
|
|
|
|
_set_config(controller, {"sports": {"enabled": True}})
|
|
controller._reconcile_enabled_plugins()
|
|
assert "sports" in controller._plugin_config_callbacks
|
|
|
|
controller.config_service.unsubscribe = MagicMock(side_effect=RuntimeError("boom"))
|
|
|
|
_set_config(controller, {"sports": {"enabled": False}})
|
|
controller._reconcile_enabled_plugins()
|
|
|
|
assert "sports" not in controller.plugin_display_modes
|
|
assert "sports" in controller._plugin_config_callbacks
|
|
|
|
|
|
class TestReconcileReturnValue:
|
|
"""_reconcile_enabled_plugins() returns True/False so the caller (run()'s
|
|
loop) only clears _pending_plugin_reconcile on success, keeping a
|
|
retryable failure's request alive instead of silently dropping it."""
|
|
|
|
def test_returns_true_on_success(self, test_display_controller):
|
|
controller = test_display_controller
|
|
plugin = _make_plugin(["foo"])
|
|
_wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
|
|
_set_config(controller, {"foo": {"enabled": True}})
|
|
assert controller._reconcile_enabled_plugins() is True
|
|
|
|
def test_returns_true_for_noop(self, test_display_controller):
|
|
controller = test_display_controller
|
|
_wire_plugin_manager(controller, {}, discovered=[])
|
|
_set_config(controller, {})
|
|
assert controller._reconcile_enabled_plugins() is True
|
|
|
|
def test_returns_false_on_discovery_failure(self, test_display_controller):
|
|
controller = test_display_controller
|
|
controller.plugin_manager.discover_plugins.side_effect = RuntimeError("boom")
|
|
_set_config(controller, {})
|
|
assert controller._reconcile_enabled_plugins() is False
|
|
|
|
def test_returns_true_when_no_plugin_manager(self, test_display_controller):
|
|
controller = test_display_controller
|
|
controller.plugin_manager = None
|
|
assert controller._reconcile_enabled_plugins() is True
|
|
|
|
|
|
class TestRunWithNoModesEnabled:
|
|
"""Before hot-reload, an empty available_modes at startup was permanent
|
|
-- the display never came back without a restart. Now that a plugin can
|
|
be enabled live from the web UI, run() must idle rather than exit."""
|
|
|
|
def test_idles_instead_of_exiting(self, test_display_controller):
|
|
controller = test_display_controller
|
|
assert controller.available_modes == []
|
|
|
|
sleep_calls = []
|
|
|
|
def fake_sleep(duration, tick_interval=1.0):
|
|
sleep_calls.append(duration)
|
|
if len(sleep_calls) >= 3:
|
|
# Stand in for the process being torn down; run() catches
|
|
# this via its broad except + finally, same as any other
|
|
# unexpected error during the loop.
|
|
raise RuntimeError("stop-test-loop")
|
|
|
|
controller._sleep_with_plugin_updates = fake_sleep
|
|
|
|
controller.run()
|
|
|
|
# Old behavior returned before ever reaching the loop body, so
|
|
# _sleep_with_plugin_updates would never have been called. The idle
|
|
# tick is short (not a long sleep) so a plugin enabled via the web
|
|
# UI while idle is picked up about as promptly as it would be once
|
|
# modes exist and the loop is iterating per-frame.
|
|
assert sleep_calls == [1, 1, 1]
|
|
|
|
|
|
class TestEnabledSetChanged:
|
|
def test_detects_toggle(self, test_display_controller):
|
|
c = test_display_controller
|
|
assert c._enabled_set_changed({"a": {"enabled": True}}, {"a": {"enabled": False}}) is True
|
|
|
|
def test_no_change(self, test_display_controller):
|
|
c = test_display_controller
|
|
cfg = {"a": {"enabled": True}, "b": {"enabled": False}}
|
|
assert c._enabled_set_changed(cfg, dict(cfg)) is False
|
|
|
|
def test_new_enabled_section(self, test_display_controller):
|
|
c = test_display_controller
|
|
assert c._enabled_set_changed(
|
|
{"a": {"enabled": True}},
|
|
{"a": {"enabled": True}, "b": {"enabled": True}},
|
|
) is True
|
|
|
|
def test_ignores_non_enabled_value_edits(self, test_display_controller):
|
|
c = test_display_controller
|
|
assert c._enabled_set_changed(
|
|
{"a": {"enabled": True, "duration": 30}},
|
|
{"a": {"enabled": True, "duration": 45}},
|
|
) is False
|