Skip to content

Encapsulate wled.cpp's own runtime state as static, not global - #5778

Open
netmindz wants to merge 1 commit into
wled:mainfrom
netmindz:refactor/wledcpp-globals
Open

Encapsulate wled.cpp's own runtime state as static, not global#5778
netmindz wants to merge 1 commit into
wled:mainfrom
netmindz:refactor/wledcpp-globals

Conversation

@netmindz

@netmindz netmindz commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

Part of an ongoing pass identifying WLED_GLOBAL declarations that are actually only referenced in one file (see #5777 for the first of these). 8 more turned out to be private to wled.cpp itself: ddp, e131 (the ESPAsyncE131 sender objects), lastMqttReconnectAttempt, lastReconnectAttempt, ledStatusLastMillis, ledStatusState, ledStatusType, udpRgbPort.

Converted all 8 to file-local static:

  • The three ledStatus* variables keep their original #if defined(STATUSLED) guard.
  • ddp/e131 use direct constructor syntax (static ESPAsyncE131 e131(handleE131Packet);), equivalent to what WLED_GLOBAL ESPAsyncE131 e131 _INIT_N(((handleE131Packet))); expanded to when wled.cpp itself 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 via pio run -e esp32dev — 1,320,323 bytes flash, no warnings.
  • Confirmed via repo-wide grep (wled00/, usermods/) that none of the 8 converted variables are referenced outside wled.cpp.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Improved internal state management for MQTT and WiFi reconnections, UDP communication, lighting protocol handling, and status LED tracking.
    • No user-facing features or behavior changes were introduced.

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
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The change moves MQTT, WiFi, UDP RGB, E1.31/DDP, and status LED runtime state into file-local declarations in wled.cpp. Their exported declarations are removed from wled.h.

Changes

Runtime state privacy

Layer / File(s) Summary
Move runtime state to file-local ownership
wled00/wled.cpp, wled00/wled.h
wled.cpp defines the runtime state as file-local static declarations. wled.h no longer exports the reconnect timestamps, UDP RGB port, status LED state, or E1.31/DDP instances.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • wled/WLED#5737: Privatizes related ESP-NOW state across wled.cpp and wled.h.
  • wled/WLED#5777: Applies similar file-local encapsulation to implementation-only runtime state.

Suggested labels: minor

Suggested reviewers: softhack007, dedehai, smitty078

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making wled.cpp runtime state file-local static instead of global.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread wled00/wled.cpp

// Runtime state private to this file - previously WLED_GLOBAL, a leftover from
// when all state lived in one big extern block regardless of who used it.
static unsigned long lastMqttReconnectAttempt = 0; // used for other periodic tasks too

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@coderabbitai please double-check if any of these now static variables is used in any usermod/

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.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Usermod directories ---'
fd --type d --ignore-case 'usermod' . || true

printf '%s\n' '--- Direct identifier references in usermod-related C++ files ---'
rg -n -P -i \
  --glob '*.{c,cc,cpp,cxx,h,hpp,ino}' \
  --glob '*usermod*/*' \
  '\b(ddp|e131|lastMqttReconnectAttempt|lastReconnectAttempt|ledStatusLastMillis|ledStatusState|ledStatusType|udpRgbPort)\b' \
  . || true

printf '%s\n' '--- Usermod-related C++ filenames found by path ---'
fd --type f --ignore-case 'usermod' . -e c -e cc -e cpp -e cxx -e h -e hpp -e ino || true

Length of output: 9445


@softhack007, I found no direct references to any of the eight variables in usermods/.

The only usermod search result was a comment in usermods/audioreactive/audio_reactive.cpp that mentions the E1.31 protocol. It does not reference the e131 object.

The new file-local static declarations do not break an in-tree usermod reference.

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants