Compare commits

...
Author SHA1 Message Date
fce1fdac57 Add panel orientation setting for upside-down mounting (#455)
Adds a display.hardware.orientation config field ("normal" / "180")
so panels mounted upside down (e.g. to put the Pi/wiring on a more
convenient side) render correctly without custom pixel_mapper_config
edits. Composes onto the existing pixel_mapper_config as a trailing
"Rotate:180" mapper, so it stays independent of any custom mapper
string (e.g. U-mapper chain layouts) already in use.

Exposed as a "Panel Orientation" dropdown in the web UI's Display
settings, validated server-side, and documented in README and
CONFIG_REFERENCE.


Claude-Session: https://claude.ai/code/session_01FakipqMDHQLpsFjTuBdSFQ

Co-authored-by: Claude <noreply@anthropic.com>
2026-08-12 09:40:48 -04:00
7171e6c022 fix(cache): one cleanup thread per cache directory, not per manager (#453)
The display process ran three cleanup threads over one directory:

    14:22:59.954  display_controller        (the real manager)
    14:22:59.973  startup validation, run 1 (discarded)
    14:23:01.055  startup validation, run 2 (discarded)

Two of those managers existed only to read a directory path.
StartupValidator._validate_cache_directory built a whole CacheManager to
call get_cache_dir(), and validation runs twice -- once before the
plugin manager exists and again after. Each construction also probes
writability by writing and deleting .writetest on the card.

The discarded ones never went away. cleanup_loop closes over `self`, so
the thread keeps its manager alive: two objects that could never be
collected, waking every 24 hours to re-scan the same 9,000-file
directory. Nothing stopped them either -- stop_cleanup_thread had no
callers anywhere in the tree.

Two changes. The validator now takes the CacheManager the application
actually uses, which is also the more correct thing to validate; when
no caller supplies one it still builds its own, but stops the thread
afterwards. And CacheManager now tracks which directory it is sweeping,
so the second manager over a directory skips starting a thread at all.
That is the right granularity regardless of call sites: the sweep lists
a directory and deletes from it, so a second thread only duplicates the
scan. Ownership is released on stop, so a survivor can take over rather
than leaving the directory permanently unclaimed by a dead owner.

Measured directly, three managers over one directory: 3 threads before,
1 after.


Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 08:42:56 -04:00
9fbdd71941 fix(cache): collect the temp files abandoned writes leave behind (#452)
* fix(cache): collect the temp files abandoned writes leave behind

DiskCache.set() writes through mkstemp then os.replace, and removes its
own temp file in a finally. That covers a write that fails, but not a
process that dies between the two -- a SIGKILL, a lost restart race, a
power cut, all ordinary on a Pi.

Nothing ever collected what was left. The temp names are
".<key>.json.<random>", and cleanup_expired_files listed only names
ending in .json, so every one of them was invisible to the sweep for as
long as the card had been in service. On the dev rig: 76 files,
1,050 MB, 81% of the whole cache directory, the oldest six months old.
The startup sweep reported "18/8864 files deleted, 0.01 MB freed" while
sitting on top of a gigabyte it could not see.

They are removed after an hour. A real write holds its temp file for
milliseconds, so that is far outside any in-flight write while still
clearing the same day's debris, and it is deliberately not tied to the
retention policies: those say how long data stays useful, and a
half-written file never was.

The predicate is tested harder than the sweep, because a false positive
deletes real data. It matches the shape set() creates rather than just a
leading dot, so a completed ".json", a stray .gitignore, and a
"weather.json.bak" are all left alone -- and one test drives set()
itself and asserts the names it produces are matched, so the writer and
the predicate cannot drift apart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* fix(cache): count swept temp files as scanned

files_scanned only counted completed .json files, so a sweep that
removed orphans reported more deleted than it had looked at -- the
summary line renders "<deleted>/<scanned>", which came out as "76/1".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 08:40:07 -04:00
2add759f40 fix(odds): identify the odds requests to ESPN (#451)
The odds fetch used a bare requests.get, so it went out as
python-requests/x.y -- the one agent ESPN is known to reject. Around
2026-08-04 it began 403ing browser strings and bare custom tokens alike;
what it accepts is a token carrying a URL that says who is calling.
Every other ESPN caller in the tree already sends that header
(src/common/api_helper.py, src/base_classes/data_sources.py); this path
was simply missed.

It is the worst one to miss. Odds are fetched per live game from inside
the live update loop, so its failures are the ones that cost the caller
its whole update budget -- the same path the 5s timeout and the cooldown
were added to protect.

Sent via a session rather than per-call, which also reuses the
connection across a slate. Deliberately no retry adapter, unlike
api_helper: retries multiply request_timeout, which is 5s precisely to
stay inside the 30s operation budget.

The existing tests patched the module's requests.get, which this change
bypasses -- test_base_odds_manager was consequently reaching the real
ESPN and taking 404s. Both files now patch the session, and the new
tests pin the agent against api_helper's live value so the two cannot
drift apart the next time ESPN moves the goalposts.


Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-11 16:10:56 -04:00
16 changed files with 669 additions and 67 deletions
+8
View File
@@ -600,6 +600,14 @@ These settings are typically only needed for non-standard panels or custom confi
- Leave empty unless you need custom mapping
- See rpi-rgb-led-matrix documentation for full options
- **`orientation`** (string, default: "normal")
- Rotates the rendered image to match how the panel is physically mounted
- Set to `"180"` (or use the "Upside Down" option in the web UI's Display
settings) if the panel is mounted upside down — useful for optimizing
where the Raspberry Pi and wiring sit relative to the mounting location
- Applied independently of `pixel_mapper_config` (appended as a trailing
`Rotate:180` mapper), so custom mapper configs keep working alongside it
- **`row_address_type`** (integer, default: 0)
- How rows are addressed on the panel
- Most panels use 0 (direct addressing)
+1
View File
@@ -112,6 +112,7 @@
"led_rgb_sequence": "RGB",
"limit_refresh_rate_hz": 100,
"pixel_mapper_config": "",
"orientation": "normal",
"row_address_type": 0,
"multiplexing": 0,
"panel_type": ""
+1
View File
@@ -66,6 +66,7 @@ in `DisplayManager` (`src/display_manager.py`, ~lines 270295).
| `led_rgb_sequence` | string, `"RGB"` |
| `limit_refresh_rate_hz` | int, `100` (code default 90) |
| `pixel_mapper_config` | string, `""` — e.g. `"U-mapper"` / `"Rotate:90"` |
| `orientation` | string, `"normal"``"180"` rotates the rendered image 180° for panels physically mounted upside down (e.g. to move the Pi/wiring to a more convenient side); composed onto `pixel_mapper_config` as a trailing `Rotate:180` mapper, so it stays independent of any custom `pixel_mapper_config` value |
| `row_address_type` | int, `0` — non-standard panel row addressing |
| `multiplexing` | int, `0` — panel multiplexing scheme |
| `panel_type` | string, `""` — set to `"FM6126A"` or `"FM6127"` for panels needing init |
+19 -1
View File
@@ -45,6 +45,24 @@ class BaseOddsManager:
self.logger = logging.getLogger(__name__)
self.base_url = "https://sports.core.api.espn.com/v2/sports"
# This path used a bare requests.get, so it identified itself as
# python-requests/x.y -- the one thing ESPN is known to reject. Around
# 2026-08-04 it began 403ing browser strings and bare custom tokens
# alike; what it accepts is a token with a URL that says who is
# calling. Every other ESPN caller in the tree already sends this
# (src/common/api_helper.py, src/base_classes/data_sources.py); the
# odds path was simply missed, and it is the one whose failures cost
# the caller its whole update budget.
#
# Deliberately no retry adapter, unlike api_helper: retries multiply
# request_timeout, which is set to 5s precisely to stay inside that
# budget. One try, then the cooldown below.
self.session = requests.Session()
self.session.headers.update({
'User-Agent': 'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)',
'Accept': 'application/json',
})
# Configuration with defaults
self.update_interval = 3600 # 1 hour default
# Well under the plugin executor's 30s operation budget. At 30s a
@@ -144,7 +162,7 @@ class BaseOddsManager:
url = f"{self.base_url}/{sport}/leagues/{espn_league}/events/{event_id}/competitions/{event_id}/odds"
self.logger.info(f"Requesting odds from URL: {url}")
response = requests.get(url, timeout=self.request_timeout)
response = self.session.get(url, timeout=self.request_timeout)
response.raise_for_status()
raw_data = response.json()
+64 -1
View File
@@ -14,6 +14,13 @@ import zlib
from typing import Dict, Any, Optional, Protocol
from datetime import datetime
# How old an abandoned write's temp file must be before the sweep removes it.
# A real write holds its temp file for milliseconds, so an hour is far beyond
# any in-flight write while still clearing the same day's debris. Deliberately
# not tied to the retention policies: those describe how long data stays
# useful, and a half-written file was never useful.
_ORPHAN_TEMP_MAX_AGE_SECONDS = 3600
class CacheStrategyProtocol(Protocol):
@@ -347,6 +354,23 @@ class DiskCache:
"""Get the cache directory path."""
return self.cache_dir
@staticmethod
def _is_orphaned_temp(filename: str) -> bool:
"""Whether a name is one of set()'s temp files rather than real data.
Matches only what this class creates: mkstemp with a prefix of
".<cache filename>." , so ".weather.json.a1b2c3d4". The shape is
checked rather than just the leading dot, because this predicate
deletes things -- a stray dotfile someone left in the cache directory
is not ours to remove, and a completed ".json" never is either.
"""
if not filename.startswith('.') or filename.endswith('.json'):
return False
head, sep, suffix = filename.rpartition('.json.')
# head is the key (non-empty after the leading dot), suffix is
# mkstemp's random component.
return bool(sep) and len(head) > 1 and bool(suffix)
def cleanup_expired_files(self, cache_strategy: CacheStrategyProtocol, retention_policies: Dict[str, int]) -> Dict[str, Any]:
"""
Clean up expired cache files based on retention policies.
@@ -381,12 +405,51 @@ class DiskCache:
try:
with self._lock:
# Get snapshot of files while holding lock briefly
filenames = [f for f in os.listdir(self.cache_dir) if f.endswith('.json')]
entries = os.listdir(self.cache_dir)
except OSError as list_error:
self.logger.error("Error listing cache directory %s: %s", self.cache_dir, list_error, exc_info=True)
stats['errors'] += 1
return stats
filenames = [f for f in entries if f.endswith('.json')]
# Sweep temp files abandoned by a write that never finished. set()
# removes its own in a finally, so these are the ones where the
# process died between mkstemp and os.replace -- a SIGKILL, a lost
# restart race, a power cut. Nothing ever collected them: they are
# named ".<key>.json.<random>", and the scan above only matches
# names ending in .json, so they accumulated indefinitely. Measured
# on a live rig: 76 files, 1,050 MB, 81% of the whole cache
# directory, the oldest six months old.
stats['orphan_temp_files_deleted'] = 0
for filename in (f for f in entries if self._is_orphaned_temp(f)):
# Counted as scanned like any other candidate, so files_deleted
# can never exceed files_scanned and the summary line reads
# honestly ("77/8864", not "77/0").
stats['files_scanned'] += 1
path = os.path.join(self.cache_dir, filename)
try:
# An in-flight write lives for milliseconds, so anything
# this old is certainly abandoned rather than in progress.
if (current_time - os.path.getmtime(path)) <= _ORPHAN_TEMP_MAX_AGE_SECONDS:
continue
with self._lock:
size = os.path.getsize(path)
os.remove(path)
stats['files_deleted'] += 1
stats['orphan_temp_files_deleted'] += 1
stats['space_freed_bytes'] += size
except FileNotFoundError:
continue # another sweep got there first
except OSError as e:
stats['errors'] += 1
self.logger.warning("Error deleting orphaned temp file %s: %s", filename, e)
if stats['orphan_temp_files_deleted']:
self.logger.info(
"Removed %d abandoned cache temp file(s)",
stats['orphan_temp_files_deleted'])
# Process files outside the lock to avoid blocking get/set operations
for filename in filenames:
stats['files_scanned'] += 1
+40 -1
View File
@@ -47,6 +47,20 @@ from src.cache.disk_cache import DateTimeEncoder # noqa: F401 - deliberate re-e
class CacheManager:
"""Manages caching of API responses to reduce API calls."""
# Which cache directories already have a cleanup thread in this process.
#
# The sweep is directory-scoped work -- it lists a directory and deletes
# from it -- so one per directory is the right number no matter how many
# managers exist. Nothing enforced that before: every instance started its
# own, and because the loop closes over `self`, a discarded manager could
# never be collected and its thread woke to re-scan the same directory
# every 24 hours for the life of the process. Startup validation runs
# twice and built a throwaway manager each time, so a display process
# carried three threads for one cache.
_cleanup_owners: Dict[str, 'CacheManager'] = {}
_cleanup_owners_lock = threading.Lock()
def __init__(self) -> None:
# Initialize logger first
self.logger: logging.Logger = get_logger(__name__)
@@ -718,11 +732,29 @@ class CacheManager:
}
def start_cleanup_thread(self) -> None:
"""Start background thread for periodic disk cache cleanup."""
"""Start background thread for periodic disk cache cleanup.
At most one thread per cache directory per process: the sweep is
directory-scoped, so a second one only duplicates the scan.
"""
if self._cleanup_thread and self._cleanup_thread.is_alive():
self.logger.debug("Cleanup thread already running")
return
with CacheManager._cleanup_owners_lock:
owner = CacheManager._cleanup_owners.get(self.cache_dir)
if owner is not None and owner is not self:
thread = owner._cleanup_thread
if thread is not None and thread.is_alive():
self.logger.debug(
"Cleanup thread for %s already owned by another cache "
"manager in this process; not starting a second",
self.cache_dir)
return
# The owner's thread died or was stopped -- take over.
CacheManager._cleanup_owners[self.cache_dir] = self
def cleanup_loop():
"""Background loop that runs cleanup periodically."""
self.logger.info("Disk cache cleanup thread started (interval: %d hours)",
@@ -770,6 +802,13 @@ class CacheManager:
Signals the thread to stop and waits for it to finish (with timeout).
This allows for clean shutdown during testing or application termination.
"""
# Release ownership first and unconditionally, so a manager that never
# started a thread (or whose thread already exited) cannot keep the
# directory claimed and block a live manager from sweeping it.
with CacheManager._cleanup_owners_lock:
if CacheManager._cleanup_owners.get(self.cache_dir) is self:
del CacheManager._cleanup_owners[self.cache_dir]
if not self._cleanup_thread or not self._cleanup_thread.is_alive():
self.logger.debug("Cleanup thread not running")
return
+4 -2
View File
@@ -90,7 +90,8 @@ class DisplayController:
# Validate startup configuration
try:
from src.startup_validator import StartupValidator
validator = StartupValidator(self.config_manager)
validator = StartupValidator(self.config_manager,
cache_manager=self.cache_manager)
is_valid, errors, warnings = validator.validate_all()
if warnings:
@@ -258,7 +259,8 @@ class DisplayController:
# Validate plugins after plugin manager is created
try:
from src.startup_validator import StartupValidator
validator = StartupValidator(self.config_manager, self.plugin_manager)
validator = StartupValidator(self.config_manager, self.plugin_manager,
cache_manager=self.cache_manager)
is_valid, errors, warnings = validator.validate_all()
if warnings:
+21 -1
View File
@@ -258,6 +258,26 @@ class DisplayManager:
# Initialize managers
# Calendar manager is now initialized by DisplayController
# Orientation setting -> rpi-rgb-led-matrix "Rotate:<deg>" pixel-mapper suffix.
# "normal" needs no suffix since 0 degrees is the identity transform.
_ORIENTATION_ROTATE_DEGREES = {'normal': None, '90': 90, '180': 180, '270': 270}
def _build_pixel_mapper_config(self, hardware_config: dict) -> str:
"""Compose the raw pixel_mapper_config string with the orientation setting.
`pixel_mapper_config` stays available as a free-form advanced field (e.g.
for "U-mapper" chain layouts); `orientation` is the user-facing dropdown
for physical mounting (e.g. panels mounted upside down) and is appended as
a "Rotate:<deg>" mapper rather than overwriting any existing config.
"""
base_mapper = (hardware_config.get('pixel_mapper_config') or '').strip()
orientation = hardware_config.get('orientation', 'normal')
degrees = self._ORIENTATION_ROTATE_DEGREES.get(orientation)
if degrees is None:
return base_mapper
rotate_mapper = f'Rotate:{degrees}'
return f'{base_mapper};{rotate_mapper}' if base_mapper else rotate_mapper
def _setup_matrix(self):
"""Initialize the RGB matrix with configuration settings."""
_init_error_str = None
@@ -283,7 +303,7 @@ class DisplayManager:
options.pwm_bits = hardware_config.get('pwm_bits', 10)
options.pwm_lsb_nanoseconds = hardware_config.get('pwm_lsb_nanoseconds', 150)
options.led_rgb_sequence = hardware_config.get('led_rgb_sequence', 'RGB')
options.pixel_mapper_config = hardware_config.get('pixel_mapper_config', '')
options.pixel_mapper_config = self._build_pixel_mapper_config(hardware_config)
options.row_address_type = hardware_config.get('row_address_type', 0)
options.multiplexing = hardware_config.get('multiplexing', 0)
options.panel_type = hardware_config.get('panel_type', '')
+20 -1
View File
@@ -15,16 +15,23 @@ from src.logging_config import get_logger
class StartupValidator:
"""Validates system state on startup."""
def __init__(self, config_manager: Any, plugin_manager: Optional[Any] = None) -> None:
def __init__(self, config_manager: Any, plugin_manager: Optional[Any] = None,
cache_manager: Optional[Any] = None) -> None:
"""
Initialize the startup validator.
Args:
config_manager: ConfigManager instance
plugin_manager: Optional PluginManager instance
cache_manager: The CacheManager the application will actually use.
Pass it. Without one this validator builds its own just to read
a directory path, which reports on a cache the app does not
use and leaves behind a cleanup thread that nothing stops --
validation runs twice per startup, so that was two of them.
"""
self.config_manager = config_manager
self.plugin_manager = plugin_manager
self.cache_manager = cache_manager
self.logger = get_logger(__name__)
self.errors: List[str] = []
self.warnings: List[str] = []
@@ -91,8 +98,20 @@ class StartupValidator:
def _validate_cache_directory(self) -> None:
"""Validate cache directory permissions."""
try:
cache_manager = self.cache_manager
if cache_manager is None:
# No caller supplied one (older embedders, direct use in a
# script). Build one, but do not leave its cleanup thread
# running behind us -- this instance is discarded on the next
# line but the thread is a closure over it, so it would never
# be collected.
from src.cache_manager import CacheManager
cache_manager = CacheManager()
try:
cache_dir = cache_manager.get_cache_dir()
finally:
cache_manager.stop_cleanup_thread()
else:
cache_dir = cache_manager.get_cache_dir()
if not cache_dir:
+4 -2
View File
@@ -8,7 +8,9 @@ is_odds_available's ML-blind truth table, the fixed format_odds_summary
gate (money-line-only odds now format), get_odds_for_games, and
configuration loading.
No real network: src.base_odds_manager.requests.get is always patched.
No real network: requests.Session.get is always patched. The odds path sends
its requests through a session so it can identify itself to ESPN, so patching
the module-level requests.get would no longer intercept anything.
"""
from unittest.mock import MagicMock, patch
@@ -59,7 +61,7 @@ def manager(cache_manager):
@pytest.fixture
def mock_get():
with patch('src.base_odds_manager.requests.get') as m:
with patch('src.base_odds_manager.requests.Session.get') as m:
m.return_value = _make_response({'items': [dict(FULL_ITEM)]})
yield m
+146
View File
@@ -0,0 +1,146 @@
"""Tests that one cache directory gets one cleanup thread per process.
The sweep lists a directory and deletes from it, so a second thread over the
same directory only duplicates the scan. Nothing enforced that: every
CacheManager started its own, and since the loop closes over `self`, a
discarded manager could never be collected -- its thread stayed alive and
re-scanned the same directory every 24 hours for the life of the process.
On the dev rig a display process carried three, for one cache directory:
14:22:59.954 display_controller (the real one)
14:22:59.973 startup validation, run 1 (discarded)
14:23:01.055 startup validation, run 2 (discarded)
Startup validation runs twice and built a throwaway manager each time, purely
to read a directory path.
"""
import threading
import pytest
from src.cache_manager import CacheManager
@pytest.fixture(autouse=True)
def _clean_registry():
CacheManager._cleanup_owners.clear()
yield
for owner in list(CacheManager._cleanup_owners.values()):
owner.stop_cleanup_thread()
CacheManager._cleanup_owners.clear()
def _live_cleanup_threads():
return [t for t in threading.enumerate()
if t.name == 'DiskCacheCleanup' and t.is_alive()]
@pytest.fixture
def manager(tmp_path, monkeypatch):
"""A CacheManager pinned to a temp dir, so tests never touch the real one."""
monkeypatch.setattr(CacheManager, '_get_writable_cache_dir',
lambda self: str(tmp_path))
return CacheManager
class TestOneThreadPerDirectory:
def test_a_single_manager_starts_one(self, manager):
before = len(_live_cleanup_threads())
m = manager()
try:
assert len(_live_cleanup_threads()) == before + 1
finally:
m.stop_cleanup_thread()
def test_three_managers_still_start_one(self, manager):
# Exactly the rig's shape: the real manager plus two throwaways.
before = len(_live_cleanup_threads())
managers = [manager() for _ in range(3)]
try:
assert len(_live_cleanup_threads()) == before + 1
finally:
for m in managers:
m.stop_cleanup_thread()
def test_the_first_one_owns_it(self, manager):
first, second = manager(), manager()
try:
assert CacheManager._cleanup_owners[first.cache_dir] is first
assert second._cleanup_thread is None
finally:
first.stop_cleanup_thread()
second.stop_cleanup_thread()
def test_the_survivor_can_take_over(self, manager):
first = manager()
first.stop_cleanup_thread()
assert not _live_cleanup_threads()
second = manager()
try:
# Ownership was released, so the directory is swept again rather
# than being left permanently unclaimed by a dead owner.
assert len(_live_cleanup_threads()) == 1
assert CacheManager._cleanup_owners[second.cache_dir] is second
finally:
second.stop_cleanup_thread()
def test_stopping_a_non_owner_does_not_unclaim_the_directory(self, manager):
first, second = manager(), manager()
try:
second.stop_cleanup_thread() # never owned it
assert CacheManager._cleanup_owners[first.cache_dir] is first
assert len(_live_cleanup_threads()) == 1
finally:
first.stop_cleanup_thread()
def test_separate_directories_get_separate_threads(self, tmp_path, monkeypatch):
a, b = tmp_path / 'a', tmp_path / 'b'
a.mkdir()
b.mkdir()
dirs = iter([str(a), str(b)])
monkeypatch.setattr(CacheManager, '_get_writable_cache_dir',
lambda self: next(dirs))
first, second = CacheManager(), CacheManager()
try:
assert first.cache_dir != second.cache_dir
assert len(_live_cleanup_threads()) == 2
finally:
first.stop_cleanup_thread()
second.stop_cleanup_thread()
def test_no_thread_leaks_across_many_constructions(self, manager):
before = len(_live_cleanup_threads())
made = [manager() for _ in range(12)]
try:
assert len(_live_cleanup_threads()) == before + 1
finally:
for m in made:
m.stop_cleanup_thread()
assert len(_live_cleanup_threads()) == before
class TestValidatorDoesNotBuildItsOwn:
def test_it_uses_the_cache_manager_it_is_given(self, manager):
from src.startup_validator import StartupValidator
shared = manager()
try:
before = len(_live_cleanup_threads())
v = StartupValidator(config_manager=object(), cache_manager=shared)
v._validate_cache_directory()
assert len(_live_cleanup_threads()) == before, (
"validation started another cleanup thread")
finally:
shared.stop_cleanup_thread()
def test_without_one_it_cleans_up_after_itself(self, manager):
from src.startup_validator import StartupValidator
before = len(_live_cleanup_threads())
v = StartupValidator(config_manager=object())
v._validate_cache_directory()
assert len(_live_cleanup_threads()) == before, (
"the fallback manager left its cleanup thread running")
+188
View File
@@ -0,0 +1,188 @@
"""Tests that abandoned cache temp files get collected.
DiskCache.set() writes through tempfile.mkstemp and os.replace, removing its
own temp file in a finally. That covers a failed write, but not a process that
dies between the two -- a SIGKILL, a lost restart race, a power cut, all
ordinary on a Pi. Nothing collected what was left behind: the temp names are
".<key>.json.<random>", and the expiry sweep only listed names ending in
.json, so they accumulated for as long as the card had been in service.
Measured on a live rig before this fix: 76 orphans totalling 1,050 MB -- 81%
of the entire cache directory -- the oldest six months old.
The predicate that decides what to delete is tested harder than the sweep
itself, because a false positive here destroys real data.
"""
import os
import time
import pytest
from src.cache.disk_cache import DiskCache, _ORPHAN_TEMP_MAX_AGE_SECONDS
class FakeStrategy:
@staticmethod
def get_data_type_from_key(key):
return 'default'
POLICIES = {'default': 30}
@pytest.fixture
def cache(tmp_path):
return DiskCache(str(tmp_path))
def _age(path, seconds):
old = time.time() - seconds
os.utime(path, (old, old))
def _write(tmp_path, name, body='{}'):
p = tmp_path / name
p.write_text(body, encoding='utf-8')
return p
class TestWhatCountsAsAnOrphan:
@pytest.mark.parametrize('name', [
'.weather.json.a1b2c3d4',
'.odds_espn_football_nfl_401.json.xyz00000',
'.a.json.b',
])
def test_our_temp_files_are_orphans(self, name):
assert DiskCache._is_orphaned_temp(name)
@pytest.mark.parametrize('name', [
'weather.json', # real data
'.weather.json', # a dotted key that completed
'.gitignore', # not ours
'.hidden', # not ours
'weather.json.bak', # no leading dot: someone else's
'.json.abc', # no key between the dot and .json.
'.weather.json.', # no random component
'notes.txt',
])
def test_everything_else_is_left_alone(self, name):
assert not DiskCache._is_orphaned_temp(name)
def test_the_names_set_actually_creates_are_matched(self, cache, tmp_path):
"""Guard against the predicate and the writer drifting apart."""
created = []
real = os.replace
def capture(src, dst):
created.append(os.path.basename(src))
return real(src, dst)
import src.cache.disk_cache as mod
mod.os.replace = capture
try:
cache.set('weather', {'v': 1})
finally:
mod.os.replace = real
assert created, "set() did not go through the temp-file path"
assert all(DiskCache._is_orphaned_temp(n) for n in created), created
class TestTheSweep:
def test_an_old_orphan_is_removed(self, cache, tmp_path):
p = _write(tmp_path, '.weather.json.a1b2c3d4', 'x' * 5000)
_age(p, _ORPHAN_TEMP_MAX_AGE_SECONDS + 60)
stats = cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert not p.exists()
assert stats['orphan_temp_files_deleted'] == 1
assert stats['space_freed_bytes'] >= 5000
def test_an_in_flight_write_is_not_snatched_away(self, cache, tmp_path):
# The whole risk of this sweep: deleting a temp file another thread is
# about to os.replace into place.
p = _write(tmp_path, '.weather.json.inflight')
cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert p.exists()
def test_real_cache_files_survive(self, cache, tmp_path):
fresh = _write(tmp_path, 'weather.json')
dotted = _write(tmp_path, '.weather.json')
_age(dotted, _ORPHAN_TEMP_MAX_AGE_SECONDS + 60)
cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert fresh.exists()
assert dotted.exists(), "a completed .json was treated as a temp file"
def test_unrelated_dotfiles_survive(self, cache, tmp_path):
keep = _write(tmp_path, '.gitignore')
_age(keep, 400 * 86400)
cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert keep.exists()
def test_expiry_still_works_alongside_it(self, cache, tmp_path):
stale = _write(tmp_path, 'old.json')
_age(stale, 40 * 86400) # past the 30-day default
orphan = _write(tmp_path, '.old.json.zz999999')
_age(orphan, _ORPHAN_TEMP_MAX_AGE_SECONDS + 60)
stats = cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert not stale.exists()
assert not orphan.exists()
assert stats['files_deleted'] == 2
assert stats['orphan_temp_files_deleted'] == 1
def test_the_rig_scenario(self, cache, tmp_path):
"""76 orphans of assorted ages, none of them reachable before."""
for i in range(76):
p = _write(tmp_path, '.sched_%d.json.r%06d' % (i, i), 'x' * 1000)
_age(p, (i + 2) * 86400)
keep = _write(tmp_path, 'sched.json')
stats = cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert stats['orphan_temp_files_deleted'] == 76
assert keep.exists()
assert not list(tmp_path.glob('.sched_*'))
# The summary line is "<deleted>/<scanned>", so an orphan that is
# deleted but never counted as scanned renders as "76/1".
assert stats['files_scanned'] == 77
assert stats['files_deleted'] <= stats['files_scanned']
def test_deleted_never_exceeds_scanned(self, cache, tmp_path):
p = _write(tmp_path, '.only.json.a1b2c3d4')
_age(p, _ORPHAN_TEMP_MAX_AGE_SECONDS + 60)
stats = cache.cleanup_expired_files(FakeStrategy(), POLICIES)
assert stats['files_deleted'] == 1
assert stats['files_scanned'] == 1
def test_a_missing_file_mid_sweep_is_not_an_error(self, cache, tmp_path):
p = _write(tmp_path, '.weather.json.a1b2c3d4')
_age(p, _ORPHAN_TEMP_MAX_AGE_SECONDS + 60)
import src.cache.disk_cache as mod
real = mod.os.path.getsize
def vanish(path):
if path.endswith('.a1b2c3d4'):
os.remove(path)
raise FileNotFoundError(path)
return real(path)
mod.os.path.getsize = vanish
try:
stats = cache.cleanup_expired_files(FakeStrategy(), POLICIES)
finally:
mod.os.path.getsize = real
assert stats['errors'] == 0
+42
View File
@@ -237,3 +237,45 @@ class TestDisplayManagerDoubleSided:
suppress_test_pattern=True)
assert dm.set_brightness(70) is True
assert mock_rgb_matrix['matrix_instance'].brightness == 70
class TestDisplayManagerOrientation:
"""The orientation setting composes onto pixel_mapper_config for panels
mounted upside down, without disturbing a custom pixel_mapper_config."""
def _config(self, **hardware_overrides):
config = {
'display': {
'hardware': {
'rows': 32, 'cols': 64, 'chain_length': 2, 'parallel': 1,
'hardware_mapping': 'adafruit-hat-pwm', 'brightness': 90,
},
'runtime': {'gpio_slowdown': 2},
},
'timezone': 'UTC',
'plugin_system': {'plugins_directory': 'plugins'},
}
config['display']['hardware'].update(hardware_overrides)
return config
def test_default_orientation_leaves_pixel_mapper_config_untouched(self, mock_rgb_matrix):
DisplayManager._instance = None
with patch.dict('os.environ', {'EMULATOR': 'false'}):
DisplayManager(self._config(), suppress_test_pattern=True)
options = mock_rgb_matrix['options_class'].return_value
assert options.pixel_mapper_config == ''
def test_orientation_180_appends_rotate_mapper(self, mock_rgb_matrix):
DisplayManager._instance = None
with patch.dict('os.environ', {'EMULATOR': 'false'}):
DisplayManager(self._config(orientation='180'), suppress_test_pattern=True)
options = mock_rgb_matrix['options_class'].return_value
assert options.pixel_mapper_config == 'Rotate:180'
def test_orientation_180_composes_with_existing_pixel_mapper_config(self, mock_rgb_matrix):
DisplayManager._instance = None
with patch.dict('os.environ', {'EMULATOR': 'false'}):
DisplayManager(self._config(orientation='180', pixel_mapper_config='U-mapper'),
suppress_test_pattern=True)
options = mock_rgb_matrix['options_class'].return_value
assert options.pixel_mapper_config == 'U-mapper;Rotate:180'
+84 -44
View File
@@ -10,10 +10,15 @@ and the update carrying every game's score was killed:
Invisible out of season -- preseason week 1 returns a single game -- and a
Sunday slate is around sixteen.
The request now goes through a session that identifies the caller, so the
tests patch `manager.session.get` rather than the module's `requests.get`.
"""
from unittest.mock import Mock
import requests
from src.base_odds_manager import BaseOddsManager
PLUGIN_BUDGET = 30.0 # PluginExecutor(default_timeout=30.0)
@@ -25,43 +30,83 @@ def _manager(cache=None):
return BaseOddsManager(cache_manager=cache, config_manager=None)
def _timing_out(manager):
"""Point the manager's session at a request that always times out."""
manager.session.get = Mock(side_effect=requests.exceptions.Timeout("x"))
return manager.session.get
def _returning(manager, payload):
resp = Mock()
resp.json.return_value = payload
resp.raise_for_status.return_value = None
manager.session.get = Mock(return_value=resp)
return manager.session.get
class TestRequestTimeout:
def test_leaves_room_in_the_operation_budget(self):
assert _manager().request_timeout < PLUGIN_BUDGET / 2
def test_the_timeout_is_the_one_actually_used(self):
m = _manager()
import src.base_odds_manager as mod
real = mod.requests.get
try:
mod.requests.get = Mock(side_effect=mod.requests.exceptions.Timeout("x"))
get = _timing_out(m)
m.get_odds("football", "nfl", "401")
assert mod.requests.get.call_args.kwargs["timeout"] == m.request_timeout
finally:
mod.requests.get = real
assert get.call_args.kwargs["timeout"] == m.request_timeout
class TestIdentifiesItselfToEspn:
"""ESPN 403s python-requests' default agent, and bare custom tokens.
What it accepts is a token carrying a URL that says who is calling. This
path used a bare requests.get and so sent the default -- the one thing
known to be rejected. Everything else in the tree that talks to ESPN
already sends the header below.
"""
def test_the_user_agent_names_the_project_and_links_to_it(self):
ua = _manager().session.headers["User-Agent"]
assert "python-requests" not in ua
assert "LEDMatrix" in ua
assert "github.com/ChuckBuilds/LEDMatrix" in ua
def test_it_is_the_same_agent_the_rest_of_the_tree_sends(self):
# Compared against the live value rather than a copied literal, so the
# two cannot drift apart the next time ESPN moves the goalposts.
from src.common.api_helper import APIHelper
assert (_manager().session.headers["User-Agent"]
== APIHelper().session.headers["User-Agent"])
def test_the_header_reaches_the_request(self):
m = _manager()
get = _returning(m, {})
m._extract_espn_data = Mock(return_value=None)
m.get_odds("football", "nfl", "401")
# Sent via the session, so it applies without being passed per-call.
assert get.call_count == 1
assert "User-Agent" in m.session.headers
def test_no_retry_adapter_multiplies_the_timeout(self):
# api_helper mounts a retrying adapter; this path must not, or a 5s
# timeout becomes 15s and the budget fix is undone.
m = _manager()
for adapter in m.session.adapters.values():
retries = getattr(adapter, "max_retries", None)
assert getattr(retries, "total", 0) in (0, None), (
"odds session mounts a retrying adapter (total=%r); retries "
"multiply request_timeout" % getattr(retries, "total", None))
class TestSlowEspnCannotKillTheUpdate:
def test_one_failure_stops_the_rest_of_the_slate_hitting_the_network(self):
m = _manager()
import src.base_odds_manager as mod
real = mod.requests.get
calls = {"n": 0}
def timeout(*a, **k):
calls["n"] += 1
raise mod.requests.exceptions.Timeout("timed out")
try:
mod.requests.get = timeout
get = _timing_out(m)
for i in range(16): # a full slate, one game at a time
m.get_odds("football", "nfl", "4018730%02d" % i)
finally:
mod.requests.get = real
assert calls["n"] == 1, (
assert get.call_count == 1, (
"%d games each paid the timeout; the breaker should have stopped "
"after the first" % calls["n"])
"after the first" % get.call_count)
def test_worst_case_slate_stays_inside_the_budget(self):
m = _manager()
@@ -70,53 +115,48 @@ class TestSlowEspnCannotKillTheUpdate:
def test_recovery_is_automatic(self):
m = _manager()
import src.base_odds_manager as mod
real_get, real_monotonic = mod.requests.get, mod.time.monotonic
real_monotonic = mod.time.monotonic
clock = {"t": 1000.0}
try:
mod.time.monotonic = lambda: clock["t"]
mod.requests.get = Mock(
side_effect=mod.requests.exceptions.Timeout("timed out"))
get = _timing_out(m)
m.get_odds("football", "nfl", "401")
assert m._skip_network_until > clock["t"], "breaker did not open"
clock["t"] += 1
before = mod.requests.get.call_count
before = get.call_count
m.get_odds("football", "nfl", "402")
assert mod.requests.get.call_count == before, "should not have retried"
assert get.call_count == before, "should not have retried"
clock["t"] += m._FAILURE_COOLDOWN
m.get_odds("football", "nfl", "403")
assert mod.requests.get.call_count > before, "never retried"
assert get.call_count > before, "never retried"
finally:
mod.requests.get, mod.time.monotonic = real_get, real_monotonic
mod.time.monotonic = real_monotonic
def test_a_healthy_fetch_clears_the_breaker(self):
m = _manager()
m._skip_network_until = 0.0
m._extract_espn_data = Mock(return_value=None)
import src.base_odds_manager as mod
real = mod.requests.get
try:
resp = Mock()
resp.json.return_value = {}
resp.raise_for_status.return_value = None
mod.requests.get = Mock(return_value=resp)
_returning(m, {})
m.get_odds("football", "nfl", "401")
finally:
mod.requests.get = real
assert m._skip_network_until == 0.0
def test_a_403_opens_the_breaker_rather_than_hammering(self):
# raise_for_status raises HTTPError, a RequestException -- so a wrong
# or missing agent backs off instead of 403ing once per game.
m = _manager()
resp = Mock()
resp.raise_for_status.side_effect = requests.exceptions.HTTPError("403")
m.session.get = Mock(return_value=resp)
m.get_odds("football", "nfl", "401")
assert m._skip_network_until > 0.0
def test_the_stale_cache_fallback_still_works(self):
# The failing request must still hand back whatever was cached; only
# the *subsequent* games skip the network.
cache = Mock()
cache.get_with_auto_strategy.side_effect = [None, {"details": "stale"}]
m = BaseOddsManager(cache_manager=cache, config_manager=None)
import src.base_odds_manager as mod
real = mod.requests.get
try:
mod.requests.get = Mock(
side_effect=mod.requests.exceptions.Timeout("timed out"))
_timing_out(m)
assert m.get_odds("football", "nfl", "401") == {"details": "stale"}
finally:
mod.requests.get = real
+7 -2
View File
@@ -796,7 +796,7 @@ def save_main_config():
'gpio_slowdown', 'rp1_rio', 'scan_mode', 'disable_hardware_pulsing', 'inverse_colors', 'show_refresh_rate',
'pwm_bits', 'pwm_dither_bits', 'pwm_lsb_nanoseconds', 'limit_refresh_rate_hz', 'use_short_date_format',
'max_dynamic_duration_seconds', 'led_rgb_sequence', 'multiplexing', 'panel_type',
'row_address_type', 'pixel_mapper_config']
'row_address_type', 'pixel_mapper_config', 'orientation']
if any(k in data for k in display_fields):
if 'display' not in current_config:
@@ -831,6 +831,11 @@ def save_main_config():
if 'pixel_mapper_config' in data and not isinstance(data['pixel_mapper_config'], str):
return jsonify({'status': 'error', 'message': 'pixel_mapper_config must be a string (e.g. "U-mapper;Rotate:90" or empty)'}), 400
# Validate orientation (physical mounting rotation; composed onto pixel_mapper_config at runtime)
ORIENTATION_ALLOWED = {'normal', '180'}
if 'orientation' in data and data['orientation'] not in ORIENTATION_ALLOWED:
return jsonify({'status': 'error', 'message': f"Invalid orientation '{data['orientation']}'. Allowed values: {', '.join(sorted(ORIENTATION_ALLOWED))}"}), 400
# Validate row_address_type
if 'row_address_type' in data:
try:
@@ -844,7 +849,7 @@ def save_main_config():
for field in ['rows', 'cols', 'chain_length', 'parallel', 'brightness', 'hardware_mapping', 'scan_mode',
'pwm_bits', 'pwm_dither_bits', 'pwm_lsb_nanoseconds', 'limit_refresh_rate_hz',
'led_rgb_sequence', 'multiplexing', 'panel_type', 'row_address_type',
'pixel_mapper_config']:
'pixel_mapper_config', 'orientation']:
if field in data:
if field in ['rows', 'cols', 'chain_length', 'parallel', 'brightness', 'scan_mode',
'pwm_bits', 'pwm_dither_bits', 'pwm_lsb_nanoseconds', 'limit_refresh_rate_hz',
@@ -117,6 +117,14 @@
</select>
</div>
<div class="form-group" id="setting-display-orientation" data-setting-key="display.hardware.orientation">
<label for="orientation" class="block text-sm font-medium text-gray-700">Panel Orientation{{ ui.help_tip('Rotates the rendered image to match how the panel is physically mounted.\nUse "Upside Down" if you flipped the panel 180° to move the Raspberry Pi / wiring to a more convenient side.', 'Panel Orientation') }}</label>
<select id="orientation" name="orientation" class="form-control">
<option value="normal" {% if main_config.display.hardware.get('orientation', 'normal') == "normal" %}selected{% endif %}>Normal</option>
<option value="180" {% if main_config.display.hardware.get('orientation', 'normal') == "180" %}selected{% endif %}>Upside Down (180°)</option>
</select>
</div>
<div class="form-group" id="setting-display-led_rgb_sequence" data-setting-key="display.hardware.led_rgb_sequence">
<label for="led_rgb_sequence" class="block text-sm font-medium text-gray-700">LED RGB Sequence{{ ui.help_tip('Order the panel expects color channels in.\nChange this only if reds/greens/blues look swapped. Default: RGB.', 'LED RGB Sequence') }}</label>
<select id="led_rgb_sequence" name="led_rgb_sequence" class="form-control">