3.0.0 Network Refactoring #8760

me-no-dev · 2023-10-13T12:39:03Z

Better explanation will come later

Now all libs have been checked yet. More to do on WiFi side

libraries/ESPmDNS/src/ESPmDNS.cpp

github-actions · 2023-12-15T13:30:15Z

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message `"Add extra interface info for Printable"`: summary looks empty type/action looks empty the commit message `"Add periman string for network and "fix" mdns on the first ETH"`: summary looks empty type/action looks empty the commit message `"Apply suggestions from code review"`: summary looks empty type/action looks empty the commit message `"Cleanup"`: summary looks empty type/action looks empty the commit message `"Create ESP_NetworkInterface class and have Ethernet extending it"`: summary looks empty type/action looks empty the commit message `"Decouple network related libraries from WiFi"`: summary looks empty type/action looks empty the commit message `"Fix CMake issues"`: summary looks empty type/action looks empty the commit message `"Fix CMakekLists and rework lib menu dependencies"`: summary looks empty type/action looks empty the commit message `"Fix build errors"`: summary looks empty type/action looks empty the commit message `"Fix examples and WiFiUpdate"`: summary looks empty type/action looks empty the commit message `"Fix periman"`: summary looks empty type/action looks empty the commit message `"Fix web server missing fs.h"`: summary looks empty type/action looks empty the commit message `"Get the correct interface MAC address for mDND Workstation service"`: summary looks empty type/action looks empty the commit message `"Guard WiFi classes and fix RMII ETH examples"`: summary looks empty type/action looks empty the commit message `"Guard WiFiProv lib to compile only on WiFi chips"`: summary looks empty type/action looks empty the commit message `"More fixes"`: summary looks empty type/action looks empty the commit message `"More fixes"`: summary looks empty type/action looks empty the commit message `"Move Client, Server and Udp out of WiFi"`: summary looks empty type/action looks empty the commit message `"Move back WiFiClient"`: summary looks empty type/action looks empty the commit message `"Remove unnecessary guard"`: summary looks empty type/action looks empty the commit message `"Rename AP methods"`: summary looks empty type/action looks empty the commit message `"Rename IPv6 getters to clarify that they are returning LinkLocal address"`: summary looks empty type/action looks empty the commit message `"Rename Network library"`: summary looks empty type/action looks empty the commit message `"Rename WiFi Server and UDP"`: summary looks empty type/action looks empty the commit message `"Rename WiFiClient and WiFiClientSecure"`: summary looks empty type/action looks empty the commit message `"Rename network classes"`: summary looks empty type/action looks empty the commit message `"Some fixes from merging master"`: summary looks empty type/action looks empty the commit message `"Split networking from WiFi (H2 can now use Ethernet)"`: summary looks empty type/action looks empty the commit message `"Switch AP to the new interface"`: summary looks empty type/action looks empty the commit message `"Update CMakeLists.txt"`: summary looks empty type/action looks empty the commit message `"Update CMakeLists.txt"`: summary looks empty type/action looks empty the commit message `"Update ETH_TLK110.ino"`: summary looks empty type/action looks empty the commit message `"Update NetworkManager.h"`: summary looks empty type/action looks empty the commit message `"Update WiFiGeneric.cpp"`: summary looks empty type/action looks empty the commit message `"Update on-push.sh"`: summary looks empty type/action looks empty the commit message `"Update progress"`: summary looks empty type/action looks empty the commit message `"more fixes"`: summary looks empty type/action looks empty the commit message `"move back WiFiClient to rebase with master"`: summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: `<type/action>(<scope/component>): <summary>`, for example `fix(esp32): Fixed startup timeout issue` allowed types are: `change,ci,docs,feat,fix,refactor,remove,revert,test` sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) `TIP:` Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).
⚠️	Please consider squashing your 59 commits (simplifying branch history).
⚠️	The Pull Request description looks very brief, please check if more details can be added.

	Messages
📖	This PR seems to be quite large (total lines of code: 7256), you might consider splitting it into smaller PRs

👋 Hello me-no-dev, we appreciate your contribution to this project!

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against fb0041c

VojtechBartoska · 2023-12-21T13:27:17Z

Closes #8735

libraries/WiFi/src/AP.cpp

sgryphon · 2024-03-20T11:52:10Z

libraries/Networking/src/NetworkInterface.cpp

+ return true;
+}
+
+bool NetworkInterface::config(IPAddress local_ip, IPAddress gateway, IPAddress subnet, IPAddress dns1, IPAddress dns2, IPAddress dns3)


The notes in the header say // For server interfaces (WiFi AP), dns1 is the DHCP lease range start and dns2 is the DNS. dns3 is not used.

That is a bit weird, having the fourth parameter be the DHCP range, and only one DNS.

Maybe two separate functions for config() [normal] and configServer(local_ip, gateway, subnet, dhcp_range, dns). The functions could check _is_server_if and return an error / failure if it is not the right type (e.g. call server config if not a server).

it's more than we have now. We can think of something less weird later. I don't even guarantee that the DNS server is properly advertised by the DHCPS

sgryphon · 2024-03-20T11:56:00Z

libraries/Networking/src/NetworkInterface.cpp

+ d3.ip.u_addr.ip4.addr = 0;
+ }
+
+ if(_is_server_if){


Things missing from the server config is IPv6, which uses a different approach (RA instead of DHCPS).

I don't think adding IPv6 AP should be part of this change, but a separate thing. Actually maybe the first thing to add would be IPv6 STA config with a static address, DNS, etc. (also a separate thing).

A long journey, step by step.

libraries/Networking/src/NetworkInterface.cpp

SuGlider · 2024-03-25T04:19:58Z

libraries/WiFiProv/src/WiFiProv.cpp

@@ -17,6 +17,10 @@
 Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA

 */
+#pragma once


#pragma once is necessary in a cpp file?

Suggested change

#pragma once

it seems to be a copy&paste result.
Nice work anyway! Kudos!

funny... I do not remember touching this file at all 😆 maybe it felt left out and decided to make a statement

lucasssvaz

Looks very good. Just some suggestions. Everything I've tested so far works flawlessly. Will do more testing and report if anything is out of the ordinary.

libraries/Ethernet/examples/ETH_LAN8720/ETH_LAN8720.ino

libraries/Ethernet/examples/ETH_TLK110/ETH_TLK110.ino

libraries/Ethernet/src/ETH.h

libraries/Network/src/NetworkClient.h

libraries/WiFi/src/WiFiGeneric.cpp

lucasssvaz · 2024-03-25T12:39:11Z

libraries/WiFi/src/WiFiGeneric.cpp

- * else error code
- */
+/*
+ * Deprecated Methods


Can we mark all the newly deprecated functions with the deprecated attribute ?
https://en.cppreference.com/w/cpp/language/attributes/deprecated

No. There is too much code out there that uses those functions. Let's roll out as-is and re-think deprecation with 3.1.

lucasssvaz · 2024-03-25T12:41:03Z

package/package_esp32_index.template.json

Just confirming. Is this supposed updated in this PR or is it a leftover from tests ?

I had to update the libs for some things to work

lucasssvaz · 2024-03-25T12:44:15Z

libraries/Network/src/NetworkServer.h

@@ -0,0 +1,62 @@
+/*
+ Server.h - Server class for Raspberry Pi


Maybe we should update this line to reflect the actual description of this file. I think as long as the copyright and the license stays untouched it should be fine. (might be wrong though)

lucasssvaz · 2024-03-25T12:44:37Z

libraries/Network/src/NetworkClient.h

@@ -0,0 +1,115 @@
+/*
+ Client.h - Base class that provides Client


Maybe we should update this line to reflect the actual description of this file. I think as long as the copyright and the license stays untouched it should be fine. (might be wrong though)

yeah, I am also a bit scared of touching other's headers, so I would rather not

Co-authored-by: Lucas Saavedra Vaz <[email protected]>

@sgryphon

* Create ESP_NetworkInterface class and have Ethernet extending it * Update CMakeLists.txt * Split networking from WiFi (H2 can now use Ethernet) Now all libs have been checked yet. More to do on WiFi side * Fix build errors * Guard WiFi classes and fix RMII ETH examples * Decouple network related libraries from WiFi * Fix examples and WiFiUpdate * Guard WiFiProv lib to compile only on WiFi chips * Add periman string for network and "fix" mdns on the first ETH * Revert back location of Client/Server/Udp in order to accept some PRs * Fix periman * Some fixes from merging master * Fix web server missing fs.h * Move Client, Server and Udp out of WiFi * More fixes * more fixes * Fix CMakekLists and rework lib menu dependencies * Fix CMake issues * move back WiFiClient to rebase with master * Update ETH_TLK110.ino * Move back WiFiClient * Update progress * Update WiFiGeneric.cpp * More fixes * Switch AP to the new interface * Cleanup * Rename AP methods * Add extra interface info for Printable * Rename IPv6 getters to clarify that they are returning LinkLocal address cc @sgryphon * Rename network classes cc @sgryphon * Update NetworkManager.h * Rename WiFi Server and UDP * Rename WiFiClient and WiFiClientSecure * Update CMakeLists.txt * Update on-push.sh * Rename Network library * Remove unnecessary guard * Get the correct interface MAC address for mDND Workstation service * Apply suggestions from code review --------- Co-authored-by: Me No Dev <[email protected]> Co-authored-by: Lucas Saavedra Vaz <[email protected]>

@sgryphon

* Create ESP_NetworkInterface class and have Ethernet extending it * Update CMakeLists.txt * Split networking from WiFi (H2 can now use Ethernet) Now all libs have been checked yet. More to do on WiFi side * Fix build errors * Guard WiFi classes and fix RMII ETH examples * Decouple network related libraries from WiFi * Fix examples and WiFiUpdate * Guard WiFiProv lib to compile only on WiFi chips * Add periman string for network and "fix" mdns on the first ETH * Revert back location of Client/Server/Udp in order to accept some PRs * Fix periman * Some fixes from merging master * Fix web server missing fs.h * Move Client, Server and Udp out of WiFi * More fixes * more fixes * Fix CMakekLists and rework lib menu dependencies * Fix CMake issues * move back WiFiClient to rebase with master * Update ETH_TLK110.ino * Move back WiFiClient * Update progress * Update WiFiGeneric.cpp * More fixes * Switch AP to the new interface * Cleanup * Rename AP methods * Add extra interface info for Printable * Rename IPv6 getters to clarify that they are returning LinkLocal address cc @sgryphon * Rename network classes cc @sgryphon * Update NetworkManager.h * Rename WiFi Server and UDP * Rename WiFiClient and WiFiClientSecure * Update CMakeLists.txt * Update on-push.sh * Rename Network library * Remove unnecessary guard * Get the correct interface MAC address for mDND Workstation service * Apply suggestions from code review Co-authored-by: Lucas Saavedra Vaz <[email protected]> * Tasmota changes * 3.0.0 Network Refactoring (espressif#8760) * Create ESP_NetworkInterface class and have Ethernet extending it * Update CMakeLists.txt * Split networking from WiFi (H2 can now use Ethernet) Now all libs have been checked yet. More to do on WiFi side * Fix build errors * Guard WiFi classes and fix RMII ETH examples * Decouple network related libraries from WiFi * Fix examples and WiFiUpdate * Guard WiFiProv lib to compile only on WiFi chips * Add periman string for network and "fix" mdns on the first ETH * Revert back location of Client/Server/Udp in order to accept some PRs * Fix periman * Some fixes from merging master * Fix web server missing fs.h * Move Client, Server and Udp out of WiFi * More fixes * more fixes * Fix CMakekLists and rework lib menu dependencies * Fix CMake issues * move back WiFiClient to rebase with master * Update ETH_TLK110.ino * Move back WiFiClient * Update progress * Update WiFiGeneric.cpp * More fixes * Switch AP to the new interface * Cleanup * Rename AP methods * Add extra interface info for Printable * Rename IPv6 getters to clarify that they are returning LinkLocal address cc @sgryphon * Rename network classes cc @sgryphon * Update NetworkManager.h * Rename WiFi Server and UDP * Rename WiFiClient and WiFiClientSecure * Update CMakeLists.txt * Update on-push.sh * Rename Network library * Remove unnecessary guard * Get the correct interface MAC address for mDND Workstation service * Apply suggestions from code review --------- Co-authored-by: Me No Dev <[email protected]> Co-authored-by: Lucas Saavedra Vaz <[email protected]> --------- Co-authored-by: me-no-dev <[email protected]> Co-authored-by: Me No Dev <[email protected]> Co-authored-by: Lucas Saavedra Vaz <[email protected]>

sgryphon · 2024-03-30T03:09:35Z

I know this has already been merged. It is still not working that well for IPv6. I tested across multiple network types:

Network	Dual-Stack	IPv6	IPv4	TLS Dual-Stack
IPv4 (Shadow)	IPv4	Not Possible	Yes	Yes
Dual-stack+NAT64 (Astral)	IPv4 (1)	DNS fail	Yes (1)	DNS fail
IPv6+NAT64 (Wildspace)	DNS fail	DNS fail	DNS fail	DNS fail

(1) Dual-stack initially worked for some, but then started failing. I suspect it is lack of support for IPv6, and because I have DNS64 everything has an IPv6 address. Could be a race condition that worked before DNS64 was resolved.

Some of the errors:

Dual-stack+NAT64 network, IPv6 destination:

[ 69767][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Button 2, scenario 1, v0.1.0-156-gaf345f5
[ 69779][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Global IPv6 2407:8800:bc61:1340:a3a:f2ff:fe65:db28
[ 69794][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: IPv4 192.168.1.154
[ 69802][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Link-Local IPv6 fe80::a3a:f2ff:fe65:db28%st1
sta: <UP>
      ether 08:3A:F2:65:DB:28
      inet 192.168.1.154 netmask 255.255.255.0 broadcast 192.168.1.255
      gateway 192.168.1.1 dns 192.168.1.1
      inet6 fe80::a3a:f2ff:fe65:db28%st1 type LINK_LOCAL
      inet6 2407:8800:bc61:1340:a3a:f2ff:fe65:db28 type GLOBAL
      inet6 fd7c:e25e:67e8:40:a3a:f2ff:fe65:db28 type UNIQUE_LOCAL
[ 69840][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: DNS0 192.168.1.1
[ 69854][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: DNS1 0.0.0.0
[ 69862][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: URL: https://v6.ipv6-test.com/api/myip.php
[ 69875][D][HTTPClient.cpp:303] beginInternal(): protocol: http, host: v6.ipv6-test.com port: 80 url: /api/myip.php
[ 69885][D][HTTPClient.cpp:598] sendRequest(): request type: 'GET' redirCount: 0

[ 76427][E][NetworkManager.cpp:125] hostByName(): DNS Failed for 'v6.ipv6-test.com' with error '-5' and result '-1'
[ 81441][I][NetworkClient.cpp:269] connect(): select returned due to timeout 5000 ms for fd 48
[ 81450][D][HTTPClient.cpp:1163] connect(): failed connect to v6.ipv6-test.com:80
[ 81458][W][HTTPClient.cpp:1486] returnError(): error(-1): connection refused
[ 81465][E][Core2Logger.cpp:192] log(): [Core2Logger] CORE2: HTTP GET error -1: connection refused

me-no-dev · 2024-03-30T08:56:34Z

@sgryphon given that you have a full environment and test case, maybe you can figure out what is missing? Try maybe some menuconfig options and rebuild the libs?

Create ESP_NetworkInterface class and have Ethernet extending it

ca7ec90

VojtechBartoska linked an issue Oct 16, 2023 that may be closed by this pull request

Refactor the network stack to allow chips without WiFi and better Ethernet and ppp compatibility #8735

Closed

1 task

me-no-dev added 10 commits October 18, 2023 12:08

Update CMakeLists.txt

e42b99a

Merge branch 'master' into feature/network_refactoring

044da55

Merge branch 'master' into feature/network_refactoring

f2824bc

Merge branch 'master' into feature/network_refactoring

74a18b6

Merge branch 'master' into feature/network_refactoring

0e6d289

Split networking from WiFi (H2 can now use Ethernet)

0c561bc

Now all libs have been checked yet. More to do on WiFi side

Fix build errors

0edaa59

Guard WiFi classes and fix RMII ETH examples

dfc59e7

Decouple network related libraries from WiFi

da4d198

Fix examples and WiFiUpdate

105c739

TD-er reviewed Oct 31, 2023

View reviewed changes

libraries/ESPmDNS/src/ESPmDNS.cpp Outdated Show resolved Hide resolved

me-no-dev and others added 9 commits October 31, 2023 13:26

Guard WiFiProv lib to compile only on WiFi chips

44f899f

Merge branch 'master' into feature/network_refactoring

f4d8cba

Merge branch 'master' into feature/network_refactoring

78cc5ec

Merge branch 'master' into feature/network_refactoring

61b8675

Merge branch 'master' into feature/network_refactoring

4f087ee

Add periman string for network and "fix" mdns on the first ETH

638abc3

Revert back location of Client/Server/Udp in order to accept some PRs

7f4b1ec

Merge branch 'master' into feature/network_refactoring

3a83cfa

Fix periman

c8a4d5a

VojtechBartoska added the Status: In Progress Issue is in progress label Nov 28, 2023

VojtechBartoska added this to the 3.0.0-RC1 milestone Nov 28, 2023

me-no-dev added 2 commits November 29, 2023 16:50

Merge branch 'master' into feature/network_refactoring

8ace49a

Merge branch 'master' into feature/network_refactoring

102a776

me-no-dev added 2 commits January 24, 2024 11:12

Merge branch 'master' into feature/network_refactoring

864cec1

Some fixes from merging master

ae2a889

sgryphon reviewed Mar 20, 2024

View reviewed changes

libraries/WiFi/src/AP.cpp Show resolved Hide resolved

sgryphon reviewed Mar 20, 2024

View reviewed changes

libraries/Networking/src/NetworkInterface.cpp Outdated Show resolved Hide resolved

me-no-dev added Status: Test needed Issue needs testing Status: Review needed Issue or PR is awaiting review and removed Status: In Progress Issue is in progress labels Mar 20, 2024

me-no-dev added 5 commits March 21, 2024 13:50

Rename WiFi Server and UDP

a39dcfd

Rename WiFiClient and WiFiClientSecure

df90716

Update CMakeLists.txt

9db3df9

Update on-push.sh

224e5eb

Rename Network library

c3efaa5

lucasssvaz added the hil_test Run Hardware Tests label Mar 22, 2024

SuGlider reviewed Mar 25, 2024

View reviewed changes

me-no-dev and others added 3 commits March 25, 2024 10:36

Remove unnecessary guard

29ab925

Merge branch 'master' into feature/network_refactoring

d5d59d3

Get the correct interface MAC address for mDND Workstation service

8f79cde

lucasssvaz reviewed Mar 25, 2024

View reviewed changes

Apply suggestions from code review

7e893e9

Co-authored-by: Lucas Saavedra Vaz <[email protected]>

lucasssvaz removed the hil_test Run Hardware Tests label Mar 25, 2024

P-R-O-C-H-Y approved these changes Mar 26, 2024

View reviewed changes

lucasssvaz approved these changes Mar 26, 2024

View reviewed changes

Merge branch 'master' into feature/network_refactoring

fb0041c

me-no-dev merged commit f2026f1 into master Mar 26, 2024
40 checks passed

me-no-dev deleted the feature/network_refactoring branch March 26, 2024 21:31

mhendriks mentioned this pull request Jun 27, 2024

WifiProv BLE doesn't auto connect Wifi after initial provisioning #9943

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3.0.0 Network Refactoring #8760

3.0.0 Network Refactoring #8760

me-no-dev commented Oct 13, 2023

github-actions bot commented Dec 15, 2023 •

edited

Loading

VojtechBartoska commented Dec 21, 2023

sgryphon Mar 20, 2024

me-no-dev Mar 20, 2024

sgryphon Mar 20, 2024

SuGlider Mar 25, 2024

SuGlider Mar 25, 2024 •

edited

Loading

me-no-dev Mar 25, 2024

lucasssvaz left a comment

lucasssvaz Mar 25, 2024

me-no-dev Mar 25, 2024

lucasssvaz Mar 25, 2024

me-no-dev Mar 25, 2024

lucasssvaz Mar 25, 2024

lucasssvaz Mar 25, 2024

me-no-dev Mar 25, 2024

sgryphon commented Mar 30, 2024 •

edited

Loading

me-no-dev commented Mar 30, 2024

		@@ -0,0 +1,115 @@
		/*
		Client.h - Base class that provides Client

3.0.0 Network Refactoring #8760

3.0.0 Network Refactoring #8760

Conversation

me-no-dev commented Oct 13, 2023

github-actions bot commented Dec 15, 2023 • edited Loading

VojtechBartoska commented Dec 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuGlider Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasssvaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgryphon commented Mar 30, 2024 • edited Loading

me-no-dev commented Mar 30, 2024

github-actions bot commented Dec 15, 2023 •

edited

Loading

SuGlider Mar 25, 2024 •

edited

Loading

sgryphon commented Mar 30, 2024 •

edited

Loading