Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/config.template.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -88,6 +88,7 @@
}
},
"timezone": "America/New_York",
"target_fps": 100,
"location": {
"city": "Tampa",
"state": "Florida",
Expand Down
71 changes: 71 additions & 0 deletions src/plugin_system/base_plugin.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -145,6 +145,77 @@ def display(self, force_clear=False):
"""
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.
#
# An empty dict is treated as "nothing here yet" rather than a
# valid answer, so resolution continues to the next source. Both
# managers default to the same config/config.json, so falling
# through cannot pick up a different file's settings -- but it does
# rescue the case where the first manager simply hasn't loaded yet,
# which would otherwise return {} and silently disable every
# setting read through this property.
if isinstance(config, dict) and config:
return config
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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)
# -------------------------------------------------------------------------
Expand Down
122 changes: 121 additions & 1 deletion test/test_plugin_system.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -250,5 +250,125 @@ def display(self, force_clear=False): pass

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_empty_plugin_manager_config_falls_through(self, mock_display_manager):
"""An empty first source means "not loaded yet", not "the answer".

Both managers default to the same config/config.json, so falling
through cannot pick up a different file. Returning {} here instead
would silently disable every setting read through this property --
the exact failure this property exists to fix.
"""
plugin = self._plugin(
mock_display_manager,
self._manager_with({"target_fps": 100}), # cache_manager
self._manager_with({}), # plugin_manager: empty
)
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 template.get("target_fps") == 100
72 changes: 72 additions & 0 deletions test/test_web_api.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -167,6 +167,78 @@ def test_save_double_sided_settings(self, client, mock_config_manager):
'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', [90.5, 90.0, True])
def test_save_target_fps_rejects_non_integer_json(self, client, mock_config_manager, value):
"""int() would truncate silently: 90.5 -> 90, True -> 1.

Only JSON can carry these; a form post sends '90.5', which int()
already rejects.
"""
response = client.post(
'/api/v3/config/main',
data=json.dumps({'target_fps': value}),
content_type='application/json',
)

assert response.status_code == 400

@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(
Expand Down
32 changes: 31 additions & 1 deletion web_interface/blueprints/api_v3.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -747,6 +747,36 @@ 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):
raw_target_fps = data['target_fps']
# A JSON body can carry real floats and bools, where int() would
# silently truncate: 90.5 would save as 90, and true as 1. Reject
# them rather than storing a value the user did not ask for. Form
# posts arrive as strings, so '90.5' still fails in int() below.
if isinstance(raw_target_fps, (bool, float)):
return jsonify({
'status': 'error',
'message': "Invalid value for target_fps: must be an integer"
}), 400
try:
target_fps = int(raw_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
Comment thread
coderabbitai[bot] marked this conversation as resolved.

# Handle location settings
if 'city' in data or 'state' in data or 'country' in data:
if 'location' not in current_config:
Expand DownExpand Up@@ -1282,7 +1312,7 @@ def separate_secrets(config, secrets_set, prefix=''):
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.
Expand Down
12 changes: 12 additions & 0 deletions web_interface/templates/v3/partials/general.html
Original file line numberDiff line numberDiff line change
Expand Up@@ -49,6 +49,18 @@ <h2 class="text-lg font-semibold text-gray-900">General Settings</h2>
<label for="timezone" class="block text-sm font-medium text-gray-700">Timezone{{ ui.help_tip('Time zone used for clocks, schedules, and time-based content.\nChoose the zone where the display physically lives so on/off schedules fire at the correct local time.', 'Timezone') }}</label>
<div id="timezone_container" class="mt-1"></div>
</div>

<!-- Scroll frame rate (device-wide) -->
<div class="form-group" id="setting-general-target-fps" data-setting-key="target_fps">
<label for="target_fps" class="block text-sm font-medium text-gray-700">Scroll Frame Rate{{ ui.help_tip('Frames per second for scrolling content, applied across plugins that scroll.\nHigher is smoother but uses more CPU; lower frees CPU but looks steppier.\nRange 30-200. Default: 100.', 'Scroll Frame Rate') }}</label>
<input type="number"
id="target_fps"
name="target_fps"
value="{{ main_config.target_fps or 100 }}"
min="30"
max="200"
class="form-control">
</div>
<script>
(function() {
// Track if already initialized to prevent re-render
Expand Down
Loading