Merge origin/main into #426 after #424/#425 landed

#425 was squash-merged, so #426's base content now lives in main under a
different SHA. Reconciled the three resulting conflicts:

- .github/workflows/test.yml: kept #426's superset test list (its unification
  suites plus test_element_style.py); main had only element_style from #425.
- CHANGELOG.md: kept #426's version, which promotes main's "Unreleased"
  section into the "3.2.0" release and is a verified superset of main's
  content (no lines dropped).
- test_sports_base_characterization.py: kept #426's side for all three blocks.
  Two are semantic, not cosmetic: #425 pinned the pre-fix behaviour
  (hockey/baseball events dropped -> *_returns_none), while #426 carries the
  actual fixes (2486bdb2, 2eea7a7f) and updated the tests to *_still_extracts.
  Taking main's side would fail against #426's fixed code. The third is the
  get_background_service import path, which is .core in #426 after the package
  split. Also refreshed a stale comment that still named the removed test.

#424's global_config plumbing merges in cleanly; verified the interlock end to
end on the merged tree (global_config target_fps -> SportsScrollDisplay 90.0).
435 tests pass across the affected suites; the lone failure
(test_get_system_status) is one of the four pre-existing on main.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
This commit is contained in:
ChuckBuilds
2026-08-02 12:09:29 -04:00
co-authored by Claude Sonnet 5
7 changed files with 312 additions and 5 deletions
+1
View File
@@ -88,6 +88,7 @@
}
},
"timezone": "America/New_York",
"target_fps": 100,
"location": {
"city": "Tampa",
"state": "Florida",
+71
View File
@@ -145,6 +145,77 @@ 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.
#
# 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
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)
# -------------------------------------------------------------------------
+121 -1
View File
@@ -250,5 +250,125 @@ 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_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
+4 -3
View File
@@ -79,9 +79,10 @@ def _competitor(abbr, team_id, score, home_away, record="30-10-5"):
"logo": None,
},
"records": [{"summary": record}],
# The hockey extractor iterates competitor["statistics"] and
# returns None for the whole event when the key is absent (see
# test_hockey_event_without_statistics_returns_none).
# The hockey extractor reads competitor["statistics"] for shot counts;
# it now defaults to an empty list when the key is absent rather than
# dropping the whole event (see
# test_hockey_event_without_statistics_still_extracts).
"statistics": [],
}
+72
View File
@@ -167,6 +167,78 @@ 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', [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(
+31 -1
View File
@@ -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
# Handle location settings
if 'city' in data or 'state' in data or 'country' in data:
if 'location' not in current_config:
@@ -1282,7 +1312,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.
@@ -49,6 +49,18 @@
<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