mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
fix(vegas): restore live plugin-update refresh dropped by sync refactor (#395)
Investigating a user report that Vegas scroll mode doesn't update scores or game status. Root cause: PR #299 (Mar 28) added a mechanism so a live score change reached the ticker within a few seconds instead of waiting for a full scroll cycle -- _tick_plugin_updates_for_vegas() diffed plugin_last_update timestamps to detect which plugins got fresh data and called coordinator.mark_plugin_updated() for each, and should_recompose() checked has_pending_updates_for_visible_segments() to trigger an immediate hot-swap. PR #330 (May 14, multi-display wireless sync) refactored both call sites while adding sync support and silently deleted this entire mechanism -- not just gated it behind the new sync-mode deferral it legitimately needed, but removed it outright. The result: VegasModeCoordinator. mark_plugin_updated() and StreamManager.has_pending_updates_for_visible_ segments() have been fully implemented but never called from anywhere since. Vegas mode's only remaining freshness sources are a 5s content cache TTL (fine) and full recompose at cycle boundaries, which depending on min/max_cycle_duration can be minutes away -- so live scores/status can sit stale far longer than a user would expect from a "live" ticker. Fix: - Restored _tick_plugin_updates_for_vegas() in display_controller.py, wired as the Vegas coordinator's update callback in place of the plain _tick_plugin_updates(). Diffs plugin_last_update before/after the tick and calls vegas_coordinator.mark_plugin_updated(plugin_id) for each plugin that actually got new data (rather than returning the list, since the callback interface no longer consumes a return value). - Restored the has_pending_updates_for_visible_segments() check in render_pipeline.should_recompose(), positioned after (not instead of) the sync-mode early return PR #330 added, so standalone installations regain immediate refresh while synced leader/follower pairs correctly keep deferring hot-swaps to cycle boundaries as PR #330 intended. Test plan: - Added test_display_controller_vegas_tick.py and test_vegas_render_pipeline_recompose.py -- neither area had any prior test coverage, which is very likely why this regression went unnoticed for ~2.5 months. - Verified both new test files fail against the pre-fix code (swapped in the current main versions of both files) with exactly the expected errors -- AttributeError for the deleted method, and the recompose assertion returning False instead of True -- then pass against the fix. - Confirmed the sync-mode deferral this restoration must not break still holds: test_sync_active_defers_pending_updates_to_cycle_boundary. - Full related suite (test_vegas_plugin_adapter, test_vegas_config, test_display_controller_plugin_toggle, test_display_controller_ optimizations, test_plugin_system): 108 passed, 1 pre-existing failure unrelated to this change (test_circuit_breaker, stale mock signature). - Full CI plugin-safety suite (test_harness, test_visual_rendering, test_plugin_matrix): 52 passed, 2 pre-existing skips.
This commit is contained in:
@@ -502,7 +502,10 @@ class DisplayController:
|
|||||||
|
|
||||||
# Run plugin updates inside the Vegas loop so the inter-iteration
|
# Run plugin updates inside the Vegas loop so the inter-iteration
|
||||||
# gap is <1 ms (nothing left for _tick_plugin_updates() to do).
|
# gap is <1 ms (nothing left for _tick_plugin_updates() to do).
|
||||||
self.vegas_coordinator.set_update_callback(self._tick_plugin_updates)
|
# Use the Vegas-aware variant so plugins that got fresh data are
|
||||||
|
# hot-swapped into the scroll promptly instead of waiting for the
|
||||||
|
# next full cycle.
|
||||||
|
self.vegas_coordinator.set_update_callback(self._tick_plugin_updates_for_vegas)
|
||||||
|
|
||||||
# Wire multi-display sync into Vegas render pipeline
|
# Wire multi-display sync into Vegas render pipeline
|
||||||
follower_pos = self.config.get("sync", {}).get("follower_position", "left")
|
follower_pos = self.config.get("sync", {}).get("follower_position", "left")
|
||||||
@@ -828,6 +831,42 @@ class DisplayController:
|
|||||||
if hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker:
|
if hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker:
|
||||||
self.plugin_manager.health_tracker.record_failure(plugin_id, exc)
|
self.plugin_manager.health_tracker.record_failure(plugin_id, exc)
|
||||||
|
|
||||||
|
def _tick_plugin_updates_for_vegas(self):
|
||||||
|
"""Run scheduled plugin updates and tell Vegas mode which plugins
|
||||||
|
actually got fresh data, so it can hot-swap them into the scroll
|
||||||
|
without waiting for a full cycle to complete.
|
||||||
|
|
||||||
|
Used as the Vegas coordinator's update callback instead of the plain
|
||||||
|
_tick_plugin_updates() so that a live score change is reflected in
|
||||||
|
the ticker within a few seconds rather than at the next cycle
|
||||||
|
boundary (which, depending on min/max_cycle_duration, can be
|
||||||
|
minutes away). Restores wiring that PR #299 added and PR #330's
|
||||||
|
sync-mode refactor inadvertently dropped: coordinator.mark_plugin_updated()
|
||||||
|
has been unreachable dead code since.
|
||||||
|
"""
|
||||||
|
if not self.plugin_manager or not hasattr(self.plugin_manager, "plugin_last_update"):
|
||||||
|
self._tick_plugin_updates()
|
||||||
|
return
|
||||||
|
|
||||||
|
old_times = dict(self.plugin_manager.plugin_last_update)
|
||||||
|
self._tick_plugin_updates()
|
||||||
|
|
||||||
|
vc = getattr(self, "vegas_coordinator", None)
|
||||||
|
if vc is None:
|
||||||
|
return
|
||||||
|
|
||||||
|
updated = [
|
||||||
|
plugin_id for plugin_id, new_time in self.plugin_manager.plugin_last_update.items()
|
||||||
|
if new_time > old_times.get(plugin_id, 0.0)
|
||||||
|
]
|
||||||
|
if updated:
|
||||||
|
logger.info("Vegas update tick: %d plugin(s) updated: %s", len(updated), updated)
|
||||||
|
for plugin_id in updated:
|
||||||
|
try:
|
||||||
|
vc.mark_plugin_updated(plugin_id)
|
||||||
|
except Exception: # pylint: disable=broad-except
|
||||||
|
logger.exception("Error marking plugin %s updated for Vegas", plugin_id)
|
||||||
|
|
||||||
def _tick_plugin_updates(self):
|
def _tick_plugin_updates(self):
|
||||||
"""Run scheduled plugin updates if the plugin manager supports them."""
|
"""Run scheduled plugin updates if the plugin manager supports them."""
|
||||||
if not self.plugin_manager:
|
if not self.plugin_manager:
|
||||||
|
|||||||
@@ -297,6 +297,8 @@ class RenderPipeline:
|
|||||||
Returns True when:
|
Returns True when:
|
||||||
- Cycle is complete and we should start fresh
|
- Cycle is complete and we should start fresh
|
||||||
- Staging buffer has new content
|
- Staging buffer has new content
|
||||||
|
- A plugin currently visible in the scroll has pending updated data
|
||||||
|
(e.g. a live score changed) — standalone (non-sync) mode only
|
||||||
"""
|
"""
|
||||||
if self._cycle_complete:
|
if self._cycle_complete:
|
||||||
return True
|
return True
|
||||||
@@ -314,6 +316,12 @@ class RenderPipeline:
|
|||||||
if buffer_status['staging_count'] > 0:
|
if buffer_status['staging_count'] > 0:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
# Trigger recompose when pending updates affect visible segments, so
|
||||||
|
# live score/status changes reach the display within a few seconds
|
||||||
|
# instead of waiting for the next full cycle.
|
||||||
|
if self.stream_manager.has_pending_updates_for_visible_segments():
|
||||||
|
return True
|
||||||
|
|
||||||
return False
|
return False
|
||||||
|
|
||||||
def hot_swap_content(self) -> bool:
|
def hot_swap_content(self) -> bool:
|
||||||
|
|||||||
@@ -0,0 +1,76 @@
|
|||||||
|
"""
|
||||||
|
Regression tests for DisplayController._tick_plugin_updates_for_vegas().
|
||||||
|
|
||||||
|
PR #299 added logic to detect which plugins actually got fresh data on a
|
||||||
|
scheduled-update tick and notify Vegas mode via
|
||||||
|
vegas_coordinator.mark_plugin_updated(), so a live score change reaches the
|
||||||
|
scroll within seconds instead of waiting for a full cycle. PR #330's
|
||||||
|
multi-display sync refactor deleted this method (folding the callback back
|
||||||
|
to the plain _tick_plugin_updates(), which reports nothing), silently
|
||||||
|
orphaning VegasModeCoordinator.mark_plugin_updated() -- it has had zero
|
||||||
|
callers since.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
from src.display_controller import DisplayController
|
||||||
|
|
||||||
|
|
||||||
|
def _make_controller(plugin_last_update, vegas_coordinator=None):
|
||||||
|
dc = object.__new__(DisplayController)
|
||||||
|
dc.plugin_manager = MagicMock()
|
||||||
|
dc.plugin_manager.plugin_last_update = dict(plugin_last_update)
|
||||||
|
dc.vegas_coordinator = vegas_coordinator
|
||||||
|
return dc
|
||||||
|
|
||||||
|
|
||||||
|
class TestTickPluginUpdatesForVegas:
|
||||||
|
def test_marks_only_plugins_whose_timestamp_advanced(self):
|
||||||
|
dc = _make_controller({"stock-news": 100.0, "odds-ticker": 100.0})
|
||||||
|
vc = MagicMock()
|
||||||
|
dc.vegas_coordinator = vc
|
||||||
|
|
||||||
|
# Simulate run_scheduled_updates() advancing only stock-news.
|
||||||
|
def fake_tick():
|
||||||
|
dc.plugin_manager.plugin_last_update["stock-news"] = 200.0
|
||||||
|
dc._tick_plugin_updates = fake_tick
|
||||||
|
|
||||||
|
dc._tick_plugin_updates_for_vegas()
|
||||||
|
|
||||||
|
vc.mark_plugin_updated.assert_called_once_with("stock-news")
|
||||||
|
|
||||||
|
def test_no_advance_marks_nothing(self):
|
||||||
|
dc = _make_controller({"stock-news": 100.0})
|
||||||
|
vc = MagicMock()
|
||||||
|
dc.vegas_coordinator = vc
|
||||||
|
dc._tick_plugin_updates = lambda: None
|
||||||
|
|
||||||
|
dc._tick_plugin_updates_for_vegas()
|
||||||
|
|
||||||
|
vc.mark_plugin_updated.assert_not_called()
|
||||||
|
|
||||||
|
def test_no_vegas_coordinator_does_not_raise(self):
|
||||||
|
dc = _make_controller({"stock-news": 100.0}, vegas_coordinator=None)
|
||||||
|
|
||||||
|
def fake_tick():
|
||||||
|
dc.plugin_manager.plugin_last_update["stock-news"] = 200.0
|
||||||
|
dc._tick_plugin_updates = fake_tick
|
||||||
|
|
||||||
|
dc._tick_plugin_updates_for_vegas() # must not raise
|
||||||
|
|
||||||
|
def test_mark_plugin_updated_exception_does_not_propagate(self):
|
||||||
|
"""One plugin's mark_plugin_updated failing must not stop the tick
|
||||||
|
or crash the update loop it runs in."""
|
||||||
|
dc = _make_controller({"a": 1.0, "b": 1.0})
|
||||||
|
vc = MagicMock()
|
||||||
|
vc.mark_plugin_updated.side_effect = [RuntimeError("boom"), None]
|
||||||
|
dc.vegas_coordinator = vc
|
||||||
|
|
||||||
|
def fake_tick():
|
||||||
|
dc.plugin_manager.plugin_last_update["a"] = 2.0
|
||||||
|
dc.plugin_manager.plugin_last_update["b"] = 2.0
|
||||||
|
dc._tick_plugin_updates = fake_tick
|
||||||
|
|
||||||
|
dc._tick_plugin_updates_for_vegas() # must not raise
|
||||||
|
|
||||||
|
assert vc.mark_plugin_updated.call_count == 2
|
||||||
@@ -0,0 +1,72 @@
|
|||||||
|
"""
|
||||||
|
Regression tests for RenderPipeline.should_recompose()'s pending-updates check.
|
||||||
|
|
||||||
|
PR #299 added a check so a plugin's live score/status change (a "pending
|
||||||
|
update" in StreamManager) triggers a hot-swap within a few seconds instead
|
||||||
|
of waiting for a full scroll cycle to complete. PR #330 (multi-display sync)
|
||||||
|
refactored should_recompose() and dropped that check entirely -- not just
|
||||||
|
gated behind the new sync-mode deferral it added, but removed outright, so
|
||||||
|
even standalone (non-sync) installations silently lost live-refresh and fell
|
||||||
|
back to waiting for full cycle boundaries (which, depending on
|
||||||
|
min/max_cycle_duration, can be minutes).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
from src.vegas_mode.config import VegasModeConfig
|
||||||
|
from src.vegas_mode.render_pipeline import RenderPipeline
|
||||||
|
|
||||||
|
|
||||||
|
class FakeDisplayManager:
|
||||||
|
width = 64
|
||||||
|
height = 32
|
||||||
|
|
||||||
|
|
||||||
|
def _make_pipeline(sync_manager=None):
|
||||||
|
stream_manager = MagicMock()
|
||||||
|
stream_manager.get_buffer_status.return_value = {'staging_count': 0}
|
||||||
|
pipeline = RenderPipeline(VegasModeConfig(), FakeDisplayManager(), stream_manager)
|
||||||
|
pipeline.sync_manager = sync_manager
|
||||||
|
return pipeline, stream_manager
|
||||||
|
|
||||||
|
|
||||||
|
class TestShouldRecompose:
|
||||||
|
def test_cycle_complete_always_recomposes(self):
|
||||||
|
pipeline, stream_manager = _make_pipeline()
|
||||||
|
pipeline._cycle_complete = True
|
||||||
|
stream_manager.has_pending_updates_for_visible_segments.return_value = False
|
||||||
|
assert pipeline.should_recompose() is True
|
||||||
|
|
||||||
|
def test_no_pending_updates_no_staging_does_not_recompose(self):
|
||||||
|
pipeline, stream_manager = _make_pipeline()
|
||||||
|
stream_manager.has_pending_updates_for_visible_segments.return_value = False
|
||||||
|
assert pipeline.should_recompose() is False
|
||||||
|
|
||||||
|
def test_pending_updates_on_visible_segment_triggers_recompose(self):
|
||||||
|
"""The actual regression: a live-updated plugin currently in view
|
||||||
|
must trigger a recompose instead of waiting for cycle end."""
|
||||||
|
pipeline, stream_manager = _make_pipeline()
|
||||||
|
stream_manager.has_pending_updates_for_visible_segments.return_value = True
|
||||||
|
assert pipeline.should_recompose() is True
|
||||||
|
|
||||||
|
def test_staging_buffer_content_triggers_recompose(self):
|
||||||
|
pipeline, stream_manager = _make_pipeline()
|
||||||
|
stream_manager.get_buffer_status.return_value = {'staging_count': 1}
|
||||||
|
stream_manager.has_pending_updates_for_visible_segments.return_value = False
|
||||||
|
assert pipeline.should_recompose() is True
|
||||||
|
|
||||||
|
def test_sync_active_defers_pending_updates_to_cycle_boundary(self):
|
||||||
|
"""Sync-mode deferral (PR #330's actual intent) must still hold:
|
||||||
|
pending updates alone must NOT trigger a mid-cycle hot-swap when a
|
||||||
|
follower display is attached, since that causes a visible
|
||||||
|
freeze+jump on the follower. This must keep working after
|
||||||
|
restoring the non-sync pending-updates check above."""
|
||||||
|
pipeline, stream_manager = _make_pipeline(sync_manager=MagicMock())
|
||||||
|
stream_manager.has_pending_updates_for_visible_segments.return_value = True
|
||||||
|
assert pipeline.should_recompose() is False
|
||||||
|
|
||||||
|
def test_sync_active_still_recomposes_on_cycle_complete(self):
|
||||||
|
pipeline, stream_manager = _make_pipeline(sync_manager=MagicMock())
|
||||||
|
pipeline._cycle_complete = True
|
||||||
|
stream_manager.has_pending_updates_for_visible_segments.return_value = True
|
||||||
|
assert pipeline.should_recompose() is True
|
||||||
Reference in New Issue
Block a user