-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Hostname unification 2: The Force Push Strikes Back #4893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e2edd38
2509b1c
31cdbb1
0c791e8
929f58f
661e1ee
63ccd83
e38575f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,14 +57,22 @@ bool deserializeConfig(JsonObject doc, bool fromFS) { | |
#endif | ||
|
||
JsonObject id = doc["id"]; | ||
getStringFromJson(cmDNS, id[F("mdns")], 33); | ||
getStringFromJson(serverDescription, id[F("name")], 33); | ||
getStringFromJson(serverDescription, id["name"], sizeof(serverDescription)); | ||
// legacy behaviour | ||
getStringFromJson(hostName, id[F("mdns")], sizeof(hostName)); | ||
if (strlen(hostName) == 0) { | ||
mDNSenabled = false; // if no host name is set, disable mDNS | ||
prepareHostname(hostName, sizeof(hostName)-1); | ||
} | ||
|
||
#ifndef WLED_DISABLE_ALEXA | ||
getStringFromJson(alexaInvocationName, id[F("inv")], 33); | ||
getStringFromJson(alexaInvocationName, id[F("inv")], sizeof(alexaInvocationName)); | ||
#endif | ||
CJSON(simplifiedUI, id[F("sui")]); | ||
|
||
JsonObject nw = doc["nw"]; | ||
CJSON(mDNSenabled, nw[F("mdns")]); | ||
getStringFromJson(hostName, nw["name"], sizeof(hostName)); | ||
Comment on lines
+74
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Order of operations issue: mDNS flag might be overwritten The mDNS enabled flag is set on line 74 from the new location Consider reordering to handle legacy migration after reading the new values: JsonObject nw = doc["nw"];
- CJSON(mDNSenabled, nw[F("mdns")]);
- getStringFromJson(hostName, nw["name"], sizeof(hostName));
+ // Read new format first
+ bool hasNewHostname = nw["name"] != nullptr;
+ if (hasNewHostname) {
+ getStringFromJson(hostName, nw["name"], sizeof(hostName));
+ CJSON(mDNSenabled, nw[F("mdns")]);
+ }
+ // Only apply legacy if new format not present
+ else if (id[F("mdns")] != nullptr) {
+ getStringFromJson(hostName, id[F("mdns")], sizeof(hostName));
+ // Don't force mDNS disabled for legacy configs
+ }
+ // Generate hostname if still empty
+ if (strlen(hostName) == 0) {
+ prepareHostname(hostName, sizeof(hostName)-1);
+ }
🤖 Prompt for AI Agents
|
||
#ifndef WLED_DISABLE_ESPNOW | ||
CJSON(enableESPNow, nw[F("espnow")]); | ||
linked_remotes.clear(); | ||
|
@@ -143,13 +151,23 @@ bool deserializeConfig(JsonObject doc, bool fromFS) { | |
|
||
JsonObject wifi = doc[F("wifi")]; | ||
noWifiSleep = !(wifi[F("sleep")] | !noWifiSleep); // inverted | ||
//noWifiSleep = !noWifiSleep; | ||
CJSON(force802_3g, wifi[F("phy")]); //force phy mode g? | ||
#ifdef ARDUINO_ARCH_ESP32 | ||
CJSON(txPower, wifi[F("txpwr")]); | ||
txPower = min(max((int)txPower, (int)WIFI_POWER_2dBm), (int)WIFI_POWER_19_5dBm); | ||
#endif | ||
|
||
// apply WiFi options from above (regardless of fromFS or not) | ||
#ifdef ARDUINO_ARCH_ESP32 | ||
WiFi.setSleep(!noWifiSleep); | ||
WiFi.setHostname(hostName); | ||
WiFi.setTxPower(wifi_power_t(txPower)); | ||
#else | ||
WiFi.setPhyMode(force802_3g ? WIFI_PHY_MODE_11G : WIFI_PHY_MODE_11N); | ||
wifi_set_sleep_type((noWifiSleep) ? NONE_SLEEP_T : MODEM_SLEEP_T); | ||
WiFi.hostname(hostName); | ||
#endif | ||
|
||
JsonObject hw = doc[F("hw")]; | ||
|
||
// initialize LED pins and lengths prior to other HW (except for ethernet) | ||
|
@@ -637,8 +655,9 @@ bool deserializeConfig(JsonObject doc, bool fromFS) { | |
getStringFromJson(mqttUser, if_mqtt[F("user")], 41); | ||
getStringFromJson(mqttPass, if_mqtt["psk"], 65); //normally not present due to security | ||
getStringFromJson(mqttClientID, if_mqtt[F("cid")], 41); | ||
|
||
if (mqttClientID[0] == 0) sprintf_P(mqttClientID, PSTR("WLED-%*s"), 6, escapedMac.c_str() + 6); | ||
getStringFromJson(mqttDeviceTopic, if_mqtt[F("topics")][F("device")], MQTT_MAX_TOPIC_LEN+1); // "wled/test" | ||
if (mqttDeviceTopic[0] == 0) sprintf_P(mqttDeviceTopic, PSTR("wled/%*s"), 6, escapedMac.c_str() + 6); | ||
getStringFromJson(mqttGroupTopic, if_mqtt[F("topics")][F("group")], MQTT_MAX_TOPIC_LEN+1); // "" | ||
CJSON(retainMqttMsg, if_mqtt[F("rtn")]); | ||
#endif | ||
|
@@ -849,14 +868,15 @@ void serializeConfig(JsonObject root) { | |
root[F("vid")] = VERSION; | ||
|
||
JsonObject id = root.createNestedObject("id"); | ||
id[F("mdns")] = cmDNS; | ||
id[F("name")] = serverDescription; | ||
id["name"] = serverDescription; | ||
#ifndef WLED_DISABLE_ALEXA | ||
id[F("inv")] = alexaInvocationName; | ||
#endif | ||
id[F("sui")] = simplifiedUI; | ||
|
||
JsonObject nw = root.createNestedObject("nw"); | ||
nw["name"] = hostName; | ||
nw[F("mdns")] = mDNSenabled; | ||
#ifndef WLED_DISABLE_ESPNOW | ||
nw[F("espnow")] = enableESPNow; | ||
JsonArray lrem = nw.createNestedArray(F("linked_remote")); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,7 +64,7 @@ button.sml { | |
.hide { | ||
display: none; | ||
} | ||
.err { | ||
.err, input:invalid { | ||
color: #f00; | ||
} | ||
.warn { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential issue with legacy hostname migration logic
The current implementation reads from
id["mdns"]
intohostName
for legacy compatibility, but if that field is empty, it disables mDNS and generates a new hostname. This could unexpectedly disable mDNS for users who had it enabled but didn't have an explicit hostname set in their legacy configuration.Consider preserving the existing mDNS enabled state when migrating legacy configurations:
🤖 Prompt for AI Agents