Add support for UDP broadcast and multicast for ESP32-S2 and ESP32-C3 #250
Conversation
…ESP32-C3 hinzu Added Broadcast for ESP32s2, i got Multicast never to work.
WalkthroughThe code updates UDP audio synchronization logic to distinguish between ESP32, ESP32-S2, and ESP32-C3 platforms. For ESP32-S2 and ESP32-C3, UDP broadcast is used instead of multicast for audio data transmission and connection setup. Conditional compilation and debug output were also adjusted to include these variants. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant AudioReactive
participant UDP
participant Network
App->>AudioReactive: loop()
AudioReactive->>AudioReactive: (conditional) transmitAudioData()
alt ESP32-S2 or ESP32-C3
AudioReactive->>UDP: Send broadcast packet (255.255.255.255:audioSyncPort)
else ESP32 or others
AudioReactive->>UDP: Send multicast packet (multicastIP:audioSyncPort)
end
UDP->>Network: Transmit packet
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsusermods/audioreactive/audio_reactive.h (2)undefined <retrieved_learning> <retrieved_learning> 🔇 Additional comments (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Rather than base the change based on suspected bug with certain esp32 varients, if we are going to have broadcast, then it should a a checkbox option as part of the sync settings so that it can also help with networks with broken multicast (which is why that option was added to the WLED Audio Server) |
| #endif | ||
|
|
||
| #ifdef ARDUINO_ARCH_ESP32 | ||
| #if defined(ARDUINO_ARCH_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32C3) |
There was a problem hiding this comment.
this change is not necessary, because ARDUINO_ARCH_ESP32 is defined on all esp32 boads (not just on the classic esp32)
| transmitData.FFT_MajorPeak = FFT_MajorPeak; | ||
|
|
||
| #if defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32C3) | ||
| // ESP32-S2 and ESP32-C3: Use broadcast like the working Python script |
There was a problem hiding this comment.
what is "the working python script" ?
I fully agree with @netmindz here. The problem cannot be related to chip type as some people have working setups - in fact my "educated guess" is that the wifi router starts to block our multicast packets at some time. For example, older german Telekom "speedport" routers are regularly cutting the WLED audiosync Multicast connection also with esp32 or -S3. It would be a nice enhancement to offer "use Broadcast instead of Multicast" for all chip types. |
| } | ||
|
|
||
| if ((audioSyncPort > 0) && (audioSyncEnabled > AUDIOSYNC_NONE)) { | ||
| #ifdef ARDUINO_ARCH_ESP32 |
There was a problem hiding this comment.
not necessary to remove this (see my other comment)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
3 participants
Added Broadcast for ESP32s2, i got Multicast never to work.
Summary by CodeRabbit