Conversation
| #ifndef WLED_DISABLE_WEBSOCKETS | ||
| #define WLED_ENABLE_WEBSOCKETS | ||
| #endif | ||
| #define WLED_ENABLE_OPC |
There was a problem hiding this comment.
You could add #ifndef WLED_DISABLE_OPC to allow complation without OPC. Similar as a line above.
|
Hey! This issue has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs. |
stale
Bot
added
the
stale
|
This still looks potentially interesting. While I don't think realtime over TCP makes enough sense to add this by default, an OPC usermod might be a great idea. |
Aircoookie
added
keep
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR introduces Open Pixel Control (OPC) protocol support to WLED. It adds a new realtime mode constant, an async TCP server accepting client connections on port 7890, packet parsing logic with circular buffer management, LED data extraction, and integration into the main WLED event loop. Changes
Sequence DiagramsequenceDiagram
participant Client as OPC Client
participant Server as AsyncServer
participant Buffer as Circular Buffer
participant Parser as handleOPCPacket
participant LEDs as LED Strip
Client->>Server: Connect on port 7890
activate Server
Server->>Server: Accept & register client
deactivate Server
loop Incoming Data
Client->>Server: Send OPC frame
Server->>Buffer: handleOPCData()<br/>(write to circular buffer)
activate Buffer
Buffer-->>Server: Data buffered
deactivate Buffer
end
loop Main Event Loop
Parser->>Buffer: Check for ≥4 byte header
activate Buffer
Buffer-->>Parser: Read header (cmd, channel, length)
deactivate Buffer
alt Complete Frame Available
Parser->>Parser: parseOPCLedValues()<br/>(extract RGB data)
Parser->>LEDs: Apply realtime LED values
LEDs->>LEDs: strip.show()
Parser->>Buffer: Consume frame bytes
else Incomplete Frame
Parser->>Parser: Wait for more data
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
wled00/opc.cpp (1)
135-143:⚠️ Potential issue | 🔴 CriticalStub function signatures don't match declarations.
When
WLED_ENABLE_OPCis not defined, the stub functions have mismatched signatures compared tofcn_declare.h. For example,handleOPCClient()is declared with(void* arg, AsyncClient* client)but stubbed asvoid handleOPCClient() {}. This causes linker errors.See the fix proposed in the
fcn_declare.hreview comment — either wrap the declarations in#ifdefor provide properly-signed stubs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/opc.cpp` around lines 135 - 143, The stub functions under the `#else` block currently have wrong signatures; update them to exactly match the declarations in fcn_declare.h (or conditionally compile those declarations) — specifically change initOPCServer, handleOPCClient, freeOPCClient, handleOPCData, handleOPCTimeOut, handleOPCError, and handleOPCDisconnect to the same parameter lists and return types as declared (e.g., handleOPCClient(void* arg, AsyncClient* client) etc.), or surround the related declarations in fcn_declare.h with `#ifdef` WLED_ENABLE_OPC so that the no-OP stubs and the declarations stay consistent.wled00/wled.h (1)
43-43:⚠️ Potential issue | 🟠 MajorOPC is unconditionally enabled — should be opt-in.
WLED_ENABLE_OPCis defined without any conditional guard, meaning OPC will be compiled into every build. Given the maintainer's feedback about OPC potentially being better suited as a usermod rather than a default feature, this should follow the pattern of other optional features likeWLED_ENABLE_DMX(line 35) which is commented out by default.Proposed fix: make OPC opt-in
-#define WLED_ENABLE_OPC +//#define WLED_ENABLE_OPC // uses ~Xkb (TCP server for Open Pixel Control protocol)Users who want OPC can then enable it via build flags (
-D WLED_ENABLE_OPC) or by uncommenting this line.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.h` at line 43, WLED_ENABLE_OPC is currently unconditionally defined causing OPC to be compiled into every build; make it opt-in by removing the unconditional definition and following the same pattern as WLED_ENABLE_DMX — either comment out the line defining WLED_ENABLE_OPC in wled.h so users must enable it manually or wrap it behind a guarded macro check so it is only active when passed via build flags (e.g., -D WLED_ENABLE_OPC); update any accompanying comment to instruct users to enable OPC via build flags or by uncommenting the define.
🧹 Nitpick comments (1)
wled00/opc.cpp (1)
114-133: Potential race: server accepts clients before buffer is ready.
opcServer->begin()(line 129) starts accepting connections immediately, butopcCircularBufPtris allocated on line 125. If a client connects and sends data extremely quickly, callbacks could fire while initialization is still in progress. Consider movingbegin()after all state is initialized, or add null guards in callbacks (as noted above).Also, use 2-space indentation per coding guidelines (tabs detected on lines 22-23, 26, 43, 49-54).
Suggested reordering
void initOPCServer() { if (opcServer != nullptr) { opcServer->end(); delete opcServer; opcServer = nullptr; } if (opcCircularBufPtr != nullptr) { opcCircularBufPtr->flush(); delete opcCircularBufPtr; opcCircularBufPtr = nullptr; } + + // Initialize all state before starting server opcCircularBufPtr = new cbuf(OPC_MAX_FRAME_STORAGE * (ledCount + OPC_HEADER_SIZE)); - opcServer = new AsyncServer(OPC_SERVER_PORT); - opcServer->onClient(&handleOPCClient, opcServer); - opcServer->setNoDelay(true); - opcServer->begin(); - opcDataLen = 0; opcParserWaitingForData = false; + + // Now start server + opcServer = new AsyncServer(OPC_SERVER_PORT); + opcServer->onClient(&handleOPCClient, opcServer); + opcServer->setNoDelay(true); + opcServer->begin(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/opc.cpp` around lines 114 - 133, The initOPCServer function starts accepting clients before all state is ready (opcServer->begin() is called before ensuring opcCircularBufPtr is fully initialized), which can race with handleOPCClient callbacks; fix by allocating and initializing opcCircularBufPtr and setting opcDataLen/opcParserWaitingForData and any other state first, then construct opcServer, register onClient (handleOPCClient, opcServer) and only call opcServer->begin() after all state is ready, and/or add defensive null checks inside handleOPCClient to guard against opcCircularBufPtr/opcServer being null; also reformat the function to use 2-space indentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/fcn_declare.h`:
- Around line 190-198: The OPC-related function declarations (initOPCServer,
freeOPCClient, handleOPCClient, handleOPCData, handleOPCTimeOut, handleOPCError,
handleOPCDisconnect, handleOPCPacket) must be wrapped with `#ifdef`
WLED_ENABLE_OPC ... `#endif` in fcn_declare.h so they are only declared when OPC
is enabled, and the stub implementations in opc.cpp must have matching
signatures (e.g., change empty-parameter stubs like void handleOPCClient() {} to
void handleOPCClient(void* arg, AsyncClient* client) {} and similarly for
handleOPCData, handleOPCTimeOut, handleOPCError, handleOPCDisconnect) so the
declarations and definitions match and avoid linker errors when WLED_ENABLE_OPC
is not defined.
In `@wled00/opc.cpp`:
- Around line 33-38: handleOPCData currently dereferences opcCircularBufPtr
without verifying it is valid; add a null check at the top of handleOPCData to
return immediately if opcCircularBufPtr is nullptr (or otherwise not
initialized) before calling opcCircularBufPtr->room() or ->write(), so the
callback cannot crash if invoked before initialization or after teardown; ensure
the early return covers both the room() size check and the write loop to avoid
any race where opcCircularBufPtr becomes null between checks.
- Around line 40-55: In handleOPCClient: do not call freeOPCClient(client) when
clientServed != nullptr — that closes the new client and corrupts the served
pointer; instead immediately reject the incoming connection by closing/stopping
the incoming AsyncClient (e.g., client->close(true) or client->stop()) and
return without changing clientServed; only assign clientServed = client and
initialize state (opcDataLen, opcParserWaitingForData) and register callbacks
when accepting a client; also guard the buffer flush call with a null check (if
(opcCircularBufPtr) opcCircularBufPtr->flush()) so opcCircularBufPtr->flush()
cannot crash when uninitialized.
- Around line 15-19: freeOPCClient deletes the global clientServed instead of
the passed-in AsyncClient* client causing undefined behavior; change the
function to consistently operate on the passed parameter by calling
client->close(true), deleting client, and only nulling clientServed if the
pointer being deleted equals clientServed (i.e., if (clientServed == client)
clientServed = nullptr), or alternatively make the function operate only on the
global by removing the parameter and using clientServed throughout — update
freeOPCClient, its signature/usages, and any callers to match the chosen
consistent approach.
- Around line 65-83: In parseOPCLedValues, replace the undeclared variable
ledCount with strip.getLengthTotal() in all uses (the loop bound, the later
comparison for discardInvalidData, and any other occurrence) so the code
compiles; keep the logic using opcDataLen/opcDataLenDiv,
opcCircularBufPtr->read(), setRealtimePixel(...), realtimeLock(...,
REALTIME_MODE_OPC) and strip.show() unchanged. After the replacement, ensure the
call to discardInvalidData(opcDataLen - strip.getLengthTotal() * 3) is safe by
confirming discardInvalidData correctly bounds/checks large or negative lengths
when opcDataLen is corrupted or malformed.
---
Duplicate comments:
In `@wled00/opc.cpp`:
- Around line 135-143: The stub functions under the `#else` block currently have
wrong signatures; update them to exactly match the declarations in fcn_declare.h
(or conditionally compile those declarations) — specifically change
initOPCServer, handleOPCClient, freeOPCClient, handleOPCData, handleOPCTimeOut,
handleOPCError, and handleOPCDisconnect to the same parameter lists and return
types as declared (e.g., handleOPCClient(void* arg, AsyncClient* client) etc.),
or surround the related declarations in fcn_declare.h with `#ifdef`
WLED_ENABLE_OPC so that the no-OP stubs and the declarations stay consistent.
In `@wled00/wled.h`:
- Line 43: WLED_ENABLE_OPC is currently unconditionally defined causing OPC to
be compiled into every build; make it opt-in by removing the unconditional
definition and following the same pattern as WLED_ENABLE_DMX — either comment
out the line defining WLED_ENABLE_OPC in wled.h so users must enable it manually
or wrap it behind a guarded macro check so it is only active when passed via
build flags (e.g., -D WLED_ENABLE_OPC); update any accompanying comment to
instruct users to enable OPC via build flags or by uncommenting the define.
---
Nitpick comments:
In `@wled00/opc.cpp`:
- Around line 114-133: The initOPCServer function starts accepting clients
before all state is ready (opcServer->begin() is called before ensuring
opcCircularBufPtr is fully initialized), which can race with handleOPCClient
callbacks; fix by allocating and initializing opcCircularBufPtr and setting
opcDataLen/opcParserWaitingForData and any other state first, then construct
opcServer, register onClient (handleOPCClient, opcServer) and only call
opcServer->begin() after all state is ready, and/or add defensive null checks
inside handleOPCClient to guard against opcCircularBufPtr/opcServer being null;
also reformat the function to use 2-space indentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 048ca7c2-8c48-4e46-9456-5588b9859218
📒 Files selected for processing (6)
wled00/const.hwled00/fcn_declare.hwled00/json.cppwled00/opc.cppwled00/wled.cppwled00/wled.h
| //opc.cpp | ||
| void initOPCServer(); | ||
| void freeOPCClient(AsyncClient* client); | ||
| void handleOPCClient(void* arg, AsyncClient* client); | ||
| void handleOPCData(void* arg, AsyncClient* client, void *data, size_t len); | ||
| void handleOPCTimeOut(void* arg, AsyncClient* client, uint32_t time); | ||
| void handleOPCError(void* arg, AsyncClient* client, int8_t error); | ||
| void handleOPCDisconnect(void* arg, AsyncClient* client); | ||
| void handleOPCPacket(); |
There was a problem hiding this comment.
Function declarations lack #ifdef WLED_ENABLE_OPC guard, and stub signatures mismatch.
The OPC function declarations here have full signatures (e.g., void handleOPCClient(void* arg, AsyncClient* client)), but the stubs in opc.cpp (lines 137-142) have empty parameter lists (e.g., void handleOPCClient() {}). This will cause linker errors when WLED_ENABLE_OPC is not defined.
Proposed fix: wrap declarations in ifdef and fix stub signatures
In fcn_declare.h:
//opc.cpp
+#ifdef WLED_ENABLE_OPC
void initOPCServer();
void freeOPCClient(AsyncClient* client);
void handleOPCClient(void* arg, AsyncClient* client);
void handleOPCData(void* arg, AsyncClient* client, void *data, size_t len);
void handleOPCTimeOut(void* arg, AsyncClient* client, uint32_t time);
void handleOPCError(void* arg, AsyncClient* client, int8_t error);
void handleOPCDisconnect(void* arg, AsyncClient* client);
-void handleOPCPacket();
+#endif
+void handleOPCPacket(); // Always declared, has stub when OPC disabledIn opc.cpp, fix stubs:
`#else`
void initOPCServer(){}
-void handleOPCClient() {}
-void freeOPCClient(){}
-void handleOPCData() {}
-void handleOPCTimeOut (){}
-void handleOPCError (){}
-void handleOPCDisconnect (){}
+void handleOPCPacket(){}
`#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/fcn_declare.h` around lines 190 - 198, The OPC-related function
declarations (initOPCServer, freeOPCClient, handleOPCClient, handleOPCData,
handleOPCTimeOut, handleOPCError, handleOPCDisconnect, handleOPCPacket) must be
wrapped with `#ifdef` WLED_ENABLE_OPC ... `#endif` in fcn_declare.h so they are only
declared when OPC is enabled, and the stub implementations in opc.cpp must have
matching signatures (e.g., change empty-parameter stubs like void
handleOPCClient() {} to void handleOPCClient(void* arg, AsyncClient* client) {}
and similarly for handleOPCData, handleOPCTimeOut, handleOPCError,
handleOPCDisconnect) so the declarations and definitions match and avoid linker
errors when WLED_ENABLE_OPC is not defined.
| void freeOPCClient(AsyncClient* client) { | ||
| client->close(true); | ||
| delete clientServed; | ||
| clientServed = nullptr; | ||
| } |
There was a problem hiding this comment.
Critical bug: freeOPCClient deletes wrong pointer.
The function receives client as a parameter but deletes clientServed (the global). This causes undefined behavior when client != clientServed. The function should consistently operate on either the parameter or the global, not mix them.
Proposed fix
void freeOPCClient(AsyncClient* client) {
- client->close(true);
- delete clientServed;
+ if (client == nullptr) return;
+ client->close(true);
+ client->free();
+ delete client;
+ if (client == clientServed) clientServed = nullptr;
- clientServed = nullptr;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/opc.cpp` around lines 15 - 19, freeOPCClient deletes the global
clientServed instead of the passed-in AsyncClient* client causing undefined
behavior; change the function to consistently operate on the passed parameter by
calling client->close(true), deleting client, and only nulling clientServed if
the pointer being deleted equals clientServed (i.e., if (clientServed == client)
clientServed = nullptr), or alternatively make the function operate only on the
global by removing the parameter and using clientServed throughout — update
freeOPCClient, its signature/usages, and any callers to match the chosen
consistent approach.
| void handleOPCData(void* arg, AsyncClient* client, void *data, size_t len) { | ||
| if (len > opcCircularBufPtr->room()) return; | ||
| for (unsigned int i = 0; i < len; i++) { | ||
| opcCircularBufPtr->write(*((uint8_t*)data + i)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing null check on opcCircularBufPtr — potential crash.
handleOPCData is registered as a callback and can be invoked asynchronously. If data arrives before opcCircularBufPtr is fully initialized (or after it's been deleted during re-initialization), dereferencing it will crash. Based on learnings, UDP/TCP packet processing must validate state before accessing buffers.
Proposed fix
void handleOPCData(void* arg, AsyncClient* client, void *data, size_t len) {
+ if (opcCircularBufPtr == nullptr) return;
if (len > opcCircularBufPtr->room()) return;
for (unsigned int i = 0; i < len; i++) {
opcCircularBufPtr->write(*((uint8_t*)data + i));
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/opc.cpp` around lines 33 - 38, handleOPCData currently dereferences
opcCircularBufPtr without verifying it is valid; add a null check at the top of
handleOPCData to return immediately if opcCircularBufPtr is nullptr (or
otherwise not initialized) before calling opcCircularBufPtr->room() or
->write(), so the callback cannot crash if invoked before initialization or
after teardown; ensure the early return covers both the room() size check and
the write loop to avoid any race where opcCircularBufPtr becomes null between
checks.
| void handleOPCClient(void* arg, AsyncClient* client) { | ||
| //add client | ||
| if (clientServed != nullptr) freeOPCClient(client); | ||
| clientServed = client; | ||
|
|
||
| //clean and initialize buffer | ||
| opcDataLen = 0; | ||
| opcCircularBufPtr->flush(); | ||
| opcParserWaitingForData = false; | ||
|
|
||
| // register client events | ||
| client->onData(&handleOPCData, NULL); | ||
| client->onError(&handleOPCError, NULL); | ||
| client->onDisconnect(&handleOPCDisconnect, NULL); | ||
| client->onTimeout(&handleOPCTimeOut, NULL); | ||
| } |
There was a problem hiding this comment.
Critical bug: frees wrong client when rejecting new connection.
Line 42 calls freeOPCClient(client) when clientServed != nullptr, intending to reject the new client. However, combined with the bug in freeOPCClient, this closes the new client but deletes the old clientServed pointer — leaving memory corruption and the new (closed) client assigned to clientServed on line 43.
Additionally, line 47 calls opcCircularBufPtr->flush() without a null check, which can crash if opcCircularBufPtr hasn't been initialized yet.
Proposed fix for client handling
void handleOPCClient(void* arg, AsyncClient* client) {
- //add client
- if (clientServed != nullptr) freeOPCClient(client);
- clientServed = client;
+ // Reject new client if one is already being served
+ if (clientServed != nullptr) {
+ client->close(true);
+ delete client;
+ return;
+ }
+ clientServed = client;
- //clean and initialize buffer
+ // Clean and initialize buffer
opcDataLen = 0;
- opcCircularBufPtr->flush();
+ if (opcCircularBufPtr != nullptr) opcCircularBufPtr->flush();
opcParserWaitingForData = false;
-
- // register client events
- client->onData(&handleOPCData, NULL);
- client->onError(&handleOPCError, NULL);
- client->onDisconnect(&handleOPCDisconnect, NULL);
- client->onTimeout(&handleOPCTimeOut, NULL);
+
+ // Register client events
+ client->onData(&handleOPCData, NULL);
+ client->onError(&handleOPCError, NULL);
+ client->onDisconnect(&handleOPCDisconnect, NULL);
+ client->onTimeout(&handleOPCTimeOut, NULL);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/opc.cpp` around lines 40 - 55, In handleOPCClient: do not call
freeOPCClient(client) when clientServed != nullptr — that closes the new client
and corrupts the served pointer; instead immediately reject the incoming
connection by closing/stopping the incoming AsyncClient (e.g.,
client->close(true) or client->stop()) and return without changing clientServed;
only assign clientServed = client and initialize state (opcDataLen,
opcParserWaitingForData) and register callbacks when accepting a client; also
guard the buffer flush call with a null check (if (opcCircularBufPtr)
opcCircularBufPtr->flush()) so opcCircularBufPtr->flush() cannot crash when
uninitialized.
| void parseOPCLedValues(uint8_t chan, uint8_t cmd) { | ||
| const uint16_t opcDataLenDiv = opcDataLen / 3; | ||
| uint8_t r, g, b; | ||
| if ((chan == 0 || chan == OPC_LISTEN_CHANNEL) && cmd == 0) { | ||
| realtimeLock(realtimeTimeoutMs, REALTIME_MODE_OPC); | ||
|
|
||
| for (uint16_t i = 0; i < opcDataLenDiv && i < ledCount; i++) { | ||
| r = opcCircularBufPtr->read(); | ||
| g = opcCircularBufPtr->read(); | ||
| b = opcCircularBufPtr->read(); | ||
| setRealtimePixel(i, r, g, b, 0); | ||
|
|
||
| } | ||
| strip.show(); | ||
| if (opcDataLen > ledCount * 3) discardInvalidData(opcDataLen - ledCount * 3); | ||
| } else { | ||
| discardInvalidData(opcDataLen); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ledCount is defined as a global/accessible variable
rg -n '\bledCount\b' --type=cpp --type-add 'cpp:*.h' -g '!opc.cpp' | head -30Repository: wled/WLED
Length of output: 595
🏁 Script executed:
#!/bin/bash
# Search for ledCount declaration (extern, variable definition)
rg -n 'extern.*ledCount|^\s*uint.*ledCount|^uint.*ledCount' --type=cpp --type-add 'cpp:*.h'Repository: wled/WLED
Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check includes in opc.cpp
head -30 wled00/opc.cpp | grep -E '^\s*#include'Repository: wled/WLED
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Search for ledCount in FX.h
rg -n 'ledCount' wled00/FX.h | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Search for ledCount in wled.h
rg -n 'ledCount' wled00/wled.hRepository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Search for all ledCount declarations/definitions (excluding comments)
rg -n '^\s*(extern\s+)?(uint\d+_t|uint|int)\s+ledCount' --type=cpp --type-add 'cpp:*.h'Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Search for ledCount usage pattern - what does the strip object have?
rg -n 'strip\.getLedCount|ledCount\s*=' wled00/ --type=cpp --type-add 'cpp:*.h' | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Search for how LED count is accessed in the codebase
rg -n 'strip\.(ledCount|getLedCount|length|maxLeds)' wled00/ --type=cpp -A 1 | head -30Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Check for any variable that might store LED count
rg -n '\blenstrip\b|\bstrip_len\b|\blastLed\b' wled00/ --type=cpp | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Look at actual opc.cpp full context around line 65-83
sed -n '60,90p' wled00/opc.cppRepository: wled/WLED
Length of output: 1058
🏁 Script executed:
#!/bin/bash
# Search for ledCount as macro or in any context
rg -n 'ledCount' wled00/ --type=cpp --type-add 'cpp:*.h' -iRepository: wled/WLED
Length of output: 885
🏁 Script executed:
#!/bin/bash
# Check how other protocol handlers (like e131, mqtt, etc.) access LED count
rg -n 'for.*<.*ledCount|for.*<.*strip\.|setRealtimePixel' wled00/e131.cpp wled00/mqtt.cpp 2>/dev/null | head -20Repository: wled/WLED
Length of output: 708
🏁 Script executed:
#!/bin/bash
# Search for LED count access in FX_fcn.cpp (where setRealtimePixel is likely defined)
rg -n 'setRealtimePixel|ledCount|SEGLEN' wled00/FX_fcn.cpp | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Search the entire opc.cpp for ledCount declaration
cat wled00/opc.cpp | head -70 | tail -40Repository: wled/WLED
Length of output: 1202
🏁 Script executed:
#!/bin/bash
# Look for what strip object provides for LED access
rg -n 'strip\.' wled00/opc.cpp -o | sort -uRepository: wled/WLED
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Check what type of LED count the strip object has
rg -n 'class.*WS2812FX|uint.*_ledcount|_leds' wled00/FX.h | head -10Repository: wled/WLED
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Get the full beginning of opc.cpp to see all globals
head -150 wled00/opc.cppRepository: wled/WLED
Length of output: 4018
🏁 Script executed:
#!/bin/bash
# Check WS2812FX class members around ledcount
rg -n '_leds|_ledcount|ledcount|getLedCount|length' wled00/FX.h -A 1 -B 1 | head -40Repository: wled/WLED
Length of output: 1666
🏁 Script executed:
#!/bin/bash
# Look for how many LEDs are stored - check strip initialization
rg -n 'WS2812FX.*strip|new WS2812FX' wled00/ --type=cpp -A 2 | head -20Repository: wled/WLED
Length of output: 331
🏁 Script executed:
#!/bin/bash
# Search for ledCount extern or global declaration in header files
rg -n 'extern.*ledCount|WLED_GLOBAL.*ledCount' wled00/ --type=cpp --type-add 'cpp:*.h'Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Check if ledCount might be in a previous version or another branch
# Let's look for what globals are defined in wled.h that relate to LED count
rg -n 'WLED_GLOBAL.*uint.*LED|WLED_GLOBAL.*ledCount' wled00/wled.hRepository: wled/WLED
Length of output: 308
🏁 Script executed:
#!/bin/bash
# Verify getLengthTotal() works and is how other code accesses LED count
rg -n 'getLengthTotal\|_length' wled00/ --type=cpp | grep -v 'MIN_SHOW_DELAY\|groupLength' | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Search for any declaration of ledCount as a variable (not in comments)
rg -n '(uint\d+_t|uint|int)\s+ledCount\s*[;=]' wled00/ --type=cpp --type-add 'cpp:*.h'Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Check if ledCount might be a macro
rg -n '#define ledCount' wled00/ --type=cpp --type-add 'cpp:*.h'Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Look at the entire wled.h for global declarations
grep -n 'ledCount\|_length' wled00/wled.h | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Check if ledCount is perhaps defined conditionally or in another included file
rg -n 'ledCount' wled00/ --type=cpp -B 2 -A 2 | grep -E '^\s*(extern|static|const|uint)' | head -20Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Do a comprehensive search - ledCount must be used somewhere or code won't compile
# Let's check if it's perhaps a very recent addition or if code is broken
git log --oneline -20 wled00/opc.cpp 2>/dev/null || echo "Git not available"Repository: wled/WLED
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Check what the correct method should be - strip has getLengthTotal()
# Let's see if other realtime handlers use it
rg -n 'realtimeTimeoutMs|setRealtimePixel|getLengthTotal' wled00/mqtt.cpp wled00/ws.cpp 2>/dev/null | head -30Repository: wled/WLED
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Final check: look at compilation context - search for any conditional defines
# that might create ledCount
rg -n 'ledCount' wled00/ -B 5 -A 1 | head -50Repository: wled/WLED
Length of output: 2957
🏁 Script executed:
#!/bin/bash
# Check if there's a forward declaration or extern in any header included by opc.cpp
rg -n 'extern.*ledCount' wled00/fcn_declare.h wled00/wled.h wled00/FX.h 2>/dev/null || echo "No extern ledCount found"Repository: wled/WLED
Length of output: 77
🏁 Script executed:
#!/bin/bash
# Final verification: check the actual current state of the OPC branch
# Look for any variable or macro that might shadow ledCount
rg -n '\bledCount\b' wled00/opc.cpp -B 10 | head -40Repository: wled/WLED
Length of output: 1130
🏁 Script executed:
#!/bin/bash
# Verify ws.cpp approach works - confirm getLengthTotal() is the right method
rg -n 'class WS2812FX' wled00/FX.h -A 150 | grep -E 'getLengthTotal|getLength' | head -5Repository: wled/WLED
Length of output: 87
🏁 Script executed:
#!/bin/bash
# One final check - search entire repository for any ledCount declaration or definition
find wled00 -name "*.h" -o -name "*.cpp" | xargs grep -l 'ledCount' | xargs grep -n '^[^/]*\(extern\|static\|uint\|int\).*ledCount'Repository: wled/WLED
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Verify the exact error - see if ledCount could possibly be defined anywhere
rg -n 'ledCount\s*=' wled00/ --type=cpp --type-add 'cpp:*.h' -B 2Repository: wled/WLED
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Check if there's a using statement or alias
rg -n 'using.*ledCount|#define.*ledCount' wled00/ --type=cpp --type-add 'cpp:*.h'Repository: wled/WLED
Length of output: 479
Replace undeclared ledCount with strip.getLengthTotal().
ledCount is not declared anywhere in the codebase and will cause a compilation error. The WLED codebase uses strip.getLengthTotal() to access LED count (as seen in ws.cpp). Replace all three occurrences (lines 71, 79, and 125) with strip.getLengthTotal().
Additionally, verify that discardInvalidData() handles the case where opcDataLen - ledCount * 3 might exceed available buffer bytes—the function already includes this check, but ensure it's correct when opcDataLen is corrupted or malformed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/opc.cpp` around lines 65 - 83, In parseOPCLedValues, replace the
undeclared variable ledCount with strip.getLengthTotal() in all uses (the loop
bound, the later comparison for discardInvalidData, and any other occurrence) so
the code compiles; keep the logic using opcDataLen/opcDataLenDiv,
opcCircularBufPtr->read(), setRealtimePixel(...), realtimeLock(...,
REALTIME_MODE_OPC) and strip.show() unchanged. After the replacement, ensure the
call to discardInvalidData(opcDataLen - strip.getLengthTotal() * 3) is safe by
confirming discardInvalidData correctly bounds/checks large or negative lengths
when opcDataLen is corrupted or malformed.
softhack007
added
connectivity
|
It seems a bit of a dead protocol that didn't really take off, folk can just use ArtNet, sACN or DDP |
5 participants
Tested implementation of Open Pixel Control protocol for setting pixel colors.
Test setup: NodeMCU v1.0 (ESP8266-12) with 24 LED matrix
Highlights:
Improvements I've thought of (but don't know how feasible / useful they are):
Notes
Only supports one client at a time and listens on one channel (besides from broadcast)
Default port is 7890 (as per opc test scripts default port)
Open to suggestions and feedback!
Summary by CodeRabbit