Skip to content

Stream /json/effects via respondModeData(namesOnly=true), eliminating JSON buffer lock #5519

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

Closed
Copilot wants to merge 5 commits into from
Closed

Stream /json/effects via respondModeData(namesOnly=true), eliminating JSON buffer lock #5519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5497f7e
Stream /json/effects via respondModeData(namesOnly=true), eliminating…
Copilot Apr 19, 2026
b7c863a
Add clarifying comments per code review feedback
Copilot Apr 19, 2026
ca7f8d4
Chunk-process 2d-gaps.json in setUpMatrix(), eliminating JSON buffer …
Copilot Apr 19, 2026
028902e
Improve 2d-gaps.json end-of-array handling and align with deserialize…
Copilot Apr 19, 2026
c8087e0
Restore gap-value format description comment (lines 81-84)
Copilot Apr 19, 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
43 changes: 32 additions & 11 deletions wled00/FX_2Dfcn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,29 +71,50 @@ 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)) {
gapTable = static_cast<int8_t*>(p_malloc(matrixSize));
if (gapTable) {
// 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 entries directly from the file, one number at a time
// (no JSON buffer / pDoc needed — the file is a plain JSON array)
// follows the same parsing pattern used by deserializeMap() for ledmap.json
size_t gapIdx = 0;
File f = WLED_FS.open(fileName, "r");
if (f) {
f.find('['); // skip to start of array
while (f.available() && gapIdx < matrixSize) {
char number[8];
size_t numRead = f.readBytesUntil(',', number, sizeof(number) - 1); // last entry reads up to ']' (no trailing comma)
number[numRead] = 0;
if (numRead > 0) {
char *end = strchr(number, ']'); // check for end-of-array marker
bool foundDigit = (end == nullptr); // no ']' means the whole token is a number
if (end != nullptr) {
// ']' present — only accept if a digit (or '-') appears before it
for (int k = 0; &number[k] != end; k++) {
if (number[k] >= '0' && number[k] <= '9') { foundDigit = true; break; }
if (number[k] == '-') { foundDigit = true; break; }
}
}
if (!foundDigit) break; // ']' with no number — array ended
gapTable[gapIdx++] = constrain(atoi(number), -1, 1);
} else break;
Comment on lines +92 to +108
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use a larger token buffer to avoid corrupting valid whitespace-heavy JSON.

Line 93 only allows 7 bytes before the delimiter. readBytesUntil() will split a longer JSON token, so indentation/whitespace before -1, 0, or 1 can be parsed as an extra 0 entry and shift the rest of the gap table.

🐛 Proposed fix 
-              char number[8];
+              char number[32]; // match deserializeMap(); handles JSON whitespace around -1/0/1
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@wled00/FX_2Dfcn.cpp` around lines 92 - 108, The token buffer "number" used in
the loop that fills gapTable (where f.readBytesUntil(',', number, sizeof(number)
- 1) is called) is too small (7 bytes) and can split whitespace-heavy JSON
tokens; increase the buffer size (e.g., make number a larger array like 32+
bytes or use a std::string/dynamic buffer) and adjust the readBytesUntil call
accordingly, then trim leading/trailing whitespace from the token before
checking for ']' and before calling atoi/ constrain so whitespace does not
produce spurious zero entries for gapTable; keep the existing checks around
strchr(number, ']') and foundDigit logic but operate on the larger/trimmed
token.

}
f.close();
}
if (gapIdx < matrixSize) { // file was too short or could not be read — discard
p_free(gapTable);
gapTable = nullptr;
}
}
DEBUG_PRINTLN(F("Gaps loaded."));
releaseJSONBufferLock();
}

unsigned x, y, pix=0; //pixel
Expand Down
2 changes: 1 addition & 1 deletion wled00/const.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,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
// JSON_LOCK 20 formerly used for LEDGAP (now parsed without JSON buffer)
#define JSON_LOCK_LEDMAP_ENUM 21
#define JSON_LOCK_REMOTE 22

Expand Down
35 changes: 21 additions & 14 deletions wled00/json.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,23 +1213,32 @@ static size_t writeJSONStringElement(uint8_t* dest, size_t maxLen, const char* s
return 1 + n;
}

// Generate a streamed JSON response for the mode data
// This uses sendChunked to send the reply in blocks based on how much fit in the outbound
// packet buffer, minimizing the required state (ie. just the next index to send). This
// allows us to send an arbitrarily large response without using any significant amount of
// memory (so no worries about buffer limits).
void respondModeData(AsyncWebServerRequest* request) {
// Generate a streamed JSON response for the mode data (namesOnly=false) or mode names
// (namesOnly=true). This uses sendChunked to send the reply in blocks based on how much
// fit in the outbound packet buffer, minimizing the required state (ie. just the next index
// to send). This allows us to send an arbitrarily large response without using any
// significant amount of memory (so no worries about buffer limits).
void respondModeData(AsyncWebServerRequest* request, bool namesOnly = false) {
size_t fx_index = 0;
request->sendChunked(FPSTR(CONTENT_TYPE_JSON),
[fx_index](uint8_t* data, size_t len, size_t) mutable {
[fx_index, namesOnly](uint8_t* data, size_t len, size_t) mutable {
size_t bytes_written = 0;
char lineBuffer[256];
while (fx_index < strip.getModeCount()) {
strncpy_P(lineBuffer, strip.getModeData(fx_index), sizeof(lineBuffer)-1); // Copy to stack buffer for strchr
if (lineBuffer[0] != 0) {
lineBuffer[sizeof(lineBuffer)-1] = '\0'; // terminate string (only needed if strncpy filled the buffer)
const char* dataPtr = strchr(lineBuffer,'@'); // Find '@', if there is one
size_t mode_bytes = writeJSONStringElement(data, len, dataPtr ? dataPtr + 1 : "");
char* dataPtr = strchr(lineBuffer,'@'); // Find '@', if there is one; non-const so namesOnly mode can truncate lineBuffer here
const char* value;
// namesOnly=true → emit the display name (everything before '@')
// namesOnly=false → emit the fx-data string (everything after '@')
if (namesOnly) {
if (dataPtr) *dataPtr = '\0'; // truncate at '@' to get name only
value = lineBuffer;
} else {
value = dataPtr ? dataPtr + 1 : ""; // everything after '@' is the fx data
}
size_t mode_bytes = writeJSONStringElement(data, len, value);
if (mode_bytes == 0) break; // didn't fit; break loop and try again next packet
if (fx_index == 0) *data = '[';
Comment on lines 1227 to 1243
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Track the first emitted element, not fx_index, when opening the array.

Line 1229 intentionally skips empty mode descriptors, but Line 1243 only replaces the leading comma with [ when the skipped index is 0. If index 0 is empty, the response starts with ,; if all entries are empty, it returns just ].

🐛 Proposed fix 
 void respondModeData(AsyncWebServerRequest* request, bool namesOnly = false) {
   size_t fx_index = 0;
+  bool arrayStarted = false;
+  bool wroteElement = false;
   request->sendChunked(FPSTR(CONTENT_TYPE_JSON),
-    [fx_index, namesOnly](uint8_t* data, size_t len, size_t) mutable {
+    [fx_index, namesOnly, arrayStarted, wroteElement](uint8_t* data, size_t len, size_t) mutable {
       size_t bytes_written = 0;
       char lineBuffer[256];
+
+      if (!arrayStarted) {
+        if (len == 0) return 0;
+        *data++ = '[';
+        --len;
+        ++bytes_written;
+        arrayStarted = true;
+      }
+
       while (fx_index < strip.getModeCount()) {
         strncpy_P(lineBuffer, strip.getModeData(fx_index), sizeof(lineBuffer)-1); // Copy to stack buffer for strchr
         if (lineBuffer[0] != 0) {
           lineBuffer[sizeof(lineBuffer)-1] = '\0'; // terminate string (only needed if strncpy filled the buffer)
           char* dataPtr = strchr(lineBuffer,'@'); // Find '@', if there is one; non-const so namesOnly mode can truncate lineBuffer here
@@
           } else {
             value = dataPtr ? dataPtr + 1 : ""; // everything after '@' is the fx data
           }
-          size_t mode_bytes = writeJSONStringElement(data, len, value);
+          size_t mode_bytes = wroteElement ? writeJSONStringElement(data, len, value) : writeJSONString(data, len, value);
           if (mode_bytes == 0) break;  // didn't fit; break loop and try again next packet
-          if (fx_index == 0) *data = '[';
           data += mode_bytes;
           len -= mode_bytes;
           bytes_written += mode_bytes;
+          wroteElement = true;
         }
         ++fx_index;        
       }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@wled00/json.cpp` around lines 1227 - 1243, The code currently uses fx_index
to decide when to replace the leading comma with '[' (if fx_index == 0) which
fails when early fx entries are empty; change to track whether any element has
been emitted (e.g., an emitted_count or firstEmitted flag) and use that to
decide array-opening behavior: after a successful writeJSONStringElement
(mode_bytes > 0) if no element has been emitted yet write '[' instead of ',' and
mark emitted; at the end, if no elements were emitted emit "[]" or handle
closing ']' accordingly; update references around fx_index,
writeJSONStringElement, and data pointer logic to use this new emitted tracker.

data += mode_bytes;
Expand Down Expand Up @@ -1276,7 +1285,7 @@ class LockedJsonResponse: public AsyncJsonResponse {
void serveJson(AsyncWebServerRequest* request)
{
enum class json_target {
all, state, info, state_info, nodes, effects, palettes, networks, config, pins
all, state, info, state_info, nodes, palettes, networks, config, pins
};
json_target subJson = json_target::all;

Expand All @@ -1285,7 +1294,7 @@ void serveJson(AsyncWebServerRequest* request)
else if (url.indexOf("info") > 0) subJson = json_target::info;
else if (url.indexOf("si") > 0) subJson = json_target::state_info;
else if (url.indexOf(F("nodes")) > 0) subJson = json_target::nodes;
else if (url.indexOf(F("eff")) > 0) subJson = json_target::effects;
else if (url.indexOf(F("eff")) > 0) { respondModeData(request, true); return; }
else if (url.indexOf(F("palx")) > 0) subJson = json_target::palettes;
else if (url.indexOf(F("fxda")) > 0) { respondModeData(request); return; }
else if (url.indexOf(F("net")) > 0) subJson = json_target::networks;
Expand All @@ -1312,7 +1321,7 @@ void serveJson(AsyncWebServerRequest* request)
}
// releaseJSONBufferLock() will be called when "response" is destroyed (from AsyncWebServer)
// make sure you delete "response" if no "request->send(response);" is made
LockedJsonResponse *response = new LockedJsonResponse(pDoc, subJson==json_target::effects); // will clear and convert JsonDocument into JsonArray if necessary
LockedJsonResponse *response = new LockedJsonResponse(pDoc, false); // will clear JsonDocument

JsonVariant lDoc = response->getRoot();

Expand All @@ -1326,8 +1335,6 @@ void serveJson(AsyncWebServerRequest* request)
serializeNodes(lDoc); break;
case json_target::palettes:
serializePalettes(lDoc, request->hasParam(F("page")) ? request->getParam(F("page"))->value().toInt() : 0); break;
case json_target::effects:
serializeModeNames(lDoc); break;
case json_target::networks:
serializeNetworks(lDoc); break;
case json_target::config:
Expand Down
Loading