Skip to content

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

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

Open
netmindz wants to merge 1 commit into
base: main
Choose a base branch
from
+18 −10
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions wled00/wled.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@
#endif
extern "C" void usePWMFixedNMI();

// 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
Comment thread
netmindz marked this conversation as resolved.
static unsigned long lastReconnectAttempt = 0;
static uint16_t udpRgbPort = 19446; // Hyperion port
static ESPAsyncE131 e131(handleE131Packet);
static ESPAsyncE131 ddp(handleE131Packet);
#if defined(STATUSLED)
static unsigned long ledStatusLastMillis = 0;
static uint8_t ledStatusType = 0; // current status type - corresponds to number of blinks per second
static bool ledStatusState = false; // the current LED state
#endif

/*
* Main WLED class implementation. Mostly initialization and connection logic
*/
Expand Down
15 changes: 5 additions & 10 deletions wled00/wled.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ WLED_GLOBAL bool e131SkipOutOfSequence _INIT(false); // freeze inst
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

@willmmiles willmmiles Aug 13, 2026

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.

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.

@softhack007 softhack007 Sep 11, 2026

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.

@willmmiles I agree 👍

@softhack007 softhack007 Sep 11, 2026

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.

implemented in b823312

@willmmiles willmmiles Sep 11, 2026 •
edited
Loading

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.

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."

@softhack007 softhack007 Sep 11, 2026

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.

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".

// lastMqttReconnectAttempt is private to wled.cpp - see there.
#ifndef WLED_DISABLE_MQTT
#ifndef MQTT_MAX_TOPIC_LEN
#define MQTT_MAX_TOPIC_LEN 32
Expand Down Expand Up @@ -580,7 +580,7 @@ WLED_GLOBAL uint16_t userVar0 _INIT(0), userVar1 _INIT(0); //available for use i
WLED_GLOBAL bool apActive _INIT(false);
WLED_GLOBAL byte apClients _INIT(0);
WLED_GLOBAL bool forceReconnect _INIT(false);
WLED_GLOBAL unsigned long lastReconnectAttempt _INIT(0);
// lastReconnectAttempt is private to wled.cpp - see there.
WLED_GLOBAL bool interfacesInited _INIT(false);
WLED_GLOBAL bool wasConnected _INIT(false);

Expand Down Expand Up @@ -680,7 +680,7 @@ WLED_GLOBAL bool stateChanged _INIT(false);
// network
WLED_GLOBAL uint16_t udpPort _INIT(21324); // WLED notifier default port
WLED_GLOBAL uint16_t udpPort2 _INIT(65506); // WLED notifier supplemental port
WLED_GLOBAL uint16_t udpRgbPort _INIT(19446); // Hyperion port
// udpRgbPort is private to wled.cpp - see there.
WLED_GLOBAL uint8_t udpNumRetries _INIT(0); // Number of times a UDP sync message is retransmitted. Increase to increase reliability
WLED_GLOBAL bool udpConnected _INIT(false);
WLED_GLOBAL bool udp2Connected _INIT(false);
Expand Down Expand Up @@ -783,11 +783,7 @@ WLED_GLOBAL bool configNeedsWrite _INIT(false); // flag to initiate savi
WLED_GLOBAL bool doReboot _INIT(false); // flag to initiate reboot from async handlers

// status led
#if defined(STATUSLED)
WLED_GLOBAL unsigned long ledStatusLastMillis _INIT(0);
WLED_GLOBAL uint8_t ledStatusType _INIT(0); // current status type - corresponds to number of blinks per second
WLED_GLOBAL bool ledStatusState _INIT(false); // the current LED state
#endif
// ledStatusLastMillis/ledStatusType/ledStatusState are private to wled.cpp - see there.

// server library objects
WLED_GLOBAL AsyncWebServer server _INIT_N(((80, {0, WLED_REQUEST_MAX_QUEUE, WLED_REQUEST_MIN_HEAP, WLED_REQUEST_HEAP_USAGE})));
Expand All @@ -802,8 +798,7 @@ WLED_GLOBAL AsyncWebHandler *editHandler _INIT(nullptr);
// udp interface objects
WLED_GLOBAL WiFiUDP notifierUdp, rgbUdp, notifier2Udp;
WLED_GLOBAL WiFiUDP ntpUdp;
WLED_GLOBAL ESPAsyncE131 e131 _INIT_N(((handleE131Packet)));
WLED_GLOBAL ESPAsyncE131 ddp _INIT_N(((handleE131Packet)));
// e131/ddp (the ESPAsyncE131 objects) are private to wled.cpp - see there.
WLED_GLOBAL bool e131NewData _INIT(false);

// led fx library object
Expand Down
Loading