Skip to content

fix(logos): register NCAA lacrosse in logo downloader#308

Merged
ChuckBuilds merged 1 commit intomainfrom
fix/lacrosse-logo-directory
Apr 8, 2026
Merged

fix(logos): register NCAA lacrosse in logo downloader#308
ChuckBuilds merged 1 commit intomainfrom
fix/lacrosse-logo-directory

Conversation

@ChuckBuilds
Copy link
Copy Markdown
Owner

@ChuckBuilds ChuckBuilds commented Apr 8, 2026

Summary

Fixes the lacrosse-scoreboard plugin's "missing layout + missing
school logos" symptom on hardware. Single-file change in
`src/logo_downloader.py` adds two NCAA lacrosse rows (and one
missing women's hockey row) to the canonical sport_key registry.

Root cause

The lacrosse-scoreboard plugin renders broken on the LED matrix:
school logos never appear, and the scoreboard collapses to a tiny
"Logo Error" text instead of the normal logo-centric scorebug
that hockey, basketball, football, and baseball all draw.

Tracing the rendering pipeline shows the apparent
"two issues" (no layout + no logos) are one bug:

  1. `plugins/lacrosse-scoreboard/ncaam_lacrosse_managers.py:29` and
    `ncaaw_lacrosse_managers.py:29` set
    `sport_key="ncaam_lacrosse"` / `"ncaaw_lacrosse"` —
    matching the same naming pattern hockey uses
    (`ncaam_hockey`, `ncaaw_hockey`).
  2. The plugin's vendored `sports.py:71-80` then asks the main
    `src/logo_downloader.py`:
    `MainLogoDownloader().get_logo_directory(sport_key)` to
    resolve where logo PNGs live on disk — exactly the same
    pattern every other sports plugin uses.
  3. `LOGO_DIRECTORIES` had no entries for `ncaam_lacrosse` or
    `ncaaw_lacrosse`. `get_logo_directory()` fell through at
    line 165 to the safe fallback
    `f'assets/sports/{league}_logos'` =
    `assets/sports/ncaam_lacrosse_logos`, a directory that does
    not exist
    and is not the shared `ncaa_logos/` directory
    where every other NCAA sport stores its logos.
  4. With the directory pointing nowhere, every per-team
    `_load_and_resize_logo()` call returned `None`. Both
    `SportsRecent._draw_scorebug_layout()` (line 1191) and
    `SportsUpcoming._draw_scorebug_layout()` (line 1698) bail out
    on missing logos and draw the literal text "Logo Error"
    before returning. That is the user-visible symptom.

The plugin's `sports.py` is otherwise nearly identical to
hockey's (a `diff` shows only 3 lines different — all about
`period >= 4` for lacrosse quarters vs `period >= 3` for
hockey periods). The layout code is fine; it just never gets
executed because logo loading fails first.

Pattern verification

Every sports plugin in `ledmatrix-plugins/plugins/` follows the
same vendored architecture and resolves `sport_key` through the
main `LogoDownloader`. Every NCAA `sport_key` was already in
`LOGO_DIRECTORIES` — except lacrosse:

Plugin sport_key Was in registry?
football `nfl`
football `ncaa_fb`
basketball `nba`
basketball `wnba`
basketball `ncaam`
basketball `ncaaw`
baseball `mlb`
baseball `ncaa_baseball`
hockey `nhl`
hockey `ncaam_hockey`
hockey `ncaaw_hockey` ✓ (LOGO_DIR only — see below)
lacrosse `ncaam_lacrosse` ✗ THIS BUG
lacrosse `ncaaw_lacrosse` ✗ THIS BUG

Changes

`src/logo_downloader.py` only:

  • `API_ENDPOINTS`: add `ncaam_lacrosse`, `ncaaw_lacrosse`,
    and (drive-by) `ncaaw_hockey` — the women's hockey entry was
    already in `LOGO_DIRECTORIES` but its corresponding ESPN URL
    was missing, leaving `fetch_teams_data('ncaaw_hockey')`
    silently broken.
  • `LOGO_DIRECTORIES`: add `ncaam_lacrosse` and
    `ncaaw_lacrosse` pointing at the existing shared
    `assets/sports/ncaa_logos` directory.

5 lines added, 0 removed. No structural changes, no plugin-side
edits, no version bumps, no asset commits.

Why no plugin-side change

The plugin's `sports.py` already imports the main
`LogoDownloader` and asks it to resolve the directory. Once
the registry has the right answer, every sport plugin (current
and future) gets it for free. This is the canonical pattern —
adding lacrosse here makes it a first-class peer.

Why no prefetch script

Existing NCAA football/hockey schools that also play lacrosse
(DUKE, UVA, MD, NAVY, ARMY, YALE, SYR, …) are already
present in `assets/sports/ncaa_logos/` and get picked up
immediately on first render with zero downloads. Lacrosse-only
schools (JHU, Loyola, Princeton, Cornell, Stony Brook, …)
download lazily into the shared directory the first time they
appear in an ESPN scoreboard payload, via the existing
`download_missing_logo(logo_url=…)` path the plugin already
exercises at runtime.

Verification done locally

  1. Directory resolution (no network):
    ```
    ncaam_lacrosse -> .../assets/sports/ncaa_logos
    ncaaw_lacrosse -> .../assets/sports/ncaa_logos
    ncaam_hockey -> .../assets/sports/ncaa_logos
    ncaaw_hockey -> .../assets/sports/ncaa_logos
    ```
    All four collapse to the shared NCAA directory.

  2. Lazy single-team download with a real ESPN logo URL
    (Johns Hopkins, team_id=120):
    ```
    downloaded: True | exists: True | size: 40787
    ```
    Real PNG (~40KB), saved to `assets/sports/ncaa_logos/`,
    then cleaned up.

Test plan

  • Pi pulls main, restarts `ledmatrix` service
  • Enable `lacrosse-scoreboard` with at least one in-season
    favorite team
  • Confirm the scoreboard shows the logo-centric layout
    (home/away logos at the edges, score in the center,
    period/clock on top, records at the bottom) instead of the
    "Logo Error" fallback
  • Confirm at least one school logo renders for a team that
    isn't already in `ncaa_logos/` from the football days
    (e.g. JHU, Loyola, Princeton, Cornell) — proving the lazy
    download path is now live
  • Confirm `journalctl -u ledmatrix -f` logs
    `Logo directory for sport_key='ncaam_lacrosse': …/ncaa_logos`
    (the existing INFO log at `sports.py:75`) instead of the
    old `…/ncaam_lacrosse_logos` fallback path

Out of scope

  • Bulk-prefetching every NCAA lacrosse logo via a script (lazy
    download covers it)
  • Refactoring the lacrosse plugin to drop its vendored
    `sports.py` / `game_renderer.py` in favor of the main
    `src/base_classes/` (hockey doesn't either; that's a separate
    architectural cleanup)
  • The latent `milb` (Minor League Baseball) registry gap
    surfaced while verifying the pattern — same shape of bug, but
    separate report

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • Added support for downloading team logos for three additional NCAA leagues: women's hockey, men's lacrosse, and women's lacrosse.

The lacrosse-scoreboard plugin renders broken on hardware: school
logos never appear, and SportsRecent/SportsUpcoming
_draw_scorebug_layout() falls into its "Logo Error" fallback
branch instead of drawing the normal logo-centric scorebug.

Root cause: src/logo_downloader.py LOGO_DIRECTORIES and
API_ENDPOINTS were missing entries for ncaam_lacrosse and
ncaaw_lacrosse, even though the plugin's manager files set those
exact sport_key values (ncaam_lacrosse_managers.py:29,
ncaaw_lacrosse_managers.py:29). The plugin's vendored sports.py
asks the main LogoDownloader to resolve sport_key →
on-disk directory the same way every other sports plugin does
(football, basketball, baseball, hockey), and
get_logo_directory() fell through to the safe fallback
f'assets/sports/{league}_logos' = 'assets/sports/ncaam_lacrosse_logos',
a directory that does not exist. Logo loads then failed for
every team and the scorebug layout collapsed to "Logo Error".

Adding the two lacrosse rows (and the missing ncaaw_hockey row
in API_ENDPOINTS, while we're here) makes lacrosse a first-class
peer to the other NCAA sports — same shared assets/sports/ncaa_logos
directory, same canonical ESPN team-list endpoint pattern. No
plugin-side change is needed because the plugin already imports
the main LogoDownloader.

Existing NCAA football/hockey schools that also play lacrosse
(DUKE, UVA, MD, NAVY, ARMY, YALE, SYR, …) get picked up
immediately on first render. Lacrosse-specific schools (JHU,
Loyola, Princeton, Cornell, Stony Brook, …) lazily download
into the shared directory via download_missing_logo() the first
time they appear in a scoreboard payload — verified locally
with both the team_id fallback path (ESPN sports.core.api) and
the direct logo_url path used by the plugin at runtime.

Verification (all from a clean clone):

  python3 -c "
  from src.logo_downloader import LogoDownloader
  d = LogoDownloader()
  for k in ('ncaam_lacrosse','ncaaw_lacrosse','ncaam_hockey','ncaaw_hockey'):
      print(k, '->', d.get_logo_directory(k))
  "
  # All four print .../assets/sports/ncaa_logos

  python3 -c "
  from pathlib import Path
  from src.logo_downloader import download_missing_logo
  ok = download_missing_logo(
      'ncaam_lacrosse', team_id='120', team_abbreviation='JHU',
      logo_path=Path('assets/sports/ncaa_logos/_jhu_test.png'),
      logo_url='https://a.espncdn.com/i/teamlogos/ncaa/500/120.png',
  )
  print('downloaded:', ok)  # True, ~40KB PNG
  "

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6723d38-693f-4a1e-ad5d-deb70fa016b3

📥 Commits

Reviewing files that changed from the base of the PR and between 6812dfe and ab3fd4d.

📒 Files selected for processing (1)
  • src/logo_downloader.py

📝 Walkthrough

Walkthrough

Extends the LogoDownloader class with three new NCAA league API endpoint mappings for women's hockey and men's and women's lacrosse, along with logo directory configurations for the lacrosse leagues to support additional sports data resolution.

Changes

Cohort / File(s) Summary
NCAA League Endpoint & Directory Mappings
src/logo_downloader.py
Added three new ESPN API endpoint entries to API_ENDPOINTS for ncaaw_hockey, ncaam_lacrosse, and ncaaw_lacrosse. Added two new logo directory mappings to LOGO_DIRECTORIES for ncaam_lacrosse and ncaaw_lacrosse, both pointing to assets/sports/ncaa_logos.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(logos): register NCAA lacrosse in logo downloader' accurately summarizes the main change: registering NCAA lacrosse sport keys in the logo downloader to fix missing logo resolution.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/lacrosse-logo-directory

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ChuckBuilds ChuckBuilds merged commit 601fedb into main Apr 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant