mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-11 17:46:37 +00:00
Compare commits
5
Commits
e395ebe008
...
734adfba7d
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
734adfba7d | ||
|
|
58aff7b7f0 | ||
|
|
967f3a0567 | ||
|
|
21c8a54f68 | ||
|
|
cf02538d2e |
@@ -36,6 +36,12 @@ jobs:
|
|||||||
uses: anthropics/claude-code-action@v1
|
uses: anthropics/claude-code-action@v1
|
||||||
with:
|
with:
|
||||||
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||||
|
# Review PRs opened by the Claude GitHub App. Without this the action
|
||||||
|
# aborts before reading the diff ("Workflow initiated by non-human
|
||||||
|
# actor"), so every such PR shows this check red. Named rather than
|
||||||
|
# '*': the allow-list is matched against the triggering actor, so
|
||||||
|
# this admits claude[bot] alone and no other app.
|
||||||
|
allowed_bots: 'claude'
|
||||||
plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
|
plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
|
||||||
plugins: 'code-review@claude-code-plugins'
|
plugins: 'code-review@claude-code-plugins'
|
||||||
prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}'
|
prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}'
|
||||||
|
|||||||
+123
-35
@@ -19,7 +19,32 @@ accepts both, but the store flags the old spelling as deprecated
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
Config saves and plugin config preparation:
|
## 3.5.0
|
||||||
|
|
||||||
|
New modules a plugin may import via `src.*` (floor on 3.5.0):
|
||||||
|
|
||||||
|
- `src/common/sports_helpers.py` — the helpers the scoreboards' `sports.py`
|
||||||
|
carry byte-identical copies of: `clamp_window`, `clamp_seconds`,
|
||||||
|
`logo_needs_refresh`, `spread_weighted_order` (+ `MIN_WINDOW_DAYS`,
|
||||||
|
`MAX_WINDOW_DAYS`), and `SportsHelpersMixin` with `_mode_customization`,
|
||||||
|
`_setting_int`, `_reset_dwell_on_reentry`, `_next_switch_index`,
|
||||||
|
`_spread_weighted_order`, `_odds_color`, `_upcoming_date_and_time_text` under
|
||||||
|
the plugins' names and signatures, plus the `_favorite_key` override point.
|
||||||
|
Constructor-free; keeps lazy state on its host (see the module docstring,
|
||||||
|
which also gives the host contract).
|
||||||
|
A new module rather than more methods on `sports_shared`: a plugin that
|
||||||
|
deletes a copy and leans on an older module having grown the method fails at
|
||||||
|
runtime with `AttributeError`, which no load-time check sees, while a missing
|
||||||
|
module fails at load. Nothing in core uses it yet.
|
||||||
|
- `test/test_common_is_hardware_free.py` — `src/common` must import without
|
||||||
|
`rgbmatrix` and never import `src.base_classes`, `src.display_manager` or
|
||||||
|
`src.plugin_system` at module level.
|
||||||
|
- `src/common/espn_dates.py` — `fetch_espn_scoreboard`,
|
||||||
|
`fetch_espn_date_chunks`, `espn_date_chunks`, `clamp_espn_limit`,
|
||||||
|
`ESPN_MAX_LIMIT`: fetch an ESPN scoreboard date range now that ESPN rejects
|
||||||
|
ranges (see Sports data below). Plugins bundle a copy of it.
|
||||||
|
|
||||||
|
### Config saves and plugin config preparation
|
||||||
|
|
||||||
- A JSON `POST /api/v3/config/main` changes only the keys it sends. The MQTT
|
- A JSON `POST /api/v3/config/main` changes only the keys it sends. The MQTT
|
||||||
bridge's brightness slider used to turn off `disable_hardware_pulsing`,
|
bridge's brightness slider used to turn off `disable_hardware_pulsing`,
|
||||||
@@ -65,31 +90,7 @@ Config saves and plugin config preparation:
|
|||||||
with any core config section are flagged: the last private copies of the
|
with any core config section are flagged: the last private copies of the
|
||||||
core-key list now use `src/core_config_keys.py`.
|
core-key list now use `src/core_config_keys.py`.
|
||||||
|
|
||||||
New module a plugin may import via `src.*` (floor on the release that ships
|
### Sports data
|
||||||
this):
|
|
||||||
|
|
||||||
- `src/common/sports_helpers.py` — the helpers the scoreboards' `sports.py`
|
|
||||||
carry byte-identical copies of: `clamp_window`, `clamp_seconds`,
|
|
||||||
`logo_needs_refresh`, `spread_weighted_order` (+ `MIN_WINDOW_DAYS`,
|
|
||||||
`MAX_WINDOW_DAYS`), and `SportsHelpersMixin` with `_mode_customization`,
|
|
||||||
`_setting_int`, `_reset_dwell_on_reentry`, `_next_switch_index`,
|
|
||||||
`_spread_weighted_order`, `_odds_color`, `_upcoming_date_and_time_text` under
|
|
||||||
the plugins' names and signatures, plus the `_favorite_key` override point.
|
|
||||||
Constructor-free; keeps lazy state on its host (see the module docstring,
|
|
||||||
which also gives the host contract).
|
|
||||||
A new module rather than more methods on `sports_shared`: a plugin that
|
|
||||||
deletes a copy and leans on an older module having grown the method fails at
|
|
||||||
runtime with `AttributeError`, which no load-time check sees, while a missing
|
|
||||||
module fails at load. Nothing in core uses it yet.
|
|
||||||
- `test/test_common_is_hardware_free.py` — `src/common` must import without
|
|
||||||
`rgbmatrix` and never import `src.base_classes`, `src.display_manager` or
|
|
||||||
`src.plugin_system` at module level.
|
|
||||||
- `src/common/espn_dates.py` — `fetch_espn_scoreboard`,
|
|
||||||
`fetch_espn_date_chunks`, `espn_date_chunks`, `clamp_espn_limit`,
|
|
||||||
`ESPN_MAX_LIMIT`: fetch an ESPN scoreboard date range now that ESPN rejects
|
|
||||||
ranges (see Sports data below). Plugins bundle a copy of it.
|
|
||||||
|
|
||||||
Sports data:
|
|
||||||
|
|
||||||
- Since 2026-09-15 ESPN answers `dates=YYYYMMDD-YYYYMMDD` scoreboard queries
|
- Since 2026-09-15 ESPN answers `dates=YYYYMMDD-YYYYMMDD` scoreboard queries
|
||||||
with `400 Bad Request` for every sport, so season schedules, the weeks window
|
with `400 Bad Request` for every sport, so season schedules, the weeks window
|
||||||
@@ -104,8 +105,41 @@ Sports data:
|
|||||||
- `BackgroundDataService.handles_espn_date_ranges` is `True`. Plugins check it
|
- `BackgroundDataService.handles_espn_date_ranges` is `True`. Plugins check it
|
||||||
to decide whether to submit a season range to the service or fetch it
|
to decide whether to submit a season range to the service or fetch it
|
||||||
themselves on an older core.
|
themselves on an older core.
|
||||||
|
- A league with no live games no longer backs its poll off past the next
|
||||||
|
kickoff. The escalation counted empty looks and nothing else, so a league
|
||||||
|
three hours before kickoff was indistinguishable from one out of season and
|
||||||
|
both reached `live_idle_max_interval`: measured gaps of up to 928 seconds,
|
||||||
|
and a rig that sat for a quarter of an hour with eight NFL games in progress
|
||||||
|
without noticing any of them. The wait is now clamped so it cannot run past
|
||||||
|
the earliest start still ahead, which the live fetch already downloads, so
|
||||||
|
it costs no extra request. Just after a kickoff the live cadence is held for
|
||||||
|
a grace window, because a provider that has not yet flipped the status would
|
||||||
|
otherwise read as another empty check and escalate the back-off again.
|
||||||
|
- ESPN date chunks are fetched six at a time (`ESPN_CHUNK_WORKERS`) in two
|
||||||
|
passes: months and edge days first, then the days of any month that came
|
||||||
|
back at the cap. A cold college-baseball season is about 130 requests, and
|
||||||
|
they went out one at a time; March and April measured on a Pi 4 (63
|
||||||
|
requests, 3101 events) went from 11.2s to 1.6s. Merged events still follow
|
||||||
|
`espn_date_chunks` order, so the payload does not depend on which request
|
||||||
|
won the race, and a capped month's payload is dropped before its days are
|
||||||
|
fetched, which keeps the peak memory of a four-capped-month fetch to about
|
||||||
|
16 MB over the sequential path rather than 43 MB — `docs/LOW_MEMORY_BOARDS.md`
|
||||||
|
puts a 1 GB Pi 3B+ at under 200 MB of headroom.
|
||||||
|
- `ESPNDataSource.fetch_standings` asks each league the endpoint that league
|
||||||
|
actually publishes. It tried `/standings` first whatever the league and fell
|
||||||
|
back to `/rankings` only on a 404, but college leagues answer `/standings`
|
||||||
|
with a 200 that carries no poll, so the fallback never fired: the rank badge
|
||||||
|
simply never appeared and anything keyed off rankings quietly did nothing.
|
||||||
|
Endpoints are now ordered by whether the league publishes a poll, and a 200
|
||||||
|
that lacks the key counts as a miss, so a league answering both still ends up
|
||||||
|
with whichever carries the poll. Only a 404 is routine — that is how a league
|
||||||
|
says it has none; a connection error, a timeout or an unparseable body is
|
||||||
|
logged as an error again, and a bug raised while inspecting the payload is no
|
||||||
|
longer swallowed as a missing poll. This is the implementation the football,
|
||||||
|
baseball and hockey boards already ship; core was the last copy on the old
|
||||||
|
one.
|
||||||
|
|
||||||
Scrolling:
|
### Scrolling
|
||||||
|
|
||||||
- **Scoreboard scroll speed no longer changes with the General tab's "Scroll
|
- **Scoreboard scroll speed no longer changes with the General tab's "Scroll
|
||||||
Frame Rate" (`target_fps`).** Scoreboards on `src.common.sports_scroll`
|
Frame Rate" (`target_fps`).** Scoreboards on `src.common.sports_scroll`
|
||||||
@@ -135,7 +169,7 @@ Scrolling:
|
|||||||
`scroll_delay` are described as the speed clamp they are rather than frame
|
`scroll_delay` are described as the speed clamp they are rather than frame
|
||||||
stepping. Scoreboard `scroll_delay` is documented as ignored for pacing.
|
stepping. Scoreboard `scroll_delay` is documented as ignored for pacing.
|
||||||
|
|
||||||
Web interface:
|
### Web interface
|
||||||
|
|
||||||
- The plugin settings form honours `"x-display": "hidden"` in config schemas:
|
- The plugin settings form honours `"x-display": "hidden"` in config schemas:
|
||||||
the property gets no control at any depth (top level, nested objects, array
|
the property gets no control at any depth (top level, nested objects, array
|
||||||
@@ -187,8 +221,19 @@ Web interface:
|
|||||||
request that gets no HTTP answer (e.g. the web service restarting mid-run) is
|
request that gets no HTTP answer (e.g. the web service restarting mid-run) is
|
||||||
re-sent with backoff instead of being counted as failed and skipped — that is
|
re-sent with backoff instead of being counted as failed and skipped — that is
|
||||||
how a disabled plugin with an update waiting was silently left out.
|
how a disabled plugin with an update waiting was silently left out.
|
||||||
|
- Three routes consulted the web process's plugin manifests without
|
||||||
|
discovering plugins first, so they misbehaved from every `ledmatrix-web`
|
||||||
|
restart until something else ran a discovery — in practice until someone
|
||||||
|
opened the dashboard, measured at over three minutes on one rig.
|
||||||
|
`POST /display/on-demand/start` and `POST /plugins/toggle` answered 404
|
||||||
|
"Plugin not found", and `POST /config/main` did not recognise a plugin
|
||||||
|
section, so it skipped secret separation and wrote the plugin's API key to
|
||||||
|
`config.json` in plain text instead of `config_secrets.json`. The routes now
|
||||||
|
discover when nothing has been discovered yet, and rescan once when a
|
||||||
|
specific plugin id (or, for on-demand by mode, a mode) is not found, so a
|
||||||
|
plugin installed since the last scan is found too.
|
||||||
|
|
||||||
Security (request paths and inline handlers, siblings of #561):
|
### Security (request paths and inline handlers, siblings of #561)
|
||||||
|
|
||||||
- `POST /api/v3/plugins/assets/upload`, `GET .../assets/list` and
|
- `POST /api/v3/plugins/assets/upload`, `GET .../assets/list` and
|
||||||
`POST .../assets/delete` validate `plugin_id` with `src/common/path_safety`
|
`POST .../assets/delete` validate `plugin_id` with `src/common/path_safety`
|
||||||
@@ -208,7 +253,7 @@ Security (request paths and inline handlers, siblings of #561):
|
|||||||
- The uploaded-images list escapes each file's original name, path and ids; a
|
- The uploaded-images list escapes each file's original name, path and ids; a
|
||||||
name like `<img src=x onerror=...>.png` was inserted as markup.
|
name like `<img src=x onerror=...>.png` was inserted as markup.
|
||||||
|
|
||||||
Display hardware settings the library refuses:
|
### Display hardware settings the library refuses
|
||||||
|
|
||||||
- The rgbmatrix library answers several settings with no matrix or `abort()`
|
- The rgbmatrix library answers several settings with no matrix or `abort()`
|
||||||
rather than an error, on every board, so the display service crash-looped
|
rather than an error, on every board, so the display service crash-looped
|
||||||
@@ -239,7 +284,7 @@ Display hardware settings the library refuses:
|
|||||||
values as the defaults; the "code default" values they listed never apply,
|
values as the defaults; the "code default" values they listed never apply,
|
||||||
because config migration fills missing keys from the template.
|
because config migration fills missing keys from the template.
|
||||||
|
|
||||||
Plugin system:
|
### Plugin system
|
||||||
|
|
||||||
- A plugin no longer starts with a schema warning and a degraded flag because
|
- A plugin no longer starts with a schema warning and a degraded flag because
|
||||||
config.json still holds a boolean where its schema now has an object with an
|
config.json still holds a boolean where its schema now has an object with an
|
||||||
@@ -250,7 +295,7 @@ Plugin system:
|
|||||||
is written at load; the next save of that plugin's settings stores the object.
|
is written at load; the next save of that plugin's settings stores the object.
|
||||||
Other type mismatches still warn.
|
Other type mismatches still warn.
|
||||||
|
|
||||||
Core:
|
### Core
|
||||||
|
|
||||||
- `ConfigManager.load_config()` no longer raises on a host without the POSIX
|
- `ConfigManager.load_config()` no longer raises on a host without the POSIX
|
||||||
ownership APIs. The self-heal that chgrp's `config_secrets.json` to the
|
ownership APIs. The self-heal that chgrp's `config_secrets.json` to the
|
||||||
@@ -268,7 +313,37 @@ Core:
|
|||||||
ownership step is now skipped where `os.chown` is missing. No behaviour
|
ownership step is now skipped where `os.chown` is missing. No behaviour
|
||||||
change on the Pi.
|
change on the Pi.
|
||||||
|
|
||||||
Automatic updates and Update Code:
|
### Cache permissions
|
||||||
|
|
||||||
|
- The web interface can read what the display service caches again.
|
||||||
|
`ledmatrix-web.service` carried `CacheDirectory=ledmatrix`, and systemd
|
||||||
|
re-owns `/var/cache/ledmatrix` and its contents to the unit's `User=`
|
||||||
|
whenever the directory's owner differs, which erased the `root:ledmatrix`
|
||||||
|
setgid layout the installers set up: every file the root display service
|
||||||
|
wrote afterwards was `root:root` 0660 and unreadable by the web interface
|
||||||
|
(392 unreadable files on one rig, with display status, on-demand state and
|
||||||
|
plugin health empty). Since #547 the web unit is rendered from its template
|
||||||
|
on every install, so every fresh install hit this.
|
||||||
|
`DiskCache.set` now gives each file the directory's group (when that
|
||||||
|
directory is group-writable) and 0660 on the open descriptor before the
|
||||||
|
rename, independent of setgid, which also closes a window where a fresh
|
||||||
|
file was visible as mkstemp's 0600. `DiskCache.share_existing_files`
|
||||||
|
repairs files an older version left behind, once per process, through
|
||||||
|
`O_NOFOLLOW` descriptors, skipping hard links and other users' files.
|
||||||
|
Existing installs only ever receive `git pull`, so that repair is the fix
|
||||||
|
for them; new installs also drop `CacheDirectory=` and
|
||||||
|
`CacheDirectoryMode=` from the web unit.
|
||||||
|
- `install_web_service.sh` replaces an existing cache directory's group
|
||||||
|
whenever the installing user is not in it. It used to replace only root's,
|
||||||
|
so a `root:ledmatrix` directory belonging to a user outside that group was
|
||||||
|
left alone and everything root wrote there stayed unreadable.
|
||||||
|
- `/display/on-demand/status` and the current-display status read the display
|
||||||
|
service's keys with `memory_ttl=0`, as every other cross-process reader
|
||||||
|
already does. They served the first copy the web process had read for the
|
||||||
|
full 120s `max_age`, so on-demand reported "active" for over 100 seconds
|
||||||
|
after the file on disk said "idle".
|
||||||
|
|
||||||
|
### Automatic updates and Update Code
|
||||||
|
|
||||||
- An update that changes `web_interface/requirements.txt` is no longer rolled
|
- An update that changes `web_interface/requirements.txt` is no longer rolled
|
||||||
back on every auto-updating device. `safe_pip_install.sh` allowed only the
|
back on every auto-updating device. `safe_pip_install.sh` allowed only the
|
||||||
@@ -292,7 +367,20 @@ Automatic updates and Update Code:
|
|||||||
budget, so a rollback finishes inside the unit's 30-minute limit instead of
|
budget, so a rollback finishes inside the unit's 30-minute limit instead of
|
||||||
being killed mid-way.
|
being killed mid-way.
|
||||||
|
|
||||||
Small fixes (update-all, plugin system settings, scripts):
|
### Installers
|
||||||
|
|
||||||
|
- The generated `ledmatrix_web` sudoers rules are parsed before they are
|
||||||
|
installed. Both installers built the drop-in from `which` lookups and copied
|
||||||
|
it into `/etc/sudoers.d` without ever checking it, and a malformed file there
|
||||||
|
makes sudo refuse every command for every user — on a headless Pi, that is
|
||||||
|
unrecoverable over SSH. `first_time_install.sh` now runs `visudo -c` on the
|
||||||
|
generated file and, if it does not parse, prints what visudo said and leaves
|
||||||
|
the installed file untouched instead of replacing it with a broken one;
|
||||||
|
`configure_web_sudo.sh` does the same before offering the rules for
|
||||||
|
confirmation. `first_time_install.sh` also built that file at a fixed `/tmp`
|
||||||
|
path as root; `mktemp` now picks the name.
|
||||||
|
|
||||||
|
### Small fixes (update-all, plugin system settings, scripts)
|
||||||
|
|
||||||
- **Check & Update All** counts a plugin that had nothing to update as
|
- **Check & Update All** counts a plugin that had nothing to update as
|
||||||
"already up to date" instead of "updated". ZIP-installed monorepo plugins
|
"already up to date" instead of "updated". ZIP-installed monorepo plugins
|
||||||
@@ -330,7 +418,7 @@ Small fixes (update-all, plugin system settings, scripts):
|
|||||||
check `web_interface/blueprints/api_v3/`, which became a package, instead of
|
check `web_interface/blueprints/api_v3/`, which became a package, instead of
|
||||||
reporting `api_v3.py` as missing.
|
reporting `api_v3.py` as missing.
|
||||||
|
|
||||||
Docs and developer tools:
|
### Docs and developer tools
|
||||||
|
|
||||||
- `docs/REST_API_REFERENCE.md` rechecked against every handler: request
|
- `docs/REST_API_REFERENCE.md` rechecked against every handler: request
|
||||||
fields that made documented calls fail (`repo_url`, `action_id`/`params`,
|
fields that made documented calls fail (`repo_url`, `action_id`/`params`,
|
||||||
|
|||||||
@@ -81,7 +81,7 @@ src/common/
|
|||||||
sports_scroll.py SportsScrollDisplay / …Manager — scroll orchestration
|
sports_scroll.py SportsScrollDisplay / …Manager — scroll orchestration
|
||||||
(content building stays in the plugins)
|
(content building stays in the plugins)
|
||||||
sports_helpers.py clamp/logo/rotation free functions + SportsHelpersMixin
|
sports_helpers.py clamp/logo/rotation free functions + SportsHelpersMixin
|
||||||
(unreleased) — the helpers byte-identical in the
|
(3.5.0) — the helpers byte-identical in the
|
||||||
plugins' sports.py, and the _favorite_key seam
|
plugins' sports.py, and the _favorite_key seam
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|||||||
+34
-10
@@ -1504,6 +1504,9 @@ echo "------------------------------------------------"
|
|||||||
# Create sudoers configuration for the web interface
|
# Create sudoers configuration for the web interface
|
||||||
echo "Creating sudoers configuration..."
|
echo "Creating sudoers configuration..."
|
||||||
SUDOERS_FILE="/etc/sudoers.d/ledmatrix_web"
|
SUDOERS_FILE="/etc/sudoers.d/ledmatrix_web"
|
||||||
|
# A predictable name in a world-writable directory is a symlink target;
|
||||||
|
# root writes the rules here, so let mktemp pick the name.
|
||||||
|
SUDOERS_TMP=$(mktemp "${TMPDIR:-/tmp}/ledmatrix_web_sudoers.XXXXXX")
|
||||||
|
|
||||||
# Get command paths
|
# Get command paths
|
||||||
PYTHON_PATH=$(which python3)
|
PYTHON_PATH=$(which python3)
|
||||||
@@ -1514,7 +1517,7 @@ BASH_PATH=$(which bash)
|
|||||||
JOURNALCTL_PATH=$(which journalctl 2>/dev/null || true)
|
JOURNALCTL_PATH=$(which journalctl 2>/dev/null || true)
|
||||||
|
|
||||||
# Create sudoers content
|
# Create sudoers content
|
||||||
cat > /tmp/ledmatrix_web_sudoers << EOF
|
cat > "$SUDOERS_TMP" << EOF
|
||||||
# LED Matrix Web Interface passwordless sudo configuration
|
# LED Matrix Web Interface passwordless sudo configuration
|
||||||
# This allows the web interface user to run specific commands without a password
|
# This allows the web interface user to run specific commands without a password
|
||||||
|
|
||||||
@@ -1541,7 +1544,7 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/
|
|||||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_pip_install.sh *
|
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_pip_install.sh *
|
||||||
EOF
|
EOF
|
||||||
if [ -n "$JOURNALCTL_PATH" ]; then
|
if [ -n "$JOURNALCTL_PATH" ]; then
|
||||||
cat >> /tmp/ledmatrix_web_sudoers << EOF
|
cat >> "$SUDOERS_TMP" << EOF
|
||||||
# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
|
# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
|
||||||
# when its output is a terminal. From that pager (less) a "!sh" is a root
|
# when its output is a terminal. From that pager (less) a "!sh" is a root
|
||||||
# shell -- the standard journalctl escalation. The web interface always passes
|
# shell -- the standard journalctl escalation. The web interface always passes
|
||||||
@@ -1555,17 +1558,38 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
|
|||||||
EOF
|
EOF
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [ -f "$SUDOERS_FILE" ] && cmp -s /tmp/ledmatrix_web_sudoers "$SUDOERS_FILE"; then
|
# Never install rules we have not parsed. A malformed drop-in in
|
||||||
echo "Sudoers configuration already up to date"
|
# /etc/sudoers.d makes sudo refuse every command for every user, which on a
|
||||||
rm /tmp/ledmatrix_web_sudoers
|
# headless Pi leaves no way in at all. If the rules do not parse, say so and
|
||||||
|
# keep whatever is already installed.
|
||||||
|
SUDOERS_VALID=1
|
||||||
|
if command -v visudo >/dev/null 2>&1; then
|
||||||
|
if ! visudo -c -f "$SUDOERS_TMP" >/dev/null 2>&1; then
|
||||||
|
SUDOERS_VALID=0
|
||||||
|
echo "⚠ The generated sudoers rules did not parse:" >&2
|
||||||
|
visudo -c -f "$SUDOERS_TMP" >&2 || true
|
||||||
|
echo "⚠ Leaving $SUDOERS_FILE unchanged. The web interface cannot control" >&2
|
||||||
|
echo " the display service until this is fixed." >&2
|
||||||
|
fi
|
||||||
else
|
else
|
||||||
echo "Installing/updating sudoers configuration..."
|
echo "⚠ visudo not found; installing the sudoers rules unvalidated"
|
||||||
cp /tmp/ledmatrix_web_sudoers "$SUDOERS_FILE"
|
|
||||||
chmod 440 "$SUDOERS_FILE"
|
|
||||||
rm /tmp/ledmatrix_web_sudoers
|
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "✓ Passwordless sudo access configured"
|
if [ "$SUDOERS_VALID" = "0" ]; then
|
||||||
|
rm -f "$SUDOERS_TMP"
|
||||||
|
elif [ -f "$SUDOERS_FILE" ] && cmp -s "$SUDOERS_TMP" "$SUDOERS_FILE"; then
|
||||||
|
echo "Sudoers configuration already up to date"
|
||||||
|
rm -f "$SUDOERS_TMP"
|
||||||
|
else
|
||||||
|
echo "Installing/updating sudoers configuration..."
|
||||||
|
cp "$SUDOERS_TMP" "$SUDOERS_FILE"
|
||||||
|
chmod 440 "$SUDOERS_FILE"
|
||||||
|
rm -f "$SUDOERS_TMP"
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ "$SUDOERS_VALID" = "1" ]; then
|
||||||
|
echo "✓ Passwordless sudo access configured"
|
||||||
|
fi
|
||||||
echo ""
|
echo ""
|
||||||
|
|
||||||
CURRENT_STEP="Configure WiFi management permissions"
|
CURRENT_STEP="Configure WiFi management permissions"
|
||||||
|
|||||||
@@ -130,6 +130,19 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
|
|||||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_PATH *"
|
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_PATH *"
|
||||||
} > "$TEMP_SUDOERS"
|
} > "$TEMP_SUDOERS"
|
||||||
|
|
||||||
|
# Never offer to install rules we have not parsed. A malformed drop-in in
|
||||||
|
# /etc/sudoers.d makes sudo refuse every command for every user.
|
||||||
|
if command -v visudo >/dev/null 2>&1; then
|
||||||
|
if ! visudo -c -f "$TEMP_SUDOERS" >/dev/null 2>&1; then
|
||||||
|
echo ""
|
||||||
|
echo "✗ The generated sudoers rules did not parse:" >&2
|
||||||
|
visudo -c -f "$TEMP_SUDOERS" >&2 || true
|
||||||
|
echo "Nothing was changed." >&2
|
||||||
|
rm -f "$TEMP_SUDOERS"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
echo ""
|
echo ""
|
||||||
echo "Generated sudoers configuration:"
|
echo "Generated sudoers configuration:"
|
||||||
echo "--------------------------------"
|
echo "--------------------------------"
|
||||||
|
|||||||
+1
-1
@@ -4,5 +4,5 @@ LEDMatrix Display System
|
|||||||
Core source package for the LED Matrix Display project.
|
Core source package for the LED Matrix Display project.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
__version__ = "3.4.0"
|
__version__ = "3.5.0"
|
||||||
|
|
||||||
|
|||||||
@@ -114,35 +114,68 @@ class ESPNDataSource(DataSource):
|
|||||||
return []
|
return []
|
||||||
|
|
||||||
def fetch_standings(self, sport: str, league: str) -> Dict:
|
def fetch_standings(self, sport: str, league: str) -> Dict:
|
||||||
"""Fetch standings from ESPN API."""
|
"""Fetch standings, or the poll for leagues that have one.
|
||||||
# Try standings endpoint first (for professional leagues like NFL, NBA, etc.)
|
|
||||||
try:
|
Order matters and used to be wrong. College leagues publish a poll at
|
||||||
url = f"{self.base_url}/{sport}/{league}/standings"
|
/rankings and a records table at /standings; professional leagues have
|
||||||
response = self.session.get(url, headers=self.get_headers(), timeout=15)
|
only /standings. The old code tried /standings first and fell back to
|
||||||
response.raise_for_status()
|
/rankings only on a 404 -- but college /standings answers 200, so the
|
||||||
|
fallback never fired and college rankings came back empty forever.
|
||||||
data = response.json()
|
Nothing failed; the AP rank badge simply never appeared, and anything
|
||||||
self.logger.debug(f"Fetched standings for {sport}/{league}")
|
else keyed off rankings quietly did nothing.
|
||||||
|
|
||||||
|
A 200 that lacks the key is treated as a miss, so a league answering
|
||||||
|
both endpoints still ends up with whichever one actually carries a poll.
|
||||||
|
"""
|
||||||
|
league_name = (league or "").lower()
|
||||||
|
wants_poll = "college" in league_name or "ncaa" in league_name
|
||||||
|
endpoints = ["rankings", "standings"] if wants_poll else ["standings", "rankings"]
|
||||||
|
|
||||||
|
for endpoint in endpoints:
|
||||||
|
url = f"{self.base_url}/{sport}/{league}/{endpoint}"
|
||||||
|
# Only the request is guarded. Inspecting the payload happens
|
||||||
|
# below, outside the handler, so that a bug in this method cannot
|
||||||
|
# be mistaken for an endpoint that failed -- that mistake would
|
||||||
|
# silently drop rankings for a league that has them, which is the
|
||||||
|
# exact failure this function was written to fix.
|
||||||
|
try:
|
||||||
|
response = self.session.get(
|
||||||
|
url, headers=self.get_headers(), timeout=15
|
||||||
|
)
|
||||||
|
response.raise_for_status()
|
||||||
|
data = response.json()
|
||||||
|
except (requests.RequestException, ValueError) as e:
|
||||||
|
status = getattr(getattr(e, "response", None), "status_code", None)
|
||||||
|
# Only a 404 is routine -- it is how a league says "no poll
|
||||||
|
# here". Everything else is worth an error, and `status is
|
||||||
|
# None` covers the ones that matter most: ConnectionError,
|
||||||
|
# Timeout, a body that would not parse. Silencing those left a
|
||||||
|
# board that could not reach ESPN with one debug line, and the
|
||||||
|
# ranked filter running on an empty table.
|
||||||
|
if status != 404:
|
||||||
|
self.logger.error(
|
||||||
|
f"Error fetching {endpoint} from ESPN for "
|
||||||
|
f"{sport}/{league}: {e}"
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
|
||||||
|
if not isinstance(data, dict):
|
||||||
|
# A list or a bare string is not something the callers can
|
||||||
|
# read. Treat it as a miss so the other endpoint still gets a
|
||||||
|
# turn, but say so -- this means ESPN changed shape.
|
||||||
|
self.logger.error(
|
||||||
|
f"Unexpected {endpoint} payload for {sport}/{league}: "
|
||||||
|
f"got {type(data).__name__}, expected an object"
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
if endpoint == "rankings" and not data.get("rankings"):
|
||||||
|
continue
|
||||||
|
self.logger.debug(f"Fetched {endpoint} for {sport}/{league}")
|
||||||
return data
|
return data
|
||||||
except Exception as e:
|
self.logger.debug(
|
||||||
# If standings doesn't exist, try rankings (for college sports)
|
f"Standings/rankings not available for {sport}/{league} from ESPN API"
|
||||||
if hasattr(e, 'response') and hasattr(e.response, 'status_code') and e.response.status_code == 404:
|
)
|
||||||
try:
|
return {}
|
||||||
url = f"{self.base_url}/{sport}/{league}/rankings"
|
|
||||||
response = self.session.get(url, headers=self.get_headers(), timeout=15)
|
|
||||||
response.raise_for_status()
|
|
||||||
|
|
||||||
data = response.json()
|
|
||||||
self.logger.debug(f"Fetched rankings for {sport}/{league}")
|
|
||||||
return data
|
|
||||||
except Exception:
|
|
||||||
# Both endpoints failed - standings/rankings may not be available for this sport/league
|
|
||||||
self.logger.debug(f"Standings/rankings not available for {sport}/{league} from ESPN API")
|
|
||||||
return {}
|
|
||||||
else:
|
|
||||||
# Non-404 error - log at debug level since standings are optional
|
|
||||||
self.logger.debug(f"Error fetching standings from ESPN for {sport}/{league}: {e}")
|
|
||||||
return {}
|
|
||||||
|
|
||||||
|
|
||||||
class MLBAPIDataSource(DataSource):
|
class MLBAPIDataSource(DataSource):
|
||||||
|
|||||||
@@ -92,10 +92,108 @@ class TestESPNDataSource:
|
|||||||
assert result == payload
|
assert result == payload
|
||||||
|
|
||||||
def test_fetch_standings_returns_empty_on_error(self):
|
def test_fetch_standings_returns_empty_on_error(self):
|
||||||
with patch.object(self.source.session, "get", side_effect=Exception("error")):
|
# A transport failure is a RequestException, not a bare Exception.
|
||||||
|
# The old stand-in passed only because the handler caught everything,
|
||||||
|
# including bugs in the method under test.
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
side_effect=requests.ConnectionError("error")):
|
||||||
result = self.source.fetch_standings("football", "nfl")
|
result = self.source.fetch_standings("football", "nfl")
|
||||||
assert result == {}
|
assert result == {}
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# fetch_standings endpoint selection
|
||||||
|
#
|
||||||
|
# College leagues publish a poll at /rankings and a records table at
|
||||||
|
# /standings; professional leagues have only /standings. Probing them in
|
||||||
|
# the wrong order still returns 200 -- just without a poll in it -- so
|
||||||
|
# nothing failed and the rank badge simply never appeared. Order is the
|
||||||
|
# behaviour here, so these tests assert it directly.
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _requested_endpoints(mock_get):
|
||||||
|
"""The endpoint names probed, in the order they were requested."""
|
||||||
|
return [call.args[0].rsplit("/", 1)[-1] for call in mock_get.call_args_list]
|
||||||
|
|
||||||
|
def test_professional_league_asks_standings_first(self):
|
||||||
|
payload = {"standings": []}
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
return_value=_mock_response(payload)) as mock_get:
|
||||||
|
result = self.source.fetch_standings("football", "nfl")
|
||||||
|
assert result == payload
|
||||||
|
assert self._requested_endpoints(mock_get) == ["standings"]
|
||||||
|
|
||||||
|
def test_college_league_asks_rankings_first(self):
|
||||||
|
poll = {"rankings": [{"name": "AP Top 25"}]}
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
return_value=_mock_response(poll)) as mock_get:
|
||||||
|
result = self.source.fetch_standings("football", "college-football")
|
||||||
|
assert result == poll
|
||||||
|
assert self._requested_endpoints(mock_get) == ["rankings"]
|
||||||
|
|
||||||
|
def test_rankings_200_without_a_poll_falls_through_to_standings(self):
|
||||||
|
"""A 200 is not the same as an answer.
|
||||||
|
|
||||||
|
This is the case the old code could not see: the endpoint responded,
|
||||||
|
so nothing raised, but the body carried no poll.
|
||||||
|
"""
|
||||||
|
empty_poll = _mock_response({"rankings": []})
|
||||||
|
table = _mock_response({"standings": [{"entries": []}]})
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
side_effect=[empty_poll, table]) as mock_get:
|
||||||
|
result = self.source.fetch_standings(
|
||||||
|
"basketball", "mens-college-basketball")
|
||||||
|
assert result == {"standings": [{"entries": []}]}
|
||||||
|
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
|
||||||
|
|
||||||
|
def test_404_on_the_first_endpoint_falls_through_quietly(self):
|
||||||
|
missing = _mock_response({}, status_code=404)
|
||||||
|
table = _mock_response({"standings": []})
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
side_effect=[missing, table]) as mock_get:
|
||||||
|
result = self.source.fetch_standings("baseball", "college-baseball")
|
||||||
|
assert result == {"standings": []}
|
||||||
|
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
|
||||||
|
|
||||||
|
def test_recovers_from_a_non_404_failure_on_the_first_endpoint(self):
|
||||||
|
table = _mock_response({"standings": [{"entries": []}]})
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
side_effect=[requests.ConnectionError("reset"), table]) as mock_get:
|
||||||
|
result = self.source.fetch_standings("football", "college-football")
|
||||||
|
assert result == {"standings": [{"entries": []}]}
|
||||||
|
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
|
||||||
|
|
||||||
|
def test_both_endpoints_failing_returns_empty(self):
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
side_effect=requests.ConnectionError("down")) as mock_get:
|
||||||
|
result = self.source.fetch_standings("football", "nfl")
|
||||||
|
assert result == {}
|
||||||
|
assert self._requested_endpoints(mock_get) == ["standings", "rankings"]
|
||||||
|
|
||||||
|
def test_a_non_object_payload_is_treated_as_a_miss(self):
|
||||||
|
odd = _mock_response(["not", "an", "object"])
|
||||||
|
table = _mock_response({"standings": []})
|
||||||
|
with patch.object(self.source.session, "get",
|
||||||
|
side_effect=[odd, table]) as mock_get:
|
||||||
|
result = self.source.fetch_standings("football", "college-football")
|
||||||
|
assert result == {"standings": []}
|
||||||
|
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
|
||||||
|
|
||||||
|
def test_a_bug_in_this_method_is_not_swallowed_as_a_failed_endpoint(self):
|
||||||
|
"""The guard for the narrowed handler.
|
||||||
|
|
||||||
|
An error raised while reading the payload used to be caught by the
|
||||||
|
endpoint handler and reported as 'no poll here', which would silently
|
||||||
|
drop rankings for a league that has them. It must surface instead.
|
||||||
|
"""
|
||||||
|
boom = Mock(spec=requests.Response)
|
||||||
|
boom.status_code = 200
|
||||||
|
boom.raise_for_status = Mock()
|
||||||
|
boom.json.side_effect = TypeError("a bug, not a network failure")
|
||||||
|
with patch.object(self.source.session, "get", return_value=boom):
|
||||||
|
with pytest.raises(TypeError):
|
||||||
|
self.source.fetch_standings("football", "nfl")
|
||||||
|
|
||||||
def test_base_url_set_correctly(self):
|
def test_base_url_set_correctly(self):
|
||||||
assert "espn.com" in self.source.base_url
|
assert "espn.com" in self.source.base_url
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,120 @@
|
|||||||
|
"""The generated sudoers rules must parse before they reach /etc/sudoers.d.
|
||||||
|
|
||||||
|
A malformed drop-in there makes sudo refuse every command for every user. On a
|
||||||
|
headless Pi that is unrecoverable without pulling the SD card, so both
|
||||||
|
installers run `visudo -c` on the file they generated before installing it.
|
||||||
|
|
||||||
|
The render test also gives us the check neither installer had: that the rules
|
||||||
|
they actually emit are valid sudoers syntax on a real Linux box.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
import tempfile
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||||
|
FIRST_TIME = os.path.join(REPO_ROOT, "first_time_install.sh")
|
||||||
|
CONFIGURE = os.path.join(REPO_ROOT, "scripts", "install", "configure_web_sudo.sh")
|
||||||
|
|
||||||
|
VISUDO = shutil.which("visudo") or (
|
||||||
|
"/usr/sbin/visudo" if os.path.exists("/usr/sbin/visudo") else None
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _read(path):
|
||||||
|
with open(path, "r", encoding="utf-8") as handle:
|
||||||
|
return handle.read()
|
||||||
|
|
||||||
|
|
||||||
|
def test_first_time_install_validates_before_installing():
|
||||||
|
body = _read(FIRST_TIME)
|
||||||
|
assert 'visudo -c -f "$SUDOERS_TMP"' in body
|
||||||
|
install = body.index('cp "$SUDOERS_TMP" "$SUDOERS_FILE"')
|
||||||
|
validate = body.index('visudo -c -f "$SUDOERS_TMP"')
|
||||||
|
assert validate < install, "the rules must be checked before they are installed"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_install_is_gated_on_the_check():
|
||||||
|
"""Checking and then installing anyway would be worse than not checking."""
|
||||||
|
body = _read(FIRST_TIME)
|
||||||
|
assert "SUDOERS_VALID=0" in body
|
||||||
|
gate = body.index('if [ "$SUDOERS_VALID" = "0" ]')
|
||||||
|
install = body.index('cp "$SUDOERS_TMP" "$SUDOERS_FILE"')
|
||||||
|
assert gate < install
|
||||||
|
|
||||||
|
|
||||||
|
def test_first_time_install_does_not_use_a_predictable_temp_file():
|
||||||
|
body = _read(FIRST_TIME)
|
||||||
|
assert "mktemp" in body
|
||||||
|
assert "> /tmp/ledmatrix_web_sudoers" not in body
|
||||||
|
assert ">> /tmp/ledmatrix_web_sudoers" not in body
|
||||||
|
|
||||||
|
|
||||||
|
def test_configure_web_sudo_validates_before_installing():
|
||||||
|
body = _read(CONFIGURE)
|
||||||
|
assert 'visudo -c -f "$TEMP_SUDOERS"' in body
|
||||||
|
install = body.index('cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web')
|
||||||
|
validate = body.index('visudo -c -f "$TEMP_SUDOERS"')
|
||||||
|
assert validate < install, "the rules must be checked before they are installed"
|
||||||
|
|
||||||
|
|
||||||
|
def _render_first_time_sudoers(project_root, user):
|
||||||
|
"""Run the installer's own sudoers heredoc with realistic values."""
|
||||||
|
body = _read(FIRST_TIME)
|
||||||
|
start = body.index("# Create sudoers content")
|
||||||
|
end = body.index("# Never install rules we have not parsed.")
|
||||||
|
block = body[start:end]
|
||||||
|
out = os.path.join(project_root, "rendered")
|
||||||
|
script = "\n".join(
|
||||||
|
[
|
||||||
|
"set -euo pipefail",
|
||||||
|
f"ACTUAL_USER={user}",
|
||||||
|
f"PROJECT_ROOT_DIR={project_root}",
|
||||||
|
'SUDOERS_TMP="$(mktemp)"',
|
||||||
|
"PYTHON_PATH=$(which python3)",
|
||||||
|
"SYSTEMCTL_PATH=/usr/bin/systemctl",
|
||||||
|
"REBOOT_PATH=/usr/sbin/reboot",
|
||||||
|
"POWEROFF_PATH=/usr/sbin/poweroff",
|
||||||
|
"BASH_PATH=$(which bash)",
|
||||||
|
"JOURNALCTL_PATH=/usr/bin/journalctl",
|
||||||
|
block,
|
||||||
|
f'cp "$SUDOERS_TMP" {out}',
|
||||||
|
]
|
||||||
|
)
|
||||||
|
subprocess.run(["bash", "-c", script], check=True)
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(sys.platform == "win32", reason="visudo is POSIX only")
|
||||||
|
@pytest.mark.skipif(VISUDO is None, reason="visudo not installed")
|
||||||
|
def test_the_rules_the_installer_emits_actually_parse():
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
rendered = _render_first_time_sudoers(tmp, "ledmatrix")
|
||||||
|
os.chmod(rendered, 0o440)
|
||||||
|
result = subprocess.run(
|
||||||
|
[VISUDO, "-c", "-f", rendered], capture_output=True, text=True
|
||||||
|
)
|
||||||
|
assert result.returncode == 0, result.stdout + result.stderr
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(sys.platform == "win32", reason="visudo is POSIX only")
|
||||||
|
@pytest.mark.skipif(VISUDO is None, reason="visudo not installed")
|
||||||
|
def test_a_broken_rule_is_caught_rather_than_installed():
|
||||||
|
"""The guard is only worth having if visudo rejects what it should."""
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
rendered = _render_first_time_sudoers(tmp, "ledmatrix")
|
||||||
|
with open(rendered, "r", encoding="utf-8") as handle:
|
||||||
|
good = handle.read()
|
||||||
|
broken = os.path.join(tmp, "broken")
|
||||||
|
with open(broken, "w", encoding="utf-8") as handle:
|
||||||
|
# An empty command path is what an unset $BASH_PATH would produce.
|
||||||
|
handle.write(good + "\nledmatrix ALL=(ALL) NOPASSWD:\n")
|
||||||
|
os.chmod(broken, 0o440)
|
||||||
|
result = subprocess.run(
|
||||||
|
[VISUDO, "-c", "-f", broken], capture_output=True, text=True
|
||||||
|
)
|
||||||
|
assert result.returncode != 0
|
||||||
Reference in New Issue
Block a user