Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 19 additions & 15 deletions wled00/FX_2Dfcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,29 +71,33 @@ void WS2812FX::setUpMatrix() {
// allowed values are: -1 (missing pixel/no LED attached), 0 (inactive/unused pixel), 1 (active/used pixel)
char fileName[32]; strcpy_P(fileName, PSTR("/2d-gaps.json"));
bool isFile = WLED_FS.exists(fileName);
size_t gapSize = 0;
int8_t *gapTable = nullptr;

if (isFile && requestJSONBufferLock(JSON_LOCK_LEDGAP)) {
if (isFile) {
DEBUG_PRINT(F("Reading LED gap from "));
DEBUG_PRINTLN(fileName);
// read the array into global JSON buffer
if (readObjectFromFile(fileName, nullptr, pDoc)) {
// the array is similar to ledmap, except it has only 3 values:
// -1 ... missing pixel (do not increase pixel count)
// 0 ... inactive pixel (it does count, but should be mapped out (-1))
// 1 ... active pixel (it will count and will be mapped)
JsonArray map = pDoc->as<JsonArray>();
gapSize = map.size();
if (!map.isNull() && gapSize >= matrixSize) { // not an empty map
gapTable = static_cast<int8_t*>(p_malloc(gapSize));
if (gapTable) for (size_t i = 0; i < gapSize; i++) {
gapTable[i] = constrain(map[i], -1, 1);
// read the gap array directly from the file, a file with fewer entries than matrix positions is ignored
// the array is similar to ledmap, except it has only 3 values:
// -1 ... missing pixel (do not increase pixel count)
// 0 ... inactive pixel (it does count, but should be mapped out (-1))
// 1 ... active pixel (it will count and will be mapped)
File f = WLED_FS.open(fileName, "r");
if (f) {
gapTable = static_cast<int8_t*>(p_malloc(matrixSize));
if (gapTable) {
int value;
unsigned count = 0;
while (count < matrixSize && readNextIntFromFile(f, value)) {
gapTable[count++] = (int8_t)constrain(value, -1, 1);
}
if (count < matrixSize) { // incomplete gap file, ignore it so below loop does not read OOB
p_free(gapTable);
gapTable = nullptr;
}
}
f.close();
}
DEBUG_PRINTLN(F("Gaps loaded."));
releaseJSONBufferLock();
}

unsigned x, y, pix=0; //pixel
Expand Down
135 changes: 94 additions & 41 deletions wled00/FX_fcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1913,6 +1913,7 @@ uint8_t WS2812FX::getActiveSegmentsNum() const {
uint16_t WS2812FX::getLengthTotal() const {
unsigned len = Segment::maxWidth * Segment::maxHeight; // will be _length for 1D (see finalizeInit()) but should cover whole matrix for 2D
if (isMatrix && _length > len) len = _length; // for 2D with trailing strip
if (isMatrix && customMappingSize > len) len = customMappingSize; // sparse matrix ledmap with gaps and trailing strip (see deserializeMap())
return len;
}

Expand Down Expand Up @@ -2058,9 +2059,9 @@ void WS2812FX::fixInvalidSegments() {
if (isMatrix) {
#ifndef WLED_DISABLE_2D
if (_segments[i].start >= Segment::maxWidth * Segment::maxHeight) {
// 1D segment at the end of matrix
if (_segments[i].start >= _length || _segments[i].startY > 0 || _segments[i].stopY > 1) { _segments.erase(_segments.begin()+i); continue; }
if (_segments[i].stop > _length) _segments[i].stop = _length;
// 1D segment at the end of matrix (trailing strip; logical length may exceed physical _length for sparse matrix ledmaps)
if (_segments[i].start >= getLengthTotal() || _segments[i].startY > 0 || _segments[i].stopY > 1) { _segments.erase(_segments.begin()+i); continue; }
if (_segments[i].stop > getLengthTotal()) _segments[i].stop = getLengthTotal();
continue;
}
if (_segments[i].start >= Segment::maxWidth || _segments[i].startY >= Segment::maxHeight) { _segments.erase(_segments.begin()+i); continue; }
Expand Down Expand Up @@ -2156,58 +2157,110 @@ bool WS2812FX::deserializeMap(unsigned n) {
isMatrix = true;
DEBUG_PRINTF_P(PSTR("LED map width=%d, height=%d\n"), Segment::maxWidth, Segment::maxHeight);
}
releaseJSONBufferLock();

d_free(customMappingTable);
customMappingTable = static_cast<uint16_t*>(d_malloc(sizeof(uint16_t)*getLengthTotal())); // prefer DRAM for speed
customMappingTable = nullptr;

if (isMatrix) {
// 2D set-up: read the file twice: first pass counts valid pixel entries (numPhy)
// then allocate matrixSize + trailingCount and fill it on the second pass including trailing pixels
// if entries are missing, they are appended (fallback)
const unsigned matrixSize = Segment::maxWidth * Segment::maxHeight;

if (customMappingTable) {
DEBUG_PRINTF_P(PSTR("ledmap allocated: %uB\n"), sizeof(uint16_t)*getLengthTotal());
// count entries and physical pixels used in the matrix, any left-over physical pixels are trailing pixels
unsigned entries = 0;
unsigned numPhy = 0;
File f = WLED_FS.open(fileName, "r");
f.find("\"map\":[");
while (f.available()) { // f.position() < f.size() - 1
char number[32];
size_t numRead = f.readBytesUntil(',', number, sizeof(number)-1); // read a single number (may include array terminating "]" but not number separator ',')
number[numRead] = 0;
if (numRead > 0) {
char *end = strchr(number,']'); // we encountered end of array so stop processing if no digit found
bool foundDigit = (end == nullptr);
int i = 0;
if (end != nullptr) do {
if (number[i] >= '0' && number[i] <= '9') foundDigit = true;
if (foundDigit || &number[i++] == end) break;
} while (i < 32);
if (!foundDigit) break;
int index = atoi(number);
if (index < 0 || index > 65535) index = 0xFFFF; // prevent integer wrap around
customMappingTable[customMappingSize++] = index;
if (end != nullptr) break; // array closing ']' was in this chunk; stop before atoi() coerces trailing JSON keys into bogus entries
if (customMappingSize >= getLengthTotal()) break;
} else break; // there was nothing to read, stop
if (f && f.find("\"map\"")) {
int value;
while (entries++ < matrixSize && readNextIntFromFile(f, value)) {
if (value >= 0 && value < (int)_length) numPhy++; // valid physical pixel entry
}
f.seek(0); // go back to the start of the file (closing and re-opening is slow)
}
// we now know the max physical pixel used in the map, check if we have unmapped pixels left (_length is total physical)
if (entries > 0) {
const unsigned trailingCount = (_length > numPhy) ? _length - numPhy : 0;
const unsigned mapSize = matrixSize + trailingCount;
customMappingTable = static_cast<uint16_t*>(d_malloc(sizeof(uint16_t) * mapSize)); // prefer DRAM for speed

if (customMappingTable) {
DEBUG_PRINTF_P(PSTR("ledmap allocated: %uB\n"), sizeof(uint16_t) * mapSize);
memset(customMappingTable, 0xFF, sizeof(uint16_t) * mapSize); // pre-fill with "-1" i.e. unmapped pixel

// second pass: fill matrix entries from file
numPhy = 0; // reset
unsigned mapindex = 0;
if (f && f.find("\"map\"")) { // advance to "map", readNextIntFromFile discards any chars up to the first number
int value;
while (mapindex < mapSize && readNextIntFromFile(f, value)) {
if (value < 0 || value >= _length) value = 0xFFFF; // set out of range mappings to unused
customMappingTable[mapindex++] = (uint16_t)value;
//if (value < 0xFFFF) numPhy++; // count valid physical pixel entries (needed for auto-trailing only, see below)
}
}

// TODO: this is a design choice: leave unmapped pixels black or append them as a strip?
// append any pixels missing in the LEDmap at the end in ascending order
// very simple walk-through search, users should map all pixels, this is a fallback
/*
if (numPhy < _length) {
for (unsigned p = 0; p < _length; p++) {
bool used = false;
// go through the whole map and check if this pixel index is not yet mapped
for (unsigned i = 0; i < mapSize; i++) {
if (customMappingTable[i] == p) { used = true; break; }
}
if (!used) customMappingTable[mapindex++] = (uint16_t)p; // append the unmapped pixel
if (mapindex >= mapSize) break; // safety check, should not happen
}
}
*/
customMappingSize = mapSize;
currentLedmap = n;
} else {
DEBUG_PRINTLN(F("ERROR LED map allocation error."));
}
}
f.close(); // all done, close the file
} else {
// 1D set-up: allocate strip length and fill with entries from file
// partial maps leave indices beyond customMappingSize unmapped (-1) TODO: see note above about appending unmapped pixels
const unsigned mapSize = getLengthTotal();
customMappingTable = static_cast<uint16_t*>(d_malloc(sizeof(uint16_t) * mapSize)); // prefer DRAM for speed

if (customMappingTable) {
memset(customMappingTable, 0xFF, sizeof(uint16_t) * mapSize); // pre-fill with "-1" i.e. unmapped pixel
DEBUG_PRINTF_P(PSTR("ledmap allocated: %uB\n"), sizeof(uint16_t)*mapSize);
File f = WLED_FS.open(fileName, "r");
if (f && f.find("\"map\":[")) {
int value;
unsigned mapindex = 0;
while (mapindex < mapSize && readNextIntFromFile(f, value)) {
if (value < 0 || value >= _length) value = 0xFFFF; // prevent integer wrap around
customMappingTable[mapindex++] = (uint16_t)value;
}
customMappingSize = mapSize;
currentLedmap = n;
f.close();
}
} else {
DEBUG_PRINTLN(F("ERROR LED map allocation error."));
}
currentLedmap = n;
f.close();
}
Comment on lines +2227 to +2251

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the 2D branch's find() target and close() placement for comparison
rg -n 'f\.find\("\\"map' wled00/FX_fcn.cpp
rg -n 'f\.close\(\)' wled00/FX_fcn.cpp

Repository: wled/WLED

Length of output: 427


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the relevant sections around both branches.
sed -n '2160,2255p' wled00/FX_fcn.cpp

printf '\n--- search for File/FS close patterns ---\n'
rg -n 'File f = WLED_FS\.open|\.close\(\); // all done, close the file|if \(f && f\.find\("\\"map"\\"\)\)' wled00 -g'*.cpp' -g'*.h'

printf '\n--- look for documentation about File destructor / close semantics ---\n'
rg -n 'destructor|close\(\)|File' docs wled00 -g'*.md' -g'*.cpp' -g'*.h' | sed -n '1,120p'

Repository: wled/WLED

Length of output: 15449


🌐 Web query:

ESP32 Arduino FS File destructor close semantics File destructor closes handle automatically

💡 Result:

In the ESP32 Arduino framework, the fs::File class uses reference counting to manage its underlying file handle [1]. When a File object goes out of scope, its internal shared pointer (_p) is destroyed [1][2]. The underlying implementation class (e.g., VFSFileImpl or similar) is designed to release its resources—including closing the file handle—when its own destructor runs [1][3]. Consequently, when the last File object referencing a specific handle is destroyed, the file is automatically closed [1]. While this automatic mechanism exists, it is widely considered best practice in embedded development to explicitly call File::close() when you are finished with a file [1][4]. Explicitly calling close() ensures that: 1. Any buffered data is immediately flushed to the storage medium. 2. The file handle and associated memory are released immediately, rather than waiting for the object to go out of scope or for the reference count to reach zero [4]. 3. You avoid potential issues related to delayed resource reclamation in memory-constrained environments [4]. In summary, while the destructor provides a safety mechanism for automatic closure [1], relying on it is not a substitute for explicit resource management via close() in robust application code [4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- occurrences of the ledmap JSON key in the repo ---\n'
rg -n '"map"\s*:|\\"map\\":\[' wled00 docs -g'*.cpp' -g'*.h' -g'*.json' -g'*.md'

printf '\n--- surrounding code where ledmap files are written/serialized ---\n'
rg -n 'customMappingTable|customMappingSize|currentLedmap|ledmap|map\]' wled00 -g'*.cpp' -g'*.h' | sed -n '1,160p'

Repository: wled/WLED

Length of output: 238


🌐 Web query:

Arduino FS File destructor close() automatically scope exit ESP32 ESP8266

💡 Result:

In the ESP32 and ESP8266 Arduino cores, the fs::File object is designed to manage its own lifecycle [1][2][3]. When a File object goes out of scope, its destructor is invoked, which eventually triggers the closure of the underlying file handle [1][2]. For the ESP32, the File class uses a reference-counted pointer to the underlying implementation class (FileImpl) [1]. When the File object is destroyed, the reference count decreases, and the implementation’s destructor handles the actual closing of the file [1][4]. For the ESP8266, the underlying implementation similarly ensures that files are closed when the File object is destroyed [2][3]. While official documentation explicitly lists close as a method to call [5], and doing so manually is considered good practice for predictable resource management, the architectural design of these cores ensures that files are not left permanently open (leaked) when the object goes out of scope [1][6][2]. In summary, while you are not strictly required to call close to prevent leaks, it remains a best practice to call close manually if you need to ensure the file is closed at a specific point in your code, such as before further processing the data or to immediately free memory resources [6][3][5].

Citations:


Relax the 1D ledmap token match f.find("\"map\":[") is stricter than the 2D path and skips valid pretty-printed ledmap files with whitespace around the array; use the same f.find("\"map\"") check here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wled00/FX_fcn.cpp` around lines 2227 - 2251, The 1D ledmap loader uses an
overly strict token match that rejects whitespace-formatted map arrays. In the
1D setup branch, update the f.find check guarding map parsing to search for the
same "\"map\"" token used by the 2D path, while leaving the subsequent integer
parsing unchanged.


#ifdef WLED_DEBUG
#ifdef WLED_DEBUG
if (customMappingSize) {
DEBUG_PRINT(F("Loaded ledmap:"));
for (unsigned i=0; i<customMappingSize; i++) {
if (!(i%Segment::maxWidth)) DEBUG_PRINTLN();
DEBUG_PRINTF_P(PSTR("%4d,"), customMappingTable[i] < 0xFFFFU ? customMappingTable[i] : -1);
}
DEBUG_PRINTLN();
#endif
/*
JsonArray map = root[F("map")];
if (!map.isNull() && map.size()) { // not an empty map
customMappingSize = min((unsigned)map.size(), (unsigned)getLengthTotal());
for (unsigned i=0; i<customMappingSize; i++) customMappingTable[i] = (uint16_t) (map[i]<0 ? 0xFFFFU : map[i]);
currentLedmap = n;
}
*/
} else {
DEBUG_PRINTLN(F("ERROR LED map allocation error."));
}
#endif

releaseJSONBufferLock();
if (strip.getLengthTotal() != lengthTotalBefore)
strip.updatePixelBuffer(); // allocate _pixels[] to match new length
return (customMappingSize > 0);
Expand Down
2 changes: 1 addition & 1 deletion wled00/const.h
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit");
#define JSON_LOCK_SERVEJSON 17
#define JSON_LOCK_NOTIFY 18
#define JSON_LOCK_PRESET_NAME 19
#define JSON_LOCK_LEDGAP 20
//#define JSON_LOCK_LEDGAP 20 // unused
#define JSON_LOCK_LEDMAP_ENUM 21
#define JSON_LOCK_REMOTE 22
#define JSON_LOCK_OTA 23
Expand Down
1 change: 1 addition & 0 deletions wled00/fcn_declare.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ inline bool writeObjectToFileUsingId(const String &file, uint16_t id, const Json
inline bool writeObjectToFile(const String &file, const char* key, const JsonDocument* content) { return writeObjectToFile(file.c_str(), key, content); };
inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFileUsingId(file.c_str(), id, dest); };
inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFile(file.c_str(), key, dest); };
bool readNextIntFromFile(File &f, int &value); // helper for reading ledmaps
bool copyFile(const char* src_path, const char* dst_path);
bool backupFile(const char* filename);
bool restoreFile(const char* filename);
Expand Down
43 changes: 43 additions & 0 deletions wled00/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,49 @@ bool readObjectFromFile(const char* file, const char* key, JsonDocument* dest, c
return true;
}

// helper to read comma-separated integers from a JSON array
// reads until it finds "-" or a number char, converts the number, reads until ',' (discarded) or ']' (kept for next call)
// returns false when the array terminator ']' or EOF is reached without a valid number
// sets value to -1 if not a number, discards garbage values following a valid number
bool readNextIntFromFile(File &f, int &value) {
value = 0;
bool foundDigit = false;
bool negative = false;
while (f.available()) {
char c = (char)f.peek();
if (c >= '0' && c <= '9') {
if (value < (0x7FFFFFFF / 10)) value = value * 10 + (c - '0'); // saturate instead of overflowing if too large
foundDigit = true;
} else if (c == '-' && !foundDigit) {
negative = true; // leading minus, return negative value of number
} else if (c == ',') {
f.read(); // consume separator comma
if (!foundDigit) {
value = -1; // invalid input, make it -1
return true; // not end of file yet
}
break; // number complete
} else if (c == ']') {
if (foundDigit) return true; // leave ']' available for the next call to terminate with "false"
f.read(); // consume array terminator (support multiple arrays in a file)
return false;
} else if (foundDigit) {
// number followed by a char/whitespace - malformed, skip everything up to the next ',' or ']'
while (f.available()) {
char d = (char)f.peek();
if (d == ',') { f.read(); return true; } // consume the ','
if (d == ']') { return true; } // leave ']' for the next call to terminate the array
f.read();
}
return true; // EOF but we have a number, let the next call return false
}
f.read(); // consume the peeked character
}
if (!foundDigit) return false;
if (negative) value = -value;
return true;
}

void updateFSInfo() {
#ifdef ARDUINO_ARCH_ESP32
#if WLED_FS == LITTLEFS || ESP_IDF_VERSION_MAJOR >= 4
Expand Down
Loading