Give plugins access to device-wide config, and a global scroll frame rate

[ChuckBuilds]

Problem

The sports scoreboards in ledmatrix-plugins read a shared scroll frame rate like this:

global_config=getattr(self, 'global_config', {}) or {}

Nothing ever sets that attribute. The loader constructs plugins with only plugin_id, config, display_manager, cache_manager, plugin_manager (plugin_loader.py:671); global_config appears zero times in all of src/; no plugin manager assigns it; and BasePlugin has no __getattr__ to synthesize it. So the lookup always evaluates to {}.

The consequence: ten scroll_display.py copies thread target_fps through to ScrollHelper, and none of it can ever fire on any core. The plumbing is complete and correct on the plugin side, waiting on a source that doesn't exist.

There was also nothing to find even if it were delivered — the only target_fps in the template is display.vegas_scroll.target_fps, which is Vegas-scoped. A device-wide setting did not exist.

Worth noting for reviewers: I first tried to confirm this from a running rig's logs, but ScrollHelper.set_target_fps logs at debug, so absent log lines prove nothing. The conclusion rests on the static chain above.

What this adds

BasePlugin.global_config — resolves the full config via plugin_manager.config_manager, then cache_manager.config_manager, then {}. That order mirrors the timezone helpers the sports plugins already ship (8d33894).

Two deliberate hardening choices:

• Exceptions are caught and logged at debug. A broken or unreadable config must never stop a plugin from loading.
• A non-dict result is rejected. Callers .get() this and feed the result into numeric code, so handing back whatever a stub or half-built manager returned would fail later and further from the cause.

A top-level target_fps (default 100), on the General tab, validated 30–200 on save. Those bounds match ScrollHelper.set_target_fps, which clamps silently — rejecting at the API means a value that would have been quietly altered gets reported, instead of appearing to save and then behaving differently.

The setter is not incidental

A read-only property would have broken six shipped plugins. news, stock-news, ledmatrix-stocks, ledmatrix-elections, ledmatrix-leaderboard and nfl-draft all do:

self.global_config = config.get('global', {})

Against a getter-only property that raises property 'global_config' of 'NewsTickerPlugin' object has no setter and those plugins stop loading. I reproduced it, then pinned it with a test. Assignment now wins over the resolved value, preserving their existing behaviour exactly.

Similarly, target_fps is deliberately kept out of the is_general_update key list. That branch treats a missing web_display_autostart as an unchecked box, so counting a target_fps-only POST as a General-tab save would silently switch autostart off. There's a regression test for that too.

Verification

End-to-end, with no plugin-side change:

config.json -> BasePlugin.global_config -> scroll_display's existing block -> ScrollHelper
ScrollHelper.target_fps: 120 -> 100

Repeated on real hardware (devpi, 512×64, 22 plugins): set target_fps: 90 in the live config.json, confirmed the property resolved it (90, and timezone alongside it), and confirmed the deployed baseball-scoreboard path drove ScrollHelper 120 -> 90. The four global_config-assigning plugins installed there (ledmatrix-stocks, news, ledmatrix-leaderboard, stock-news) all loaded clean, no tracebacks — the setter proven in situ. The rig was then restored byte-identical and restarted.

15 new tests. Each was checked to fail without the implementation rather than passing vacuously — the --apply-style trap of a test that can't fail. Suite: 1441 passed. Four failures (test_display_dirty_tracking, test_web_api::test_get_system_status, two in test_state_reconciliation) are pre-existing and reproduce identically on a clean tree, verified by stashing.

What this does not do

It does not unify scroll_display.py. The convergence plan calls for the core to ship one, and that's the reason the target_fps change had to be written ten times. But the copies have genuinely diverged along the three documented lineages:

pair	differing lines
soccer ↔ nrl	14
soccer ↔ afl	119
soccer ↔ hockey	524
soccer ↔ football	582
soccer ↔ basketball	655
soccer ↔ baseball	666

Reconciling 500–670 line divergences is a project, not a PR, and doing it badly would be worse than not doing it. This PR delivers the piece that unblocks work already written; the unification is the follow-up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ

Summary by CodeRabbit

• New Features

  • Added a device-wide scroll frame rate setting with a default of 100 FPS.
  • The setting can be configured through the general settings interface.
  • Plugins can access and override shared device configuration when needed.

• Bug Fixes

  • Added validation to accept frame rates from 30 to 200 FPS only.
  • Invalid values are rejected while other general settings remain unchanged.
  • Configuration handling now safely falls back when shared settings are unavailable.

[coderabbitai]

Warning

Review limit reached

@ChuckBuilds, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cb342de6-7450-4fb3-94c5-5f60abd0b298

📥 Commits

Reviewing files that changed from the base of the PR and between 809d676 and 99d0ee2.

📒 Files selected for processing (4)

• src/plugin_system/base_plugin.py
• test/test_plugin_system.py
• test/test_web_api.py
• web_interface/blueprints/api_v3.py

📝 Walkthrough

Walkthrough

Adds device-wide target_fps configuration with UI and API validation. Exposes BasePlugin.global_config with fallback and override behavior. Adds tests for resolution, persistence, validation, boundaries, and setting preservation.

Changes

Target FPS configuration

Layer / File(s)	Summary
Global configuration resolution config/config.template.json, src/plugin_system/base_plugin.py, test/test_plugin_system.py	Adds the target_fps template value and the BasePlugin.global_config property with fallback, validation, and override behavior.
Target FPS web configuration web_interface/templates/v3/partials/general.html, web_interface/blueprints/api_v3.py, test/test_web_api.py	Adds the 30–200 FPS form field, validates submitted values, stores valid integers in the root configuration, and preserves unrelated settings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GeneralForm
  participant save_main_config
  participant RootConfiguration
  GeneralForm->>save_main_config: Submit target_fps
  save_main_config->>save_main_config: Validate integer from 30 through 200
  save_main_config->>RootConfiguration: Store target_fps
  RootConfiguration-->>save_main_config: Persisted configuration

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both primary changes: plugin access to device-wide configuration and a global scroll frame rate.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/plugin-global-config

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity · 0 duplication

Metric	Results
Complexity	9
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/plugin_system/base_plugin.py`:
- Around line 202-203: Update the plugin-manager configuration selection in the
surrounding method to accept any dictionary, including an empty one, so it
retains priority over cache_manager.config_manager; replace the truthiness
requirement while preserving the non-dictionary fallback behavior. Add a
regression test covering an empty plugin-manager configuration alongside a
populated cache-manager configuration and assert the empty dictionary is
returned.

In `@test/test_plugin_system.py`:
- Around line 354-359: Update test_template_ships_a_global_target_fps to assert
that the shipped config’s target_fps value equals 100, rather than only checking
that it is an integer.

In `@web_interface/blueprints/api_v3.py`:
- Around line 755-768: Update the target_fps validation block to reject JSON
floats and booleans before calling int(), while preserving acceptance of valid
integer values and the existing documented 400 error response. Add an API
regression test covering target_fps=90.5 and verify it returns the
integer-validation error without updating current_config.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d3ae3d27-931f-4890-8ead-60377f4c543e

📥 Commits

Reviewing files that changed from the base of the PR and between 5b45f35 and 809d676.

📒 Files selected for processing (6)

• config/config.template.json
• src/plugin_system/base_plugin.py
• test/test_plugin_system.py
• test/test_web_api.py
• web_interface/blueprints/api_v3.py
• web_interface/templates/v3/partials/general.html