Add v6 support to IPAddress to match ArduinoCore-API

[sgryphon]

By completing this PR sufficiently, you help us to review this Pull Request quicker and also help improve the quality of Release Notes

Checklist

1. [X] Please provide specific title of the PR describing the change, including the component name (eg. „Update of Documentation link on Readme.md“)
2. [X] Please provide related links (eg. Issue which will be closed by this Pull Request)
3. [X] Please update relevant Documentation if applicable
4. [X] Please check Contributing guide

This entire section above can be deleted if all items are checked.

---

Description of Change

Please describe your proposed Pull Request and it's impact.

Update the IPAddress class to support v6 addresses, using the same interface changes as ArduinoCore-API (and thus compatible with other Arduino based platforms).

Note that the actual API changes are minimal:

• add a type() getter and corresponding enum
• add three new constructors: one for 16 octets, empty with type specifier, and byte pointer with type specifier
• add constant for IP6ADDR_ANY (similar function as IPADDR_NONE)

The changes are backward compatible, i.e. any IPv4 code will continue to get the same outputs for a given inputs. (There are extensive unit test cases in ArduinoCore-API).

Note: Whilst backwards compatible, this change does increase the memory footprint from 4 bytes to 20 bytes (assuming the enum compiles as an underlying int32_t).

Tests scenarios

Please describe on what Hardware and Software combinations you have tested this Pull Request and how.

(eg. I have tested my Pull Request on Arduino-esp32 core v2.0.2 with ESP32 and ESP32-S2 Board with this scenario)

NOTE: I am not sure how to build and test the project as I can't find instructions in the readme or contributing files. I can see the github actions scripts, but they are a bit more complicated that the ArduinoCore-API project, so I haven't reverse engineered out the steps to run all the tests yet (or the dependencies what is needed).

I am putting this code up for review while I figure this out (and maybe GitHub can run the tests).

Note that the code is mostly cut and paste from ArduinoCore-API project, which has unit tests, so should work. (The only bit not directly copied was the toString() function changes).

Related links

Please provide links to related issue, PRs etc.

(eg. Closes #number of issue)

Related to #7114, although it doesn't make it directly compatible with ESP8266 but with ArduinoCore, but does close off the issue.

Essentially fixes #6600 as it uses the RFC 5952 canonical format, which includes shortening of the left-most largest group of two or more zero fields. (The simple example in #6600 simply did the first zero fields). Note that the fromString() function parses all IPv6 representations, not just the canonical one. (There are extensive unit tests in the ArduinoCore-API project)

It is related to #6242 as it provides core support for the IPAddress class to have both v4 and v6 addresses, enabling support to be added (internally) to other classes (such as WiFi, Ethernet, etc) to handle IPv6 (without having to complicate their interfaces).

[CLAassistant]

All committers have signed the CLA.

[sgryphon]

I have not been able to run the tests locally, but did get them running (and now all successful / green) in my GitHub repo. Link to the successful run is here: https://github.com/sgryphon/arduino-esp32/actions/runs/2931290749

This PR says it needs a contributor to approve first-time running of the workflows.

[sgryphon]

Concern: Regarding the != operator.

• Usually the == and != operators should be specified together, as often they are used interchangeably, e.g. I would expect "!(a == b)" to give the same result as "a != b" (although not always the case, e.g. nullability in SQL).
• The != operator does not exist in the current IPAddress.
• When I add it, it fails some tests where the is ambiguity of "ip != 0" between implicit conversion of IPAddress to int, and then compare two int, vs conversion of 0 to IPAddress, and then comparison of IP address.
• To fix this, I think that the conversions could be marked explicit.

While adding the != operator is backwards compatible (it adds more), changing the existing conversion to be explicit would be a (fairly small) change in existing functionality.

Personally I think the missing != is a bug, that needs to be fixed, even if it breaks existing implicit conversion.

[mrengineer7777]

I see ArduinoCore does not have an IPv6 library. Opinion: If we are adding v6 support to IPAddress, we should delete IPv6Address library.

[sgryphon]

Opinion: If we are adding v6 support to IPAddress, we should delete IPv6Address

That would be a breaking change. It is okay to do this for a major version, but usually you would want to have the replacement already added (i.e. add this backwards compatible change first), then foreshadow & let people know (mark the old code deprecated, if the language supports this, and tell them to use the new).

Then a future version would remove the code.

e.g. a plan could be to add the IPv6 support first; if that is going to be release v3.0.0, then add a comment that IPv6Address will be removed in v4.0.0 and to use IPAddress instead (and then remove it in v4).

[me-no-dev]

@sgryphon are you around to rebase this? And if possible, update it to the latest API from Arduino? We would like to merge it now :)

[sgryphon]

@sgryphon are you around to rebase this? And if possible, update it to the latest API from Arduino? We would like to merge it now :)

Thanks for the ping. I will have a look, but shouldn't take too long.

[me-no-dev]

I will have a look, but shouldn't take too long.

Just copying the latest IPAddress from ArduinoCore-API should be sufficient :) there are a couple changes since you submitted this PR.

[github-actions[bot]]

	Messages
:book:	You might consider squashing your 3 commits (simplifying branch history).

👋 Hello sgryphon, 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.
- 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 :no_entry_sign: dangerJS against 12d618fd3870f72220488385cde307815e19e7d0

[sgryphon]

I have rebased, and added the recent (minor) changes from ArduinoCore... but I see you now have a commit message linter, so will also need to clean them up (I may get time over the weekend, let me see).

[me-no-dev]

but I see you now have a commit message linter

Don't worry, we are still experimenting with it :) Thanks for updating the files!

[sgryphon]

I've updated the commit messages to match the linter format. BTW when I did the initial rebase I also squashed. Before merging I did do a three-way compare and nothing had changed in arduino-esp32 since; and then I applied the changes from arduinocore api (some, e.g. toString) already existed in -esp32

[me-no-dev]

@sgryphon could you please sign the CLA (second message in the thread)

[sgryphon]

@sgryphon could you please sign the CLA (second message in the thread)

CLA is signed.

[sgryphon]

If anyone wants to have a discussion on wider IPv6 (not just IPAddress, which should be aligned with ArduinoCore), I created a general IPv6 discussion that can be used: https://github.com/espressif/arduino-esp32/discussions/9009

[s-hadinger]

I'm very annoyed by this PR that doesn't take into account what we have proposed in Tasmota. The main idea was to wrap ip ip_addr_t from LWIP to reuse as much as possible. This naive implementation completely misses the zone attribute of IPv6 addresses, hence any link-local address is unusable because of the missing zone.

I'm sorry but we just can't use this implementation at all.

For reference here are the definitions from LWIP:

/** This is the aligned version of ip6_addr_t,
    used as local variable, on the stack, etc. */
struct ip6_addr {
  u32_t addr[4];
#if LWIP_IPV6_SCOPES
  u8_t zone;
#endif /* LWIP_IPV6_SCOPES */
};

/** IPv6 address */
typedef struct ip6_addr ip6_addr_t;

[...]

/** This is the aligned version of ip4_addr_t,
   used as local variable, on the stack, etc. */
struct ip4_addr {
  u32_t addr;
};

/** ip4_addr_t uses a struct for convenience only, so that the same defines can
 * operate both on ip4_addr_t as well as on ip4_addr_p_t. */
typedef struct ip4_addr ip4_addr_t;

[...]


/**
 * @ingroup ipaddr
 * A union struct for both IP version's addresses.
 * ATTENTION: watch out for its size when adding IPv6 address scope!
 */
typedef struct ip_addr {
  union {
    ip6_addr_t ip6;
    ip4_addr_t ip4;
  } u_addr;
  /** @ref lwip_ip_addr_type */
  u8_t type;
} ip_addr_t;

[s-hadinger]

Let me explain more for those who are not familiar with link-local IPv6. Link-local addressed are used extensively by Matter protocol.

If you have multiple network interfaces, there is no way to for sure which network interface is used by which link-local addresses. Hence an additional information zone is used to track which interface must be used.

In text representation, they are separate with %, example: fe80::86cc:a8ff:fe64:b768%st1 is a link-local address on wifi st1, fe80::86cc:a8ff:fe64:c768%en2 is on Ethernet

[me-no-dev]

@s-hadinger check and comment https://github.com/espressif/arduino-esp32/pull/9016 please

[sgryphon]

PR that doesn't take into account

Small steps.

That PR was mostly written 1 year ago, to align with ArduinoCore (which does not have zone ID). ArduinoCore in turn was actually focussed for mobile (4G) devices, with a single interface assigned a global address, because carriers are moving to IPv6 only networks... so quite a different case that Matter / Thread.

The new PR (linked) integrates in the zone ID ... and I suggesting contributing it back to ArduinoCore, for consistency between platforms.

I also added a section to the discussion I created https://github.com/espressif/arduino-esp32/discussions/9009

Because yes, people misunderstand things like link-local, e.g. confusing it with local, as opposed to remote (the ends of a connection) and link-local vs global (different scopes).