diff --git a/config/config.template.json b/config/config.template.json index 6f84e048..f6330567 100644 --- a/config/config.template.json +++ b/config/config.template.json @@ -88,6 +88,7 @@ } }, "timezone": "America/New_York", + "target_fps": 100, "location": { "city": "Tampa", "state": "Florida", diff --git a/src/plugin_system/base_plugin.py b/src/plugin_system/base_plugin.py index 87056b0f..c735e024 100644 --- a/src/plugin_system/base_plugin.py +++ b/src/plugin_system/base_plugin.py @@ -145,6 +145,69 @@ class BasePlugin(ABC): """ raise NotImplementedError("Plugins must implement display()") + # ------------------------------------------------------------------------- + # Global (whole-device) configuration + # ------------------------------------------------------------------------- + @property + def global_config(self) -> Dict[str, Any]: + """ + The full LEDMatrix configuration, for reading device-wide settings. + + ``self.config`` is only this plugin's own slice, so cross-cutting + settings — ``target_fps``, ``timezone``, ``location`` — were previously + unreachable from a plugin without reaching into a manager by hand. + + Resolution order mirrors the timezone helpers the sports plugins + already ship: ``plugin_manager.config_manager`` first (the cores that + hang it there), then ``cache_manager.config_manager``. Returns ``{}`` + when neither is available, so callers can use plain ``.get()`` without + guarding, and a plugin on a core that predates this property still + loads — ``getattr(self, 'global_config', {})`` simply yields the + default. + + Treat as read-only: the returned dict is the live config the core is + using, so mutating it edits every other consumer's view and can be + persisted back to disk. + + Assignment is still allowed and wins over the resolved value. Several + shipped plugins (news, stock-news, ledmatrix-stocks, ledmatrix- + elections, ledmatrix-leaderboard, nfl-draft) set + ``self.global_config`` to their own ``config['global']`` sub-dict; a + property without a setter would raise AttributeError and stop those + plugins loading. + + Example: + fps = self.global_config.get('target_fps') + """ + override = getattr(self, '_global_config_override', None) + if override is not None: + return override + for owner in (self.plugin_manager, self.cache_manager): + config_manager = getattr(owner, 'config_manager', None) + if config_manager is None: + continue + try: + config = config_manager.get_config() + except Exception: + # A broken or unreadable config must never stop a plugin from + # loading; fall through to the next source, then to {}. + self.logger.debug( + "Could not read global config from %s", + type(owner).__name__, exc_info=True, + ) + continue + # Only a real mapping is usable: callers do .get() on this and feed + # the result to numeric code, so handing back whatever a stub or a + # half-built manager returned would fail later and further away. + if isinstance(config, dict) and config: + return config + return {} + + @global_config.setter + def global_config(self, value: Dict[str, Any]) -> None: + """Let a plugin substitute its own view (see the getter's docstring).""" + self._global_config_override = value + # ------------------------------------------------------------------------- # Adaptive layout support (opt-in) # ------------------------------------------------------------------------- diff --git a/test/test_plugin_system.py b/test/test_plugin_system.py index fce98c0f..57d3d48d 100644 --- a/test/test_plugin_system.py +++ b/test/test_plugin_system.py @@ -250,5 +250,110 @@ class TestBasePlugin: config = {"enabled": True, "live_priority": True} plugin = ConcretePlugin("test", config, mock_display_manager, mock_cache_manager, None) - + assert plugin.has_live_priority() is True + + +class TestBasePluginGlobalConfig: + """global_config exposes device-wide settings that self.config cannot. + + The sports scoreboards read `getattr(self, 'global_config', {})` to find + the shared target_fps; before this property existed nothing ever set that + attribute, so the lookup silently returned {} and the setting could never + take effect on any core. + """ + + @staticmethod + def _plugin(display_manager, cache_manager, plugin_manager=None): + from src.plugin_system.base_plugin import BasePlugin + + class ConcretePlugin(BasePlugin): + def update(self): pass + def display(self, force_clear=False): pass + + return ConcretePlugin( + "test", {"enabled": True}, display_manager, cache_manager, plugin_manager + ) + + @staticmethod + def _manager_with(config): + """A stand-in manager exposing config_manager.get_config().""" + manager = MagicMock() + manager.config_manager.get_config.return_value = config + return manager + + def test_reads_config_from_plugin_manager(self, mock_display_manager, mock_cache_manager): + plugin = self._plugin( + mock_display_manager, mock_cache_manager, + self._manager_with({"target_fps": 100}), + ) + assert plugin.global_config["target_fps"] == 100 + + def test_falls_back_to_cache_manager(self, mock_display_manager): + # The core that hangs config_manager off the cache manager instead. + cache_manager = self._manager_with({"target_fps": 75}) + plugin = self._plugin(mock_display_manager, cache_manager, plugin_manager=None) + assert plugin.global_config["target_fps"] == 75 + + def test_plugin_manager_wins_over_cache_manager(self, mock_display_manager): + plugin = self._plugin( + mock_display_manager, + self._manager_with({"target_fps": 75}), + self._manager_with({"target_fps": 100}), + ) + assert plugin.global_config["target_fps"] == 100 + + def test_returns_empty_dict_when_no_config_manager(self, mock_display_manager): + # Plain objects: no config_manager attribute at all. + plugin = self._plugin(mock_display_manager, object(), object()) + assert plugin.global_config == {} + + def test_unreadable_config_does_not_raise(self, mock_display_manager): + # A plugin must still load when the config on disk is broken. + broken = MagicMock() + broken.config_manager.get_config.side_effect = OSError("unreadable") + plugin = self._plugin(mock_display_manager, broken, broken) + assert plugin.global_config == {} + + def test_non_dict_config_is_rejected(self, mock_display_manager): + # A stub or half-built manager can return a non-mapping; handing that + # back would blow up later in numeric code, far from the cause. + plugin = self._plugin( + mock_display_manager, object(), self._manager_with("not-a-dict") + ) + assert plugin.global_config == {} + + def test_missing_property_degrades_to_default(self, mock_display_manager, mock_cache_manager): + # How plugins actually call it, so a plugin written against this core + # still loads on one that predates the property. + plugin = self._plugin(mock_display_manager, mock_cache_manager, object()) + assert getattr(plugin, "global_config", {}).get("target_fps") is None + + def test_plugin_may_still_assign_global_config(self, mock_display_manager, mock_cache_manager): + # news, stock-news, ledmatrix-stocks, ledmatrix-elections, + # ledmatrix-leaderboard and nfl-draft all do exactly this. Without a + # setter the property raises "has no setter" and those plugins stop + # loading entirely. + from src.plugin_system.base_plugin import BasePlugin + + class AssigningPlugin(BasePlugin): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.global_config = self.config.get("global", {}) + + def update(self): pass + def display(self, force_clear=False): pass + + plugin = AssigningPlugin( + "news", {"enabled": True, "global": {"scroll_speed": 2}}, + mock_display_manager, mock_cache_manager, self._manager_with({"target_fps": 100}), + ) + # The plugin's own value wins over the resolved config. + assert plugin.global_config == {"scroll_speed": 2} + + def test_template_ships_a_global_target_fps(self): + # The plumbing is useless if the setting isn't in the shipped config. + import json + with open("config/config.template.json") as fh: + template = json.load(fh) + assert isinstance(template.get("target_fps"), int) diff --git a/test/test_web_api.py b/test/test_web_api.py index d65b11ff..48524158 100644 --- a/test/test_web_api.py +++ b/test/test_web_api.py @@ -167,6 +167,63 @@ class TestConfigAPI: 'enabled': True, 'copies': 2, 'axis': 'vertical', } + def test_save_target_fps(self, client, mock_config_manager): + """The device-wide scroll frame rate persists as a top-level int.""" + response = client.post( + '/api/v3/config/main', + data={'target_fps': '90'}, + content_type='application/x-www-form-urlencoded', + ) + + assert response.status_code == 200 + saved = mock_config_manager.save_config_atomic.call_args[0][0] + # Must be the coerced int, not the raw form string -- the generic + # remaining-keys loop would otherwise write '90' back over it. + assert saved['target_fps'] == 90 + + def test_save_target_fps_alone_does_not_reset_other_general_settings( + self, client, mock_config_manager): + """A target_fps-only POST must not be treated as a full General-tab save. + + The general branch reads web_display_autostart as an unchecked-checkbox + (absent means False), so counting target_fps as a general update would + silently switch autostart off for anyone setting only the frame rate. + """ + mock_config_manager.load_config.return_value['web_display_autostart'] = True + + response = client.post( + '/api/v3/config/main', + data={'target_fps': '90'}, + content_type='application/x-www-form-urlencoded', + ) + + assert response.status_code == 200 + saved = mock_config_manager.save_config_atomic.call_args[0][0] + assert saved['web_display_autostart'] is True + + @pytest.mark.parametrize('value', ['20', '250', 'fast']) + def test_save_target_fps_rejects_out_of_range(self, client, mock_config_manager, value): + """Values ScrollHelper would silently clamp are reported instead.""" + response = client.post( + '/api/v3/config/main', + data={'target_fps': value}, + content_type='application/x-www-form-urlencoded', + ) + + assert response.status_code == 400 + + def test_save_target_fps_accepts_bounds(self, client, mock_config_manager): + """Both endpoints of the documented range are valid.""" + for value in ('30', '200'): + response = client.post( + '/api/v3/config/main', + data={'target_fps': value}, + content_type='application/x-www-form-urlencoded', + ) + assert response.status_code == 200, f"{value} should be accepted" + saved = mock_config_manager.save_config_atomic.call_args[0][0] + assert saved['target_fps'] == int(value) + def test_save_double_sided_unchecked_disables(self, client, mock_config_manager): """An omitted 'enabled' checkbox is saved as disabled, not left stale.""" response = client.post( diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index fef28649..af2b6d1f 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -747,6 +747,26 @@ def save_main_config(): if 'timezone' in data: current_config['timezone'] = data['timezone'] + # Device-wide scroll frame rate, read by plugins via + # BasePlugin.global_config. Bounds match ScrollHelper.set_target_fps, + # which clamps silently -- rejecting here instead means a value that + # would have been quietly altered is reported rather than appearing to + # save and then behaving differently. + if 'target_fps' in data and data['target_fps'] not in ('', None): + try: + target_fps = int(data['target_fps']) + except (ValueError, TypeError): + return jsonify({ + 'status': 'error', + 'message': "Invalid value for target_fps: must be an integer" + }), 400 + if not (30 <= target_fps <= 200): + return jsonify({ + 'status': 'error', + 'message': "Invalid value for target_fps: must be between 30 and 200" + }), 400 + current_config['target_fps'] = target_fps + # Handle location settings if 'city' in data or 'state' in data or 'country' in data: if 'location' not in current_config: @@ -1282,7 +1302,7 @@ def save_main_config(): if key in ['timezone', 'city', 'state', 'country', 'web_display_autostart', 'auto_discover', 'auto_load_enabled', 'development_mode', - 'plugins_directory']: + 'plugins_directory', 'target_fps']: continue # Skip fields that are already handled above in their own named sections. # Without this, every form field name lands as a top-level config key too. diff --git a/web_interface/templates/v3/partials/general.html b/web_interface/templates/v3/partials/general.html index 60cc6f6c..eaf87a2c 100644 --- a/web_interface/templates/v3/partials/general.html +++ b/web_interface/templates/v3/partials/general.html @@ -49,6 +49,18 @@
+ + +