IDF v4 Ethernet/Network Fixes

[troyhacks]

IDF v4 moves from SYSTEM_EVENT_WIFI_STA_GOT_IP to ARDUINO_EVENT_WIFI_STA_GOT_IP (for example)

Also some improvements to disable WiFi when Ethernet is connected, which ensures things like Art-Net and DDP use the Ethernet connection when present.

Summary by CodeRabbit

• New Features
  • Enhanced Ethernet connectivity: The system now manages connections more intelligently by prioritizing Ethernet and automatically adjusting WiFi behavior as needed.
• Refactor
  • Updated connection status displays now clearly indicate whether the device is connected via Ethernet or WiFi, providing improved clarity on network status.

[troyhacks]

This requires Tasmota based on v4.4.6. The tests with Tasmota on v4.4.4 were very buggy.

Use this: https://github.com/tasmota/platform-espressif32/releases/download/2023.10.04/platform-espressif32.zip

This should also improve Ethernet on builds against IDF v3 as well.

[netmindz]

@coderabbitai review

[coderabbitai[bot]]

:white_check_mark: Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai[bot]]

Walkthrough

The changes update the handling of Ethernet events in the network code. In network.cpp, new event definitions for Ethernet are introduced, and the WiFiEvent function is extended to manage Ethernet events—particularly modifying the action for SYSTEM_EVENT_ETH_GOT_IP and removing some debug output. In wled.cpp, the handleConnection() method now prints a URL with the local IP and prints a distinct message if the connection is via Ethernet, disconnecting WiFi in that case.

Changes

File(s)	Change Summary
wled00/network.cpp	Added new Ethernet event macros (CONNECTED, DISCONNECTED, START, GOT_IP) conditional on ESP-IDF v4+; updated WiFiEvent() to handle SYSTEM_EVENT_ETH_GOT_IP and adjusted logging.
wled00/wled.cpp	Modified WLED::handleConnection() to output a URL containing the IP address and to check the connection type; if Ethernet is active, prints a message and disconnects WiFi.

Sequence Diagram(s)

sequenceDiagram
    participant EVT as Event Trigger
    participant WF as WiFiEvent handler
    participant NW as Network System
    EVT->>WF: Network event (e.g., ETH_GOT_IP)
    WF->>NW: Check if Ethernet is active
    alt Ethernet active and AP inactive
        NW-->>WF: Ethernet connected
        WF->>NW: Disconnect WiFi
    else Both connections active
        NW-->>WF: Log message: 'WiFi remains active'
    end

sequenceDiagram
    participant SYS as System
    participant CON as handleConnection()
    participant NET as Network Check
    SYS->>CON: Connection established
    CON->>NET: Determine connection type
    alt Connection is Ethernet
        NET-->>CON: Ethernet detected
        CON->>SYS: Print "Connected! IP address: http://" + IP + " via Ethernet (disabling WiFi)"
        CON->>SYS: Disconnect WiFi
    else Connection is WiFi
        NET-->>CON: WiFi detected
        CON->>SYS: Print "Connected! IP address: http://" + IP + " via WiFi"
    end

✨ Finishing Touches

• [ ] 📝 Generate Docstrings (Beta)

---

🪧 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.
  • Generate unit testing code for this file.
  • 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 generate unit testing code for this file.
  • @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 generate unit testing code.
  • @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.

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. (Beta)
• @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.

[netmindz]

Hi @blazoncek are you ok to approve this PR or if not can you clarify what changes are required before you feel ok approving?

[netmindz]

Where have we got to with this folks? I'm keen to push forward with V4 and get preview/beta release of 0.16 out soon

[Arcitec]

@netmindz It was discussed on Discord in #dev-general 1 week ago. From what I remember:

@blazoncek Has local code on his computer which fixed these variable names in a cleaner way, by using the V4 variables and doing "if V3 SDK, define the V4 names to match the V3 values" rather than the reverse. That is more forward-thinking and makes more sense. The new V4 names are much better labeled too.

Apart from the variable names, @troyhacks also found some issue with the SDK itself and suggested a fixed version of the V4 SDK here in a related comment:

https://github.com/wled/WLED/issues/4502#issuecomment-2619374837

He also discovered that there's some weird issue at startup which required an extra WiFi.disconnect(true) to turn off the WiFi antenna, which was added in this PR.

I am not sure if Blaz has done similar work to fix the connection edge cases that Troy fixed here.

So this PR is in a unfinished state. Cleanup to use the V4 variable names is at least required. Blaz suggested code here:

https://github.com/wled/WLED/pull/4526#discussion_r1939991981

There's also a few still-open code review comments in this PR, about potential problems introduced by the code here, so it's definitely not merge-ready.

[blazoncek]

Has local code on his computer

https://github.com/blazoncek/WLED/blob/5a39d91cabb822d64d3b1095c89b1c8a1397a95a/wled00/network.cpp#L396-L417 https://github.com/blazoncek/WLED/blob/5a39d91cabb822d64d3b1095c89b1c8a1397a95a/wled00/network.cpp#L492-L538 https://github.com/blazoncek/WLED/blob/5a39d91cabb822d64d3b1095c89b1c8a1397a95a/wled00/src/dependencies/network/Network.cpp#L74-L81

I only keep code local when it is not complete/finished.

However my modifications are part of my ongoing effort to make ESP-NOW work for syncing. I do not yet consider that as ready since it still has some issues when multiple WiFis are configured.

[maxx-ukoo]

I try to run WLED on board with RTL8201 lan chip. I can config that this fix works on my board but for 0_15_x branch. I will try on main again.

[maxx-ukoo]

It looks like master has all changes for support ethernet with V4/ It works for me - https://github.com/wled/WLED/compare/main...maxx-ukoo:mk_add_RTL8201_support?expand=1

[jzucker2]

Definitely seems like an improvement! Thank you!

Over a longer period I had a cap on the docker compose but it was trending higher before

Thanks again!

[blazoncek]

@maxx-ukoo enclose in V4 #ifdef and submit a PR.

[maxx-ukoo]

@blazoncek I have updated my change with code like:

#if defined(ARDUINO_ARCH_ESP32) && defined(ESP_ARDUINO_VERSION_MAJOR) //ESP_IDF_VERSION_MAJOR==3
#define WLED_NUM_ETH_TYPES        14
#else
#define WLED_NUM_ETH_TYPES        13
#endif

I able to compile esp32_eth and esp32_eth_V4 versions. But what I can do with:

<option value="13">WESP32_RTL8201</option>

How I can have this option in HTML only for V4?

[blazoncek]

Borrow code from elsewhere where conditional stuff happens. See xml.cpp

[netmindz]

I able to compile esp32_eth and esp32_eth_V4 versions. But what I can do with:

<option value="13">WESP32_RTL8201</option>

How I can have this option in HTML only for V4?

Not easily is the short answer, you might just need to put (V4 Only) in the name for now

Version 0.16 will be V4 only when we actually release

[netmindz]

Can we please get this over the line please. Do you have permission to just push the changes you want @blazoncek ?

[blazoncek]

Can we please get this over the line please. Do you have permission to just push the changes you want @blazoncek ?

AFAIK some things were resolved in #4429. I'm not sure if this is still relevant.