Automatically download the right bin for the latest release and install#5561
Automatically download the right bin for the latest release and install#5561
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds an auto-update flow to the Update page: stores ChangesAuto-update from GitHub
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant WLED_API as WLED_Device_API
participant GitHub_API as GitHub_API
participant Update_Form as Update_Form
User->>Browser: Click "Auto update"
Browser->>WLED_API: GET /json/info
WLED_API-->>Browser: device info (includes repo)
Browser->>GitHub_API: GET /repos/{owner}/{repo}/releases/latest
GitHub_API-->>Browser: release metadata (assets, tag)
Browser->>Browser: Select asset matching suffix (_{tag}_{release}.bin)
Browser->>GitHub_API: GET asset binary URL
GitHub_API-->>Browser: binary blob
Browser->>Browser: Create File & DataTransfer -> set input[name="update"]
Browser->>Update_Form: Submit form (`#upd`)
Update_Form-->>Browser: Update initiated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🧹 Nitpick comments (1)
wled00/data/update.htm (1)
130-130: ⚡ Quick winMake status updates screen-reader friendly
#autoUpdStatusis dynamically updated but has no live-region semantics. Addingrole="status"+aria-live="polite"improves update progress accessibility with minimal change.Suggested markup tweak
- <span id="autoUpdStatus" style="display:block;margin-top:4px;font-size:13px;"></span> + <span id="autoUpdStatus" role="status" aria-live="polite" aria-atomic="true" style="display:block;margin-top:4px;font-size:13px;"></span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/update.htm` at line 130, The span with id "autoUpdStatus" is dynamically updated but lacks live-region semantics; update the element referenced by id autoUpdStatus to include role="status" and aria-live="polite" (and optionally aria-atomic="true") so screen readers announce progress updates — modify the element with id "autoUpdStatus" in update.htm accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/data/update.htm`:
- Around line 91-93: The unconditional rewrite of asset.browser_download_url to
'https://download.wled.me' (fwUrl) causes 404s for non-official repos; change
the logic that builds fwUrl so it only rewrites the host for official WLED
releases (e.g., when info.repo === 'wled/WLED' or another explicit check for the
official repo), otherwise use the original asset.browser_download_url as-is;
update the code around fwUrl and subsequent fetch (fwResp) to conditionally
replace the host based on info.repo (or parsed hostname) so non-official release
assets are fetched from their original URL.
---
Nitpick comments:
In `@wled00/data/update.htm`:
- Line 130: The span with id "autoUpdStatus" is dynamically updated but lacks
live-region semantics; update the element referenced by id autoUpdStatus to
include role="status" and aria-live="polite" (and optionally aria-atomic="true")
so screen readers announce progress updates — modify the element with id
"autoUpdStatus" in update.htm accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c26f991-f960-4236-abfe-c82b2f7ca710
📒 Files selected for processing (1)
wled00/data/update.htm
| const fwUrl = asset.browser_download_url.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me'); | ||
| const fwResp = await fetch(fwUrl); | ||
| if (!fwResp.ok) throw new Error(`Download failed: ${fwResp.status}`); |
There was a problem hiding this comment.
Unconditional URL rewrite breaks auto-update for non-official repos
At Line 91, replacing every release asset host with https://download.wled.me can 404 for builds whose info.repo is not wled/WLED, even when GitHub returned a valid browser_download_url.
Proposed fix
- const fwUrl = asset.browser_download_url.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me');
- const fwResp = await fetch(fwUrl);
- if (!fwResp.ok) throw new Error(`Download failed: ${fwResp.status}`);
+ const githubUrl = asset.browser_download_url;
+ const useMirror = (info.repo || '').toLowerCase() === 'wled/wled';
+ const fwUrl = useMirror
+ ? githubUrl.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me')
+ : githubUrl;
+ let fwResp = await fetch(fwUrl);
+ if (!fwResp.ok && useMirror) fwResp = await fetch(githubUrl); // mirror fallback
+ if (!fwResp.ok) throw new Error(`Download failed: ${fwResp.status}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/data/update.htm` around lines 91 - 93, The unconditional rewrite of
asset.browser_download_url to 'https://download.wled.me' (fwUrl) causes 404s for
non-official repos; change the logic that builds fwUrl so it only rewrites the
host for official WLED releases (e.g., when info.repo === 'wled/WLED' or another
explicit check for the official repo), otherwise use the original
asset.browser_download_url as-is; update the code around fwUrl and subsequent
fetch (fwResp) to conditionally replace the host based on info.repo (or parsed
hostname) so non-official release assets are fetched from their original URL.
- Release name on update page links directly to the matching bin on download.wled.me - Badge links to GitHub releases page as 'View latest' - Auto-update button renamed to 'Auto update' - Manual upload section gets its own subheading; submit button renamed to 'Upload' - 'Manual OTA Update' button in security settings renamed to 'Update WLED' to match info menu wording
|
Updated so that users can choose to either do it totally automatically or if they don't trust the download proxy, get a direct link to the bin file and install as before 
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
wled00/data/update.htm (1)
44-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not force the mirror host for all repositories.
Line 44 and Line 105 rewrite every asset URL to
download.wled.me. That can break valid GitHub assets for non-wled/WLEDrepos and fail auto-update with 404s.Proposed fix
- const binUrl = asset.browser_download_url.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me'); + const githubUrl = asset.browser_download_url; + const useMirror = (data.repo || '').toLowerCase() === 'wled/wled'; + const binUrl = useMirror + ? githubUrl.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me') + : githubUrl;- const fwUrl = asset.browser_download_url.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me'); - const fwResp = await fetch(fwUrl); - if (!fwResp.ok) throw new Error(`Download failed: ${fwResp.status}`); + const githubUrl = asset.browser_download_url; + const useMirror = (info.repo || '').toLowerCase() === 'wled/wled'; + const fwUrl = useMirror + ? githubUrl.replace(/^https?:\/\/[^/]+/, 'https://download.wled.me') + : githubUrl; + let fwResp = await fetch(fwUrl); + if (!fwResp.ok && useMirror) fwResp = await fetch(githubUrl); // mirror fallback + if (!fwResp.ok) throw new Error(`Download failed: ${fwResp.status}`);Also applies to: 105-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/update.htm` around lines 44 - 45, The code currently forces all asset URLs to use download.wled.me by blindly replacing the host when building binUrl (and the second rewrite later), which breaks non-wled repositories; update the logic in the places that compute binUrl (where asset.browser_download_url is used) and the other URL-rewrite location to only rewrite the host when the asset comes from the WLED repo (e.g. asset.browser_download_url or a repo identifier contains "github.com/wled/WLED" or repoName === "wled/WLED"); implement this by parsing asset.browser_download_url into a URL object and applying the .replace/host swap only if the URL matches the WLED repo condition, otherwise leave asset.browser_download_url unchanged and use that value when setting binUrl and when updating the DOM element selected by document.querySelector('.release-name').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/data/update.htm`:
- Around line 31-36: After detecting a valid repo (when repoUrl is truthy) the
code shows the release download UI via toggle('release-download') but does not
hide the stale "Checking…" placeholder (`#Norelease-download`), leaving old status
text visible; update the successful path to also hide or toggle off the
no-release element (the DOM node with id or selector for the "no release"
placeholder) immediately after setting
document.querySelector('.release-repo').href and
document.querySelector('.release-badge').src (i.e., alongside the existing
toggle('release-download') call), and apply the same change in the duplicate
handling near lines 138–139 so the no-release placeholder is always hidden on
success.
---
Duplicate comments:
In `@wled00/data/update.htm`:
- Around line 44-45: The code currently forces all asset URLs to use
download.wled.me by blindly replacing the host when building binUrl (and the
second rewrite later), which breaks non-wled repositories; update the logic in
the places that compute binUrl (where asset.browser_download_url is used) and
the other URL-rewrite location to only rewrite the host when the asset comes
from the WLED repo (e.g. asset.browser_download_url or a repo identifier
contains "github.com/wled/WLED" or repoName === "wled/WLED"); implement this by
parsing asset.browser_download_url into a URL object and applying the
.replace/host swap only if the URL matches the WLED repo condition, otherwise
leave asset.browser_download_url unchanged and use that value when setting
binUrl and when updating the DOM element selected by
document.querySelector('.release-name').
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c1ac98d-1b4c-4ba0-b0d1-c9a70683872a
📒 Files selected for processing (2)
wled00/data/settings_sec.htmwled00/data/update.htm
✅ Files skipped from review due to trivial changes (1)
- wled00/data/settings_sec.htm
| if (repoUrl) { | ||
| const badgeUrl = "https://img.shields.io/github/release/" + data.repo + ".svg?style=flat-square"; | ||
| document.querySelector('.release-repo').href = repoUrl; | ||
| document.querySelector('.release-badge').src = badgeUrl; | ||
| toggle('release-download'); // only show release download item after receiving a valid data.repo | ||
| // fetch latest release from GitHub to build direct bin download link |
There was a problem hiding this comment.
Hide the “Checking…” placeholder in the successful path.
After a valid repo is detected, Line 35 reveals #release-download but #Norelease-download remains visible, so stale status text can stay on screen.
Proposed fix
- toggle('release-download'); // only show release download item after receiving a valid data.repo
+ gId('Norelease-download').classList.add("hide");
+ gId('release-download').classList.remove("hide"); // explicit showAlso applies to: 138-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/data/update.htm` around lines 31 - 36, After detecting a valid repo
(when repoUrl is truthy) the code shows the release download UI via
toggle('release-download') but does not hide the stale "Checking…" placeholder
(`#Norelease-download`), leaving old status text visible; update the successful
path to also hide or toggle off the no-release element (the DOM node with id or
selector for the "no release" placeholder) immediately after setting
document.querySelector('.release-repo').href and
document.querySelector('.release-badge').src (i.e., alongside the existing
toggle('release-download') call), and apply the same change in the duplicate
handling near lines 138–139 so the no-release placeholder is always hidden on
success.
|
I haven't validated the code itself, but: thank you very much for putting this together!! This is finally the payoff for what we've been building towards with all the new metadata tagging. |
@netmindz small improvement: maybe find a different name for the second label ("Release:"), like "Variant:" or "Board Type:". |
I've swapped the second to "Latest version" as I'm reluctant to rename the release as it is what we refer to as Release name, as it's the file name of release, which yes had mention of the esp32 variant, but also things like memory type and options features like the ethernet |
3 participants
This pull request enhances the WLED software update page by introducing an "Auto-update from GitHub" feature. This allows users to automatically fetch and install the latest compatible firmware release directly from GitHub, streamlining the update process and providing real-time status feedback.
New auto-update functionality:
autoUpdateasync function that:Supporting changes:
/json/infoin a globaldeviceInfovariable for use in the auto-update logic. [1] [2]Summary by CodeRabbit
New Features
UI Improvements