Skip to content
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

Make DNS resolution order selectable during runtime or compile time #6865

Open
wants to merge 45 commits into
base: master
from

Conversation

@altelch
Copy link

altelch commented Dec 1, 2019

Modified ESP8266WifiGeneric to allow selection of DNS resolution order. Can be controlled by dns.h defines or in an extended api call Wifi.hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, uint8_t resolveType) in dual stack mode (LWIP_IPV4 and LWIP_IPV6 is set).

…only in dual stack mode).
@devyte devyte self-requested a review Dec 2, 2019
altelch added 2 commits Dec 2, 2019
Copy link
Collaborator

d-a-v left a comment

The core API is respected and extended with lwIP-v2's IPv6 API, thanks !
This however has a lack of documentation.
There is currently no documentation on IPv6, but only one example.
Could you update the IPv6.ino example and show how the new argument can be used ?

altelch and others added 7 commits Dec 4, 2019
@altelch

This comment has been minimized.

Copy link
Author

altelch commented Dec 4, 2019

Sorry for the commit-chaos. Hat no dev-environment at hand. Now IPv6.ino has an example on how to use the extended hostByName().

libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp Outdated Show resolved Hide resolved
libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp Outdated Show resolved Hide resolved
libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp Outdated Show resolved Hide resolved
altelch and others added 11 commits Dec 1, 2019
altelch
… enum class isn't used.
@altelch

This comment has been minimized.

Copy link
Author

altelch commented Dec 7, 2019

Ok, changed to using an enum class for DNS resolution type. Hope this is what you intended.

@d-a-v d-a-v requested review from devyte and d-a-v Dec 7, 2019
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

This comment has been minimized.

Copy link
@d-a-v

d-a-v Dec 10, 2019

Collaborator

What do you think of

{
    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,
}

This comment has been minimized.

Copy link
@altelch

altelch Dec 10, 2019

Author

Hi, this was exactly my first try but it caused problems with the CI built as some of the tests failed because of LWIP_DNS_ADDRTYPE_* isn't defined during compile time. I did put the typedef into an IF clause to have it only built if Dualstack is enabled but that caused a even more strange built problem with test 1. This was the only working solution I could find without breakting the tests.

@d-a-v
d-a-v approved these changes Dec 10, 2019
Copy link
Collaborator

d-a-v left a comment

Thanks !

altelch added 21 commits Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.