-
Notifications
You must be signed in to change notification settings - Fork 26
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
Arduino 3 / ESP IDF 5 compatibility #11
Conversation
c4cb765
to
226a581
Compare
Fantastic PR, much needed this, thanks! |
src/AsyncTCP.cpp
Outdated
#if ESP_IDF_VERSION_MAJOR < 5 | ||
// ip_addr_set_ip4_u32(&local_addr, _addr); | ||
local_addr.u_addr.ip4.addr = _addr; | ||
local_addr.type = IPADDR_TYPE_V4; | ||
ip_clear_no4(&local_addr); | ||
/* local_addr.type = bind_type; | ||
local_addr.u_addr.ip4.addr = (uint32_t) _addr; | ||
memcpy(local_addr.u_addr.ip6.addr, static_cast<const uint32_t*>(_addr6), sizeof(uint32_t) * 4); */ | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to ESPHome devs: this part is weird and should consider IPv6.
I've left it the way it was before, but I think there is a missing piece here in the case an IPv6Address
has been passed to the constructor. Maybe some work not completed on your side yet ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks! I just looked at the old code and web_server
in ESPHome could never have worked with IPv6 on ESP32/Arduino. I was sure I tested it, but apparently not. I'm not sure I could get it to work on current Arduino version without breaking my back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reworked this part in my fork just this evening
f4b8fe6
to
c3faef6
Compare
FYI I've just updated the PR following Arduino 3 release and a few bug reports. This PR is now more a "sync" with my fork at https://github.com/mathieucarbou/AsyncTCP Main of the changes are fixes taken from me-no-dev#173 and Ipv6 fixes plus Arduino 3 compatibility. FYI, following the IPv6 support and change to support |
:+1 👍 for merging this PR. |
This PR proposes some changes to allow the AsyncTCP lib to be compatible with Arduino 3 / ESP IDF 5 that will be released next.
Sadly, the
IPv6Address
class has been removed...I've tried to keep the API as close as possible, so that users will just have to update from
IPv6Address
toIPAddress
which is now used for both types (and this is also what many other languages also do).I would have preferred that the removal were done in a deprecating and backward compatible way instead of breaking the API like the butchery job they did. I think they should have kept this
IPv6Address
, enhanced theIPAddress
class (and make it a super class) and introduce aIPv4Address
class. The Java language has a pretty good example of abstraction already working since years.In this PR I also updated the CI job to build with both the released version and dev version of espressif / arduino.
Compilation pass in my fork, but the changes in this PR were not tested with IPv6 (only with Pv4 and Arduino 3).