Make version information consistent across update interfaces #4846
Conversation
WalkthroughReplaced UI text to show "Installed version: WLED ##VERSION##" in two HTML pages. Added C++ helper Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
wled00/xml.cpp (3)
40-55: Prefer HTML entities over JS string escaping for quotes.Minor readability nit: instead of embedding escaped quotes (\"%s\"), render quotes using HTML entities. This avoids reliance on JS string escaping semantics and is clearer to future maintainers.
Apply this diff:
- snprintf_P(buf,len,PSTR("WLED %s (%d)<br>\\\"%s\\\"<br>(Processor: %s)"), + snprintf_P(buf,len,PSTR("WLED %s (%d)<br>"%s"<br>(Processor: %s)"),
40-55: Consider HTML-escaping releaseString to prevent markup injection.releaseString is compile-time controlled, but it can still contain characters like <, >, &, or quotes. Since the string is inserted as innerHTML, escaping releaseString reduces the risk of unintended markup rendering.
If you prefer, I can provide a lightweight helper that escapes &, <, >, " before formatting the version string.
40-55: ESP8266 vs ESP8285 identification.For ESP8266 builds that run on ESP8285, the processor label will still read "ESP8266". If you want to be precise, you could detect 8285 specifically (optional).
Would you like a small helper that detects 8285 on ESP8266 cores?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
wled00/data/settings_sec.htm(1 hunks)wled00/data/update.htm(1 hunks)wled00/xml.cpp(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
🔇 Additional comments (6)
wled00/data/settings_sec.htm (1)
79-79: Consistent installed version display added (matches OTA page).Good addition. The static placeholder degrades gracefully if JS injection fails, and the shared "sip" target enables consistent dynamic content via getSettingsJS.
wled00/data/update.htm (1)
30-30: Aligned the installed version label with Security & Updates.Looks good. Keeping the static "WLED ##VERSION##" ensures a fallback before JS populates the detailed string.
wled00/xml.cpp (4)
40-55: New helper centralizes detailed version formatting across pages.Nice encapsulation. Using snprintf_P prevents overflow and the function cleanly supports both ESP32 and ESP8266 variants.
91-92: NFC: Brace style change.Moving the opening brace to a new line is fine and consistent with nearby code style.
614-616: Index 0 confirmed as the only “sip” element
Verified a single<span class="sip">in wled00/data/settings_sec.htm, so targeting index 0 is correct. Approving code changes.
676-677: Verification of version formatter consistency
- Confirmed exactly one
class="sip"occurrence inwled00/data/update.htm(line 30).- Unable to locate the
printSetClassElementHTMLdefinition or anyinnerHTMLusage inwled00/xml.cpp. Please manually verify that this function safely escapestmp_bufwhen injecting it into the page (viainnerHTMLor equivalent).Updated quick checks for your convenience:
rg -nP 'class\s*=\s*["\']sip["\']' -n -C2 wled00/data/update.htm rg -nR 'printSetClassElementHTML' -n -C3 wled00/xml.cpp
|
Regarding coderabbit's nitpicks about HTML escaping: I certainly wish that |
The duplication of logic and the formatting differences between the "OTA Updates" and "Security & Updates" pages made it very difficult to find the exact version details. With this change, both update-pages now share the same consistent and detailed formatting, making it easy for users to identify which exact version and binary of WLED they've installed. The version format has also been improved to make it much easier to understand.
Arcitec
force-pushed
the
improve-version-info
branch
from
1364c2f to
7865985
Compare
2 participants
The duplication of logic and the formatting differences between the "OTA Updates" and "Security & Updates" pages made it very difficult to find the exact version details.
With this change, both update-pages now share the same consistent and detailed formatting, making it easy for users to identify which exact version and binary of WLED they've installed.
The version format has also been improved to make it much easier to understand.
OTA Update Page (only reachable via Info -> OTA Update):
Before:
After:
Security & Updates Page:
Before (doesn't say anything useful):
After:
The new format is easier for users to understand, and only adds 10 characters. Most builds hover around 75 characters total, and I've verified that the 128 byte buffer would fit extremely long
WLED_RELEASE_NAMEnames. Much, much longer than any person would ever use. :)With these changes, it's finally easy to find the vital information about the installed build. I didn't even know that it was possible to find it in the UI until I started digging around in the code and found that it was hiding on the well-hidden OTA page, hehe. It makes sense to have it on the Security & Updates page too.
I also fixed the braces of two functions that weren't consistent with the coding style of the rest of that file.
Summary by CodeRabbit