mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
fix(vegas): a display duration that is not a number no longer cancels a static pause
The Vegas static pause compared plugin.get_display_duration() with the clock. clock-simple, calendar and countdown return their display_duration setting straight from config.json, so a value saved as "20" or null reached that comparison as a string or None. The TypeError went to the pause's broad except, which ended the pause: the plugin flashed up and the scroll went straight on, at every one of its turns. inf held the pause until something interrupted it, and NaN, False, 0 or a negative number ended it at once. The pause now reads the duration the way the rotation has since #739, with the same helper, then the rotation's fallbacks: 30 s for anything that is not a number or a get_display_duration() that raises, 15 s for a number at or below zero. Logged once per plugin. test_vegas_static_mode.py's pauses used 0 to mean "no wait"; they now use 0.01. The helper moves from display_controller (_finite_seconds) to base_plugin (finite_seconds), unchanged: the coordinator cannot import from display_controller, which imports src.vegas_mode at module level, and a new src module would turn ledmatrix-plugins' min-core table check red until it was listed. base_plugin is already loaded whenever either one is. Tests: test/test_vegas_static_pause_duration.py, on a fake clock, including TestSameAsTheRotation, which runs every value through both the pause and the rotation's _get_display_duration/_resolve_durations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -885,6 +885,18 @@ policies are unchanged.
|
||||
a runtime publisher that stops still goes `stale`, and a subscription that
|
||||
goes quiet still falls back to the cache. The cache path's 120 s rule is
|
||||
unchanged.
|
||||
- A plugin that pauses the Vegas scroll gets its pause when its display
|
||||
duration is not a plain number. Several plugins (clock-simple, calendar,
|
||||
countdown) return `display_duration` as it is in config.json, so a value
|
||||
saved as `"20"` or `null` (the raw config editor, a hand edit) reached the
|
||||
pause as a string or None; comparing it with the clock raised, and the
|
||||
plugin flashed up and the scroll went straight on, at every one of its
|
||||
turns. `inf` held the pause until something interrupted it, and 0, a
|
||||
negative number or NaN ended it at once. The pause now reads the duration
|
||||
as the rotation does (`finite_seconds()` in `base_plugin`): a numeric
|
||||
string counts, anything else that is not a finite number (or a
|
||||
`get_display_duration()` that raises) pauses for 30 s, and a number at or
|
||||
below zero for 15 s, with one warning per plugin.
|
||||
|
||||
### Scrolling
|
||||
|
||||
|
||||
@@ -25,7 +25,6 @@ import os
|
||||
import inspect
|
||||
import signal
|
||||
import json
|
||||
import math
|
||||
import threading
|
||||
import types
|
||||
from collections import deque
|
||||
@@ -57,6 +56,7 @@ from src.ipc.contract import (
|
||||
PluginReloadResult,
|
||||
)
|
||||
from src.ipc.server import ControlServer, QueuedCommand, StateHub, start_control_server
|
||||
from src.plugin_system.base_plugin import finite_seconds
|
||||
from src.vegas_mode.render_pipeline import SYNC_SEND_INTERVAL
|
||||
|
||||
# Get logger with consistent configuration
|
||||
@@ -90,19 +90,6 @@ _MIN_INITIAL_UPDATE_TIMEOUT_SECONDS = 2.0
|
||||
DEFAULT_DYNAMIC_DURATION_CAP = 180.0
|
||||
|
||||
|
||||
def _finite_seconds(value: Any) -> Optional[float]:
|
||||
"""``value`` as seconds when it is a finite number or a numeric string,
|
||||
else None. A bool is not a number here, though it is an int: True would
|
||||
read as a one-second screen."""
|
||||
if isinstance(value, bool):
|
||||
return None
|
||||
try:
|
||||
seconds = float(value)
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
return None
|
||||
return seconds if math.isfinite(seconds) else None
|
||||
|
||||
|
||||
class _PluginReloadJob:
|
||||
"""A ``plugin.reload`` whose slow half runs off the render thread.
|
||||
|
||||
@@ -1392,7 +1379,7 @@ class DisplayController:
|
||||
except Exception as err: # pylint: disable=broad-except
|
||||
problem = f"get_display_duration() raised {type(err).__name__}: {err}"
|
||||
else:
|
||||
seconds = _finite_seconds(value)
|
||||
seconds = finite_seconds(value)
|
||||
if seconds is not None:
|
||||
return seconds
|
||||
problem = f"display duration {value!r} is not a number"
|
||||
|
||||
@@ -11,6 +11,7 @@ Stability: Stable - maintains backward compatibility
|
||||
from abc import ABC, abstractmethod
|
||||
from enum import Enum
|
||||
from typing import Dict, Any, Optional, List
|
||||
import math
|
||||
import os
|
||||
import sys
|
||||
from src.deprecation import deprecated, warn_deprecated
|
||||
@@ -240,6 +241,26 @@ def resolve_vegas_participation(plugin: Any, plugin_id: Optional[str] = None) ->
|
||||
return legacy_vegas_participation(plugin)
|
||||
|
||||
|
||||
def finite_seconds(value: Any) -> Optional[float]:
|
||||
"""``value`` as seconds when it is a finite number or a numeric string,
|
||||
else None. A bool is not a number here, though it is an int: True would
|
||||
read as a one-second screen.
|
||||
|
||||
How the core reads a plugin's get_display_duration() -- the rotation
|
||||
(DisplayController._get_display_duration) and the Vegas static pause --
|
||||
which several plugins answer straight from config.json, so a value saved
|
||||
as "20" or null arrives as a string or None. A number at or below zero is
|
||||
returned as it is; each caller has its own rule for that.
|
||||
"""
|
||||
if isinstance(value, bool):
|
||||
return None
|
||||
try:
|
||||
seconds = float(value)
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
return None
|
||||
return seconds if math.isfinite(seconds) else None
|
||||
|
||||
|
||||
class BasePlugin(ABC):
|
||||
"""
|
||||
Base class that all plugins must inherit from.
|
||||
|
||||
@@ -18,10 +18,11 @@ import math
|
||||
import sys
|
||||
import time
|
||||
import threading
|
||||
from typing import Optional, Dict, Any, List, Callable, TYPE_CHECKING
|
||||
from typing import Optional, Dict, Any, FrozenSet, List, Callable, TYPE_CHECKING
|
||||
|
||||
from src import display_watchdog
|
||||
from src.common import render_gate
|
||||
from src.plugin_system.base_plugin import finite_seconds
|
||||
from src.vegas_mode.config import VegasModeConfig
|
||||
from src.vegas_mode.elements import LiveEpochs
|
||||
from src.vegas_mode.plugin_adapter import PluginAdapter
|
||||
@@ -53,6 +54,14 @@ _FPS_HEARTBEAT_INTERVAL = 300.0
|
||||
#: every plugin. Game state doesn't change within a quarter second.
|
||||
_LIVE_PRIORITY_CHECK_INTERVAL = 0.25
|
||||
|
||||
#: Seconds a static pause shows a plugin whose display duration can't be
|
||||
#: used, as long as the rotation shows it: 30 when get_display_duration()
|
||||
#: raises or answers something that is not a number
|
||||
#: (DisplayController._get_display_duration), 15 when it answers a number at
|
||||
#: or below zero (DisplayController._resolve_durations).
|
||||
_UNREADABLE_DURATION = 30.0
|
||||
_NOT_POSITIVE_DURATION = 15.0
|
||||
|
||||
|
||||
def _percentile(ordered: List[float], fraction: float) -> float:
|
||||
"""Nearest-rank percentile of an already-sorted list.
|
||||
@@ -92,6 +101,9 @@ class VegasModeCoordinator:
|
||||
_live_reason: Optional[str] = None
|
||||
# Set only while Vegas has changed the GIL switch interval; read with getattr.
|
||||
_saved_switch_interval: Optional[float]
|
||||
#: Plugins already warned about a display duration the pause can't use,
|
||||
#: so a bad setting logs once, not at every turn. Replaced, not mutated.
|
||||
_duration_warned: FrozenSet[str] = frozenset()
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
@@ -1010,7 +1022,7 @@ class VegasModeCoordinator:
|
||||
# Wait for the plugin's display duration. Monotonic, like the
|
||||
# iteration clock: an NTP step on an RTC-less Pi would otherwise
|
||||
# end the pause at once or stretch it by the correction.
|
||||
duration = plugin.get_display_duration()
|
||||
duration = self._static_pause_duration(plugin)
|
||||
start = time.monotonic()
|
||||
|
||||
while time.monotonic() - start < duration:
|
||||
@@ -1046,6 +1058,42 @@ class VegasModeCoordinator:
|
||||
|
||||
return True
|
||||
|
||||
def _static_pause_duration(self, plugin: 'BasePlugin') -> float:
|
||||
"""Seconds a static pause shows ``plugin``: its display duration,
|
||||
read the way the rotation reads it.
|
||||
|
||||
Several plugins return their display_duration setting straight from
|
||||
config.json, so one saved as "20" or null came back as a string or
|
||||
None; comparing it with the clock raised, and the pause's broad
|
||||
except ended the pause at every one of the plugin's turns. inf
|
||||
paused until something interrupted it, and NaN, False, 0 or a
|
||||
negative number ended the pause at once. A numeric string counts
|
||||
(finite_seconds); anything else, or a raise, gets
|
||||
_UNREADABLE_DURATION, and a number at or below zero
|
||||
_NOT_POSITIVE_DURATION, logged once per plugin.
|
||||
"""
|
||||
try:
|
||||
value = plugin.get_display_duration()
|
||||
except Exception as err: # pylint: disable=broad-except
|
||||
problem = f"get_display_duration() raised {type(err).__name__}: {err}"
|
||||
seconds = _UNREADABLE_DURATION
|
||||
else:
|
||||
seconds = finite_seconds(value)
|
||||
if seconds is not None and seconds > 0:
|
||||
return seconds
|
||||
if seconds is None:
|
||||
problem = f"display duration {value!r} is not a number"
|
||||
seconds = _UNREADABLE_DURATION
|
||||
else:
|
||||
problem = f"display duration {value!r} is not above zero"
|
||||
seconds = _NOT_POSITIVE_DURATION
|
||||
plugin_id = plugin.plugin_id
|
||||
if plugin_id not in self._duration_warned:
|
||||
self._duration_warned = self._duration_warned | {plugin_id}
|
||||
logger.warning("[%s] %s; its static pause lasts %.0fs (logged once)",
|
||||
plugin_id, problem, seconds)
|
||||
return seconds
|
||||
|
||||
def _end_static_pause(self) -> None:
|
||||
"""End static pause and restore scroll state."""
|
||||
should_resume_scrolling = False
|
||||
|
||||
@@ -219,7 +219,8 @@ class TestCoordinatorStaticPause:
|
||||
def _plugin(self):
|
||||
plugin = MagicMock()
|
||||
plugin.plugin_id = 'clock'
|
||||
plugin.get_display_duration.return_value = 0
|
||||
# A moment: zero would pause 15 s, as the rotation shows it.
|
||||
plugin.get_display_duration.return_value = 0.01
|
||||
return plugin
|
||||
|
||||
def test_trigger_comes_from_the_pipeline(self):
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
"""A Vegas static pause lasts as long as the rotation shows the plugin.
|
||||
|
||||
The pause asked the plugin for get_display_duration() and compared the
|
||||
answer with the clock. Several plugins (clock-simple, calendar, countdown)
|
||||
return their display_duration setting as it is in config.json, so one saved
|
||||
as "20" or null -- the raw config editor, a hand edit -- reached that
|
||||
comparison as a string or None. The TypeError went to the pause's broad
|
||||
except, which ended the pause: the plugin flashed up and the scroll went on,
|
||||
at every one of its turns. inf paused until something interrupted it, and
|
||||
NaN, False, 0 or a negative number ended the pause at once.
|
||||
|
||||
The pause now reads the answer the way the rotation does since #739, with
|
||||
the same helper (base_plugin.finite_seconds): a numeric string counts;
|
||||
anything else that is not a finite number, or a raise, gets the rotation's
|
||||
30 s; a number at or below zero gets its 15 s.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
import threading
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
os.environ.setdefault("EMULATOR", "true")
|
||||
|
||||
import pytest
|
||||
|
||||
from src.vegas_mode import coordinator
|
||||
|
||||
NOT_NUMBERS = [None, '', 'twenty', True, False, float('nan'), float('inf'),
|
||||
'inf', '1e400', [20], {'seconds': 20}]
|
||||
NOT_ABOVE_ZERO = [0, -5, '-5', '0']
|
||||
NUMBERS = [('20', 20.0), (' 7.5 ', 7.5), (12, 12.0), (12.5, 12.5)]
|
||||
|
||||
|
||||
class FakeClock:
|
||||
"""time.monotonic/time.sleep for the pause loop: sleeping moves the clock."""
|
||||
|
||||
#: A pause still going after this long never ends (inf did that).
|
||||
LIMIT = 3600.0
|
||||
|
||||
def __init__(self):
|
||||
self.now = 0.0
|
||||
|
||||
def monotonic(self):
|
||||
return self.now
|
||||
|
||||
def sleep(self, seconds):
|
||||
self.now += seconds
|
||||
if self.now > self.LIMIT:
|
||||
raise RuntimeError("the static pause never ended")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def clock(monkeypatch):
|
||||
fake = FakeClock()
|
||||
monkeypatch.setattr(coordinator, 'time', fake)
|
||||
return fake
|
||||
|
||||
|
||||
def _plugin(duration, plugin_id='clock-simple'):
|
||||
plugin = MagicMock()
|
||||
plugin.plugin_id = plugin_id
|
||||
plugin.get_display_duration.return_value = duration
|
||||
return plugin
|
||||
|
||||
|
||||
def _coord(*plugins):
|
||||
coord = coordinator.VegasModeCoordinator.__new__(coordinator.VegasModeCoordinator)
|
||||
coord.render_pipeline = MagicMock()
|
||||
coord.render_pipeline.get_scroll_position.return_value = 0
|
||||
coord.display_manager = MagicMock()
|
||||
locks = {plugin.plugin_id: threading.Lock() for plugin in plugins}
|
||||
coord.plugin_manager = SimpleNamespace(get_plugin_lock=locks.__getitem__)
|
||||
coord._state_lock = threading.Lock()
|
||||
coord._static_pause_active = False
|
||||
coord._saved_scroll_position = None
|
||||
coord._should_stop = False
|
||||
coord._live_priority_active = False
|
||||
coord._live_priority_check = None
|
||||
coord._interrupt_check = None
|
||||
coord.stats = {'static_pauses': 0}
|
||||
return coord
|
||||
|
||||
|
||||
def _pause(coord, plugin, clock):
|
||||
"""One static pause: (whether it completed, how long it lasted)."""
|
||||
start = clock.now
|
||||
completed = coord._handle_static_pause(plugin)
|
||||
return completed, clock.now - start
|
||||
|
||||
|
||||
class TestPauseLength:
|
||||
@pytest.mark.parametrize('value, seconds', NUMBERS)
|
||||
def test_numbers_and_numeric_strings_are_used(self, clock, value, seconds):
|
||||
plugin = _plugin(value)
|
||||
completed, lasted = _pause(_coord(plugin), plugin, clock)
|
||||
assert completed is True
|
||||
assert lasted == pytest.approx(seconds, abs=0.15)
|
||||
|
||||
@pytest.mark.parametrize('value', NOT_NUMBERS, ids=repr)
|
||||
def test_anything_but_a_finite_number_pauses_for_30s(self, clock, value):
|
||||
plugin = _plugin(value)
|
||||
completed, lasted = _pause(_coord(plugin), plugin, clock)
|
||||
assert completed is True
|
||||
assert lasted == pytest.approx(30.0, abs=0.15)
|
||||
plugin.display.assert_called_once_with(force_clear=True)
|
||||
|
||||
@pytest.mark.parametrize('value', NOT_ABOVE_ZERO, ids=repr)
|
||||
def test_a_number_not_above_zero_pauses_for_15s(self, clock, value):
|
||||
plugin = _plugin(value)
|
||||
completed, lasted = _pause(_coord(plugin), plugin, clock)
|
||||
assert completed is True
|
||||
assert lasted == pytest.approx(15.0, abs=0.15)
|
||||
|
||||
def test_a_raising_get_display_duration_pauses_for_30s(self, clock):
|
||||
plugin = _plugin(None)
|
||||
plugin.get_display_duration.side_effect = KeyError('display_duration')
|
||||
completed, lasted = _pause(_coord(plugin), plugin, clock)
|
||||
assert completed is True
|
||||
assert lasted == pytest.approx(30.0, abs=0.15)
|
||||
|
||||
def test_a_good_value_after_a_bad_one_is_used(self, clock):
|
||||
plugin = _plugin(None)
|
||||
coord = _coord(plugin)
|
||||
assert _pause(coord, plugin, clock)[1] == pytest.approx(30.0, abs=0.15)
|
||||
plugin.get_display_duration.return_value = 45
|
||||
assert _pause(coord, plugin, clock)[1] == pytest.approx(45.0, abs=0.15)
|
||||
|
||||
def test_the_pause_can_still_be_interrupted(self, clock):
|
||||
plugin = _plugin('twenty')
|
||||
coord = _coord(plugin)
|
||||
coord._interrupt_check = lambda: clock.now >= 5
|
||||
completed, lasted = _pause(coord, plugin, clock)
|
||||
assert completed is False
|
||||
assert lasted == pytest.approx(5.0, abs=0.15)
|
||||
|
||||
|
||||
class TestWarning:
|
||||
def test_logged_once_per_plugin(self, clock, caplog):
|
||||
clock_plugin = _plugin('twenty')
|
||||
calendar = _plugin(None, plugin_id='calendar')
|
||||
coord = _coord(clock_plugin, calendar)
|
||||
with caplog.at_level(logging.WARNING, logger='src.vegas_mode.coordinator'):
|
||||
for _ in range(3):
|
||||
for plugin in (clock_plugin, calendar):
|
||||
coord._handle_static_pause(plugin)
|
||||
warnings = [r.getMessage() for r in caplog.records
|
||||
if 'display duration' in r.getMessage()]
|
||||
assert len(warnings) == 2
|
||||
assert any('clock-simple' in m and "'twenty'" in m for m in warnings)
|
||||
assert any('calendar' in m and 'None' in m for m in warnings)
|
||||
|
||||
|
||||
class TestFiniteSeconds:
|
||||
"""The shared rule: what counts as a number of seconds."""
|
||||
|
||||
@pytest.mark.parametrize('value, seconds', NUMBERS + [(0, 0.0), ('-5', -5.0)])
|
||||
def test_numbers_and_numeric_strings(self, value, seconds):
|
||||
from src.plugin_system.base_plugin import finite_seconds
|
||||
result = finite_seconds(value)
|
||||
assert result == seconds and isinstance(result, float)
|
||||
|
||||
@pytest.mark.parametrize('value', NOT_NUMBERS + [pytest.param(10 ** 400, id='10**400')],
|
||||
ids=repr)
|
||||
def test_anything_else_is_none(self, value):
|
||||
from src.plugin_system.base_plugin import finite_seconds
|
||||
assert finite_seconds(value) is None
|
||||
|
||||
|
||||
def _rotation_seconds(plugin):
|
||||
"""How long the rotation shows ``plugin`` (no dynamic duration, no
|
||||
Rotation & Durations override): the two calls run() makes for a screen.
|
||||
"""
|
||||
from src.display_controller import DisplayController
|
||||
dc = object.__new__(DisplayController)
|
||||
dc.config = {}
|
||||
dc.plugin_modes = {'mode': plugin}
|
||||
return dc._resolve_durations(plugin, 'mode', dc._get_display_duration('mode'), False)[1]
|
||||
|
||||
|
||||
class TestSameAsTheRotation:
|
||||
"""The pause and the rotation share finite_seconds; this pins their
|
||||
fallbacks (30 s, 15 s) to each other too."""
|
||||
|
||||
@pytest.mark.parametrize('value', [value for value, _ in NUMBERS]
|
||||
+ NOT_NUMBERS + NOT_ABOVE_ZERO, ids=repr)
|
||||
def test_the_pause_lasts_as_long_as_the_rotation_shows_it(self, clock, value):
|
||||
plugin = _plugin(value)
|
||||
expected = _rotation_seconds(plugin)
|
||||
assert _pause(_coord(plugin), plugin, clock)[1] == pytest.approx(expected, abs=0.15)
|
||||
|
||||
def test_a_raise_too(self, clock):
|
||||
plugin = _plugin(None)
|
||||
plugin.get_display_duration.side_effect = KeyError('display_duration')
|
||||
expected = _rotation_seconds(plugin)
|
||||
assert _pause(_coord(plugin), plugin, clock)[1] == pytest.approx(expected, abs=0.15)
|
||||
Reference in New Issue
Block a user