Skip to content

Add option to use SafeBootloader #5426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wled-install wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add option to use SafeBootloader #5426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aed8221
Feature: Add SafeBoot compatibility
wled-develop Mar 15, 2026
d1045a3
Add SafeBoot compatibile partition table
wled-develop Mar 15, 2026
16ac74d
Use SHA for OTA instead of CRC
wled-develop Mar 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions tools/WLED_ESP32_8MB_SafeBoot.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Name, Type, SubType, Offset, Size, Flags
nvs, data, nvs, 0x9000, 0x5000,
otadata, data, ota, 0xe000, 0x2000,
app0, app, ota_0, 0x10000, 0x240000,
app1, app, ota_1, 0x250000, 0x240000,
spiffs, data, spiffs, 0x490000, 0x100000,
spiffs_backup, data, 0x40, 0x590000, 0x100000,
crc0, data, 0x41, 0x690000, 0x1000,
crc1, data, 0x42, 0x691000, 0x1000,
coredump, data, coredump,,64K
4 changes: 4 additions & 0 deletions wled00/cfg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "wled.h"
#include "wled_ethernet.h"
#include "safe_boot_functions.h"

/*
* Serializes and parses the cfg.json and wsec.json settings files, stored in internal FS.
Expand Down Expand Up @@ -832,6 +833,9 @@ void serializeConfigToFS() {
if (f) serializeJson(root, f);
f.close();
releaseJSONBufferLock();
#ifdef WLED_ENABLE_SAFE_BOOT
update_spiffs_crc();
#endif

configNeedsWrite = false;
}
Expand Down
7 changes: 7 additions & 0 deletions wled00/file.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "wled.h"
#include "safe_boot_functions.h"

/*
* Utility for SPIFFS filesystem
Expand Down Expand Up @@ -40,6 +41,9 @@ void closeFile() {
#endif
f.close(); // "if (f)" check is aleady done inside f.close(), and f cannot be nullptr -> no need for double checking before closing the file handle.
DEBUGFS_PRINTF("took %lu ms\n", millis() - s);
#ifdef WLED_ENABLE_SAFE_BOOT
update_spiffs_crc();
#endif
Comment on lines +44 to +46
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Critical performance concern: CRC calculation on every file close.

Calling update_spiffs_crc() after every closeFile() will cause significant performance degradation. Based on the implementation in wled00/safe_boot_functions.cpp:19-39, calc_crc() reads the entire SPIFFS partition in 4 KB chunks. For a typical 1-3 MB SPIFFS partition, this means reading megabytes of flash data on every single file close operation.

This will cause:

  • Visible LED stuttering during file operations
  • Slow preset saves and config writes
  • Potential watchdog timeouts on larger partitions

Consider updating the CRC only at strategic points (config save, preset save completion) rather than on every file close, or deferring CRC updates to idle time.

-  `#ifdef`  WLED_ENABLE_SAFE_BOOT
-    update_spiffs_crc();
-  `#endif`
   doCloseFile = false;
 }

The CRC updates in cfg.cpp, presets.cpp, and copyFile() should be sufficient for Safe Boot validation.

Copy link
Contributor Author

@wled-install wled-install Mar 15, 2026

Choose a reason for hiding this comment

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

Theoretically yes, practially it is fast enogh.

Copy link
Contributor

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

@wled-install, fair enough — thanks for testing this on actual hardware. The CRC read-and-compute still happens on every close even when no update is needed, but if you've verified it doesn't cause noticeable lag or watchdog issues in practice (likely because ESP32 flash reads at ~40 MHz are fast relative to the partition size), that's a reasonable call to make. Acknowledged!

🧠 Learnings used 
Learnt from: softhack007
Repo: wled/WLED PR: 5276
File: wled00/file.cpp:0-0
Timestamp: 2026-01-05T18:35:03.147Z
Learning: In WLED's file.cpp, the static File object `f` cannot be nullptr (it's not a pointer). The `if (f)` check only tests whether the file handle is open, and this check is already performed inside the `f.close()` function. Therefore, there is no need for an explicit `if (f)` check before calling `f.close()` - double-checking is redundant.

Learnt from: DedeHai
Repo: wled/WLED PR: 4956
File: wled00/wled_server.cpp:237-262
Timestamp: 2025-11-08T08:25:18.724Z
Learning: In WLED's filesystem code (ESP8266/ESP32), when iterating directory entries with `openNextFile()`, explicit `close()` calls between iterations are not required. The File class assignment operator properly handles closing the previous file handle when reassigning (e.g., `rootfile = rootdir.openNextFile()`). Both explicit and implicit patterns exist in the codebase, but relying on assignment operator cleanup is valid.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.

Learnt from: DedeHai
Repo: wled/WLED PR: 5404
File: wled00/wled_server.cpp:224-224
Timestamp: 2026-03-05T15:48:35.387Z
Learning: In WLED (wled00/wled_server.cpp), `updateFSInfo()` updates `fsBytesUsed`/`fsBytesTotal` which are purely informational ("FYI") display values for the user. No synchronization or locking is required around these calls, even when invoked from async request handlers, because: (1) the values do not need to be 100% accurate, and (2) simultaneous multi-file delete/add operations are extremely improbable on this embedded IoT platform.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-08T00:57:36.134Z
Learning: In WLED (wled00/cfg.cpp), `deserializeConfigFromFS()` only sets `needsSave=true` (triggering a write of cfg.json) when usermod settings are present in the JSON (lines 758-761). On a fresh install with no usermods, `needsSave=false` so `serializeConfigToFS()` is never called at first boot. Correct defaults are never written to cfg.json until the user manually saves. Structural fix: set `needsSave = needsSave || !cfgExists` where `cfgExists = WLED_FS.exists(s_cfg_json)` checked before reading the file.

Learnt from: DedeHai
Repo: wled/WLED PR: 4793
File: wled00/file.cpp:481-513
Timestamp: 2025-07-31T18:21:49.868Z
Learning: In WLED, essential configuration files that require backup have short, controlled names (like "/cfg.json", "/presets.json") that are well within a 32-character buffer limit. The file naming is controlled by developers, making buffer overflow in backup filename construction highly unlikely.

Learnt from: blazoncek
Repo: wled/WLED PR: 4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-08T00:57:36.134Z
Learning: In WLED (wled00/cfg.cpp), `deserializeConfig()` is called with an empty JsonObject when cfg.json doesn't exist on fresh install. Any value read without the ArduinoJSON `|` fallback operator or CJSON macro will override correct constructor defaults with 0/null. Known affected values: `strip.setTargetFps(hw_led["fps"])` at line 179 (sets FPS to 0/unlimited instead of WLED_FPS=42). Fix: `strip.setTargetFps(hw_led["fps"] | WLED_FPS)`. The gamma issue (gammaCorrectCol/Bri) had the same root cause and was fixed in commit d1d9dec402 (Jan 2026) using inline `| default` fallbacks.

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.cpp:385-387
Timestamp: 2026-02-08T19:33:02.648Z
Learning: In WLED ESP32 code, when checking whether USB CDC serial initialization delay is needed, rely on the ARDUINO_USB_CDC_ON_BOOT flag rather than explicit CONFIG_IDF_TARGET checks, because not all development boards for chips with native USB capability (ESP32-C3/C5/C6/S2/S3/H2/P4) actually use the native USB—many have external UART bridge chips instead. The ARDUINO_USB_CDC_ON_BOOT flag correctly indicates the board's actual USB configuration.

Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4893
File: wled00/set.cpp:95-95
Timestamp: 2026-03-14T20:56:46.543Z
Learning: Guideline: Ensure WiFi hostname is set after WiFi.mode() but before WiFi.begin() to avoid default esp-XXXXXX hostname being used in DHCP. This ordering only takes effect after the STA interface exists (so avoid placing hostname setting before initConnection). In WLED, place the hostname configuration inside initConnection() (after WiFi.disconnect(true) and before WiFi.begin()) rather than in earlier boot code like deserializeConfig(). This rule should be applied in code reviews for WLED’s network initialization paths in wled00/*.cpp, and note that on ESP8266 the ordering is less strict but still desirable for consistency.

Copy link
Member

@softhack007 softhack007 Mar 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Consider updating the CRC only at strategic points (config save, preset save completion) rather than on every file close, or deferring CRC updates to idle time.

@wled-install I still think this is a valid concern - especially when users have many leds, frequent flash file reads can lead to leds flickering, or effects slowing down randomly. Also there are boards with 16mb flash (9MB LittleFS) or even 32MB flash (24MB LittleFS) where it might actually cause a slow-down.

Not sure if you found all the file closing locations in the code - ledmaps/gapmaps, webserver up/download, and gif player do file access directly, without relying on file.cpp functions.

The first optimization could be to only update CRCs after file write closing, but not on file read closing.

Or - depending on how mission critical these checksums are - create a global flag crc_dirty and delay updating the checksums into the main loop, maybe only every 15 seconds.

Copy link
Contributor

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

doCloseFile = false;
}

Expand Down Expand Up @@ -478,6 +482,9 @@ bool copyFile(const char* src_path, const char* dst_path) {
DEBUG_PRINTLN(F("copy failed"));
WLED_FS.remove(dst_path); // delete incomplete file
}
#ifdef WLED_ENABLE_SAFE_BOOT
else {update_spiffs_crc();}
#endif
return success;
}

Expand Down
4 changes: 4 additions & 0 deletions wled00/presets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "wled.h"
#include "safe_boot_functions.h"

/*
* Methods to handle saving and loading presets to/from the filesystem
Expand Down Expand Up @@ -114,6 +115,9 @@ void initPresetsFile()
}
serializeJson(doc, f);
f.close();
#ifdef WLED_ENABLE_SAFE_BOOT
update_spiffs_crc();
#endif
}

bool applyPresetFromPlaylist(byte index)
Expand Down
86 changes: 86 additions & 0 deletions wled00/safe_boot_functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
This functions are used to use WLED together with "Safe Bootbootloader"
https://github.com/wled-install/SafeBootloader
*/
#ifdef WLED_ENABLE_SAFE_BOOT
#include "safe_boot_functions.h"
#include "wled.h"
#include <Arduino.h>

#define SPIFFS_CRC_MAGIC 0x53504653

typedef struct {
uint32_t magic;
uint32_t crc_spiffs; // CRC SPIFFS / Backup
uint32_t size_spiffs; // Größe SPIFFS / Backup
} crc_group_t;

uint32_t calc_crc(const esp_partition_t* part)
{
const size_t bufsize = 4096;
uint8_t buf[bufsize];

uint32_t crc = 0;
size_t offset = 0;

while (offset < part->size) {
size_t toread = bufsize;
if (offset + toread > part->size)
toread = part->size - offset;

esp_partition_read(part, offset, buf, toread);

crc = crc32_le(crc, buf, toread);
offset += toread;
}

return crc;
}

bool update_spiffs_crc()
{
const esp_partition_t* spiffs =
esp_partition_find_first(
ESP_PARTITION_TYPE_DATA,
ESP_PARTITION_SUBTYPE_DATA_SPIFFS,
"spiffs");

const esp_partition_t* crcpart =
esp_partition_find_first(
ESP_PARTITION_TYPE_DATA,
static_cast<esp_partition_subtype_t>(0x41), // crc subtype
"crc0");

if (!spiffs || !crcpart) {
DEBUG_PRINTLN(F("SPIFFS or CRC partition not found!"));
return false;
}


uint32_t crc = calc_crc(spiffs);

crc_group_t stored = {};
esp_partition_read(crcpart, 0, &stored, sizeof(stored));

// prüfen ob identisch
Copy link
Member

@softhack007 softhack007 Mar 16, 2026

Choose a reason for hiding this comment

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

minor: please translate german comments into english

if (stored.magic == SPIFFS_CRC_MAGIC &&
stored.crc_spiffs == crc &&
stored.size_spiffs == spiffs->size)
{
// nichts zu tun
Copy link
Member

@softhack007 softhack007 Mar 16, 2026

Choose a reason for hiding this comment

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

same minor comment here: german -> english please

DEBUG_PRINTLN(F("SPIFFS CRC matches stored value, no update needed."));
return true;
}

stored.magic = SPIFFS_CRC_MAGIC;
stored.crc_spiffs = crc;
stored.size_spiffs = spiffs->size;
DEBUG_PRINTLN(F("Updating SPIFFS CRC partition with new values."));
esp_partition_erase_range(crcpart, 0, 4096);
esp_partition_write(crcpart, 0, &stored, sizeof(stored));
DEBUG_PRINTLN(F("SPIFFS CRC partition updated."));

return true;
}
Comment on lines +12 to +84
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Indentation does not follow the repository C++ style rule.

Line 12 onward uses 4-space indentation in nested scopes; this repository guideline requires 2 spaces per level.

As per coding guidelines **/*.{cpp,h,c}: Use spaces (2 per level) for indentation in C++ files.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@wled00/safe_boot_functions.cpp` around lines 12 - 84, The code in
crc_group_t, calc_crc, and update_spiffs_crc uses 4-space indentation; reformat
these blocks to use 2 spaces per indentation level to match repository C++ style
(apply to the whole function bodies and any nested scopes, including the while
loop in calc_crc and the if/else blocks in update_spiffs_crc), keeping existing
line breaks and logic intact and ensuring alignment for statements like
esp_partition_find_first, esp_partition_read/write, esp_partition_erase_range
and the DEBUG_PRINTLN calls.


#endif
16 changes: 16 additions & 0 deletions wled00/safe_boot_functions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
This functions are used to use WLED together with "Safe Bootbootloader"
https://github.com/wled-install/SafeBootloader
*/

#pragma once

#ifdef WLED_ENABLE_SAFE_BOOT
#include <esp_partition.h>
#include <esp_spi_flash.h>
#include <rom/crc.h>
#include <esp_ota_ops.h>

bool update_spiffs_crc();

#endif
Loading