Skip to content

Fix/led panel permissions 224#226

Merged
ChuckBuilds merged 9 commits intomainfrom
fix/led-panel-permissions-224
Jan 30, 2026
Merged

Fix/led panel permissions 224#226
ChuckBuilds merged 9 commits intomainfrom
fix/led-panel-permissions-224

Conversation

@ChuckBuilds
Copy link
Copy Markdown
Owner

@ChuckBuilds ChuckBuilds commented Jan 30, 2026

Summary by CodeRabbit

  • Chores
    • Reorganized installation sequence so dependencies, web assets, and the main service install in a more reliable order.
    • Ensured configuration files are created with correct ownership and permissions; re-applies permissions after path changes.
    • Hardened service integration with daemon reloads and stricter unit permissions; avoided repeated web installs via marker checks.
    • Refined Python/pip handling and added capability-setting for the Python runtime.

✏️ Tip: You can customize this high-level summary in your review settings.

Chuck and others added 7 commits January 30, 2026 10:44
The permission normalization step in first_time_install.sh was running
chmod 644 on all files, which stripped executable bits from compiled
library files (librgbmatrix.so.1) after make build-python created them.

This caused LED panels to not work after fresh installation until users
manually ran chmod on the rpi-rgb-led-matrix-master directory.

Fixes #224

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Issues addressed:
- Remove redundant python3-pillow from apt (Debian maps it to python3-pil)
- Only upgrade pip, not setuptools/wheel (they conflict with apt versions)
- Remove separate apt numpy install (pip handles it from requirements.txt)
- Install web interface deps during first-time setup, not on every startup
- Add marker file (.web_deps_installed) to skip redundant pip installs
- Add user-friendly message about wait time after installation

The web UI was taking 30-60+ seconds to start because it ran pip install
on every startup. Now it only installs dependencies on first run.

Fixes #208

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Step 7 was installing web dependencies again even though they were
already installed in Step 5. Now Step 7 checks for the .web_deps_installed
marker file and skips the installation if it already exists.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The .web_deps_installed marker file should only be created when pip
install actually succeeds. Previously it was created regardless of
the pip exit status, which could cause subsequent runs to skip
installing missing dependencies.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The services were being started before config files existed, causing
the web service to fail with "config.json not found". Reordered steps
so config files are created (Step 4) before services are installed
and started (Step 4.1).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The apt-installed pip cannot be upgraded because it doesn't have a
RECORD file. Since the apt version (25.1.1) is already recent enough,
we can skip the upgrade step entirely.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Moved main LED Matrix service installation (Step 7.5) to after all
Python dependencies are installed (Steps 5-7). Previously services
were being started before pip packages and rgbmatrix were ready,
causing startup failures.

New order:
- Step 5: Python pip dependencies
- Step 6: rpi-rgb-led-matrix build
- Step 7: Web interface dependencies
- Step 7.5: Main LED Matrix service (moved here)
- Step 8: Web interface service

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Reorganized first_time_install.sh to reorder installation steps: the main LED Matrix service is now installed after Python and web dependencies (moved to Step 7.5). Added explicit creation and permissioning of config.json/config_secrets.json, improved Python capability handling (setcap targeting discovered Python binary, no pip upgrade), hardened systemd unit permissions, introduced marker-based guards for web deps, and adjusted ownership/permission reapplication logic.

Changes

Cohort / File(s) Summary
Installer script (overall flow)
first_time_install.sh
Resequenced installation tasks: Python deps → web deps → main service (new Step 7.5). Renumbered steps and updated user messaging and idempotency markers.
Python & capabilities
first_time_install.sh
Removed pip upgrade; only checks pip version. Added capability configuration block that finds the Python binary (including Python 3.13) and applies setcap when available.
Config files & permissions
first_time_install.sh
Explicitly creates config.json and config_secrets.json from templates or minimal fallbacks, sets ownership and permissions, and reapplies ownership for plugins/plugin-repos based on detected service user.
Systemd / web integration
first_time_install.sh
Moved/added systemctl daemon-reload invocations, hardened systemd unit file permissions (8.1), added conditional checks for old vs new service paths, and introduced a marker to avoid re-installing web deps.
Helper / marker logic & messaging
first_time_install.sh
Added checks for relocated scripts, marker-based web dependency guard, conditional updates for service files, and small messaging/flow tweaks for missing or moved scripts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Hopping through scripts with nimble paws,
I add a step and tighten up the laws,
Configs snugged safe, permissions set right,
Services now start after Python's light.
Installations hum — I nibble with delight. 🥕

🚥 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/led panel permissions 224' directly references the issue number and core purpose of the changeset, which involves ownership and permissions adjustments throughout the installation script.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/led-panel-permissions-224

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.

Resolved conflicts:
- Kept pip upgrade removal (apt-installed pip can't self-upgrade)
- Kept Step 7.5 service installation after dependencies

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@first_time_install.sh`:
- Around line 514-516: The initial "This script will perform the following
steps" list must be updated to reflect the reordered steps so Step 4 matches the
updated CURRENT_STEP and echo "Step 4: Ensuring configuration files exist...";
locate the header/list block (the startup echo statements that enumerate steps)
and change the items so item 4 reads "Ensure configuration files exist" and
renumber subsequent entries to match the provided suggested sequence (including
fractional steps like 7.5/8.5/10.1 where applicable) so the step numbers align
with the CURRENT_STEP variable and the runtime echo lines.
- Around line 898-931: The script prints success even when setcap fails and may
call setcap on a symlink (which fails); update the capability block to resolve
the real binary (use readlink -f on /usr/bin/python3 and /usr/bin/python3.13),
check command -v setcap first, run sudo setcap on the resolved path, capture the
exit status, and only print the "✓ Python3... capabilities configured" message
when setcap succeeds (otherwise print a clear warning/error). Reference the
PYTHON_VER variable and the /usr/bin/python3(/3.13) paths used in the existing
conditional so the fix replaces the current setcap calls and success echoes.

- Updated step list header to match actual step order after reordering
  (Step 4 is now "Ensure configuration files exist", added Step 7.5
  for main service, added Step 8.1 for systemd permissions)

- Fixed Python capabilities configuration:
  - Check if setcap command exists before attempting to use it
  - Resolve symlinks with readlink -f to get the real binary path
  - Only print success message when setcap actually succeeds
  - Print clear warning with helpful info when setcap fails

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ChuckBuilds ChuckBuilds merged commit 1c3269c into main Jan 30, 2026
1 check passed
@ChuckBuilds ChuckBuilds deleted the fix/led-panel-permissions-224 branch January 30, 2026 17:49
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