Fix: ESP32-S3 DHCP Hostname timing issue (Option 12) for OpenWrt/dnsmasq#5397
Fix: ESP32-S3 DHCP Hostname timing issue (Option 12) for OpenWrt/dnsmasq#5397
Conversation
Add hostname configuration for WiFi connection based on cmDNS.
WalkthroughThe change adds WiFi hostname configuration on ESP32 within Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/wled.cpp`:
- Around line 868-877: The DHCP hostname is being set in handleConnection but
initConnection later calls WiFi.begin() and sets a different hostname from
prepareHostname(), causing inconsistent hostnames; move the hostname setup to
initConnection before the call to WiFi.begin() and use a single source (prefer
cmDNS if non-empty else WLED_HOST_NAME or the result of prepareHostname()) by
replacing any WiFi.setHostname() in handleConnection with a single
WiFi.setHostname(...) in initConnection prior to WiFi.begin(), and remove the
redundant WiFi.setHostname calls so hostname is configured exactly once (refer
to symbols: initConnection, handleConnection, prepareHostname, cmDNS,
WLED_HOST_NAME, WiFi.begin, WiFi.setHostname).
| // The DNS name of the ESP must be set before the first connection to the DHCP server. Otherwise, the esp default name, such as “esp32s3-267D0C,” will be used. GG | ||
| #ifdef ARDUINO_ARCH_ESP32 | ||
| // Use cmDNS (this is the buffer that holds your name) | ||
| if (cmDNS && cmDNS[0] != '\0') { | ||
| WiFi.setHostname(cmDNS); | ||
| } else { | ||
| // Fallback to DEVICE_NAME if cmDNS is still empty | ||
| WiFi.setHostname(WLED_HOST_NAME); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Set DHCP hostname once, from a single source, before WiFi.begin().
This block sets cmDNS (or WLED_HOST_NAME) in handleConnection, but initConnection() later sets a different hostname derived from prepareHostname() after WiFi.begin(). Routers that cache the first DHCPDISCOVER will keep the earlier value, so the visible hostname can diverge from the configured device name. To avoid mismatches and repeated reconfiguration, set the hostname exactly once in initConnection() before WiFi.begin() using a single chosen source, then remove the later WiFi.setHostname() call.
🔧 Suggested fix (unify hostname source + move before WiFi.begin)
@@ void WLED::handleConnection()
- `#ifdef` ARDUINO_ARCH_ESP32
- // Use cmDNS (this is the buffer that holds your name)
- if (cmDNS && cmDNS[0] != '\0') {
- WiFi.setHostname(cmDNS);
- } else {
- // Fallback to DEVICE_NAME if cmDNS is still empty
- WiFi.setHostname(WLED_HOST_NAME);
- }
- `#endif`
@@ void WLED::initConnection()
- // convert the "serverDescription" into a valid DNS hostname (alphanumeric)
- char hostname[25];
- prepareHostname(hostname);
+ // convert the "serverDescription" into a valid DNS hostname (alphanumeric)
+ char hostname[25];
+ if (cmDNS && cmDNS[0] != '\0') {
+ strncpy(hostname, cmDNS, sizeof(hostname));
+ hostname[sizeof(hostname) - 1] = '\0';
+ } else {
+ prepareHostname(hostname);
+ }
+#ifdef ARDUINO_ARCH_ESP32
+ WiFi.setHostname(hostname); // set before WiFi.begin for DHCP Option 12
+#endif
@@
`#ifdef` ARDUINO_ARCH_ESP32
WiFi.setTxPower(wifi_power_t(txPower));
WiFi.setSleep(!noWifiSleep);
- WiFi.setHostname(hostname);
`#else`
wifi_set_sleep_type((noWifiSleep) ? NONE_SLEEP_T : MODEM_SLEEP_T);
WiFi.hostname(hostname);
`#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/wled.cpp` around lines 868 - 877, The DHCP hostname is being set in
handleConnection but initConnection later calls WiFi.begin() and sets a
different hostname from prepareHostname(), causing inconsistent hostnames; move
the hostname setup to initConnection before the call to WiFi.begin() and use a
single source (prefer cmDNS if non-empty else WLED_HOST_NAME or the result of
prepareHostname()) by replacing any WiFi.setHostname() in handleConnection with
a single WiFi.setHostname(...) in initConnection prior to WiFi.begin(), and
remove the redundant WiFi.setHostname calls so hostname is configured exactly
once (refer to symbols: initConnection, handleConnection, prepareHostname,
cmDNS, WLED_HOST_NAME, WiFi.begin, WiFi.setHostname).
|
Build failure |
|
This has already been (properly) addressed in #4893 quite some time ago but unfortunately project isn't very well maintained recently. |
says who? I thought that you asked not be involved in these topics any more? |
3 participants
Description:
On ESP32-S3 (and other fast S3/C3 targets), the WiFi stack starts the DHCP handshake (DHCPDISCOVER) almost immediately after WiFi.begin(). In the current WLED implementation, WiFi.setHostname() is often called too late, after the first discovery packet has already been sent with the default esp32s3-xxxxxx identifier.
This causes issues in strict network environments like OpenWrt (dnsmasq), where the router caches the first hostname it sees during the lease process and ignores subsequent updates.
Changes:
I 'moved' or better set another WiFi.setHostname() call to the very beginning of WLED::initConnection(). By doing this, the hostname (Option 12) is injected into the LwIP stack before the radio starts transmitting.
The fix uses the dynamic cmDNS buffer but falls back to the hardcoded DEVICE_NAME if the config hasn't been loaded yet (first boot).
I successfully tested the fix with my OpenWrt server (logread -f | grep -i dhcp) and an ESP32-S3-DevKitC-N16R8 (ESP32-S3 (QFN56) (revision v0.2)) as well as an ESP32-DevKitC (ESP32-D0WDQ6 (revision v1.0)).
Summary by CodeRabbit