From 248c1deeb4460010e3bfbad2b0084c3f253eb202 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Wed, 4 Jan 2023 03:49:07 +0300 Subject: [PATCH 1/8] single impl --- .../ESP8266WiFi/src/ESP8266WiFiGeneric.cpp | 145 ++++++++---------- .../ESP8266WiFi/src/ESP8266WiFiGeneric.h | 15 +- 2 files changed, 75 insertions(+), 85 deletions(-) diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp index 9b7fccc9a3..298e4e100f 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp @@ -22,8 +22,10 @@ */ +#include #include -#include +#include + #include #include #include "ESP8266WiFi.h" @@ -599,108 +601,89 @@ void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *ca static bool _dns_lookup_pending = false; -/** - * Resolve the given hostname to an IP address. - * @param aHostname Name to be resolved - * @param aResult IPAddress structure to store the returned IP address - * @return 1 if aIPAddrString was successfully converted to an IP address, - * else 0 - */ -int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult) -{ - return hostByName(aHostname, aResult, 10000); -} - +static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) { + if (_dns_lookup_pending) { + DEBUG_WIFI_GENERIC("[hostByName] DNS lookup still in progress\n"); + return 0; + } -int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms) -{ ip_addr_t addr; - aResult = static_cast(INADDR_NONE); + err_t err; + aResult = static_cast(INADDR_NONE); if(aResult.fromString(aHostname)) { - // Host name is a IP address use it! - DEBUG_WIFI_GENERIC("[hostByName] Host: %s is a IP!\n", aHostname); + DEBUG_WIFI_GENERIC("[hostByName] Host: %s is IP!\n", aHostname); return 1; } DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname); -#if LWIP_IPV4 && LWIP_IPV6 - err_t err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult,LWIP_DNS_ADDRTYPE_DEFAULT); -#else - err_t err = dns_gethostbyname(aHostname, &addr, &wifi_dns_found_callback, &aResult); -#endif - if(err == ERR_OK) { - aResult = IPAddress(&addr); - } else if(err == ERR_INPROGRESS) { + switch (resolveType) { + case DNSResolveType::DNS_AddrType_IPv4: + case DNSResolveType::DNS_AddrType_IPv6: + case DNSResolveType::DNS_AddrType_IPv4_IPv6: + case DNSResolveType::DNS_AddrType_IPv6_IPv4: + static_assert(std::is_same_v>, ""); + err = dns_gethostbyname_addrtype(aHostname, &addr, + &wifi_dns_found_callback, &aResult, + static_cast(resolveType)); _dns_lookup_pending = true; - // Will resume on timeout or when wifi_dns_found_callback fires. - // The final argument, intvl_ms, to esp_delay influences how frequently - // the scheduled recurrent functions (Schedule.h) are probed; here, to allow - // the ethernet driver perform work. - esp_delay(timeout_ms, []() { return _dns_lookup_pending; }, 1); - _dns_lookup_pending = false; - if(aResult.isSet()) { + break; + default: + return 0; + } + + switch (err) { + case ERR_OK: + aResult = IPAddress(&addr); + break; + // will resume on timeout or when wifi_dns_found_callback fires + case ERR_INPROGRESS: + esp_delay(timeout_ms, + []() { + return _dns_lookup_pending; + }); + + if (aResult.isSet()) { err = ERR_OK; + } else { + err = ERR_TIMEOUT; } } - if(err == ERR_OK) { + _dns_lookup_pending = false; + + if (err == ERR_OK) { DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str()); return 1; } - - DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n", aHostname, lwip_strerr(err), (int)err); + + DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n", + aHostname, lwip_strerr(err), static_cast(err)); + return 0; } -#if LWIP_IPV4 && LWIP_IPV6 -int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) +/** + * Resolve the given hostname to an IP address. + * @param aHostname Name to be resolved + * @param aResult IPAddress structure to store the returned IP address + * @return 1 if aIPAddrString was successfully converted to an IP address, + * else 0 + */ +int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult) { - ip_addr_t addr; - err_t err; - aResult = static_cast(INADDR_NONE); - - if(aResult.fromString(aHostname)) { - // Host name is a IP address use it! - DEBUG_WIFI_GENERIC("[hostByName] Host: %s is a IP!\n", aHostname); - return 1; - } - - DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname); - switch(resolveType) - { - // Use selected addrtype - case DNSResolveType::DNS_AddrType_IPv4: - case DNSResolveType::DNS_AddrType_IPv6: - case DNSResolveType::DNS_AddrType_IPv4_IPv6: - case DNSResolveType::DNS_AddrType_IPv6_IPv4: - err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult, (uint8_t) resolveType); - break; - default: - err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult, LWIP_DNS_ADDRTYPE_DEFAULT); // If illegal type, use default. - break; - } - - if(err == ERR_OK) { - aResult = IPAddress(&addr); - } else if(err == ERR_INPROGRESS) { - _dns_lookup_pending = true; - // will resume on timeout or when wifi_dns_found_callback fires - esp_delay(timeout_ms, []() { return _dns_lookup_pending; }); - _dns_lookup_pending = false; - // will return here when dns_found_callback fires - if(aResult.isSet()) { - err = ERR_OK; - } - } + return hostByNameImpl(aHostname, aResult, DNSDefaultTimeoutMs, DNSResolveTypeDefault); +} - if(err == ERR_OK) { - DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str()); - return 1; - } +int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms) +{ + return hostByNameImpl(aHostname, aResult, timeout_ms, DNSResolveTypeDefault); +} - DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %d!\n", aHostname, (int)err); - return 0; +#if LWIP_IPV4 && LWIP_IPV6 +int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) +{ + return hostByNameImpl(aHostname, aResult, timeout_ms, resolveType); } #endif diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h index 3d3ceb3262..d0525176bd 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h @@ -24,6 +24,10 @@ #define ESP8266WIFIGENERIC_H_ #include "ESP8266WiFiType.h" + +#include +#include + #include #include @@ -44,12 +48,15 @@ typedef void (*WiFiEventCb)(WiFiEvent_t); enum class DNSResolveType: uint8_t { - DNS_AddrType_IPv4 = 0, // LWIP_DNS_ADDRTYPE_IPV4 = 0 - DNS_AddrType_IPv6, // LWIP_DNS_ADDRTYPE_IPV6 = 1 - DNS_AddrType_IPv4_IPv6, // LWIP_DNS_ADDRTYPE_IPV4_IPV6 = 2 - DNS_AddrType_IPv6_IPv4 // LWIP_DNS_ADDRTYPE_IPV6_IPV4 = 3 + DNS_AddrType_IPv4 = LWIP_DNS_ADDRTYPE_IPV4, + DNS_AddrType_IPv6 = LWIP_DNS_ADDRTYPE_IPV6, + DNS_AddrType_IPv4_IPv6 = LWIP_DNS_ADDRTYPE_IPV4_IPV6, + DNS_AddrType_IPv6_IPv4 = LWIP_DNS_ADDRTYPE_IPV6_IPV4, }; +inline constexpr auto DNSDefaultTimeoutMs = 10000; +inline constexpr auto DNSResolveTypeDefault = static_cast(LWIP_DNS_ADDRTYPE_DEFAULT); + struct WiFiState; class ESP8266WiFiGenericClass { From 5b930c7c5bb82767d7380377ea9fc743bec0666d Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Wed, 4 Jan 2023 03:51:48 +0300 Subject: [PATCH 2/8] still pending after esp delay --- cores/esp8266/IPAddress.cpp | 5 ++ cores/esp8266/IPAddress.h | 4 +- .../ESP8266WiFi/src/ESP8266WiFiGeneric.cpp | 90 +++++++++++-------- 3 files changed, 59 insertions(+), 40 deletions(-) diff --git a/cores/esp8266/IPAddress.cpp b/cores/esp8266/IPAddress.cpp index c269d00506..25660ec0b3 100644 --- a/cores/esp8266/IPAddress.cpp +++ b/cores/esp8266/IPAddress.cpp @@ -27,6 +27,11 @@ IPAddress::IPAddress(const IPAddress& from) ip_addr_copy(_ip, from._ip); } +IPAddress::IPAddress(IPAddress&& from) +{ + ip_addr_copy(_ip, from._ip); +} + IPAddress::IPAddress() { _ip = *IP_ANY_TYPE; // lwIP's v4-or-v6 generic address } diff --git a/cores/esp8266/IPAddress.h b/cores/esp8266/IPAddress.h index 894d431c68..3c4d12965c 100644 --- a/cores/esp8266/IPAddress.h +++ b/cores/esp8266/IPAddress.h @@ -66,7 +66,9 @@ class IPAddress: public Printable { public: // Constructors IPAddress(); - IPAddress(const IPAddress& from); + IPAddress(const IPAddress&); + IPAddress(IPAddress&&); + IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet); IPAddress(uint32_t address) { ctor32(address); } IPAddress(unsigned long address) { ctor32(address); } diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp index 298e4e100f..4ea9f05c28 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -597,60 +598,68 @@ bool ESP8266WiFiGenericClass::isSleepLevelMax () { // ------------------------------------------------ Generic Network function --------------------------------------------- // ----------------------------------------------------------------------------------------------------------------------- -void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg); +namespace { -static bool _dns_lookup_pending = false; +struct _dns_found_result { + IPAddress addr; + bool done; +}; -static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) { - if (_dns_lookup_pending) { - DEBUG_WIFI_GENERIC("[hostByName] DNS lookup still in progress\n"); - return 0; - } +} - ip_addr_t addr; - err_t err; +static void _dns_found_callback(const char *, const ip_addr_t *, void *); - aResult = static_cast(INADDR_NONE); - if(aResult.fromString(aHostname)) { +static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) { + if (aResult.fromString(aHostname)) { DEBUG_WIFI_GENERIC("[hostByName] Host: %s is IP!\n", aHostname); return 1; } + static_assert(std::is_same_v>, ""); DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname); - switch (resolveType) { - case DNSResolveType::DNS_AddrType_IPv4: - case DNSResolveType::DNS_AddrType_IPv6: - case DNSResolveType::DNS_AddrType_IPv4_IPv6: - case DNSResolveType::DNS_AddrType_IPv6_IPv4: - static_assert(std::is_same_v>, ""); - err = dns_gethostbyname_addrtype(aHostname, &addr, - &wifi_dns_found_callback, &aResult, - static_cast(resolveType)); - _dns_lookup_pending = true; - break; - default: - return 0; - } + + ip_addr_t addr; + auto pending = std::make_unique<_dns_found_result>( + _dns_found_result{ + .addr = IPADDR_NONE, + .done = false, + }); + + err_t err = dns_gethostbyname_addrtype(aHostname, + &addr, &_dns_found_callback, pending.get(), + static_cast(resolveType)); switch (err) { + // Address already known case ERR_OK: - aResult = IPAddress(&addr); + aResult = addr; + break; + + // We are no longer able to issue requests + case ERR_MEM: break; - // will resume on timeout or when wifi_dns_found_callback fires + + // We need to wait for c/b to fire *or* we exit on our own timeout + // (which also requires us to notify the c/b that it is supposed to delete the pending obj) case ERR_INPROGRESS: esp_delay(timeout_ms, - []() { - return _dns_lookup_pending; + [&]() { + return !pending->done; }); - if (aResult.isSet()) { - err = ERR_OK; + if (pending->done) { + if ((pending->addr).isSet()) { + aResult = pending->addr; + err = ERR_OK; + } } else { + pending.release(); + pending->done = true; err = ERR_TIMEOUT; } - } - _dns_lookup_pending = false; + break; + } if (err == ERR_OK) { DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str()); @@ -693,16 +702,19 @@ int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResul * @param ipaddr * @param callback_arg */ -void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg) +static void _dns_found_callback(const char *, const ip_addr_t *ipaddr, void *arg) { - (void) name; - if (!_dns_lookup_pending) { + auto result = reinterpret_cast<_dns_found_result *>(arg); + if (result->done) { + delete result; return; } - if(ipaddr) { - (*reinterpret_cast(callback_arg)) = IPAddress(ipaddr); + + if (ipaddr) { + result->addr = IPAddress(ipaddr); } - _dns_lookup_pending = false; // resume hostByName + + result->done = true; esp_schedule(); } From dae257417ef1399fce48c88521a86d0ca3cbc902 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Wed, 4 Jan 2023 19:49:46 +0300 Subject: [PATCH 3/8] oops --- libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp index 4ea9f05c28..039444a4c4 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp @@ -653,8 +653,8 @@ static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t ti err = ERR_OK; } } else { - pending.release(); pending->done = true; + pending.release(); err = ERR_TIMEOUT; } From 62e3fe2e6296746c5657800d5cbaf6614fa20b2c Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Wed, 4 Jan 2023 19:56:01 +0300 Subject: [PATCH 4/8] text --- libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp index 039444a4c4..93a025cf3a 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp @@ -667,7 +667,10 @@ static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t ti } DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n", - aHostname, lwip_strerr(err), static_cast(err)); + aHostname, + (err == ERR_TIMEOUT) ? "Timeout" : + (err == ERR_INPROGRESS) ? "No response" : + "Unknown", static_cast(err)); return 0; } @@ -702,9 +705,9 @@ int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResul * @param ipaddr * @param callback_arg */ -static void _dns_found_callback(const char *, const ip_addr_t *ipaddr, void *arg) +static void _dns_found_callback(const char*, const ip_addr_t* ipaddr, void* arg) { - auto result = reinterpret_cast<_dns_found_result *>(arg); + auto result = reinterpret_cast<_dns_found_result*>(arg); if (result->done) { delete result; return; From bb1e7ba4947955cf992d658ccdf5fbce35f3dfca Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 5 Jan 2023 01:21:22 +0300 Subject: [PATCH 5/8] tests --- tests/host/common/ClientContextTools.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/host/common/ClientContextTools.cpp b/tests/host/common/ClientContextTools.cpp index cf9987fea0..0b89fb1029 100644 --- a/tests/host/common/ClientContextTools.cpp +++ b/tests/host/common/ClientContextTools.cpp @@ -37,18 +37,26 @@ #include // gethostbyname -err_t dns_gethostbyname(const char* hostname, ip_addr_t* addr, dns_found_callback found, - void* callback_arg) +err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback, void*, u8 type) { - (void)callback_arg; - (void)found; - struct hostent* hbn = gethostbyname(hostname); + auto* hbn = gethostbyname(hostname); if (!hbn) return ERR_TIMEOUT; - addr->addr = *(uint32_t*)hbn->h_addr_list[0]; + + uint32_t tmp; + std::memcpy(&tmp, hbn->h_addr_list, sizeof(tmp)); + addr->addr = tmp; + return ERR_OK; } +err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback found, + void* callback_arg) +{ + return dns_gethostbyname_addrtype(hostname, addr, found, callback_arg, LWIP_DNS_ADDRTYPE_DEFAULT); + +} + static struct tcp_pcb mock_tcp_pcb; tcp_pcb* tcp_new(void) { From c437847afa4d6a7001b21110ff9082f0e5dd2694 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 5 Jan 2023 01:25:47 +0300 Subject: [PATCH 6/8] fixup! tests --- tests/host/common/ClientContextTools.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/host/common/ClientContextTools.cpp b/tests/host/common/ClientContextTools.cpp index 0b89fb1029..62740f1bab 100644 --- a/tests/host/common/ClientContextTools.cpp +++ b/tests/host/common/ClientContextTools.cpp @@ -44,7 +44,7 @@ err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_foun return ERR_TIMEOUT; uint32_t tmp; - std::memcpy(&tmp, hbn->h_addr_list, sizeof(tmp)); + std::memcpy(&tmp, hbn->h_addr_list[0], sizeof(tmp)); addr->addr = tmp; return ERR_OK; From c36668a1d2dd7009ce83b8ba0ba7f9dbe7525a17 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 5 Jan 2023 01:27:09 +0300 Subject: [PATCH 7/8] fixup! fixup! tests --- tests/host/common/ClientContextTools.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/host/common/ClientContextTools.cpp b/tests/host/common/ClientContextTools.cpp index 62740f1bab..74f66fd6bb 100644 --- a/tests/host/common/ClientContextTools.cpp +++ b/tests/host/common/ClientContextTools.cpp @@ -37,7 +37,8 @@ #include // gethostbyname -err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback, void*, u8 type) +err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback, void*, + u8 type) { auto* hbn = gethostbyname(hostname); if (!hbn) @@ -51,10 +52,10 @@ err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_foun } err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback found, - void* callback_arg) + void* callback_arg) { - return dns_gethostbyname_addrtype(hostname, addr, found, callback_arg, LWIP_DNS_ADDRTYPE_DEFAULT); - + return dns_gethostbyname_addrtype(hostname, addr, found, callback_arg, + LWIP_DNS_ADDRTYPE_DEFAULT); } static struct tcp_pcb mock_tcp_pcb; From 6986014b5c83d384680f462b0306b0db29ec0b99 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 5 Jan 2023 02:33:38 +0300 Subject: [PATCH 8/8] fix esp_delay not doing its thing --- libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp index 93a025cf3a..be482737a8 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp @@ -642,10 +642,11 @@ static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t ti // We need to wait for c/b to fire *or* we exit on our own timeout // (which also requires us to notify the c/b that it is supposed to delete the pending obj) case ERR_INPROGRESS: + // Re-check every 10ms, we expect this to happen fast esp_delay(timeout_ms, [&]() { return !pending->done; - }); + }, 10); if (pending->done) { if ((pending->addr).isSet()) {