Skip to content

Conversation

@blazoncek
Copy link
Collaborator

@blazoncek blazoncek commented Jul 12, 2025

Fixes #4268

Only for ESP32

Summary by CodeRabbit

  • New Features

    • Added support for specifying a hostname or custom text for each network-based LED output (excluding ESP8266 devices).
    • Hostnames are now resolved to IP addresses automatically on ESP32 devices for network LED outputs.
    • The web UI for LED outputs now includes a "Host" input field for compatible devices.
  • Improvements

    • Custom text/hostname is now saved and restored with LED bus configurations.
    • Invalid input fields are visually highlighted in red for easier identification.
  • Bug Fixes

    • Minor typo and formatting corrections in comments and code.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 12, 2025

Walkthrough

The changes introduce support for specifying hostnames (including mDNS names) for network-based virtual LED buses, primarily on ESP32 devices. Hostnames can be configured via the UI and are resolved to IP addresses at runtime. The configuration, serialization, and UI logic are updated to handle this new hostname field, with conditional logic for ESP32 platforms.

Changes

Files / Groups Change Summary
wled00/bus_manager.cpp, wled00/bus_manager.h Add hostname support for network buses on ESP32: store, resolve (via DNS/mDNS), and update IP addresses dynamically.
wled00/cfg.cpp, wled00/set.cpp, wled00/xml.cpp Extend bus configuration to include a custom text/hostname field; update serialization, deserialization, and form handling.
wled00/data/settings_leds.htm Add "Host" input field for network LED outputs (except ESP8266); device detection helpers for UI; conditional display logic.
wled00/data/style.css Add CSS rule to highlight invalid input fields in red.

Assessment against linked issues

Objective Addressed Explanation
Allow specifying hostnames (including mDNS names) for DDP/network LED outputs (#4268)
Hostname field should be configurable via the settings UI and stored in configuration (#4268)
Hostname should be resolved to IP at runtime (support mDNS and DNS) on ESP32 (#4268)
No changes to non-network LED outputs or unrelated features (#4268)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes were found. All modifications are directly related to enabling hostname/mDNS support for network LED outputs as described in the linked issue.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.
    • Explain this complex logic.
    • 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. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai 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

Documentation and Community

  • 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
wled00/bus_manager.h (1)

379-392: Consider improving constructor parameter naming and efficiency.

The BusConfig constructor has been correctly extended to support the hostname feature. However, consider these improvements:

  1. The parameter name sometext could be more descriptive (e.g., hostname or customText)
  2. String parameters could be passed by const reference for better efficiency

Apply this diff to improve the constructor:

-BusConfig(uint8_t busType, uint8_t* ppins, uint16_t pstart, uint16_t len = 1, uint8_t pcolorOrder = COL_ORDER_GRB, bool rev = false, uint8_t skip = 0, byte aw=RGBW_MODE_MANUAL_ONLY, uint16_t clock_kHz=0U, uint8_t maPerLed=LED_MILLIAMPS_DEFAULT, uint16_t maMax=ABL_MILLIAMPS_DEFAULT, String sometext = "")
+BusConfig(uint8_t busType, uint8_t* ppins, uint16_t pstart, uint16_t len = 1, uint8_t pcolorOrder = COL_ORDER_GRB, bool rev = false, uint8_t skip = 0, byte aw=RGBW_MODE_MANUAL_ONLY, uint16_t clock_kHz=0U, uint8_t maPerLed=LED_MILLIAMPS_DEFAULT, uint16_t maMax=ABL_MILLIAMPS_DEFAULT, const String& customText = "")

And update the initialization:

-, text(sometext)
+, text(customText)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7bfd6f and e7157e5.

📒 Files selected for processing (7)
  • wled00/bus_manager.cpp (6 hunks)
  • wled00/bus_manager.h (6 hunks)
  • wled00/cfg.cpp (4 hunks)
  • wled00/data/settings_leds.htm (7 hunks)
  • wled00/data/style.css (2 hunks)
  • wled00/set.cpp (3 hunks)
  • wled00/xml.cpp (2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
wled00/xml.cpp (6)
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
wled00/cfg.cpp (2)
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Learnt from: ComicoX
PR: wled/WLED#4626
File: wled00/button.cpp:116-118
Timestamp: 2025-03-29T19:21:52.806Z
Learning: ESP32-C6 does not have built-in touch sensors, unlike other ESP32 variants.
wled00/set.cpp (6)
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:343-347
Timestamp: 2025-02-19T12:41:22.676Z
Learning: The setName() method in WLED handles segment name validation internally. No additional length checks or name clearing is needed before calling setName().
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, `bri` is a global variable used for brightness control.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
wled00/bus_manager.cpp (2)
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
wled00/bus_manager.h (2)
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
wled00/data/settings_leds.htm (9)
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: blazoncek
PR: wled/WLED#4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
🧬 Code Graph Analysis (3)
wled00/cfg.cpp (1)
wled00/bus_manager.h (1)
  • String (136-136)
wled00/bus_manager.cpp (1)
wled00/bus_manager.h (6)
  • _hostname (348-348)
  • b (124-124)
  • b (124-124)
  • b (194-200)
  • b (194-194)
  • b (241-241)
wled00/bus_manager.h (1)
wled00/bus_wrapper.h (4)
  • busType (471-575)
  • busType (471-471)
  • busType (1370-1490)
  • busType (1370-1370)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
  • GitHub Check: wled_build / Build Environments (esp32_wrover)
  • GitHub Check: wled_build / Build Environments (usermods)
  • GitHub Check: wled_build / Build Environments (lolin_s2_mini)
  • GitHub Check: wled_build / Build Environments (esp32c3dev)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
  • GitHub Check: wled_build / Build Environments (esp32dev_V4)
  • GitHub Check: wled_build / Build Environments (esp32_eth)
  • GitHub Check: wled_build / Build Environments (esp32dev)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_160)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_160)
  • GitHub Check: wled_build / Build Environments (esp8266_2m)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full)
  • GitHub Check: wled_build / Build Environments (nodemcuv2)
  • GitHub Check: wled_build / Build Environments (usermods)
🔇 Additional comments (22)
wled00/data/style.css (1)

82-84: LGTM! Good UX enhancement for input validation.

The CSS rule provides clear visual feedback for invalid inputs, which will be helpful for the new hostname field validation.

wled00/xml.cpp (2)

314-314: LGTM! Consistent parameter naming convention.

The hostname parameter declaration follows the established pattern used for other bus configuration parameters.


354-354: LGTM! Proper integration with bus configuration.

The form value output correctly calls the bus's getCustomText() method and follows the same pattern as other bus configuration parameters.

wled00/set.cpp (4)

144-144: LGTM! Appropriate variable declaration.

The String variable for storing hostname text is properly declared in the appropriate scope.


178-178: LGTM! Consistent parameter handling.

The hostname parameter array declaration follows the established pattern used for other bus configuration parameters.


229-229: LGTM! Good input validation with length limiting.

The substring(0,31) truncation provides appropriate length validation for hostname input, preventing potential buffer overflows while allowing reasonable hostname lengths.


232-232: LGTM! Proper integration with bus configuration.

The addition of the text parameter to busConfigs.emplace_back() correctly integrates the hostname functionality with the existing bus configuration system.

wled00/cfg.cpp (3)

238-239: Good implementation of hostname configuration deserialization.

The extraction of the "text" field from the bus configuration JSON and passing it to the BusConfig constructor is correctly implemented. This enables users to specify hostnames for network LED buses.


980-980: Proper serialization of custom text using the bus interface.

The serialization correctly uses the bus->getCustomText() virtual method, which allows different bus types to provide their custom text implementation (e.g., hostname for network buses).


383-383: Good formatting cleanup.

The removal of trailing whitespace improves code consistency.

wled00/bus_manager.h (3)

136-136: Well-designed virtual interface for custom text.

The addition of the virtual getCustomText() method to the base Bus class provides a clean interface for bus-specific custom text retrieval. The default implementation returning an empty String is appropriate for bus types that don't need custom text.


346-361: ESP32-specific hostname functionality properly implemented.

The conditional compilation for ESP32-only features is correctly applied. The resolveHostname() method and _hostname member are appropriately scoped to ESP32 where mDNS resolution is available. The override of getCustomText() to return the hostname follows good OOP practices.


219-219: Good comment correction.

The typo fix from "menaings" to "meanings" improves code documentation quality.

wled00/data/settings_leds.htm (5)

55-59: Device identification functions look well-implemented.

The helper functions correctly identify ESP platforms based on hardware limits. The pattern follows const.h definitions and provides a clean abstraction for conditional UI logic.


279-279: Good defensive programming with null check.

Adding the null check before calling querySelector prevents potential runtime errors if gT(t) returns undefined.


289-290: Conditional host field display logic is correct.

The logic properly shows the host field only for network LED types on non-ESP8266 platforms and clears the field when conditions aren't met. This aligns with the ESP32-only mDNS functionality.


473-473: HTML input field implementation looks good.

The host input field includes appropriate constraints (maxlength, pattern) and follows the existing code style. The pattern restricts input to alphanumeric characters, underscores, and hyphens, which is appropriate for hostnames.


597-597: Variable declaration fix improves code clarity.

Adding the variable declaration for p improves readability and prevents potential scope issues.

wled00/bus_manager.cpp (4)

8-9: ESP32-specific includes are appropriate.

The conditional includes for ESPmDNS and Network headers are correctly placed within the ESP32 architecture guard, ensuring they're only included when needed.


30-30: External declaration follows existing pattern.

The external declaration of cmDNS follows the same pattern as other external variables in the file.


698-701: Hostname initialization and resolution logic is sound.

The ESP32-specific code properly initializes the hostname from bus configuration and calls resolveHostname during construction. The conditional compilation ensures this only runs on ESP32 platforms.


935-940: Network bus hostname resolution in on() method is well-placed.

The logic correctly iterates through virtual buses and calls resolveHostname() to update IP addresses on system startup or bus activation. This ensures hostname resolution occurs when network connectivity changes.

Comment on lines +737 to +746
void BusNetwork::resolveHostname() {
static unsigned long nextResolve = 0;
if (Network.isConnected() && millis() > nextResolve && _hostname.length() > 0) {
nextResolve = millis() + 600000; // resolve only every 10 minutes
IPAddress clnt;
if (strlen(cmDNS) > 0) clnt = MDNS.queryHost(_hostname);
else WiFi.hostByName(_hostname.c_str(), clnt);
if (clnt != IPAddress()) _client = clnt;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Implement error handling for hostname resolution failures.

The resolveHostname() method has good throttling (10-minute intervals) but could benefit from improved error handling and logging.

Consider adding error handling and debug logging:

 void BusNetwork::resolveHostname() {
   static unsigned long nextResolve = 0;
   if (Network.isConnected() && millis() > nextResolve && _hostname.length() > 0) {
     nextResolve = millis() + 600000; // resolve only every 10 minutes
     IPAddress clnt;
+    bool resolved = false;
     if (strlen(cmDNS) > 0) clnt = MDNS.queryHost(_hostname);
+    if (clnt != IPAddress()) resolved = true;
     else WiFi.hostByName(_hostname.c_str(), clnt);
+    if (!resolved && clnt != IPAddress()) resolved = true;
-    if (clnt != IPAddress()) _client = clnt;
+    if (resolved) {
+      _client = clnt;
+      DEBUGBUS_PRINTF_P(PSTR("Bus: Resolved hostname %s to %s\n"), _hostname.c_str(), _client.toString().c_str());
+    } else {
+      DEBUGBUS_PRINTF_P(PSTR("Bus: Failed to resolve hostname %s\n"), _hostname.c_str());
+    }
   }
 }
🤖 Prompt for AI Agents
In wled00/bus_manager.cpp around lines 737 to 746, the resolveHostname() method
lacks error handling and logging for hostname resolution failures. Add checks to
detect if MDNS.queryHost or WiFi.hostByName fail to resolve the hostname, and
log appropriate error or debug messages indicating the failure. This will
improve observability and help diagnose network issues during hostname
resolution.

@DarthWeber
Copy link

Thanks - but which branch do i need? I cant find "fix-4768" and in the main branch i see no changes (only ip field).

@DedeHai
Copy link
Collaborator

DedeHai commented Jul 21, 2025

@DarthWeber you can compile this PR or donwnload the compiled bins under the "Checks" tab -> Artifacts
FYI: any (recent) commit has compiled bins for download, click on the checkmark to get them:
image

@DarthWeber
Copy link

Thanks, but also with the bin under artifacts e7157e5 i got no host field under the ddp segment. Seems to be without PR, the build is still 2412040

@DarthWeber
Copy link

DarthWeber commented Jul 22, 2025

got it compiled. But it doesn't work as expected: I need to fill in the ip. I can not let them empty. DDP works only if i fill in the right ip. Double-checked the hostname, it is right.

Screenshot 2025-07-22 122715

And another strange thing: The multirelay usermod is not available although i use -D USERMOD_MULTI_RELAY. It is available with 15-beta1. But that is maybe another issue.

@blazoncek
Copy link
Collaborator Author

enter anything in IP fields.

@DarthWeber
Copy link

i tried 8.8.8.8 and 0.0.0.0 - but it doesnt work

@blazoncek
Copy link
Collaborator Author

Well, it does in my case. And in fact it replaces IP with device's actual IP when you review settings.
I would expect that mDNS issues on your network.

@DarthWeber
Copy link

DarthWeber commented Jul 22, 2025

got it. I deleted the DDP output, entered the hostname and 8.8.8.8 and i got the resolved ip after saving. But this works only the first time an a new output. If you enter something in the ip again, it does not resolve again the hostname.

@blazoncek
Copy link
Collaborator Author

To prevent resource exhaustion new mDNS resolution is done after 10 min.
Otherwise you would see LED stuttering due to delays.

@DarthWeber
Copy link

Works since 24h like a charme. Only two minor problems:

  • in that release i cant include the multirelay usermod.
  • one or two stucks on the receiving controller, whis is still 0.15-beta1

@blazoncek blazoncek added enhancement connectivity Issue regarding protocols, WiFi connection or availability of interfaces labels Jul 26, 2025
@blazoncek
Copy link
Collaborator Author

Is everyone comfortable with merging this?
It does have its quirks. Namely, when you reboot and ESP starts, buses are created before there is WiFi connectivity. This means mDNS is not used to resolve IP address and stored IP is used instead. However, to mitigate this mDNS is retried later when you turn WLED off and on again.

Copy link
Member

@willmmiles willmmiles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to accept this as-is. We can arrange to resolve on the network coming up in a future patch.

@blazoncek blazoncek merged commit 374d906 into main Aug 2, 2025
42 checks passed
@blazoncek blazoncek deleted the fix-4268 branch August 2, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connectivity Issue regarding protocols, WiFi connection or availability of interfaces enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DDP with mdns names - feature request

5 participants