Hostname unification #4751

blazoncek · 2025-06-27T09:56:46Z

fixes Simplify "device name" across app #1242
fixes empty MQTT topic
fixes missing hostname on change ESP32-S3 and C3 hostname #4586

Summary by CodeRabbit

New Features
- Network Settings: hostname input + mDNS enable with validation and live mDNS URL preview.
- Ethernet Type selector with mutual exclusivity between Ethernet and ESP-NOW.
- MQTT client/device IDs and hostname default to unique MAC-based values when empty.
Improvements
- Consolidated hostname/mDNS handling across setup and runtime; mDNS only active when enabled.
- Safer string handling and immediate application of WiFi options.
- Streamlined network UI and device name validation.
Bug Fixes
- Fixed unsafe string/buffer handling and prevented unconditional mDNS updates.
Style
- Renamed settings buttons and enhanced invalid-input styling.

- fixes #1242 - fixes empty MQTT topic - fixes missing hostname on change #4586

coderabbitai · 2025-06-27T09:57:11Z

Walkthrough

Consolidates hostname/mDNS handling, replaces unsafe fixed-size copies with sizeof-based copying, adds mDNS enable flag, removes prepareHostname, adds resolveHostname, applies hostname immediately to network interfaces, and updates network UI (hostname input, mDNS toggle, Ethernet/ESP-NOW logic).

Changes

Cohort / File(s)	Change Summary
Core hostname & config `wled00/cfg.cpp`, `wled00/set.cpp`, `wled00/wled.cpp`, `wled00/wled.h`, `wled00/wled_server.cpp`, `wled00/xml.cpp`	Consolidated hostname usage to `hostName`, added `mDNSenabled` flag, replaced legacy fixed-copy patterns with `sizeof()`-bounded copies, gated mDNS updates/initialization on `mDNSenabled`, and updated captive portal/OTA/mDNS uses to `hostName`.
Network resolution & events `wled00/network.cpp`, `wled00/fcn_declare.h`	Added `resolveHostname` (optional mDNS resolution) and removed `prepareHostname` declaration; simplified Ethernet hostname setting to use `hostName`.
Utilities removed `wled00/util.cpp`	Removed `prepareHostname` implementation.
UI — network/settings `wled00/data/settings_wifi.htm`, `wled00/data/settings.htm`, `wled00/data/settings_ui.htm`, `wled00/data/style.css`, `wled00/xml.cpp`	Renamed/rewrote network UI to "Network Settings", added hostname input with pattern validation and mDNS checkbox, revamped Ethernet/ESP-NOW mutual exclusivity and related JS (`genUrl()`, `tE()`, `aR()`, `rstR()`), updated button labels and input validation styling.
Improv metadata `wled00/improv.cpp`	Switched to safe snprintf_P formatting and switched use from legacy cmDNS to `hostName` for Improv info payloads.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Simplify "device name" across app: unify device ID usage for server name, mDNS hostname, AP name (#1242)	✅

Possibly related PRs

Add mDNS support for MQTT server #4769 — Modifies hostname resolution and mDNS handling similar to this PR (overlap in resolveHostname and mDNS usage).
Add mDNS resolution for network bus #4768 — Also touches mDNS hostname-to-IP resolution and related hostname variables, closely related to the new resolveHostname behavior.

Suggested reviewers

willmmiles
blazoncek

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 929f58f and 661e1ee.

📒 Files selected for processing (9)

wled00/cfg.cpp (4 hunks)
wled00/data/style.css (1 hunks)
wled00/fcn_declare.h (1 hunks)
wled00/set.cpp (3 hunks)
wled00/util.cpp (0 hunks)
wled00/wled.cpp (4 hunks)
wled00/wled.h (1 hunks)
wled00/wled_server.cpp (1 hunks)
wled00/xml.cpp (3 hunks)

💤 Files with no reviewable changes (1)

wled00/util.cpp

🚧 Files skipped from review as they are similar to previous changes (8)

wled00/wled.h
wled00/fcn_declare.h
wled00/wled.cpp
wled00/xml.cpp
wled00/wled_server.cpp
wled00/cfg.cpp
wled00/set.cpp
wled00/data/style.css

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch unify-hostname

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

netmindz · 2025-07-06T15:53:07Z

wled00/network.cpp

      // convert the "serverDescription" into a valid DNS hostname (alphanumeric)
-      char hostname[64];
-      prepareHostname(hostname);
+      char hostname[33];


Why the change from 64 to 33 and this seems out of step with other parts of the code where you are using 25

None of the UI fields are 64 bytes.

netmindz · 2025-07-06T15:56:23Z

With this change, can we still have no mDNS but still the user-defined hostname? (used in DHCP request for "regular" DNS)

blazoncek · 2025-07-06T17:27:40Z

With this change, can we still have no mDNS but still the user-defined hostname?

Yes, the fallback is UI device name.

netmindz · 2025-07-09T17:52:00Z

Yeah that doesn't quite make sense then.

While I accept that not putting anything in there will maintain existing behaviour, I thought that part of the reason for this change is that current behaviour is unclear.

Most users would expect the network hostname to be with the network settings, not automatically generated from a setting under UI.

So this change to move hostname to be consistent for regular DNS and mDNS is an improvement, but it would be sad if you couldn't see your own regular hostname without being forced to enable mDNS as the only way to disable is to remove the hostname

blazoncek · 2025-07-09T19:12:45Z

Then the only option is to add a checkmark for enabling mDNS.
I would postpone that for a later time.

blazoncek · 2025-07-16T07:54:58Z

@netmindz @willmmiles @DedeHai @softhack007 @Aircoookie I would like we all agree on the proposed change since it may disrupt some people's network configuration and we should be aware of this change.
Unfortunately I do not see a solution apart from adding another field for (non-mDNS) hostname or adding checkmark for enabling mDNS (slightly bigger change).

As a pro for this change it fixes wrong/missing hostname on change or boot.

DedeHai · 2025-07-16T10:14:06Z

this is above my networking know-how. I exclusively work with IPs.

blazoncek · 2025-07-16T12:09:26Z

this is above my networking know-how. I exclusively work with IPs.

It is not really technical. This PR switches the hostname generation to mDNS name instead of UI name (by default). UI name is used as a fallback when user does not want to use mDNS and empties mDNS name.

In current situation UI name is WLED by default and if user does not change that there may be several devices with same hostname. Default mDNS name is a combination of "wled-" and MAC address.

DedeHai · 2025-07-16T16:38:14Z

Ah I get it, still don't really understand the background, did not have time yet to read the issues solved with this.
I never had any issues with all devices having the same name and adding a cryptic hex number to it does not change the situation from my point of view. If I need to know what device it is, I rename it manually. So if there is a technical reason to do so, no objection.

Can't really comment on "disrupting network". I do not use names, I assign fixed IPs.

All I want to add: it must not be convoluted, like input fields need to be clear. If I change mDNS name, it should not change the host name and vice versa (if that is how it currently is). If any naming settings are linked, they MUST be in the same settings page and synced directly in that page so its obvious.

edit:
I glanced at the issues this is solving and uinifying makes sense.

blazoncek · 2025-07-18T07:47:25Z

I'm pulling this to draft as perhaps we need to discuss the following:

mDNS name
hostname for DHCP (dnsmasq), Arduino, WiFi identification, etc
device name (UI)

IMO hostname and mDNS name should be identical (not necessarily the case with this PR) however device name should not be used in the networking context (it is with this PR if user erases mDNS).

I have modified the entire name handling and introduced explicit mDNS selection as a POC in my fork (unpublished ATM).

…hostname to "nw" key in config JSON - add new config key in "nw" for mDNS (bool) - remove `prepareHostname()` function - update mDNS calls - add hostname update for Ethernet - move WiFi configuration to cfg.json - rename LED and WiFi to Hardware and Network in settings - reorder network settings

DedeHai · 2025-07-21T12:01:06Z

wled00/data/settings_wifi.htm

+		<div id="ESPNOW">
+			Enable ESP-NOW: <input type="checkbox" name="RE" onchange="tE()"><br>
+			<i>Listen for events over ESP-NOW<br>
+			Keep disabled if not using a remote or wireless sync, increases power consumption.<br></i>


name it ESPNow-Sync instead of wireless sync?

blazoncek · 2025-07-21T18:26:45Z

@willmmiles does the latest push fulfill what we talked about (single hostname across all networking options)? It does not change UI name, though, which remains WLED by default but can be overridden during compile.

netmindz · 2025-08-09T12:03:26Z

I think we should just have an explicit hostname field, that is then used by the DHCP client to self register against your domain used for you LAN, for example wled123.fritz.box and then also the same hostname for mDNS wled123.local

By default it set to WLED dash mac address, with colons removed. For existing installs we populate this new field with the current logic of the sanitised version of the server name, to maintain compatibility

blazoncek · 2025-08-09T12:36:07Z

@netmindz this is exactly what this PR does.

netmindz · 2025-08-09T12:52:14Z

It is very close, I'm just testing at the moment, but the hostname I got for my existing install did not use the device name, but i need to double check i was in the correct state before

netmindz · 2025-08-09T12:55:22Z

If we are going to display the full URL that your mDNS address will be, then that should be a link

netmindz · 2025-08-09T12:58:35Z

I terms of placement, I think it should be just before the static ip rather than right at the top

netmindz · 2025-08-09T13:05:24Z

yeah, I've just double checked and as it stands at the moment, this PR is a breaking change, the existing way of setting the hostname does not carry over to the new setup, i just get wled-e73b10

We need to populate the new hostname value based on the existing logic

netmindz

We need to migrate people's existing hostname

netmindz · 2025-08-09T13:10:19Z

wled00/cfg.cpp

+  getStringFromJson(hostName, id[F("mdns")], sizeof(hostName));
+  if (strlen(hostName) == 0) {
+    mDNSenabled = false; // if no host name is set, disable mDNS
+    sprintf_P(hostName, PSTR("wled-%.*s"), 6, escapedMac.c_str() + 6);


This needs to be changed to use the existing logic, for example my current hostname of window is because i have the WLED device called Window

This uses existing logic. If {"id":{"mdns":....}, ...} exists, it uses that name. Only if someone disabled mDNS (by entering empty name) then new hostname is generated.

That's using the mDNS name, personally I've never edited that as I'm using regular DNS which uses the device name from the UI settings

Not sure which we choose, but whatever we choose it won't be the right option for 100% of users.

Certainly if the device name in UI settings is still "WLED" then we could definitely ignore that

the non-mDNS hostname is wled-{serverDescription}, so actually you get "wled-window" not just "window"

wled-window.ftriz.box for example - when name in server description in the UI settings is "Window"

cmDNS and serverDescription are only created during 1st boot. Each consecutive boot will use stored values if they were modified by user or not.

The only difference this PR does (regarding network naming) is the change of DHCP lease names (and dnsmasqd responses to DNS querires (non-mDNS) if your router supported that). Those were missing in case of ESP32 (producing esp-xxxxxx registration) or constructed from serverDescription (producing wled-zzzzzzz name, where zzzzzz conformed to [0-9a-zA-Z\-\_]+ regexp). DNS queries were never officially supported or advertised by WLED.

The mDNS name and hostname are now (with this PR) the same (as is usual or common practice).

However, I am loosing interest in pursuing this PR further (since none of the OP responded to my queires, these include: @tablatronix, @huggy-d1, @mateuszdrab, @b3-4r, @Rodney2K, @BBaoVanC, @schildbach and @Colbyjdx) so do whatever pleases you (merge, edit or abandon), I am not going to update it any more.

tablatronix · 2025-08-21T11:26:10Z

I must have this muted didn't see this

milhnl · 2025-08-30T12:49:57Z

Hi all!

First of all, I'd like to thank @blazoncek for all the hard work. I don't intend to 'steal' his PR in any way, but saw that he didn't want to pursue this any further, so I tried to get this to a state where it can be merged. The result is over in my fork.

There were two issues:

main had moved on, so there were some new compilation errors after rebasing. Those are fixed.
@netmindz wanted the hostname to remain wled-$serverDescription if it was not set to avoid a breaking change. This behaviour is restored.

This PR was tested on both an ESP8266 and ESP32, but the latter reported esp-xxxxxx as hostname to dnsmasq on main, so I'm not sure whether the legacy hostname even worked there.

If you need anything more, just let me know!

blazoncek · 2025-08-30T13:57:47Z

@milhnl you are welcome to PR into unify-hostname branch instead of main. That way no stealing will occur.

EDIT: FYI I disliked behaviour of prepareHostname().

milhnl · 2025-08-30T14:01:46Z

EDIT: FYI I disliked behaviour of prepareHostname().

I agree. But not breaking backwards compatibility seems more important to the maintainers, which I get, even though it's not in a documented feature.

blazoncek · 2025-08-30T19:24:42Z

Sometimes you need to break things to make something new. 😉

milhnl · 2025-08-31T11:07:42Z

After that, there's two commits at in my PR. No clue how these not-to-main PRs work in the GitHub view. But they're there.

Hostname unification

e2edd38

- fixes #1242 - fixes empty MQTT topic - fixes missing hostname on change #4586

blazoncek requested a review from willmmiles June 27, 2025 14:33

netmindz reviewed Jul 6, 2025

View reviewed changes

blazoncek marked this pull request as draft July 18, 2025 07:38

blazoncek added 2 commits July 21, 2025 12:16

Fix cherry-pick error

31cdbb1

blazoncek marked this pull request as ready for review July 21, 2025 12:04

DedeHai reviewed Jul 21, 2025

View reviewed changes

DedeHai mentioned this pull request Jul 22, 2025

Update infrared gpio setting docs wled/WLED-Docs#265

Merged

blazoncek added the connectivity Issue regarding protocols, WiFi connection or availability of interfaces label Jul 26, 2025

blazoncek added 2 commits August 2, 2025 14:38

Merge branch 'main' into unify-hostname

0c791e8

Hostname save bugfix

929f58f

netmindz requested changes Aug 9, 2025

View reviewed changes

milhnl mentioned this pull request Aug 30, 2025

Unify hostname #4888

Closed

Merge branch 'main' into unify-hostname

661e1ee

milhnl mentioned this pull request Aug 31, 2025

Hostname unification 2: The Force Push Strikes Back #4893

Open

netmindz closed this Sep 2, 2025

blazoncek deleted the unify-hostname branch September 2, 2025 06:35

Uh oh!

Hostname unification #4751

Hostname unification #4751

Uh oh!

Conversation

blazoncek commented Jun 27, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Assessment against linked issues

Possibly related PRs

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR/Issue comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Status, Documentation and Community

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netmindz commented Jul 6, 2025

Uh oh!

blazoncek commented Jul 6, 2025

Uh oh!

netmindz commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blazoncek commented Jul 9, 2025

Uh oh!

blazoncek commented Jul 16, 2025

Uh oh!

DedeHai commented Jul 16, 2025

Uh oh!

blazoncek commented Jul 16, 2025

Uh oh!

DedeHai commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blazoncek commented Jul 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blazoncek commented Jul 21, 2025

Uh oh!

netmindz commented Aug 9, 2025

Uh oh!

blazoncek commented Aug 9, 2025

Uh oh!

netmindz commented Aug 9, 2025

Uh oh!

netmindz commented Aug 9, 2025

Uh oh!

netmindz commented Aug 9, 2025

Uh oh!

netmindz commented Aug 9, 2025

Uh oh!

netmindz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tablatronix commented Aug 21, 2025

blazoncek commented Jun 27, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jun 27, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

netmindz commented Jul 9, 2025 •

edited

Loading

DedeHai commented Jul 16, 2025 •

edited

Loading

blazoncek commented Aug 30, 2025 •

edited

Loading