fix(install): only a running desktop stops the install; detect desktops by metapackage by ChuckBuilds · Pull Request #781 · ChuckBuilds/LEDMatrix

ChuckBuilds · 2026-10-06T12:53:42Z

Pull Request

Summary

Follow-up to #780. The installer's "Lite only" check is meant to stop installs where a desktop competes with the LED panel for CPU, but its package check kept firing on Lite systems: first libblockdev-* (#780), then anything starting with gnome/kde/xfce/lxde, e.g. gnome-keyring (CodeRabbit on #780).

This PR changes what each signal does:

Signal	Before	Now
Display manager running (`display-manager`, lightdm, gdm, sddm, lxdm)	stop	stop
Desktop packages installed	stop	warning, install continues
Desktop session files (`/usr/share/xsessions`, `/usr/share/raspberrypi-ui-mods`)	stop	warning, install continues

A desktop that is installed but not running costs the panel nothing. The stop message now gives the fix: sudo systemctl set-default multi-user.target, then reboot.

Other changes:

Running check made reliable, since it is now the only blocker. It used systemctl list-units | grep -q under pipefail, the pattern this script's own comment warns can turn a match into "not found". It now uses systemctl is-active, and adds display-manager, the alias every Debian display manager registers.
Package names matched exactly. Desktop metapackages and session managers are matched as whole names, so standalone parts (gnome-keyring, xfce4-terminal, lxde-icon-theme, ...) no longer count. The list now also covers Raspberry Pi OS Trixie, which replaced raspberrypi-ui-mods with rpd-wayland-core / rpd-x-core, plus Debian tasksel desktops (task-desktop, task-*-desktop) and multi-arch names (plasma-workspace:arm64).
docs/TROUBLESHOOTING.md updated for the new message.
Also carries docs(common): list espn_payload in the common README #782's README row for espn_payload (commit 42455f6), which unblocks Core unit tests on main. It becomes a no-op once docs(common): list espn_payload in the common README #782 merges.

Type of change

Bug fix

Related issues

Refs #780 (CodeRabbit's gnome-keyring finding).

Test plan

New tests in test/test_install_os_support.py. They run the installer's real OS-check section, with stubbed systemctl/dpkg-query and a fake /etc/os-release:
- a running display-manager / lightdm / gdm / sddm stops the install;
- 10 desktop packages each warn and continue (Bookworm/Trixie RPi, xfce, lxde, gnome, kde, multi-arch, tasksel);
- desktop session files warn;
- Lite with libblockdev-*, gnome-keyring, xfce4-terminal, lxde-icon-theme, etc. is confirmed Lite with no warning.
Against main's installer, 16 of the 17 new tests fail. The one that passes is the libblockdev case, already fixed in fix(install): stop libblockdev matching the desktop-environment check #780. With this change all 17 pass.
pytest test/test_install_os_support.py: 68 passed. bash -n first_time_install.sh passes.
Not run end to end on a Pi.

Documentation

I updated the relevant doc in docs/ if developer behavior changed (docs/TROUBLESHOOTING.md)

Plugin compatibility

N/A — change doesn't touch the plugin system

Checklist

My commits follow the message convention in CONTRIBUTING.md
I read CONTRIBUTING.md and CODE_OF_CONDUCT.md
I've not committed any secrets or hardcoded API keys

Notes for reviewer

The change is non-destructive: the check only decides whether the installer proceeds, and nothing is installed or removed by it. The behavioural change to review is intentional: a Pi with a desktop installed but booting to the console now installs, with a warning, where it used to stop. The README's "stops on the desktop edition" still holds, because the desktop image boots into its display manager.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VZNWFWprcFYGfuf1JAyrBJ

Summary by CodeRabbit

Bug Fixes
- Improved installation checks to distinguish an actively running desktop from desktop components that are merely installed. A running display manager now stops the OS check; installed desktop packages or session files trigger a warning while allowing installation to continue.
- Desktop package detection now matches specific package names, including Raspberry Pi OS Bookworm and Trixie metapackages, reducing false matches with unrelated packages.

The Lite check matched any installed package starting with gnome/kde/ xfce/lxde, so standalone parts (gnome-keyring, xfce4-terminal, lxde-icon-theme) rejected a Lite system. Match whole names of desktop metapackages and session managers instead. Also catch desktops the prefixes missed: Raspberry Pi OS Trixie replaced raspberrypi-ui-mods with rpd-wayland-core / rpd-x-core, Debian tasksel desktops (task-*-desktop), and multi-arch names (plasma-workspace:arm64). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VZNWFWprcFYGfuf1JAyrBJ

coderabbitai · 2026-10-06T12:54:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 54ad4821-2ae3-434a-901a-1fd50f81cd70

📥 Commits

Reviewing files that changed from the base of the PR and between 4411bb5 and 9e5cf36.

📒 Files selected for processing (4)

docs/TROUBLESHOOTING.md
first_time_install.sh
src/common/README.md
test/test_install_os_support.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The installer now distinguishes a running desktop from installed desktop components. A running display manager fails the OS check. Installed desktop packages or session files without a running display manager produce a warning and allow installation to continue. The common-module README now documents espn_payload.

Changes

Desktop detection

Layer / File(s)	Summary
Check running services and installed desktop components `first_time_install.sh`, `test/test_install_os_support.py`	The installer checks named display-manager services and matches installed desktop packages against an explicit list. The tests configure service and package responses.
Apply desktop-check outcomes `first_time_install.sh`, `test/test_install_os_support.py`, `docs/TROUBLESHOOTING.md`	A running display manager fails the OS check. Installed desktop packages or session files without a running display manager produce a warning and allow installation to continue. Tests and troubleshooting guidance cover these outcomes.

ESPN payload module documentation

Layer / File(s)	Summary
Document the ESPN payload module `src/common/README.md`	The module table and entry describe `espn_payload`, its payload and URL functions, and its documented use by `BackgroundDataService` before caching scoreboard windows.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9e5cf

The installer changes are ready to merge after normal checks. The README’s slimming claim can be qualified separately.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the installer changes: only a running desktop stops installation, and desktop detection uses metapackages.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (2 skipped: 2 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

codacy-production · 2026-10-06T12:54:33Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

#749 added src/common/espn_payload.py without a summary row or section, so test_common_readme_lists_every_module fails on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VZNWFWprcFYGfuf1JAyrBJ (cherry picked from commit 8333f23)

ChuckBuilds · 2026-10-06T13:07:44Z

Core unit tests (Python 3.11) failure isn't this PR's. The one failing test is test_common_readme_lists_every_module: no summary-table row for: ['espn_payload']. #749 added src/common/espn_payload.py to main without a README row, so main (6c533d6) fails it too. I reproduced it there, and this PR only touches first_time_install.sh.

Fix: #782 adds the README row and section. I've carried the same commit into this PR (42455f6), so this PR should go green, and that commit becomes a no-op here once #782 merges.

Generated by Claude Code

A desktop costs the panel CPU only while it runs, so a running display manager (checked with systemctl is-active, including the generic display-manager alias, instead of a grep -q pipe under pipefail) still stops the installer. Desktop packages or session files on a Pi that boots to the console now print a warning and the install continues. Adds installer OS-check tests for running, installed-only and Lite systems, including the libblockdev and gnome-keyring false positives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VZNWFWprcFYGfuf1JAyrBJ

ChuckBuilds mentioned this pull request Oct 6, 2026

docs(common): list espn_payload in the common README #782

Merged

7 tasks done

ChuckBuilds changed the title ~~fix(install): detect desktops by their metapackages, not name prefixes~~ fix(install): only a running desktop stops the install; detect desktops by metapackage Oct 6, 2026

ChuckBuilds merged commit d98b479 into main Oct 6, 2026
15 checks passed

ChuckBuilds deleted the fix/installer-desktop-package-list branch October 6, 2026 17:35

Navigation Menu

Repository navigation

fix(install): only a running desktop stops the install; detect desktops by metapackage - #781

fix(install): only a running desktop stops the install; detect desktops by metapackage#781
ChuckBuilds merged 3 commits into
mainfrom
fix/installer-desktop-package-list

ChuckBuilds commented Oct 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codacy-production Bot commented Oct 6, 2026

Uh oh!

ChuckBuilds commented Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ChuckBuilds commented Oct 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Summary

Type of change

Related issues

Test plan

Documentation

Plugin compatibility

Checklist

Notes for reviewer

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codacy-production Bot commented Oct 6, 2026

Up to standards ✅

Uh oh!

ChuckBuilds commented Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ChuckBuilds commented Oct 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading