Encapsulate wled.cpp's own runtime state as static, not global - #5778
Encapsulate wled.cpp's own runtime state as static, not global#5778netmindz wants to merge 1 commit into
Conversation
8 WLED_GLOBAL variables were referenced only in wled.cpp: ddp, e131 (the ESPAsyncE131 sender objects), lastMqttReconnectAttempt, lastReconnectAttempt, ledStatusLastMillis, ledStatusState, ledStatusType, udpRgbPort. Converted all 8 to file-local `static`, preserving the #if defined(STATUSLED) guard around the three ledStatus* variables and using direct constructor syntax for the two ESPAsyncE131 objects (equivalent to what WLED_GLOBAL/_INIT_N expanded to when wled.cpp itself was the definition TU). No behavior change - purely a storage-class change. Verified: esp32dev builds and links cleanly via `pio run -e esp32dev` (1,320,323 bytes flash, no warnings from either changed file). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtCyBD91vAYWvBzaMyQSHd
WalkthroughThe change moves MQTT, WiFi, UDP RGB, E1.31/DDP, and status LED runtime state into file-local declarations in ChangesRuntime state privacy
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
| WLED_GLOBAL uint16_t pollReplyCount _INIT(0); // count number of replies for ArtPoll node report | ||
|
|
||
| // mqtt | ||
| WLED_GLOBAL unsigned long lastMqttReconnectAttempt _INIT(0); // used for other periodic tasks too |
There was a problem hiding this comment.
We should update the agents file to avoid clutter comments like this -- there's no need to document private variables in global headers, even if they were misplaced there before. It creates a maintenance burden that will inevitably fall out of date.
There was a problem hiding this comment.
Thank you! That's maybe more specific to this specific task than I was thinking, but still good.
I'd been thinking something like:
"Document the code as it is, not as it was. When refactoring, write as if the code has always been the better way. We do not need to clutter the code with comments referencing past mistakes or describing why best practices are being followed. Instead, comment when the code seems to go against best practices -- that's when the rationale is most important. You do not need to 'leave tracks' in the code; good architecture makes relevant logic easy to find, and the commit history fills in the gaps."
There was a problem hiding this comment.
good points about "Dokument the code as it is, not as it was" and " comment when the code seems to go against best practices -- that's when the rationale is most important.".
I'll think about how to integrate them. Actually, AIs are like children - if you talk too much, they get confused 😉. So i'd need something in the style of "do this, don't that".
3 participants
Summary
Part of an ongoing pass identifying
WLED_GLOBALdeclarations that are actually only referenced in one file (see #5777 for the first of these). 8 more turned out to be private towled.cppitself:ddp,e131(theESPAsyncE131sender objects),lastMqttReconnectAttempt,lastReconnectAttempt,ledStatusLastMillis,ledStatusState,ledStatusType,udpRgbPort.Converted all 8 to file-local
static:ledStatus*variables keep their original#if defined(STATUSLED)guard.ddp/e131use direct constructor syntax (static ESPAsyncE131 e131(handleE131Packet);), equivalent to whatWLED_GLOBAL ESPAsyncE131 e131 _INIT_N(((handleE131Packet)));expanded to whenwled.cppitself was the definition translation unit (WLED_DEFINE_GLOBAL_VARS).No behavior change — purely a storage-class change, same types and initial values as before.
Test plan
esp32dev: builds and links cleanly viapio run -e esp32dev— 1,320,323 bytes flash, no warnings.wled00/,usermods/) that none of the 8 converted variables are referenced outsidewled.cpp.🤖 Generated with Claude Code
Summary by CodeRabbit