Skip to content

Stop mDNS before Wi-Fi teardown in initConnection() (#5945) - #5965

Open
sionnn9 wants to merge 3 commits into
wled:mainfrom
sionnn9:fix/mdns-stop-before-wifi-teardown
Open

sionnn9 wants to merge 3 commits into
wled:mainfrom
sionnn9:fix/mdns-stop-before-wifi-teardown

Conversation

@sionnn9

@sionnn9 sionnn9 commented Nov 10, 2026 •

Copy link
Copy Markdown

What it does

The issue was that when an ESP32-C5 switches the Wi-Fi band and reconnects, the board could crash, and a reboot fixes it. So what we did is stop mDNS before the Wi-Fi interface is destroyed. It is started again once the board has reconnected.

Additional references and context

How it works

We updated initConnection() in wled00/wled.cpp. Before WiFi.disconnect(true) it calls MDNS.end(), only if interfacesInited && strlen(cmDNS) > 0, meaning mDNS is actually running. It is inside #ifdef ARDUINO_ARCH_ESP32, so ESP8266 is unchanged. No restart code was added, because initInterfaces() already calls MDNS.end() and MDNS.begin() once the board is connected.

AI assistance: yes

  • What the AI drafted: we used AI to understand the code and the architecture and how it works to understand the error we reffered a comment onthe issue which gave us a breif understanding of where the issue was.
  • What I verified/tested myself: we reproduced the crash in an emulator simulating the ESP32-C5 board (details in the README linked above).
  • Untested / unsure about: we did not test on a real ESP32-C5, and we did not build locally.

Testing and Validation

Emulator: first we tested the unchanged code to recreate the error. It panicked on cycle 2 of 10 (backtrace: join_group in mDNS). With the change, there were no errors in three runs of 10 cycles. The emulator has one band, so there was no real 5 GHz connection. A temporary test harness triggered the band change and is not part of this PR.

  • Tested on actual hardware:
    • No, not tested on a real ESP32-C5.
  • Open ends, known limitations, breaking changes:
    • Not built locally because of a PlatformIO setup problem on my machine, so I'm relying on CI. One crash reproduction only, so no crash rate.
  • Anything you couldn't quite solve?
    • A real 2.4 GHz to 5 GHz test on hardware. It would be great if a maintainer with a C5 and a dual-band router could try it.

Checklist

  • I have personally reviewed all changes (incl. source code) before opening this PR.
  • I'm ready to explain my proposed source code changes in detail (without an AI agent answering for me).
  • I agree to follow this project's Code of Conduct

Summary by CodeRabbit

  • Bug Fixes
    • Improved local network service discovery during Wi-Fi reconnections by stopping mDNS before the network interface resets and restarting it after reconnection.
    • Improved cleanup of over-the-air update services when Wi-Fi disconnects.

@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Walkthrough

On ESP32, WLED::initConnection() now stops mDNS and, when AOTA is enabled, ArduinoOTA before disconnecting Wi-Fi and switching the interface to WIFI_MODE_NULL. The code logs this shutdown. initInterfaces restarts mDNS after reconnecting.

Changes

mDNS and Wi-Fi reconnection

Layer / File(s) Summary
Stop network services before Wi-Fi teardown
wled00/wled.cpp
On ESP32, WLED::initConnection() logs the shutdown and stops mDNS and, when enabled, ArduinoOTA before Wi-Fi teardown.

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

Change: Bug fix

Possibly related issues

Possibly related PRs

  • wled/WLED#5719 — The HomeWizard usermod uses mDNS for meter discovery, which may be affected while mDNS is stopped during Wi-Fi teardown.

Suggested labels: bug, AI

Suggested reviewers: dedehai

Merge Risk: Minimal · up to f3191

ESP32 network services are stopped before Wi-Fi teardown. No actionable merge-blocking risk was established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: stopping mDNS before Wi-Fi teardown in initConnection(). It does not mention the secondary ArduinoOTA teardown, which is not required in the title.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@softhack007

Copy link
Copy Markdown
Member

thanks, the change looks reasonable. Needs testing on a real -C5 (and on -S3 for regression tests) before merging.

@softhack007

Copy link
Copy Markdown
Member

@coderabbitai are there other protocol handlers that should be stopped before Wi-Fi teardown, for example asyncWebserver?

@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

Analysis chain

Script executed:

#!/bin/bash
echo '=== Connection lifecycle outline ==='
ast-grep outline wled00/wled.cpp --items all --match 'initConnection|initInterfaces|handleConnection|WLED' --view expanded
echo '=== Connection and service lifecycle references ==='
rg -n -C 3 'initConnection\s*\(|initInterfaces\s*\(|interfacesInited|WiFi\.disconnect\s*\(|WIFI_MODE_NULL|MDNS\.(begin|end)|ArduinoOTA\.(begin|end)' wled00
echo '=== Web server and protocol references ==='
rg -n -C 2 'AsyncWebServer|server\.(begin|end|reset)|ws\.(begin|end|closeAll)|udp\.(begin|stop)|\.stop\(\)|mqtt\.(connect|disconnect)' wled00 -g '*.{cpp,h}'
echo '=== Network library version declarations ==='
rg -n -i -C 2 'ESPAsyncWebServer|AsyncTCP|ESPAsyncUDP|PubSubClient|AsyncMqttClient|arduino-esp32|platformio' -g '*.{ini,json,yml,yaml}' . | head -n 260

Length of output: 66439


Script executed:

#!/bin/bash
echo '=== AP and Wi-Fi teardown ==='
cat -n wled00/wled.cpp | sed -n '708,810p'
echo '=== Service initialization and reconnect handling ==='
cat -n wled00/wled.cpp | sed -n '921,1115p'
echo '=== Network event handling ==='
ast-grep outline wled00/network.cpp --items all --match 'event|Event' --view expanded
cat -n wled00/network.cpp | sed -n '440,535p'
echo '=== Network service declarations ==='
rg -n -C 2 '\b(notifierUdp|udpConnected|ntpUdp|e131|ddp|dnsServer|mqtt|espalexa|ws)\b' wled00/wled.h | head -n 220
echo '=== Service lifecycle calls ==='
rg -n -C 3 '\b(notifierUdp|ntpUdp|e131|ddp|dnsServer|espalexa|mqtt|ws)\.(begin|end|stop|connect|disconnect|closeAll|cleanupClients|listen|listenMulticast)\s*\(' wled00 -g '*.{cpp,h}'
echo '=== E1.31 and DDP implementation locations ==='
rg -n -C 2 'class (ESPAsyncE131|ESPAsyncDDP|.*E131|.*DDP)|bool (ESPAsyncE131|ESPAsyncDDP)::begin|void (ESPAsyncE131|ESPAsyncDDP)::' wled00 -g '*.{cpp,h}'
echo '=== Relevant ESP32 platform configuration ==='
cat -n platformio.ini | sed -n '145,155p;260,268p;315,355p;425,455p'

Length of output: 34368


Script executed:

#!/bin/bash
set -u
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
echo '=== Pinned ESPAsyncWebServer source ==='
url='https://github.com/raw/Aircoookie/ESPAsyncWebServer/ac44e32abf2a69ae650412fb6bc193c59ccac38a/src/WebServer.cpp'
if curl -fsSL --max-time 20 "$url" -o "$tmp/WebServer.cpp"; then
  rg -n -C 5 'AsyncWebServer::(begin|end|reset)' "$tmp/WebServer.cpp"
else
  echo 'Pinned ESPAsyncWebServer source unavailable'
fi
echo '=== AsyncTCP 3.4.10 source ==='
url='https://github.com/raw/ESP32Async/AsyncTCP/v3.4.10/src/AsyncTCP.cpp'
if curl -fsSL --max-time 20 "$url" -o "$tmp/AsyncTCP.cpp"; then
  rg -n -C 12 'AsyncServer::(begin|end)|tcp_bind\(|tcp_listen|IP_ADDR_ANY' "$tmp/AsyncTCP.cpp" | head -n 240
else
  echo 'AsyncTCP 3.4.10 source unavailable at tag v3.4.10'
fi

Length of output: 5801


Script executed:

#!/bin/bash
echo '=== E1.31 implementation outline ==='
ast-grep outline wled00/src/dependencies/e131/ESPAsyncE131.cpp --items all --view expanded
ast-grep outline wled00/src/dependencies/e131/ESPAsyncE131.h --items all --match 'ESPAsyncE131' --view expanded
echo '=== E1.31 setup and teardown ==='
cat -n wled00/src/dependencies/e131/ESPAsyncE131.cpp | sed -n '1,100p'
cat -n wled00/src/dependencies/e131/ESPAsyncE131.h | sed -n '218,248p'
echo '=== Alexa initialization ==='
ast-grep outline wled00/alexa.cpp --items all --view expanded
cat -n wled00/alexa.cpp | sed -n '1,60p'
ast-grep outline wled00/src/dependencies/espalexa/Espalexa.h --items all --match 'begin|end|stop|Espalexa' --view expanded
rg -n -C 4 'udp\.(begin|stop|beginMulticast)|joinMulticast|listenMulticast|void end\(|bool begin\(' wled00/src/dependencies/espalexa/Espalexa.h
echo '=== MQTT connection handling ==='
rg -n -C 3 'mqtt->(connect|disconnect)|initMqtt\s*\(|WLEDNetwork\.isConnected\s*\(' wled00/mqtt.cpp wled00/wled.cpp | head -n 180

Length of output: 13467


Script executed:

#!/bin/bash
set -u
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
base='https://github.com/raw/Aircoookie/ESPAsyncWebServer/ac44e32abf2a69ae650412fb6bc193c59ccac38a/src'
for file in WebServer.cpp ESPAsyncWebServer.h; do
  if curl -fsSL --max-time 20 "$base/$file" -o "$tmp/$file"; then
    echo "=== Pinned web server: $file ==="
    rg -n -C 5 'AsyncWebServer::AsyncWebServer|AsyncWebServer\s*\(' "$tmp/$file" | head -n 100
  else
    echo "$file unavailable"
  fi
done
url='https://github.com/raw/ESP32Async/AsyncTCP/v3.4.10/src/AsyncTCP.cpp'
if curl -fsSL --max-time 20 "$url" -o "$tmp/AsyncTCP.cpp"; then
  echo '=== AsyncTCP 3.4.10 server constructors ==='
  rg -n -C 7 'AsyncServer::AsyncServer' "$tmp/AsyncTCP.cpp"
fi

Length of output: 3977


@softhack007 I do not see evidence that AsyncWebServer must be stopped before Wi-Fi teardown. The pinned server binds its listener to IPADDR_ANY, and AsyncTCP 3.4.10 leaves an existing listener in place on a repeated begin() (server constructor, AsyncTCP). That does not prove active HTTP or WebSocket clients survive teardown safely.

The best candidate for further investigation is E1.31 multicast. ESPAsyncE131::initMulticast() joins groups using the local interface address, and the wrapper has no public stop method (wled00/src/dependencies/e131/ESPAsyncE131.cpp Line 69; ESPAsyncE131.h Line 242). I cannot confirm that it needs explicit teardown or causes the reported crash.

Other services worth checking are the UDP receivers, MQTT, and the captive-portal DNS server. initConnection() does not explicitly stop them. A global shutdown also needs care because the web server and UDP receivers are shared by AP and station initialization.

Before merging, I recommend the requested real ESP32-C5 and ESP32-S3 tests. Exercise repeated reconnects with active HTTP/WebSocket traffic and E1.31 multicast, then verify service recovery, including AP fallback. I have not run those tests.


Learnings used

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-10-03T14:59:43.661Z
Learning: In wled/WLED, softhack007 prefers contributors to answer maintainer questions directly with short explanations in their own words, even if the explanations are technically imprecise or the English is imperfect. Contributors should state uncertainty and actual test results rather than relay unverified AI-generated technical answers. AI may assist with translation or wording, but should not manufacture explanations that shift fact-checking and AI-session supervision onto maintainers.

You are interacting with an AI system.

@sionnn9

sionnn9 commented Nov 11, 2026

Copy link
Copy Markdown
Author

thanks, the change looks reasonable. Needs testing on a real -C5 (and on -S3 for regression tests) before merging.

Hello and thank you for taking a look at it, currently i do not have an ESP32-C5 or an s3 to test it on which is why i went for an emulator testing .so i was wondering if someone with an actual board could possibly test it ?ill update the pr according to the result.

@softhack007 softhack007 added Awaiting testing AI Partly generated by an AI. Make sure that the contributor fully understands the code! labels Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Partly generated by an AI. Make sure that the contributor fully understands the code! Awaiting testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants