fix(install): resolve install issues and speed up web UI startup#225
fix(install): resolve install issues and speed up web UI startup#225ChuckBuilds merged 4 commits intomainfrom
Conversation
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>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaced apt-based numpy and related steps with pip-based installation from requirements; added a Changes
Sequence DiagramsequenceDiagram
participant Startup as Startup Process
participant Checker as Marker Check
participant FS as File System
participant Installer as Pip Installer
participant WebUI as Web Interface
Startup->>Checker: dependencies_installed()?
Checker->>FS: stat/.web_deps_installed
alt marker exists
FS-->>Checker: exists
Checker-->>Startup: skip install
Startup->>WebUI: launch web UI
else marker missing
FS-->>Checker: not found
Checker-->>Startup: run installer
Startup->>Installer: pip install -r requirements.txt (web deps)
Installer-->>Startup: install result (success/fail)
alt success
Startup->>FS: create .web_deps_installed
FS-->>Startup: marker created
Startup->>WebUI: launch web UI
else failure
Startup-->>Startup: exit with error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@first_time_install.sh`:
- Around line 804-816: The web dependencies are being installed twice because
Step 6 writes a marker file ".web_deps_installed" but Step 7 still runs
install_dependencies_apt.py; modify the script to check for the marker before
running the Step 7 block and skip install_dependencies_apt.py if
"$PROJECT_ROOT_DIR/.web_deps_installed" exists (or alternatively remove the Step
6 installation and instead rely solely on install_dependencies_apt.py), ensuring
the check references the marker file name and the install_dependencies_apt.py
invocation to prevent duplicate installs.
🧹 Nitpick comments (1)
first_time_install.sh (1)
1549-1553: Good user communication about first-boot expectations.The patience message helps users understand that the first boot may take longer. The 5-minute estimate is reasonable for worst-case scenarios on slower SD cards or with many dependencies.
Minor suggestion: Consider clarifying that subsequent boots will be much faster (<10 seconds) thanks to the marker file optimization.
📝 Suggested enhancement for clarity
echo "1. PLEASE BE PATIENT after reboot!" echo " - The web interface may take up to 5 minutes to start on first boot" echo " - Services need time to initialize after installation" echo " - Wait at least 2-3 minutes before checking service status" +echo " - Subsequent boots will start much faster (<10 seconds)" echo ""
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>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@first_time_install.sh`:
- Around line 804-816: The script currently creates the marker
.web_deps_installed regardless of pip install success; change the control flow
so the marker is only created when the install succeeds by checking the pip
command exit status (use the same conditional that prints "✓ Web interface
dependencies installed") and move the touch
"$PROJECT_ROOT_DIR/.web_deps_installed" into the success branch (or wrap it in
an if after the pip command). Reference variables/strings: PROJECT_ROOT_DIR,
web_interface/requirements.txt, the pip install command, the warning echo "⚠
Warning: Some web interface dependencies failed to install", and the marker name
.web_deps_installed to locate and update the logic.
🧹 Nitpick comments (2)
first_time_install.sh (2)
909-933: Good: Marker check prevents duplicate installation; same marker creation concern applies.The marker file check at line 910 correctly prevents redundant web dependency installation, addressing the previous review concern. However, similar to Step 5, the marker at line 931 is created regardless of whether the installation actually succeeds. This could mask failures in subsequent runs.
Consider making the marker creation conditional on successful installation in both fallback paths.
♻️ Proposed improvement for conditional marker creation
# Try to install dependencies using the smart installer if available if [ -f "$PROJECT_ROOT_DIR/scripts/install_dependencies_apt.py" ]; then echo "Using smart dependency installer..." - python3 "$PROJECT_ROOT_DIR/scripts/install_dependencies_apt.py" + if python3 "$PROJECT_ROOT_DIR/scripts/install_dependencies_apt.py"; then + touch "$PROJECT_ROOT_DIR/.web_deps_installed" + echo "✓ Web interface dependencies installed" + else + echo "⚠ Warning: Smart dependency installer failed" + fi else echo "Using pip to install dependencies..." if [ -f "$PROJECT_ROOT_DIR/requirements_web_v2.txt" ]; then - python3 -m pip install --break-system-packages -r requirements_web_v2.txt + if python3 -m pip install --break-system-packages -r requirements_web_v2.txt; then + touch "$PROJECT_ROOT_DIR/.web_deps_installed" + echo "✓ Web interface dependencies installed" + else + echo "⚠ Warning: pip install failed" + fi else echo "⚠ requirements_web_v2.txt not found; skipping web dependency install" fi fi - - # Create marker file to indicate dependencies are installed - touch "$PROJECT_ROOT_DIR/.web_deps_installed" - echo "✓ Web interface dependencies installed" fi
1556-1560: LGTM! Consider clarifying the timing expectation.The patience note addresses the PR objective for user-facing messaging. The "up to 5 minutes" estimate is conservative, which is reasonable for first boot on varying hardware. Given that the PR objective states subsequent boots should be under 5 seconds, you might consider clarifying this distinction to set better user expectations:
📝 Optional: Clarify first boot vs subsequent boot timing
echo "1. PLEASE BE PATIENT after reboot!" -echo " - The web interface may take up to 5 minutes to start on first boot" +echo " - The web interface may take 1-2 minutes to start on first boot" +echo " - Subsequent boots should be much faster (under 10 seconds)" echo " - Services need time to initialize after installation" echo " - Wait at least 2-3 minutes before checking service status"
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>
1 participant
Summary
Changes
Issue #224 - LED Panels Not Working After Fresh Install
rpi-rgb-led-matrix-masterfrom permission normalization in Step 11.1chmod 644was stripping executable bits from compiled library filesIssue #208 - Duplicate Package Wheel Errors
python3-pillowfrom apt install (Debian maps it topython3-pil)pip, notsetuptools/wheel(they conflict with apt versions)Web UI Slow Startup
.web_deps_installed) to skip redundant pip installspip installon every startup, causing 30-60+ second delaysUser Experience
Test plan
sudo systemctl status ledmatrix ledmatrix-webFixes #224
Fixes #208
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.