audioreactive: add UDP broadcast as an alternative sync transport - #5863
efranceschi wants to merge 2 commits into
Conversation
Multicast sound sync depends on the network keeping the group membership alive. Access points that enable IGMP snooping without providing an IGMP querier stop forwarding the group once the membership times out (~260s), which silently breaks sound sync for every receiver until they reboot. Add a 'Transport' setting to the AudioReactive sync section that lets the sending node use UDP broadcast instead of multicast. Broadcast does not rely on group membership, so it is not affected by that behaviour. Default is unchanged (multicast). Receivers need no change: their socket is bound to the sync port, so it accepts broadcast packets as well. The setting is a uint8_t rather than a bool so further transports can be added later without a config migration, as suggested in wled#5637. Written with AI assistance; every line reviewed and tested on hardware.
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: wled/WLED/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe AudioReactive usermod adds a persisted multicast or broadcast setting for UDP sound synchronization. The network layer provides a shared broadcast address for audio sync and UDP notifications. ChangesAudio sync transport
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant User
participant AudioReactive
participant Config
participant WLEDNetwork
participant UDPReceiver
User->>AudioReactive: Select multicast or broadcast
AudioReactive->>Config: Store sync.transport
Config-->>AudioReactive: Load sync.transport
AudioReactive->>WLEDNetwork: Request broadcastIP() when broadcast is selected
WLEDNetwork-->>AudioReactive: Return broadcast address
AudioReactive->>UDPReceiver: Send audio sync packet
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
similar to the proposed solution in MoonModules#250 If the root cause is confirmed to be "IGMP snooping" by the access point, we should first investigate if there is a way to keep IGMP happy. Adding broadcast as a "transport" option is a useful addition any way. |
|
@coderabbitai look for technical references that support "IGMP snooping by the access point" as a root cause for UDP multicast receiver-side connection dropout - IPv4 standards and RFCs related to UDP broadcast, plus official documentation from espressif. Are there other explanations for the observed behaviour, especially affecting older "consumer grade" Wi-Fi internet gateways? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🤖 web_search executed:
Length of output: 10229 🏁 Script executed: #!/bin/bash
set -euo pipefail
target=$(fd -a '^audio_reactive\.cpp$' usermods/audioreactive | head -n 1)
if [ -z "$target" ]; then
echo "audio_reactive.cpp not found" >&2
exit 1
fi
ast-grep outline "$target" --items all --match 'transmitAudioData|audioSyncTransport|addToConfig|readFromConfig' --view expanded
printf '\n--- Relevant occurrences ---\n'
rg -n -C 12 'audioSyncTransport|transmitAudioData\s*\(' "$target"Length of output: 9751
This sequence is technically credible:
The PR comment should say that this is a possible network cause, not an established root cause, until a capture confirms it. Capture these packets on the LAN and, if possible, on the affected Wi-Fi segment:
A useful confirmation test is to enable an IGMP querier on the relevant VLAN, or disable IGMP snooping temporarily. If multicast remains stable beyond 260 seconds, the hypothesis becomes strong. Broadcast standards and this implementationThe implementation calculates: ~uint32_t(WLEDNetwork.subnetMask()) | uint32_t(WLEDNetwork.gatewayIP())This produces the broadcast address for the gateway subnet, assuming the gateway is in the active interface subnet. This is a subnet-directed IPv4 broadcast address, but the sender uses it on its local LAN. It must not depend on router forwarding to reach local receivers.
RFC 2644 does not prevent local-LAN broadcast delivery. It means this transport should not be expected to cross routed subnet or VLAN boundaries. That limitation is appropriate for this sound-sync protocol. Espressif referencesEspressif documents UDP multicast support through lwIP and its multicast socket APIs: Espressif also documents a separate Wi-Fi delivery risk:
See ESP-IDF Wi-Fi documentation: power-save behavior. This means UDP broadcast is a workaround for IGMP-state loss, not a universal reliability improvement. Broadcast and multicast both remain vulnerable to Wi-Fi loss and access-point policy. Other plausible causes on older consumer gatewaysOlder consumer gateways can also cause the observed behavior through:
The selectable transport is therefore a useful compatibility feature. Multicast should remain the default because it limits traffic to group members. Broadcast provides a practical fallback for local networks where multicast forwarding is unreliable. ✏️ Learnings added
You are interacting with an AI system. |
softhack007
added
connectivity
Is there a way to implement this using the arduino-esp32 WifiUDP class?
How would we implement this? is there an API in arduino-esp32 WifiUDP? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🤖 web_search executed:
Length of output: 9861
For this problem, use one of these network-side actions:
There is an important Arduino-ESP32 regression risk. The current Arduino-ESP32 issue espressif/arduino-esp32#12408 reports that some recent versions can fail to receive the For Wi-Fi power save, The Arduino-ESP32 wrapper is: `#include` <WiFi.h>
// Call after the station interface has started.
bool ok = WiFi.setSleep(false); // Requests WIFI_PS_NONEThe direct ESP-IDF call is: `#include` <esp_wifi.h>
// Call after the station interface has started.
esp_err_t err = esp_wifi_set_ps(WIFI_PS_NONE);
if (err != ESP_OK) {
// Handle or log the failure.
}To restore the normal default mode: esp_wifi_set_ps(WIFI_PS_MIN_MODEM);Espressif documents For WLED, this should be a global Wi-Fi policy, not an operation inside This setting affects only Wi-Fi station operation. It does not apply to Ethernet receivers. It can increase power consumption, so it should remain opt-in. It can reduce DTIM-related receive latency, but it will not repair missing IGMP queries or access-point multicast filtering. 🧠 Learnings usedYou are interacting with an AI system. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Short summary: yes it is. Its a strong recommendation to use "disable wifi sleep" whenever sound sync is used => improvement for WLED-Docs. |
|
The AudioReactive broadcast transport was computing the broadcast address inline, which softhack007 rightly called fragile - and checking the core turned up two different open-coded ways of doing it rather than a pattern to follow. Add broadcastIP() to WLEDNetworkClass, next to localIP(), subnetMask() and gatewayIP(). It returns the directed broadcast of whichever interface is up, falls back to the limited broadcast while disconnected, and gives us a single place to revisit for IPv6. The state notifier in udp.cpp now uses it too, replacing its open-coded '~subnetMask | gatewayIP'.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- PlatformIO configuration candidates ---'
fd -a -t f -i 'platformio|platform|package' . | head -n 160
printf '%s\n' '--- V5 environment definitions and platform references ---'
rg -n -i -C 4 '^\[env:.*v5|v5|tasmota|pioarduino|platform(_packages)?\s*=|framework-arduinoespressif32' \
-g 'platformio*.ini' -g '*.ini' -g '*.json' -g '*.yml' -g '*.yaml' .
printf '%s\n' '--- Git submodules (if any) ---'
git submodule status || true
printf '%s\n' '--- Current repository revision ---'
git rev-parse HEAD
git remote -v | head -n 4Length of output: 75396 🤖 web_search executed:
Length of output: 13121 🏁 Script executed: #!/bin/bash
set -euo pipefail
workdir=$(mktemp -d)
trap 'rm -rf "$workdir"' EXIT
inspect_platform() {
local name="$1"
local url="$2"
local archive="$workdir/$name.zip"
local dir="$workdir/$name"
echo "--- $name: download and package metadata ---"
curl --fail --location --silent --show-error "$url" --output "$archive"
unzip -q "$archive" -d "$dir"
echo "Archive SHA-256:"
sha256sum "$archive"
echo "Relevant package declarations:"
rg -n -i -C 3 \
'framework-arduinoespressif32|arduino-esp32|version|url|package' \
"$dir" -g 'platform.json' -g 'package.json' -g 'package.py' -g '*.json' \
| head -n 220 || true
echo "Embedded Ethernet fix symbols, if framework source is present:"
rg -n -C 3 \
'receiveAllMulticast|addMulticastFilter|ETH_CMD_S_ALL_MULTICAST' \
"$dir" | head -n 160 || true
}
inspect_platform \
tasmota \
https://github.com/tasmota/platform-espressif32/releases/download/2026.05.50/platform-espressif32.zip
inspect_platform \
pioarduino \
https://github.com/pioarduino/platform-espressif32/releases/download/55.03.39/platform-espressif32.zip
echo "--- Upstream Arduino-ESP32 issue, PR, releases, and fix commit ancestry ---"
gh api repos/espressif/arduino-esp32/issues/12408 \
--jq '{number, state, created_at, closed_at, body, pull_request}'
gh api repos/espressif/arduino-esp32/pulls/12409 \
--jq '{number, state, merged_at, merge_commit_sha, base:{ref:.base.ref,sha:.base.sha}, body}'
merge_sha=$(gh api repos/espressif/arduino-esp32/pulls/12409 --jq '.merge_commit_sha')
echo "merge_sha=$merge_sha"
gh api "repos/espressif/arduino-esp32/commits/$merge_sha/branches-where-head" --paginate \
--jq '.[] | {name}'
gh api repos/espressif/arduino-esp32/releases --paginate \
--jq '.[] | select(.published_at >= "2026-02-28T00:00:00Z") | {tag_name, published_at, target_commitish, prerelease}' \
| head -n 80Length of output: 54530 🏁 Script executed: #!/bin/bash
set -euo pipefail
workdir=$(mktemp -d)
trap 'rm -rf "$workdir"' EXIT
inspect_framework() {
local name="$1"
local url="$2"
local archive="$workdir/$name.tar.xz"
local dir="$workdir/$name"
echo "--- $name ---"
curl --fail --location --silent --show-error "$url" --output "$archive"
echo "SHA-256: $(sha256sum "$archive")"
mkdir -p "$dir"
tar -xJf "$archive" -C "$dir"
echo "Package metadata:"
fd -a -t f '^package\.json$' "$dir" -x sh -c 'echo "### $1"; cat "$1"' sh {} \; || true
echo "Ethernet multicast filter implementation:"
rg -n -C 8 \
'receiveAllMulticast|addMulticastFilter|removeMulticastFilter|ETH_CMD_S_ALL_MULTICAST' \
"$dir" -g 'ETH.cpp' -g 'ETH.h' || true
}
inspect_framework \
tasmota_3_3_8_260506 \
https://github.com/tasmota/arduino-esp32/releases/download/v3.3.8.260506/framework-arduinoespressif32.tar.xz
inspect_framework \
official_3_3_9 \
https://github.com/espressif/arduino-esp32/releases/download/3.3.9/esp32-core-3.3.9.tar.xz
echo "--- Official Arduino-ESP32 2.0.18 tag ---"
git clone --quiet --filter=blob:none --no-checkout https://github.com/espressif/arduino-esp32.git "$workdir/arduino"
git -C "$workdir/arduino" sparse-checkout set --no-cone libraries/Ethernet/src/ETH.cpp libraries/Ethernet/src/ETH.h package/package.json
git -C "$workdir/arduino" checkout --quiet 2.0.18
echo "Relevant 2.0.18 Ethernet initialization and multicast APIs:"
rg -n -C 10 \
'begin\s*\(|receiveAllMulticast|addMulticastFilter|ETH_CMD_S_ALL_MULTICAST|esp_eth_ioctl' \
"$workdir/arduino/libraries/Ethernet/src/ETH.cpp" \
"$workdir/arduino/libraries/Ethernet/src/ETH.h" || trueLength of output: 2294 🏁 Script executed: #!/bin/bash
set -euo pipefail
workdir=$(mktemp -d)
trap 'rm -rf "$workdir"' EXIT
inspect_framework() {
local name="$1"
local url="$2"
local archive="$workdir/$name.tar.xz"
local dir="$workdir/$name"
echo "--- $name ---"
curl --fail --location --silent --show-error "$url" --output "$archive"
echo "SHA-256: $(sha256sum "$archive")"
mkdir -p "$dir"
python3 - "$archive" "$dir" <<'PY'
import sys
import tarfile
archive, destination = sys.argv[1:]
with tarfile.open(archive, mode="r:xz") as tar:
tar.extractall(destination, filter="data")
PY
echo "Package metadata:"
fd -a -t f '^package\.json$' "$dir" -x sh -c 'echo "### $1"; cat "$1"' sh {} \; || true
echo "Ethernet multicast filter implementation:"
rg -n -C 8 \
'receiveAllMulticast|addMulticastFilter|removeMulticastFilter|ETH_CMD_S_ALL_MULTICAST' \
"$dir" -g 'ETH.cpp' -g 'ETH.h' || true
}
inspect_framework \
tasmota_3_3_8_260506 \
https://github.com/tasmota/arduino-esp32/releases/download/v3.3.8.260506/framework-arduinoespressif32.tar.xz
inspect_framework \
official_3_3_9 \
https://github.com/espressif/arduino-esp32/releases/download/3.3.9/esp32-core-3.3.9.tar.xz
echo "--- Official Arduino-ESP32 2.0.18 tag ---"
git clone --quiet --filter=blob:none --no-checkout https://github.com/espressif/arduino-esp32.git "$workdir/arduino"
git -C "$workdir/arduino" sparse-checkout set --no-cone libraries/Ethernet/src/ETH.cpp libraries/Ethernet/src/ETH.h
git -C "$workdir/arduino" checkout --quiet 2.0.18
echo "Relevant 2.0.18 Ethernet initialization and multicast APIs:"
rg -n -C 10 \
'begin\s*\(|receiveAllMulticast|addMulticastFilter|ETH_CMD_S_ALL_MULTICAST|esp_eth_ioctl' \
"$workdir/arduino/libraries/Ethernet/src/ETH.cpp" \
"$workdir/arduino/libraries/Ethernet/src/ETH.h" || trueLength of output: 28586 🏁 Script executed: #!/bin/bash
set -euo pipefail
workdir=$(mktemp -d)
trap 'rm -rf "$workdir"' EXIT
platform_url='https://github.com/tasmota/platform-espressif32/releases/download/2024.06.00/platform-espressif32.zip'
platform_zip="$workdir/platform.zip"
platform_dir="$workdir/platform"
curl --fail --location --silent --show-error "$platform_url" --output "$platform_zip"
unzip -q "$platform_zip" -d "$platform_dir"
platform_json=$(fd -a -t f '^platform\.json$' "$platform_dir" | head -n 1)
echo '--- Tasmota platform 2024.06.00 framework declaration ---'
python3 - "$platform_json" <<'PY'
import json
import sys
with open(sys.argv[1], encoding="utf-8") as f:
p = json.load(f)
print(p["packages"]["framework-arduinoespressif32"]["version"])
print(p["packages"]["framework-espidf"]["version"])
PY
framework_url=$(python3 - "$platform_json" <<'PY'
import json
import sys
with open(sys.argv[1], encoding="utf-8") as f:
print(json.load(f)["packages"]["framework-arduinoespressif32"]["version"])
PY
)
framework_archive="$workdir/framework.tar.xz"
framework_dir="$workdir/framework"
curl --fail --location --silent --show-error "$framework_url" --output "$framework_archive"
echo "Framework SHA-256: $(sha256sum "$framework_archive")"
mkdir -p "$framework_dir"
python3 - "$framework_archive" "$framework_dir" <<'PY'
import sys
import tarfile
with tarfile.open(sys.argv[1], mode="r:xz") as tar:
tar.extractall(sys.argv[2], filter="data")
PY
echo '--- Package metadata ---'
fd -a -t f '^package\.json$' "$framework_dir" -x sh -c 'echo "### $1"; cat "$1"' sh {} \;
echo '--- Ethernet multicast-related implementation ---'
rg -n -C 10 \
'receiveAllMulticast|addMulticastFilter|ETH_CMD_S_ALL_MULTICAST|ETH_CMD_[A-Z_]*MULTICAST|esp_eth_ioctl|esp_eth_start' \
"$framework_dir" -g 'ETH.cpp' -g 'ETH.h' || trueLength of output: 3997 🏁 Script executed: #!/bin/bash
set -euo pipefail
workdir=$(mktemp -d)
trap 'rm -rf "$workdir"' EXIT
framework_url='https://github.com/tasmota/arduino-esp32/releases/download/2.0.18/framework-arduinoespressif32.zip'
framework_archive="$workdir/framework.zip"
framework_dir="$workdir/framework"
curl --fail --location --silent --show-error "$framework_url" --output "$framework_archive"
echo "Framework SHA-256: $(sha256sum "$framework_archive")"
mkdir -p "$framework_dir"
python3 - "$framework_archive" "$framework_dir" <<'PY'
import shutil
import sys
shutil.unpack_archive(sys.argv[1], sys.argv[2])
PY
echo '--- Package metadata ---'
fd -a -t f '^package\.json$' "$framework_dir" -x sh -c 'echo "### $1"; cat "$1"' sh {} \;
echo '--- Ethernet initialization and multicast-related implementation ---'
rg -n -C 10 \
'receiveAllMulticast|addMulticastFilter|ETH_CMD_S_ALL_MULTICAST|ETH_CMD_[A-Z_]*MULTICAST|esp_eth_ioctl|esp_eth_start' \
"$framework_dir" -g 'ETH.cpp' -g 'ETH.h' || true
echo '--- Ethernet begin implementations ---'
rg -n -C 20 'bool ETHClass::begin' "$framework_dir" -g 'ETH.cpp' || trueLength of output: 16617
1. V5 buildsBoth V5 platform choices include the fix for Arduino-ESP32 issue
In both framework archives, receiveAllMulticast(true);after Ethernet initialization. The call is enabled for ESP-IDF 5.5.0 and later. It configures the Ethernet MAC to receive all multicast frames, including The Tasmota V5 package contains this call at Lines 469 and 945. The pioarduino Arduino-ESP32 3.3.9 package contains it at Lines 467 and 943. 2. Tasmota Arduino-ESP32 2.0.18The Tasmota V4 platform uses:
This framework does not contain That is expected. The reported regression started in Arduino-ESP32 3.3.6 with the newer ESP-IDF 5.5 Ethernet behavior. The upstream fix is also conditionally compiled only for ESP-IDF 5.5.0 and later. Therefore, the specific The upstream report is also Ethernet-specific. It used an ESP32 with a LAN8720 PHY and the 🧠 Learnings usedYou are interacting with an AI system. |
| if (ip[0] == 0) { | ||
| return IPAddress(255, 255, 255, 255); // not connected yet: limited broadcast | ||
| } | ||
| return IPAddress(uint32_t(ip) | ~uint32_t(subnetMask())); |
There was a problem hiding this comment.
🤔 not sure this is the right solution. @netmindz @willmmiles what do you think?
There was a problem hiding this comment.
@coderabbitai other network features in the WLED core - especially UDP sync and real-time protocols - also include broadcast support. How do these modules find the right "broadcast" IP address to use?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== WLEDNetwork broadcastIP declarations and call sites =='
rg -n -C 4 '\bbroadcastIP\s*\(' wled00 usermods --glob '*.{cpp,h,hpp,ino}' || true
printf '%s\n' '== Existing directed/limited broadcast address construction and broadcast send destinations =='
rg -n -i -C 5 'broadcast|255\s*,\s*255\s*,\s*255\s*,\s*255|subnetMask\s*\(|gatewayIP\s*\(' wled00 --glob '*.{cpp,h,hpp,ino}' || true
printf '%s\n' '== UDP notifier and real-time protocol send paths =='
rg -n -i -C 4 'notify|realtime|E1\.31|Art-?Net|DDP|TPM2' wled00 --glob '*.{cpp,h,hpp,ino}' || trueLength of output: 169880
@softhack007, the core does not use one common broadcast-address method in all modules.
- WLED UDP sync notifications now send to
WLEDNetwork.broadcastIP()inwled00/udp.cppat Line 196. This produces the directed broadcast address from the active interface IP and subnet mask. - Node discovery sends to the fixed limited broadcast address
255.255.255.255insendSysInfoUDP()atwled00/udp.cppLines 730-733. - Network LED buses for DDP and Art-Net do not discover a broadcast address.
BusNetworkstores a configured_clientIP address and passes it torealtimeBroadcast(). The DDP and Art-Net send paths use that address directly. A user can configure a broadcast address as the destination. - E1.31, DDP, Art-Net, TPM2.NET, and Hyperion are primarily receiver paths in WLED. They bind and receive packets. They do not select a broadcast destination.
- Art-Net poll replies go back to the poll sender’s IP address. They are not broadcast.
So, before this change, there was no consistent core helper for directed IPv4 broadcast. The new WLEDNetwork.broadcastIP() centralizes the correct calculation for the active Wi-Fi or Ethernet interface. It also replaces the state notifier’s previous inline calculation. The fixed 255.255.255.255 node-discovery behavior remains separate by design.
You are interacting with an AI system.
There was a problem hiding this comment.
@coderabbitai broadcast for the protocols you listed already works in the main branch.
a) is the proposed addition correct from UDP and IP protocol point of view?
b) is the suggested addition consistent with other protocols that already support broadcast?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== PR checkout: broadcast-related sender/address code =='
rg -n -i -C 5 'broadcast|INADDR_BROADCAST|255\s*,\s*255\s*,\s*255\s*,\s*255|subnetMask\s*\(|gatewayIP\s*\(' \
wled00 usermods \
--glob '*.{cpp,h,hpp,ino}' \
-g '!**/html_*.h' || true
printf '%s\n' '== GitHub main: candidate files and relevant lines =='
for path in \
wled00/udp.cpp \
wled00/src/dependencies/network/Network.cpp \
wled00/src/dependencies/network/Network.h \
wled00/bus_manager.cpp \
wled00/e131.cpp \
wled00/src/dependencies/network/Network.h
do
printf '\n-- main:%s --\n' "$path"
gh api "repos/wled/WLED/contents/$path?ref=main" --jq .content 2>/dev/null \
| base64 -d 2>/dev/null \
| rg -n -i -C 5 'broadcast|INADDR_BROADCAST|255\s*,\s*255\s*,\s*255\s*,\s*255|subnetMask\s*\(|gatewayIP\s*\(' || true
done
printf '%s\n' '== GitHub main: locate likely broadcast send code =='
gh api 'search/code?q=repo:wled/WLED+%28broadcast+OR+INADDR_BROADCAST%29' --paginate \
--jq '.items[] | [.path, .html_url] | `@tsv`' 2>/dev/null || trueLength of output: 57817
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== PR checkout: broadcast-related sender/address code =='
rg -n -i -C 5 'broadcast|INADDR_BROADCAST|255\s*,\s*255\s*,\s*255\s*,\s*255|subnetMask\s*\(|gatewayIP\s*\(' \
wled00 usermods \
--glob '*.{cpp,h,hpp,ino}' \
-g '!**/html_*.h' || true
printf '%s\n' '== GitHub main: candidate files and relevant lines =='
for path in \
wled00/udp.cpp \
wled00/src/dependencies/network/Network.cpp \
wled00/src/dependencies/network/Network.h \
wled00/bus_manager.cpp \
wled00/e131.cpp
do
printf '\n-- main:%s --\n' "$path"
gh api "repos/wled/WLED/contents/$path?ref=main" --jq .content 2>/dev/null \
| base64 -d 2>/dev/null \
| rg -n -i -C 5 'broadcast|INADDR_BROADCAST|255\s*,\s*255\s*,\s*255\s*,\s*255|subnetMask\s*\(|gatewayIP\s*\(' || true
done
printf '%s\n' '== GitHub main: locate likely broadcast send code =='
gh api 'search/code?q=repo:wled/WLED+%28broadcast+OR+INADDR_BROADCAST%29' --paginate \
--jq '.items[] | [.path, .html_url] | `@tsv`' 2>/dev/null || trueLength of output: 57495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir=$(mktemp -d)
trap 'rm -rf "$tmpdir"' EXIT
for path in wled00/udp.cpp wled00/src/dependencies/network/Network.cpp wled00/bus_manager.cpp wled00/e131.cpp; do
out="$tmpdir/$(basename "$path")"
if gh api "repos/wled/WLED/contents/$path?ref=main" --jq .content | base64 -d > "$out"; then
echo "===== main/$path ====="
rg -n -i -C 6 'broadcast|255\.255\.255\.255|255\s*,\s*255\s*,\s*255\s*,\s*255|subnetMask\s*\(|gatewayIP\s*\(' "$out" || true
fi
done
echo '===== PR checkout: exact prior state-notifier code context ====='
sed -n '175,210p' wled00/udp.cppLength of output: 9694
@softhack007, the inspection confirms that the proposed calculation is both protocol-correct and already used by an existing WLED UDP sender.
mainuses~subnetMask() | gatewayIP()for core UDP state notifications inwled00/udp.cppat Line 196.usermods/udp_name_sync/udp_name_sync.cppalready useslocalIP() | ~subnetMask()at Line 39.- The PR moves the state notifier to
WLEDNetwork.broadcastIP()and uses the samelocalIP() | ~subnetMask()calculation.
For a normal configuration, where the gateway is inside the active interface subnet, these expressions produce the same directed broadcast address. For example, both produce 192.168.1.255 on 192.168.1.0/24.
The localIP() form is the better source for this helper. A directed broadcast address derives from the active interface network, not from the default gateway. It also works when the interface has no gateway, and it aligns with the existing udp_name_sync usermod.
The fixed 255.255.255.255 address in sendSysInfoUDP() is intentionally different. It is a limited broadcast for node discovery. The network LED buses also support broadcast because BusNetwork forwards its configured _client destination to realtimeBroadcast(). They do not calculate a destination address internally.
Therefore, this PR is consistent with existing WLED behavior:
- It preserves node discovery’s limited broadcast.
- It preserves configured broadcast destinations for network LED buses.
- It centralizes the directed-broadcast calculation already used by UDP name sync.
- It replaces the core notifier’s gateway-based variant with the more direct interface-based variant.
You are interacting with an AI system.
There was a problem hiding this comment.
@efranceschi OK, this looks reasonable from my perspective, especially as you have already done some confidence testing with your code. But I would like to hear the opinion of other maintainers (@DedeHai @willmmiles @netmindz) before we add it to the WLED core.
There was a problem hiding this comment.
I'm happy to take this addition. Moving this calculation to our network utils library so it can be used consistently is a good idea, and localIP() is strictly superior to gatewayIP() for this purpose.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Thanks. One detail that matters for the case @softhack007 was asking about: when there is no station connection - AP mode, or STA down - localIP() is 0.0.0.0, so broadcastIP() returns the limited broadcast 255.255.255.255 instead. The old expression produced ~mask | 0 = 0.0.0.255 there, which goes nowhere.
| DEBUG_PRINTLN(F("UDP sending packet.")); | ||
| IPAddress broadcastIp = ~uint32_t(WLEDNetwork.subnetMask()) | uint32_t(WLEDNetwork.gatewayIP()); | ||
| notifierUdp.beginPacket(broadcastIp, udpPort); | ||
| notifierUdp.beginPacket(WLEDNetwork.broadcastIP(), udpPort); |
There was a problem hiding this comment.
@coderabbitai please suggest a testcase to validate this change (common case = same behaviour, special case = behaviour improvement)
There was a problem hiding this comment.
@softhack007, use these two tests for notify().
-
Common case: retain the current directed-broadcast behavior
Configure a connected interface with:
- Local IP:
192.168.10.42 - Subnet mask:
255.255.255.0 - Gateway:
192.168.10.1 udpConnected = true
Trigger
notify()with an enabled notification mode.Verify that
notifierUdp.beginPacket()receives:destination = 192.168.10.255 port = udpPortThis validates that the new
WLEDNetwork.broadcastIP()call preserves the previous result on a normal/24network. - Local IP:
-
Special case: no gateway, but a valid local interface address
Configure a connected interface with:
- Local IP:
192.168.10.42 - Subnet mask:
255.255.255.0 - Gateway:
0.0.0.0 udpConnected = true
Trigger
notify()again.Verify that
notifierUdp.beginPacket()receives:destination = 192.168.10.255 port = udpPortThe previous expression would produce
0.0.0.255because it usesgatewayIP(). The new implementation must derive the directed broadcast address fromlocalIP()andsubnetMask(). This is the behavior improvement. - Local IP:
A useful additional assertion is a non-/24 subnet, for example 192.168.10.42 with mask 255.255.252.0. The expected destination is 192.168.11.255. This confirms that the mask calculation is not limited to /24 networks.
You are interacting with an AI system.
There was a problem hiding this comment.
Ran both cases on hardware. Sender: ESP32 running this branch, four stock 16.0.1 receivers, colour sync measured by polling each receiver after every change.
Common case (local IP 192.168.68.95, mask 255.255.255.0, gateway 192.168.68.1): 12/12 changes delivered to all four receivers, before and after the change. Same behaviour, as expected - both expressions yield 192.168.68.255 here.
Special case as suggested (gateway 0.0.0.0 with a static IP): not reachable through WLED's own config. I set it up expecting the old expression to break, and it did not - 12/12 again. The reason is wled.cpp:794:
if (uint32_t(multiWiFi[selectedWiFi].staticIP) != 0U && uint32_t(multiWiFi[selectedWiFi].staticGW) != 0U) {
WiFi.config(...);
}A static IP without a gateway is silently ignored and the node falls back to DHCP, so gatewayIP() never actually returns zero that way.
Where the two do diverge is when there is no station connection at all - AP mode, or the STA interface down - because gatewayIP() then returns INADDR_NONE:
| old expression | WLEDNetwork.broadcastIP() |
|
|---|---|---|
| STA up | 192.168.68.255 |
192.168.68.255 |
| no STA (AP mode) | ~mask | 0 = 0.0.0.255 |
255.255.255.255 |
So the regression test is "same address while connected", which I verified, and the improvement shows up in AP mode. I did not exercise that end to end, since it would mean moving the receivers onto the sender's own AP.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@coderabbitai does this line also need adjustment for the broadcast case? |
This comment was marked as resolved.
This comment was marked as resolved.
@coderabbitai I think this was the intention of the PR: either use multicast, or use broadcast. Otherwise there would be no difference on the receiver side, and the "transport" option would only affect behaviour of the sender. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target=$(fd -a -t f '^audio_reactive\.cpp$' usermods | head -n 1)
if [ -z "$target" ]; then
echo "audio_reactive.cpp not found" >&2
exit 1
fi
echo "--- relevant symbol outline ---"
ast-grep outline "$target" --items all --match 'audioSyncTransport|beginMulticast|transmitAudioData|readFromConfig|addToConfig' --view expanded
echo "--- transport, socket, receive, and config contexts ---"
rg -n -C 14 'audioSyncTransport|beginMulticast|fftUdp\.begin\s*\(|parsePacket\s*\(|remoteIP\s*\(|transmitAudioData\s*\(|readFromConfig\s*\(|addToConfig\s*\(' "$target"
echo "--- PR files and diff ---"
git diff --find-renames origin/main...HEAD -- "$target" 2>/dev/null || git diff --find-renames HEAD^...HEAD -- "$target"Length of output: 23798
For the intended mutually exclusive transport mode, if (audioSyncTransport == 1) {
udpSyncConnected = fftUdp.begin(audioSyncPort);
} else {
`#ifdef` ARDUINO_ARCH_ESP32
udpSyncConnected = fftUdp.beginMulticast(IPAddress(239, 0, 0, 1), audioSyncPort);
`#else`
udpSyncConnected = fftUdp.beginMulticast(WiFi.localIP(), IPAddress(239, 0, 0, 1), audioSyncPort);
`#endif`
}This change ensures that broadcast-mode receivers do not join the multicast group. It also makes the setting a true transport selection on both sender and receiver. The setting takes effect when 🧠 Learnings usedYou are interacting with an AI system. |
@netmindz this means the receiver can actually handle everything (the UDP port is always used correctly). Maybe we should keep this behaviour - its fail-safe and users only need to set the transport option on the sender side. What do you think? |
|
Confirmed the root cause, with a test rather than an inference. The network has no IGMP querier. I ran an IGMPv2 General Query from a laptop on the same subnet. The detail that matters for anyone reproducing this: the query is ignored unless it carries the IP Router Alert option (RFC 2113) - my first attempt without it changed nothing at all. With a proper query every 30 s, receivers that had already been pruned came back on their own, without rebooting: Reception went from 50% of samples to 98%, purely by adding queries to the network. That also settles the regression question. They only recovered because they answered the query, so on this firmware (16.0.1, core 4.4.8.240628) the IGMP responder works and espressif/arduino-esp32#12408 does not explain the behaviour here. Still worth checking separately on V5 builds, where the core is 3.x based. On power save: all five nodes already run with On keeping IGMP happy from the firmware side: the usermod joins once in |
@efranceschi not necessary to add a PR for this - we will soon migrate to the out-of-tree audioreactive (WLED-MM based). The out-of-tree AR already has reconnection logic. |
@efranceschi "I" = your AI coding agent? I'm just asking because we usually prefer to talk directly, instead of remotely driving an AI session... |
|
@softhack007 We are duplicating trying to address issues we have already addressed in the MM version. AC is missing reconnection code and i have also added broadcast support to work around various multicast issues with some networks |
|
@softhack007 I agree with @netmindz, if any changes from this PR can be used in upstream MM AR, they should be added there. AC AR is currently in a limbo state, I could merge my two pending PR's then update the code to use the ADC manager just to serve as a reference for the changes to port them to MM later. |
netmindz
left a comment
netmindz
left a comment
There was a problem hiding this comment.
Not an issue with the code as such, just duplication, so don't then want issue of breaking change when we swap
5 participants
audioreactive: add UDP broadcast as an alternative sync transport
Problem
Sound sync only sends over multicast (
239.0.0.1), which depends on the network keeping ourgroup membership alive. Access points that run IGMP snooping without an IGMP querier never ask
members to renew, so the membership expires after the default ~260 s group membership interval
and the AP stops forwarding the group. Nothing looks wrong in WLED - the socket is still open,
the info page still says "receive mode" - but no audio arrives until the node reboots.
Same symptom as #4408, closed as stale. Reproducible on demand here.
Change
A
Transportdropdown in the AudioReactive sync settings lets the sender use UDP broadcastinstead of multicast. Broadcast has no group membership, so IGMP snooping does not affect it.
transport = 0is multicast.Verified with four receivers on stock 16.0.1.
uint8_trather thanboolso a third transport can be added later without a configmigration, as suggested in Adding ESPNOW connection to sync multiple hardware with sound reactive. #5637. Any value other than
1falls back to multicast.usermods/audioreactive/.This is not a new pattern for WLED: the state notifier already broadcasts, and builds its
address the same way. On this network colour sync keeps working while sound sync dies - same
devices, different transport.
Testing
5x ESP32 DevKit, INMP441 mic on the sender, TP-Link Deco mesh (4 APs), receivers on stock
16.0.1 while the sender runs this branch.
Before: every receiver stops ~260 s after joining the group and never recovers. A 6 minute
tcpdump -i en0 igmprecorded zero IGMP packets - the mesh never queries.After (broadcast): ran for 9 hours overnight, polling all four receivers every 2 minutes.
Not once did a receiver report
idle- the transport was never pruned - and the sender ran thewhole period without a reboot, with stable heap.
Also verified: multicast still works with
transport = 0(664 packets in 15 s), the settingsurvives a reboot and saves from the Usermods page, and
esp32dev,esp32_wrover,esp32s3dev_8MB_qspiandesp32c3devall build clean.Not tested: ESP8266 receivers, AP mode, and networks that rate limit broadcast - which is
why this is an option rather than a new default.
Related
259 s) was rejected because it restarts audio processing; this change touches neither
sampling nor FFT.
@softhack007 suggested a "transport" option with UDP broadcast as a candidate.
AI disclosure
Written with AI assistance, reviewed line by line and tested on my own hardware.
Summary by CodeRabbit
New Features
Bug Fixes