From a196171ac427ea43be7ea182513a65dadab301d6 Mon Sep 17 00:00:00 2001 From: Will Tatam Date: Sat, 8 Aug 2026 10:22:32 +0100 Subject: [PATCH] Encapsulate notifier/realtime UDP socket state behind accessors notifierUdp/notifier2Udp/rgbUdp and their udpConnected/udp2Connected/ udpRgbConnected flags were WLED_GLOBAL. Once udp.cpp was split into per-protocol files (wled00/sync/), each protocol file ended up reaching into these directly - fan-out grew from a clean 2-file pair to 3-5 files each, exactly the kind of implicit cross-file coupling an accessor boundary is meant to prevent. Moved them into a NotifierSockets struct owned by wled.cpp (where the sockets are actually opened via .begin()), exposed to every other file via six small accessors: getNotifierUdp()/getNotifier2Udp()/getRgbUdp() (returning WiFiUDP& so callers can call methods directly, no copies) and isUdpConnected()/isUdp2Connected()/isUdpRgbConnected(). Call sites that use a socket more than once alias it to a local reference at the top of the function rather than repeating the accessor call. Found and fixed one real external consumer: usermods/udp_name_sync reached into notifierUdp/udpConnected directly. It isn't part of the default esp32dev usermod set, so this would have been a silent build break for anyone enabling it - caught by building the dedicated `usermods` env, which compiles every usermod. No behavior change - purely a storage/access-pattern change. Verified: - esp32dev builds and links cleanly via `pio run -e esp32dev` (1,320,311 bytes flash). - usermods env (builds all 59 usermods, including the fixed udp_name_sync) builds and links cleanly via `pio run -e usermods`. - Repo-wide grep confirms no remaining raw references to any of the six converted identifiers outside wled.cpp's NotifierSockets struct. Co-Authored-By: Claude Sonnet 5 --- usermods/udp_name_sync/udp_name_sync.cpp | 3 +- wled00/e131.cpp | 1 + wled00/fcn_declare.h | 8 +++++ wled00/sync/realtime_udp.cpp | 6 ++-- wled00/sync/sync_nodes.cpp | 5 ++-- wled00/sync/sync_notifier.cpp | 9 +++--- wled00/udp.cpp | 8 +++-- wled00/wled.cpp | 38 +++++++++++++++++++----- wled00/wled.h | 7 ++--- 9 files changed, 61 insertions(+), 24 deletions(-) diff --git a/usermods/udp_name_sync/udp_name_sync.cpp b/usermods/udp_name_sync/udp_name_sync.cpp index 34197bb345..9e36f63657 100644 --- a/usermods/udp_name_sync/udp_name_sync.cpp +++ b/usermods/udp_name_sync/udp_name_sync.cpp @@ -29,7 +29,7 @@ class UdpNameSync : public Usermod { void loop() override { if (!enabled) return; if (!WLED_CONNECTED) return; - if (!udpConnected) return; + if (!isUdpConnected()) return; Segment& mainseg = strip.getMainSegment(); if (segmentName[0] == '\0' && !mainseg.name) return; //name was never set, do nothing @@ -39,6 +39,7 @@ class UdpNameSync : public Usermod { IPAddress broadcastIp = uint32_t(WLEDNetwork.localIP()) | ~uint32_t(WLEDNetwork.subnetMask()); byte udpOut[WLED_MAX_SEGNAME_LEN + 2]; udpOut[0] = kPacketType; // custom usermod packet type (avoid 0..5 used by core protocols) + WiFiUDP& notifierUdp = getNotifierUdp(); if (segmentName[0] != '\0' && !mainseg.name) { // name cleared notifierUdp.beginPacket(broadcastIp, udpPort); diff --git a/wled00/e131.cpp b/wled00/e131.cpp index b6b04a1dc7..cd726cf2e0 100644 --- a/wled00/e131.cpp +++ b/wled00/e131.cpp @@ -576,6 +576,7 @@ static void sendArtnetPollReply(ArtPollReply *reply, IPAddress ipAddress, uint16 pollReplyCount = 0; } + WiFiUDP& notifierUdp = getNotifierUdp(); notifierUdp.beginPacket(ipAddress, ARTNET_DEFAULT_PORT); notifierUdp.write(reply->raw, sizeof(ArtPollReply)); notifierUdp.endPacket(); diff --git a/wled00/fcn_declare.h b/wled00/fcn_declare.h index 9fdd7d72c6..4683d544d7 100644 --- a/wled00/fcn_declare.h +++ b/wled00/fcn_declare.h @@ -294,6 +294,14 @@ bool isAsterisksOnly(const char* str, byte maxLen); void handleSettingsSet(AsyncWebServerRequest *request, byte subPage); bool handleSet(AsyncWebServerRequest *request, const String& req, bool apply=true); +//wled.cpp +WiFiUDP& getNotifierUdp(); +WiFiUDP& getNotifier2Udp(); +WiFiUDP& getRgbUdp(); +bool isUdpConnected(); +bool isUdp2Connected(); +bool isUdpRgbConnected(); + //udp.cpp void handleNotifications(); diff --git a/wled00/sync/realtime_udp.cpp b/wled00/sync/realtime_udp.cpp index 69721cb9db..e928b50b88 100644 --- a/wled00/sync/realtime_udp.cpp +++ b/wled00/sync/realtime_udp.cpp @@ -16,6 +16,7 @@ static uint8_t tpmPacketCount = 0; static uint16_t tpmPayloadFrameSize = 0; static void sendTPM2Ack() { + WiFiUDP& notifierUdp = getNotifierUdp(); notifierUdp.beginPacket(notifierUdp.remoteIP(), TMP2NET_OUT_PORT); uint8_t response_ack = 0xac; notifierUdp.write(&response_ack, 1); @@ -26,6 +27,7 @@ static void sendTPM2Ack() { // Returns true if a packet was read (handled or discarded), false if nothing was pending. bool handleHyperionPacket() { + WiFiUDP& rgbUdp = getRgbUdp(); size_t packetSize = rgbUdp.parsePacket(); if (!packetSize) return false; @@ -62,7 +64,7 @@ bool handleDirectRealtimePacket(uint8_t *udpIn, size_t packetSize, bool isSupp) } if (tpmType != 0xda) return true; //ignore, not TPM2.NET data - realtimeIP = (isSupp) ? notifier2Udp.remoteIP() : notifierUdp.remoteIP(); + realtimeIP = (isSupp) ? getNotifier2Udp().remoteIP() : getNotifierUdp().remoteIP(); realtimeLock(realtimeTimeoutMs, REALTIME_MODE_TPM2NET); if (realtimeOverride) return true; @@ -88,7 +90,7 @@ bool handleDirectRealtimePacket(uint8_t *udpIn, size_t packetSize, bool isSupp) //UDP realtime: 1 warls 2 drgb 3 drgbw 4 dnrgb 5 dnrgbw if (udpIn[0] > 0 && udpIn[0] < 6) { - realtimeIP = (isSupp) ? notifier2Udp.remoteIP() : notifierUdp.remoteIP(); + realtimeIP = (isSupp) ? getNotifier2Udp().remoteIP() : getNotifierUdp().remoteIP(); DEBUG_PRINTLN(realtimeIP); if (packetSize < 2) return true; diff --git a/wled00/sync/sync_nodes.cpp b/wled00/sync/sync_nodes.cpp index 55be843f29..2f1bcf9507 100644 --- a/wled00/sync/sync_nodes.cpp +++ b/wled00/sync/sync_nodes.cpp @@ -11,7 +11,7 @@ bool parseNodeInfoPacket(const uint8_t *udpIn, unsigned len, bool isSupp, const { if (!(isSupp && udpIn[0] == 255 && udpIn[1] == 1 && len >= 40)) return false; - if (!nodeListEnabled || notifier2Udp.remoteIP() == localIP) return true; + if (!nodeListEnabled || getNotifier2Udp().remoteIP() == localIP) return true; unsigned unit = udpIn[39]; NodesMap::iterator it = Nodes.find(unit); @@ -67,7 +67,7 @@ void refreshNodeList() \*********************************************************************************************/ void sendSysInfoUDP() { - if (!udp2Connected) return; + if (!isUdp2Connected()) return; IPAddress ip = WLEDNetwork.localIP(); if (!ip || ip == IPAddress(255,255,255,255)) ip = IPAddress(4,3,2,1); @@ -100,6 +100,7 @@ void sendSysInfoUDP() data[40+i] = (build>>(8*i)) & 0xFF; IPAddress broadcastIP(255, 255, 255, 255); + WiFiUDP& notifier2Udp = getNotifier2Udp(); notifier2Udp.beginPacket(broadcastIP, udpPort2); notifier2Udp.write(data, sizeof(data)); notifier2Udp.endPacket(); diff --git a/wled00/sync/sync_notifier.cpp b/wled00/sync/sync_notifier.cpp index ed87a4f176..62490cb612 100644 --- a/wled00/sync/sync_notifier.cpp +++ b/wled00/sync/sync_notifier.cpp @@ -20,9 +20,9 @@ static NotifierSendState notifierSend; void notify(byte callMode, bool followUp) { #ifndef WLED_DISABLE_ESPNOW - if (!udpConnected && !useESPNowSync) return; + if (!isUdpConnected() && !useESPNowSync) return; #else - if (!udpConnected) return; + if (!isUdpConnected()) return; #endif if (!syncGroups || !sendNotificationsRT) return; switch (callMode) @@ -189,11 +189,12 @@ void notify(byte callMode, bool followUp) DEBUG_PRINTLN(F("ESP-NOW sending packet failed.")); } } - if (udpConnected) + if (isUdpConnected()) #endif { DEBUG_PRINTLN(F("UDP sending packet.")); IPAddress broadcastIp = ~uint32_t(WLEDNetwork.subnetMask()) | uint32_t(WLEDNetwork.gatewayIP()); + WiFiUDP& notifierUdp = getNotifierUdp(); notifierUdp.beginPacket(broadcastIp, udpPort); notifierUdp.write(udpOut, WLEDPACKETSIZE); // TODO: add actual used buffer size notifierUdp.endPacket(); @@ -207,7 +208,7 @@ void notify(byte callMode, bool followUp) // send-retry check so the dispatcher doesn't need to know notify()'s internal state. void notifyRetryIfNeeded() { - if (udpConnected && (notifierSend.count < udpNumRetries) && ((millis() - notifierSend.lastSentTime) > 250)) { + if (isUdpConnected() && (notifierSend.count < udpNumRetries) && ((millis() - notifierSend.lastSentTime) > 250)) { notify(notifierSend.lastCallMode, true); } } diff --git a/wled00/udp.cpp b/wled00/udp.cpp index f40714db40..36e9012a50 100644 --- a/wled00/udp.cpp +++ b/wled00/udp.cpp @@ -17,6 +17,8 @@ void handleNotifications() { IPAddress localIP; + WiFiUDP& notifierUdp = getNotifierUdp(); + WiFiUDP& notifier2Udp = getNotifier2Udp(); //send second notification if enabled notifyRetryIfNeeded(); @@ -32,17 +34,17 @@ void handleNotifications() if (realtimeMode && millis() > realtimeTimeout) exitRealtime(); //receive UDP notifications - if (!udpConnected) return; + if (!isUdpConnected()) return; bool isSupp = false; size_t packetSize = notifierUdp.parsePacket(); - if (!packetSize && udp2Connected) { + if (!packetSize && isUdp2Connected()) { packetSize = notifier2Udp.parsePacket(); isSupp = true; } //hyperion / raw RGB - if (!packetSize && udpRgbConnected) { + if (!packetSize && isUdpRgbConnected()) { if (handleHyperionPacket()) return; } diff --git a/wled00/wled.cpp b/wled00/wled.cpp index d5125c30e2..dd8a44e016 100644 --- a/wled00/wled.cpp +++ b/wled00/wled.cpp @@ -25,6 +25,28 @@ #endif extern "C" void usePWMFixedNMI(); +// The notifier/realtime UDP sockets and their connection state are owned here +// (this is where they're opened) and exposed to the rest of the codebase via +// the accessors below instead of as raw WLED_GLOBAL externs. Previously these +// were plain globals - once udp.cpp was split into per-protocol files (see +// wled00/sync/), each protocol file ended up reaching into them directly, +// which is exactly the kind of implicit cross-file coupling an accessor +// boundary is meant to prevent. +struct NotifierSockets { + WiFiUDP notifierUdp, notifier2Udp, rgbUdp; + bool udpConnected = false; + bool udp2Connected = false; + bool udpRgbConnected = false; +}; +static NotifierSockets notifierSockets; + +WiFiUDP& getNotifierUdp() { return notifierSockets.notifierUdp; } +WiFiUDP& getNotifier2Udp() { return notifierSockets.notifier2Udp; } +WiFiUDP& getRgbUdp() { return notifierSockets.rgbUdp; } +bool isUdpConnected() { return notifierSockets.udpConnected; } +bool isUdp2Connected() { return notifierSockets.udp2Connected; } +bool isUdpRgbConnected() { return notifierSockets.udpRgbConnected; } + /* * Main WLED class implementation. Mostly initialization and connection logic */ @@ -723,13 +745,13 @@ void WLED::initAP(bool resetAP) DEBUG_PRINTLN(F("Init AP interfaces")); server.begin(); if (udpPort > 0 && udpPort != ntpLocalPort) { - udpConnected = notifierUdp.begin(udpPort); + notifierSockets.udpConnected = notifierSockets.notifierUdp.begin(udpPort); } if (udpRgbPort > 0 && udpRgbPort != ntpLocalPort && udpRgbPort != udpPort) { - udpRgbConnected = rgbUdp.begin(udpRgbPort); + notifierSockets.udpRgbConnected = notifierSockets.rgbUdp.begin(udpRgbPort); } if (udpPort2 > 0 && udpPort2 != ntpLocalPort && udpPort2 != udpPort && udpPort2 != udpRgbPort) { - udp2Connected = notifier2Udp.begin(udpPort2); + notifierSockets.udp2Connected = notifierSockets.notifier2Udp.begin(udpPort2); } e131.begin(false, e131Port, e131Universe, E131_MAX_UNIVERSE_COUNT); ddp.begin(false, DDP_DEFAULT_PORT); @@ -931,11 +953,11 @@ void WLED::initInterfaces() server.begin(); if (udpPort > 0 && udpPort != ntpLocalPort) { - udpConnected = notifierUdp.begin(udpPort); - if (udpConnected && udpRgbPort != udpPort) - udpRgbConnected = rgbUdp.begin(udpRgbPort); - if (udpConnected && udpPort2 != udpPort && udpPort2 != udpRgbPort) - udp2Connected = notifier2Udp.begin(udpPort2); + notifierSockets.udpConnected = notifierSockets.notifierUdp.begin(udpPort); + if (notifierSockets.udpConnected && udpRgbPort != udpPort) + notifierSockets.udpRgbConnected = notifierSockets.rgbUdp.begin(udpRgbPort); + if (notifierSockets.udpConnected && udpPort2 != udpPort && udpPort2 != udpRgbPort) + notifierSockets.udp2Connected = notifierSockets.notifier2Udp.begin(udpPort2); } if (ntpEnabled) ntpConnected = ntpUdp.begin(ntpLocalPort); diff --git a/wled00/wled.h b/wled00/wled.h index 8ae04f2384..b8b47a21c2 100644 --- a/wled00/wled.h +++ b/wled00/wled.h @@ -681,9 +681,9 @@ 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 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); -WLED_GLOBAL bool udpRgbConnected _INIT(false); +// notifierUdp/notifier2Udp/rgbUdp and udpConnected/udp2Connected/udpRgbConnected +// are private to wled.cpp (NotifierSockets) - use getNotifierUdp()/getNotifier2Udp()/ +// getRgbUdp()/isUdpConnected()/isUdp2Connected()/isUdpRgbConnected() instead. // ui style WLED_GLOBAL bool showWelcomePage _INIT(false); @@ -797,7 +797,6 @@ WLED_GLOBAL AsyncClient *hueClient _INIT(NULL); 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)));