refactor(scroll): route every scrolling plugin through the shared resolver - #462
Merged
Conversation
scripts/test_scroll_card_renders.py has been red on main since #409, which made the safety check fail on every open PR regardless of what that PR touched -- #428 fails it while changing only one plugin's README and manifest. The signal was useless: a real render regression would have looked identical to this noise. #409 deliberately changed which font several text elements draw in, so the goldens committed on 2026-08-02 describe the old, wrong rendering. Measured: at the parent of #409 the guard reports 3 problems, on main it reports 42. These 39 are that difference. Checked rather than assumed. Every regenerated card was compared against its predecessor pixel by pixel, and the four sampled visually: the text content is identical and only the glyph shapes change, which is what a font swap should look like. --update also rewrote 24 cards whose pixels had not changed at all, presumably encoder metadata; those are reverted so the diff is only the 39 that actually differ. Baseball's three upcoming cards are deliberately NOT refreshed. They are the 3 that were already failing before #409, and they fail for an unrelated reason: game_renderer.py formats the date with %-m/%-d and %-I:%M%p, which are glibc extensions. Off glibc they raise, the except falls back to a raw ISO string, and the card loses its start time and shows 2026-09-19 instead of Sep 19. The committed goldens are correct; regenerating them anywhere but glibc would bake the broken output in. Filed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…olver Eleven plugins each hand-rolled scroll configuration and disagreed with one another about what identical config meant. odds-ticker ranked the deprecated scroll_pixels_per_second ABOVE the documented scroll_speed/scroll_delay pair, and because that key carries a schema default the documented settings were dead for every user (#408). ledmatrix-leaderboard read the same key only as a fallback, so the same config produced different speeds in the two plugins. stock-news derived px/frame through its own arithmetic. Nobody was wrong locally; they were just eleven answers to one question. All of them now call src.common.scroll_config.configure(), which resolves every supported config shape in one place, snaps the speed to one the panel can render in whole pixels, and reports the frame hold needed to keep slow speeds crisp. Identical config now means an identical speed everywhere. Each plugin keeps its original logic as a fallback behind try: from src.common import scroll_config as _scroll_config except ImportError: _scroll_config = None because plugins update independently of the core and must keep working against one that predates the helper. Where the legacy block sits inline in __init__ the shared call runs after it and wins, rather than re-indenting logic other config shapes still depend on; those plugins log a line saying so, because two scroll speeds in the journal with no indication which took effect is exactly the confusion this change exists to remove. Plugins pass settings.frame_hold to set_scrolling_state(True, ...) when they begin scrolling. The hold cannot be applied once at construction: plugins share one display manager, and it is reset whenever any other plugin finishes its scroll. Verified on hardware (Pi 4, 2x128x64, 100Hz). Four enabled plugins resolve through the helper with no tracebacks, and ledmatrix-stocks at 50 px/s measures 50.0 fps, median 20.00ms -- exactly two refreshes per frame, with the panel still refreshing at 100Hz so flicker is unchanged. Requires core support: ChuckBuilds/LEDMatrix#523. Without it every plugin takes the fallback path and behaves exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 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. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 36 |
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.
Owner
Author
|
This branch is deployed on the hdpi soak rig (256x64, core 1.
|
Two defects meant display_options.scroll_speed / scroll_delay -- the format
the plugin documents and recommends -- could never take effect.
The priority-1 branch read scroll_pixels_per_second into self rather than
clearing it the way the display_config branch does. config_schema.json gives
that deprecated key a default of 50.0 and schema defaults are merged into
plugin config, so it was never None and use_frame_based() below was never
True. Separately, use_frame_based() only ever inspected display_config, so
even with that fixed the recommended shape still could not select frame-based
mode. Both halves were needed.
The visible symptom was the plugin logging its scroll configuration twice on
startup, ~0.36s apart, the second line quietly overriding the first:
Using display_options.scroll_speed=1.0 px/frame ... (frame-based mode)
Using scroll_pixels_per_second: 50.0 px/s (time-based mode)
with the second one being what actually ran. On a 100Hz panel that meant
0.5px per frame, so every second frame rendered identically, dirty tracking
skipped the panel swap, and the ticker juddered. Editing scroll_speed had no
effect, and deleting scroll_pixels_per_second only restored the schema
default.
Also takes the live-game check off the render path. display() called
_get_current_update_interval() every frame, and its slow path reads the
scoreboard cache from disk and parses JSON per enabled league -- producing a
single ~15ms frame every few minutes, measurable as a stall mid-scroll. The
interval is now memoised for 15s; the scoreboard re-check underneath is rate
limited to 300s regardless, so live detection is unaffected. The neighbouring
debug line used an f-string, so it called _has_live_games() on every skipped
update even with debug logging off; it now uses %s args.
Verified on hardware: odds-ticker selects frame-based mode with no time-based
line following it, and resolves to 100.0 px/s at 1px per refresh.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
A third item for this branch, same shape as the odds-ticker one above.
Same cause as So the branch currently breaks a test double in two of the nine plugins it touches — Worth noting neither shows up in CI today: both are pytest-style files, and |
This was referenced Sep 5, 2026
…o main Three display-manager doubles still declared set_scrolling_state with a single argument, so the frame_hold this PR passes raised TypeError mid-render. display() swallows that in its own except Exception, leaving only "no frame reached the display" -- odds-ticker's cache-invalidation test failed for a reason that had nothing to do with cache invalidation. CI caught one of the three; the other two never reach the scrolling branch. scripts/test_scroll_state_doubles.py checks the invariant instead, and catches all three (including the *args one, which looks tolerant but cannot take a keyword). odds-ticker's double also needed _scroll_frame_hold itself: it borrows only the methods under test, and display() now calls that one. Fixes a continuation line in odds-ticker/manager.py that was indented back to column 12 inside the call -- legal, but not what anyone meant. Merges main. This branch was two commits behind, which made its diff look like it deleted versions[] entries main had gained. Six plugins then needed version bumps they did not need before, because main had bumped past them. ledmatrix-leaderboard and nfl-draft sit one patch above #466's numbers rather than duplicating them -- two PRs must never publish the same version, and a user could not tell which build they have. That pins the merge order these two already needed. Verified against a core with #523 and #534 merged: safety harness clean on all 11 changed plugins, 23 unit tests pass, version and collision gates pass. The one failing repo guard, test_odds_centre_collision.py, fails identically on main and is untouched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
ChuckBuilds
added a commit
that referenced
this pull request
Sep 7, 2026
Resolves the conflicts #462 created when it landed on main. All three were version bookkeeping; no code conflicted, and both branches' code changes are present in the result. plugins/ledmatrix-leaderboard/manifest.json plugins/nfl-draft/manifest.json Both branches bumped the version and added a changelog entry. Neither change supersedes the other -- main's is the shared scroll resolver, this branch's is a validation-sweep fix -- so both entries are kept and this branch's is renumbered above main's (1.3.5/1.3.6 -> 1.3.7 and 1.4.5/1.4.6 -> 1.4.7). Taking either side alone would have dropped a shipped change from the changelog and left a version number that either collides with main or goes backwards. plugins.json Generated, so it is not hand-merged: reset to main's copy and regenerated with update_registry.py from the resolved manifests. Verified after merging: both branches' code survives in the two files git auto-merged (leaderboard's scroll-shortfall measurement and nfl-draft's boolean-False display() return alongside the shared resolver and frame-hold accessor from main), every plugin source parses, every manifest and the registry are valid JSON, and no conflict markers remain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds
added a commit
that referenced
this pull request
Sep 7, 2026
…are (#466) * fix(plugins): eight validation findings — fonts, fit-checks, skip contract, registry ids Found while installing and validating every catalogued plugin on a 256x64 rig. Each fix is independent; grouped because they are all small. hello-world called font_manager.get_font(element_key) with no size. get_font takes (family, size_px), so every frame raised TypeError into a swallowing except and the plugin always fell back to its bundled BDF -- in the template plugin other people copy. Now uses resolve_font(), the accessor for a registered element, with the family and size pulled from one constant so the register and lookup calls cannot drift. Closes #448. christmas-countdown hardcoded lines = ["MERRY", "CHRISTMAS"] on the day itself while the countdown path a few lines below already shortens to "XMAS" when use_xmas says it will not fit. On 64-wide panels the word ran 3px off the edge, for the one day a year that branch is reached. Closes #450. static-image's harness fixture was addressed as "plugins/static-image/..." which only resolves when the plugins directory is called "plugins". On a rig using plugin-repos/ the image was missing, the render was blank, and all eight goldens reported drift -- which reads as a rendering regression, not a missing file. The path is now relative to the plugin, and _resolve_image_path() gained the plugin's own directory as a base so any bundled asset resolves. Closes #453. ledmatrix-leaderboard's scroll-budget warning measured the shortfall over image_width while triggering on image_width + display_width inflated by the safety buffer, so it under-reported the loss and, when only the buffer pushed it over, printed "roughly the last 0% of the list will not be reached". It now measures over the same distance it triggers on and stays quiet when the content actually fits. Closes #454. plugins.json listed leaderboard/music/stocks/weather under ids that disagree with the manifests in their plugin_path. The store therefore offered four already-installed plugins, and installing one deleted the working copy outside the rollback net. Closes #455. nfl-draft returned None from display() on its off-season path. The controller only skips a mode on a boolean False, so "off-season silence" became a black panel held for display_duration -- 60s by default, every rotation, for the ~11 months a year that are not draft season. Closes #456 (plugins side). cozette.bdf was removed from the core's bundled fonts but was still offered in seven plugins' font pickers, where choosing it can only fail. The same cleanup already landed for news and clock-simple. Closes #460. baseball-scoreboard's _make_render_live fixture builds its subject with object.__new__ and omits _font_cache, which SportsCore.__init__ creates and the at-bat card's font ladder needs, so four checks failed on an empty canvas. Closes #463. Verified on the rig: hello-world renders with no TypeError and its fill extent goes 58%->98% (the font-manager face is finally used), christmas-countdown passes all 8 sizes frozen to Dec 25 where 2 failed before, with no change on other dates. * fix(on-air): hold the blank frame for 1s, not the configured display_duration display() deliberately returns True when the light is off, because returning False during an on-demand request drops the controller into its "Initializing" state before the stop is processed. The comment justifying that said the black frame is cycled past "nearly instantly" because get_display_duration() is 1s. It never was: the method returns the configured display_duration, which defaults to 5 and allows up to 60, and has done since v1.0.0. So a rig with on-air enabled but not broadcasting -- the normal state most of the time -- went black for 5s on every rotation. get_display_duration() now reports IDLE_DURATION while the light is off, which makes the existing comment true, and the comment is corrected to say what it actually does. The configured duration still applies when there is a tally to show. Closes #457. * fix(layout): size 7-segment digits around their spacing, fit geochron's 12h clock 7-segment-clock's _calculate_scale_factor solved the scale against the digits and separators only, then the renderer added spacing_gaps * digit_spacing * scale on top. The clock was therefore sized to the panel and *then* made wider, so it ran off the right edge for any digit_spacing >= 3 -- a range config_schema advertises all the way to 10. The gaps scale with the digits, so they belong in the width the scale is solved against. Measured across the full schema range (failures out of 8 sizes): spacing 0 2 3 5 8 10 before 0 0 2 3 6 6 after 0 0 0 0 0 0 including the worst case, 12-hour format with a leading zero at spacing 10. This changes the rendering at the default spacing of 2 as well -- correctly, the digits are now sized to leave room for the gaps -- so two goldens are refreshed. Only 64x64 and 128x96 moved; the other six are byte-identical, which is the expected shape: those are the sizes where the width constraint binds. Both refreshed renders sit well inside the panel (64x64 spans x=12..56, 128x96 x=21..118). geochron's sidebar clock ran off a 128x32 panel with clock_format "12h": the meridiem adds two characters the 24-hour string does not carry, and the sidebar rows were never measured. build_readout now takes a measure callable -- the manager passes draw.textlength with the real font -- and drops the seconds when the full string will not fit, which is the same trade the featured-city row already makes a few lines below. 1 failure -> 0, with the default 24h path unchanged and its goldens still passing. Closes #449, closes #458. * fix(calendar): load .bdf through FreeType, not the legacy PIL bitmap reader calendar routed .bdf files to ImageFont.load(), which is Pillow's legacy PIL-bitmap reader: it expects a .pil metrics file plus a .pbm/.gif/.png glyph image and has never read BDF. Every .bdf in the font picker failed with "cannot find glyph data file" and fell back to the default font, so three of the six offered faces rendered identically and choosing one appeared to do nothing. FreeType reads BDF, so one loader covers .ttf and .bdf alike -- with a retry at the size the file declares, because a bitmap face exists at exactly the one pixel size it was drawn at and FreeType rejects any other with "invalid pixel size". That is the same shape clock-simple, news, countdown, tide-display and jellyfin-now-playing already use; this brings calendar in line. Verified on the rig by hashing the 128x64 render for each face, twice: before after PressStart2P-Regular.ttf 5c68bd5b distinct 4x6-font.ttf 8e0533b7 distinct 5by7.regular.ttf c0ae264c distinct 5x7.bdf \ both the default b8789a63 distinct 4x6.bdf / font fallback 4619e214 distinct Five faces, five distinct renders, identical across both runs, and no load warnings left. Previously the two .bdf faces were byte-identical to each other. Refs #459. The other half of that issue -- baseball/football/hockey/lacrosse expecting a pre-converted .pil that ships nowhere -- is left for now: the test rig's core is on a branch that predates src/common/sports_shared, so those four plugins cannot currently be loaded there to verify a change. * fix(olympics): roll over to the next Games, and fit the countdown to the panel The Games were a single CURRENT_OLYMPICS dict pinned to Milano Cortina 2026. Once those closed nothing advanced, so the countdown went negative and the plugin rendered "-210 DAYS UNTIL WINTER OLYMPICS" -- counting further down every day, and naming the wrong season. OLYMPIC_GAMES is now an ordered table and current_games() returns the Games in progress or the next still to come. Milano Cortina 2026 and Los Angeles 2028 are listed; when the table is exhausted the fetches and the data package report no Games and log which file to add the next one to, so the failure mode is a skipped screen rather than a wrong one. _calculate_days_until also clamps at zero, so a stale table cannot put a negative on the panel whatever the caller does. The countdown card assumed a 256px panel: the logo took the left half and the text was drawn into the right half without ever being measured, so "DAYS UNTIL" and "OLYMPICS" ran off the edge on six of eight sizes. Lines are now measured in the font that will draw them and stepped down to shorter wording, and where even that cannot fit -- at 64px wide the right half is about three characters -- the logo is dropped and the text takes the full width. A countdown nobody can read is worth less than the rings. failures across the 8 sizes: 6 -> 0 Verified on the rig: 256x128 now reads "679 DAYS UNTIL SUMMER OLYMPICS", which matches (date(2028,7,14) - today).days exactly, with the rings intact. Closes #451, closes #452. * fix(flights): bring two stale test doubles back in line with the code test_vegas_map_parity.py failed 4 of 21 on main, both causes stand-ins that had drifted from what they stand in for. fake_fetch took (x, y, zoom) while _fetch_tile gained an allow_network keyword that manager.py passes by name, so every path reaching it raised TypeError. The keyword is now accepted and recorded alongside the coordinates, so a test can assert that a cache-only render never asks to open a socket -- which is the whole point of that parameter and nothing checks it today. Underneath that, FakeLogger implemented debug/info/warning only, and the plugin guards expensive debug formatting behind isEnabledFor. Added, reporting False, along with error/exception. 4 failed, 17 passed -> 21 passed Worth recording that the map-cache code itself was never at fault: with the doubles corrected every assertion in TestMapBackgroundCacheIsSizeAware passes as written. Closes #465. * fix(scripts): stop reporting pytest modules as passing without running them run_plugin_tests.py runs each discovered test_*.py as a subprocess and reads the exit code. That is right for the standalone scripts most plugin tests are, but 11 files at plugin root are pytest modules -- `def test_*` / `class Test*` with no `__main__` guard. Running one of those as a script imports it, defines the tests, executes none, and exits 0, which this runner reported as a pass. Two of the eleven were failing the whole time: four map-cache tests in ledmatrix-flights and one in news. Files are now classified and pytest modules go to pytest, with its outcome mapped onto the same PASS/SKIP/FAIL codes so the summary and the CI gate are unchanged in shape. pytest's "no tests collected" (exit 5) is reported as a failure rather than a pass: for a file we classified as a pytest module it means the classification was wrong, which is exactly the condition this bug hid behind. Same two plugins, before and after: old: [pass] test_vegas_map_parity.py [pass] test_news_ticker.py 13 passed, 0 skipped, 0 failed new: [pass] test_vegas_map_parity.py [FAIL] test_news_ticker.py -- FAILED test_parked_page_rolls_over_when_the_controller_never_resets 19 passed, 0 skipped, 1 failed (The flights file passes on this branch because its stale doubles are fixed in the preceding commit; the news failure belongs to PR #462, which changes news/manager.py without updating its FakeDisplayManager.) Closes #464. * fix(soccer): declare the league-qualified display modes the plugin actually registers The manifest declared soccer_live / soccer_recent / soccer_upcoming. At runtime the plugin registers a mode per configured league -- soccer_eng.1_live, soccer_esp.1_recent and so on -- so none of the three declared names existed. Anything driven off the manifest pointed at modes that are not there: on-demand requests by mode, display.plugin_rotation_order, and the UI's mode lists. The schema defines ten built-in leagues, so the manifest now enumerates all thirty modes, matching what the sibling multi-league plugins already do (baseball declares its milb and ncaa modes even though both are disabled by default). The seven leagues enabled out of the box produce the 21 modes observed on a live rig; the remaining three are declared for when they are switched on. Not covered: custom_leagues lets a user add any ESPN league code, and those modes cannot be enumerated ahead of time. Declaring the built-ins fixes the default install and every preset league; an open-ended set would need the core to accept a prefix or pattern, which is a larger change than this. Refs #461. * fix(sports): load .bdf through FreeType in the four remaining renderers baseball and football looked for a pre-converted .pil/.pbm pair next to the .bdf; hockey and lacrosse did not attempt BDF at all, warning "not supported; convert to PILfont format" and falling back. No .pil ships anywhere in the project, so in practice every .bdf face in those pickers rendered as the default font. The premise was wrong: FreeType reads BDF, which is what the core's FontManager and clock-simple, news, countdown, tide-display, jellyfin-now-playing and calendar all already do. All four now call ImageFont.truetype() and retry at the size the file declares, since a bitmap face exists at exactly one pixel size and FreeType rejects any other with "invalid pixel size". The three stale comments asserting truetype() cannot read BDF are corrected, since that claim is what kept propagating. Verified against a core checkout of main (the soak rig's core is on a branch predating src/common/sports_shared, so these four cannot load there): _bdf_pixel_size("5x7.bdf") = 7, _bdf_pixel_size("4x6.bdf") = 6 truetype(5x7.bdf, 8) -> OSError "invalid pixel size"; at native 7 -> loads truetype(4x6.bdf, 8) -> OSError "invalid pixel size"; at native 6 -> loads so the retry is doing exactly the work the fallback used to skip. Honest limit on the verification: the render-level A/B was inconclusive. With no live game data the 128x64 card does not draw the customized text elements, so all three font choices hash identically before and after. The loader is unit-verified; a render-level check wants seeded fixture data, which the harness files for these plugins do not currently carry. Closes #459. * chore: bump versions for the 19 plugins changed here, and sync the registry CLAUDE.md is explicit that a monorepo plugin change needs a manifest version bump plus update_registry.py, or the store never offers the update and users never receive the fix. Nine commits of fixes went in without it; this catches them all up. ledmatrix-flights is deliberately not bumped: its only change is to a test file (the stale fake_fetch / FakeLogger doubles), which ships nothing to users. The CI workflow draws the same line, skipping the version gate for test-only changes. 7-segment-clock 1.0.5 -> 1.0.6 ledmatrix-leaderboard 1.3.4 -> 1.3.5 afl-scoreboard 1.22.3 -> 1.22.4 ledmatrix-music 1.4.0 -> 1.4.1 baseball-scoreboard 1.40.3 -> 1.40.4 nfl-draft 1.4.4 -> 1.4.5 basketball-scoreboard 1.29.4 -> 1.29.5 nrl-scoreboard 1.21.3 -> 1.21.4 calendar 1.2.5 -> 1.2.6 olympics 2.0.3 -> 2.0.4 christmas-countdown 1.0.3 -> 1.0.4 on-air 1.2.7 -> 1.2.8 football-scoreboard 3.5.1 -> 3.5.2 soccer-scoreboard 2.24.4 -> 2.24.5 geochron 1.0.4 -> 1.0.5 static-image 1.0.7 -> 1.0.8 hello-world 1.1.0 -> 1.1.1 hockey-scoreboard 1.25.3 -> 1.25.4 lacrosse-scoreboard 1.24.4 -> 1.24.5 * fix(hello-world): fit the greeting to the panel; add the versions[] entries CI gates on Two things the on-hardware validation of this branch turned up. hello-world regressed to 3 overflow failures at 64x32, 64x64 and 96x48. The #448 fix is correct -- the font-manager branch finally executes -- but that means the registered 10px face is now used where the bundled BDF fallback used to be, and the plugin never measured its text. It now steps the size down until the string fits, which is what the template plugin should have been demonstrating. 5 PASS / 3 FAIL -> 8 PASS / 0 FAIL. The manifest bumps in the previous commit changed `version` without adding the matching `versions[]` entry. CLAUDE.md is explicit that versions[0] is the entry the store and loader actually read, and scripts/check_manifest_version_fields.py gates it. Each of the 19 now carries a new top entry with its floor renamed to `ledmatrix_min_version`, which is what that gate asks for. scripts/check_manifest_version_fields.py <19 ids> OK: 19 plugin(s) checked, newest version entries are current. ledmatrix-flights still gets no bump. Its only change is a root-level test_*.py, which the CI workflow explicitly excludes from the shipped-code set alongside test/ -- "they change nothing a user receives". * fix: resolve the CI failures on #466 The `safety` job failed on two steps. Both are fixed, and one of them was a bug of mine that no test could have caught. _bdf_pixel_size() was called in five sports plugins and defined in one. The other four raise NameError -- and the call sits inside an `except OSError:` handler that only runs when a BDF font is requested at a size it was not drawn at, which is precisely the case the fix exists for and precisely the case nothing exercised. Every unit test passed, every plugin imported, and the fix was simply dead in four of five. pylint --errors-only reports it as E0602; it was two of Codacy's four new issues. scripts/test_module_helpers_defined.py now checks the general invariant across all 493 plugin files. Placing that helper broke scripts/test_logo_placeholder_refresh.py, which captures _logo_needs_refresh with a regex running to the next "\n\nclass ", so anything between the two reads as part of the helper and the byte-identity check fails. Moved above it. Worth knowing the guard carries that implicit layout constraint. The soccer favourite-team tests failed with ModuleNotFoundError on src.common.sports_shared. Seven of the eight soccer tests point src.__path__ at a real core so genuine submodules resolve while the stubs stay stubbed; this file never got that block, so it broke the day sports.py started importing sports_shared. It read as passing because the CI runner was reporting pytest files without running them -- fixing that runner is what made these 22 failures visible. Closes #468. christmas-countdown and geochron golden drift is NOT from this PR: unmodified main produces byte-identical drift (180/227px and 200/243px, same max deltas), and CI reports those same numbers. The goldens predate the harness setting fontmode="1" to match the panel, so they carry antialiased text the panel cannot show. Refreshed; the rendered text is unchanged, only its antialiasing. This PR is simply the first to touch those two plugins and make CI render them. Codacy's other two: B110 in the olympics measurement guard, narrowed from a bare `except Exception: pass` so a real display-manager fault surfaces instead of silently disabling text fitting; and B603 on the pytest subprocess, annotated with the repo's existing nosec convention (fixed interpreter, argument list, no shell). Bandit is clean. Verified as CI runs it: safety harness passes for all 20 changed plugins, 212 passed / 1 skipped / 0 failed on the unit tests, all repo guards pass except test_odds_centre_collision.py, which fails identically on main and is untouched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * chore: satisfy Codacy's subprocess checks on the test runner Codacy runs Bandit and Opengrep (its Semgrep fork). The pytest subprocess.run this PR adds to scripts/run_plugin_tests.py trips two Opengrep patterns on two different lines -- dangerous-subprocess-use-audit on the run( line and dangerous-subprocess-use-tainted-env-args on the argv line -- and a nosemgrep applies only to its own line, so each needs one. Suppression rather than a rewrite: fixed interpreter, argument list, no shell, so nothing is word-split or expanded. Applied to both call sites so they read consistently. The subprocess import is left exactly as main has it. Annotating it turned an untouched line into a changed one and made Codacy count a pre-existing finding as new. Codacy: 0 new issues, up to standards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eleven plugins each hand-rolled scroll configuration and disagreed with one another about what identical config meant.
odds-tickerranked the deprecatedscroll_pixels_per_secondabove the documentedscroll_speed/scroll_delaypair. Because that key carries a schema default, the documented settings were unreachable for every user — see odds-ticker: display_options.scroll_speed/scroll_delay are silently ignored; frame-based mode is unreachable #408.ledmatrix-leaderboardread the same key only as a fallback, so identical config produced different speeds in the two plugins.stock-newsderived px/frame through its own arithmetic.None of them was wrong locally. They were eleven answers to one question.
What changed
All eleven now call
src.common.scroll_config.configure(), which resolves every supported config shape in one place, snaps the speed to one the panel can render in whole pixels, and reports the frame hold needed to keep slow speeds crisp.odds-tickerledmatrix-leaderboardnewstext-displayledmatrix-stocksstock-newsledmatrix-elections,march-madnessf1-scoreboard,ufc-scoreboardnfl-draftBackwards compatibility
Every plugin keeps its original logic behind:
Plugins update independently of the core, so one running against an un-upgraded core must keep working. Without core support every plugin takes the fallback path and behaves exactly as before.
Where the legacy block sits inline in
__init__, the shared call runs after it and wins, rather than re-indenting logic other config shapes still depend on. Those plugins log a line saying so — two scroll speeds in the journal with no indication which took effect is precisely the confusion this change exists to remove.Frame hold
Plugins pass
settings.frame_holdtoset_scrolling_state(True, ...)when they begin scrolling. It cannot be applied once at construction: plugins share one display manager, and the hold is reset whenever any other plugin finishes its scroll. That bug was caught on hardware — a plugin logged50.0 fpswhile the panel measured100.0 fps.Verified on hardware
Pi 4, 2×128×64 at 100 Hz. Four enabled plugins resolve through the helper, no tracebacks, and
ledmatrix-stocksat 50 px/s measures:median 20.00msis exactly two panel refreshes — the hold is applied, and the panel still refreshes at 100 Hz underneath so flicker is unchanged.Requires
Core support in ChuckBuilds/LEDMatrix#523. Merge that first; this is inert without it.
All 11 manifests bumped and
plugins.jsonregenerated viaupdate_registry.py.🤖 Generated with Claude Code