Revert "Add old version check to OTA update"

[willmmiles]

Reverts #5057

This merge breaks OTA updates from older nightlies.

Summary by CodeRabbit

• Bug Fixes

  • Implemented stricter firmware compatibility validation during OTA updates, ensuring firmware versions meet precise compatibility requirements.
  • Updated error messages to provide clearer feedback when firmware compatibility issues are encountered.

• Chores

  • Simplified firmware validation logic by removing legacy compatibility checks.

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

[coderabbitai]

Walkthrough

This PR simplifies firmware metadata validation by removing the safe-update version checking mechanism. Changes include reverting the metadata version constant, eliminating platform-specific build description initialization, removing the safe_update_version struct field, and updating version validation logic to require exact descriptor version matching instead of greater-than comparisons.

Changes

Cohort / File(s)	Summary
Metadata struct definition wled00/wled_metadata.h	Removed uint8_t safe_update_version[3] field from wled_metadata_t struct, reducing struct size and altering layout.
Metadata implementation logic wled00/wled_metadata.cpp	Downgraded WLED_CUSTOM_DESC_VERSION from 2 to 1; removed platform-conditional initialization for WLED_BUILD_DESCRIPTION; changed version validation from greater-than (>) to exact-match (!=); removed safe-update version parsing and validation block; updated OTA error message from "Firmware release name mismatch" to "Firmware compatibility mismatch."

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring attention:

• Verify implications of changing version check from > to != logic and ensure backward compatibility expectations are met
• Confirm struct layout and alignment changes from field removal do not affect binary metadata serialization or deserialization
• Validate that all references to safe_update_version have been removed throughout the codebase
• Review the intended behavior shift from version range checking to strict equality matching

Possibly related PRs

• Add old version check to OTA update #5057: Directly related PR that introduced the safe-update version field, version bump to 2, and safe-update enforcement logic that this PR removes

Suggested reviewers

• netmindz

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: reverting a previous commit that added old version checks to OTA updates, which aligns with the file modifications removing version checks and validation logic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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 revert-5057-update-source-version-check

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fdb85d8 and d67d8f5.

📒 Files selected for processing (2)

• wled00/wled_metadata.cpp (3 hunks)
• wled00/wled_metadata.h (0 hunks)

💤 Files with no reviewable changes (1)

• wled00/wled_metadata.h

🧰 Additional context used

📓 Path-based instructions (1)

wled00/**/*.cpp

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use 2-space indentation for C++ source files (.cpp)

Files:

• wled00/wled_metadata.cpp

🧠 Learnings (15)

📓 Common learnings

Learnt from: willmmiles
Repo: wled/WLED PR: 4998
File: wled00/ota_update.cpp:120-124
Timestamp: 2025-10-11T17:56:24.923Z
Learning: In the WLED OTA update implementation (wled00/ota_update.cpp), the parameter name for bypassing OTA validation is "skipValidation", not "ignoreRelease". The intent evolved from the original spec in issue #4929 to disable all validation checks, not just release name checks, which is why the broader parameter name "skipValidation" is used.

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.

📚 Learning: 2025-10-11T17:56:24.923Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4998
File: wled00/ota_update.cpp:120-124
Timestamp: 2025-10-11T17:56:24.923Z
Learning: In the WLED OTA update implementation (wled00/ota_update.cpp), the parameter name for bypassing OTA validation is "skipValidation", not "ignoreRelease". The intent evolved from the original spec in issue #4929 to disable all validation checks, not just release name checks, which is why the broader parameter name "skipValidation" is used.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-11-14T13:37:11.994Z

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-11-14T13:37:30.955Z

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-02-19T12:43:34.199Z

Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-11-20T00:04:04.829Z

Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-08-28T08:09:20.630Z

Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-08-26T11:51:21.817Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-10-10T18:34:06.550Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-04-26T19:19:07.600Z

Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-05-09T18:43:15.355Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-11-16T19:40:46.260Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-02-19T12:43:34.200Z

Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-09-16T18:08:42.848Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.

Applied to files:

• wled00/wled_metadata.cpp

📚 Learning: 2025-08-21T00:00:17.196Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.

Applied to files:

• wled00/wled_metadata.cpp

⏰ 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). (20)

• GitHub Check: wled_build / Build Environments (esp01_1m_full)
• GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
• GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
• GitHub Check: wled_build / Build Environments (esp32c3dev)
• GitHub Check: wled_build / Build Environments (esp32_eth)
• GitHub Check: wled_build / Build Environments (esp32_wrover)
• GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
• GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
• GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
• GitHub Check: wled_build / Build Environments (lolin_s2_mini)
• GitHub Check: wled_build / Build Environments (usermods)
• GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
• GitHub Check: wled_build / Build Environments (esp32dev_debug)
• GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
• GitHub Check: wled_build / Build Environments (esp8266_2m)
• GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
• GitHub Check: wled_build / Build Environments (nodemcuv2)
• GitHub Check: Analyze (c-cpp)
• GitHub Check: wled_build / Build Environments (nodemcuv2)
• GitHub Check: wled_build / Build Environments (usermods)

🔇 Additional comments (3)

wled00/wled_metadata.cpp (3)

99-103: Simplified validation is appropriate for this revert.

The change from > (allowing older or equal versions) to != (exact match only) makes the validation more restrictive but simpler and less error-prone. This approach eliminates the complex safe-update version logic that was causing issues with older nightlies.

The exact matching strategy trades flexibility for reliability, which is appropriate given the issues encountered in PR #5057. Future enhancements to support version compatibility can be addressed separately with proper testing across nightly builds.

---

154-156: Improved error message clarity.

The updated error message "Firmware compatibility mismatch" is more accurate and user-friendly than "Firmware release name mismatch", as it better reflects the scope of validation being performed.

---

19-19: No backward compatibility concerns with this revert.

The descriptor version 2 introduced in PR #5057 was reverted the same day before being included in any official build. No nightly or release builds were published with version 2, so there are no deployed instances requiring migration. The revert to version 1 safely restores the original validation logic without affecting any users.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}