arduino-esp32 icon indicating copy to clipboard operation
arduino-esp32 copied to clipboard

Add "Get Global IPv6 Address" and update the example.

Open fryefryefrye opened this issue 2 years ago • 6 comments

https://github.com/espressif/arduino-esp32/issues/5624#issuecomment-1200153422


Description of Change

Help to Get "Global IPv6 Address"

Tests scenarios

I have tested my Pull Request on Arduino-esp32 core v2.0.4 with ESP32.

log out put:

STA Local IPv6: fe80:0000:0000:0000:a6cf:12ff:fe99:eabc
STA Global IPv6: 2409:8920:0270:3d67:a6cf:12ff:fe99:eabc

fryefryefrye avatar Jul 30 '22 14:07 fryefryefrye

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: me-no-dev
:x: fryefryefrye
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jul 30 '22 14:07 CLAassistant

I think this needs a short conversation about what "localIP" really means here. If it is just the assigned address, then scope should be a parameter in localIPv6 rather than making a new method.

lbernstone avatar Jul 30 '22 15:07 lbernstone

I think this needs a short conversation about what "localIP" really means here. If it is just the assigned address, then scope should be a parameter in localIPv6 rather than making a new method.

In IPv4 , only have remote address and local address.

In IPv6, address which call "Local Link address" which is start with "fe80", is calculated by MAC address ,and can only used in LAN. Global IPv6 Address can be access on the internet. But they are all "local address".

But I think making a new method is more easy for the user.

fryefryefrye avatar Jul 30 '22 15:07 fryefryefrye

Thanks for the PR @fryefryefrye.

I'm adding this to 2.1.0 milestone because this needs more investigation and work. We will evaluate this PR soon.

VojtechBartoska avatar Aug 08 '22 12:08 VojtechBartoska

Related PRs: #6712 & #6667

VojtechBartoska avatar Aug 08 '22 12:08 VojtechBartoska

I suggest not trying to patch in support for just one additional address, but recognise that with IPv6 a device will usually have varying numbers of multiple addresses.

e.g. Have a signature like IPAddress localIP(int8_t index = 0) that returns as many addresses as are available (or NONE once they have run out). Maybe also have a signature like int8_t getLocalIPs(IPAddress ipAddresses[], int8_t max) that will fill an array (up to max), and return the number of addresses.

Note that ArduinoCore-API now has IPAddress that supports both v4 and v6. Generally the client code doesn't want to care about which version of IP is being used.

Note that local means this end of the connection (as opposed to the remote end), and is different from link-local.

As discussed above, link-local are addresses starting with "fe80:". The equivalent in IPv4 is a "169.254" address, which is only assigned when there is no network, whereas with IPv6 they are always assigned.

In IPv4 and endpoint usually only has one address, e.g. either a global address, private network address, or if no network then a link-local address (e.g. used in point-to-point connections).

With IPv6 an endpoint will usually have multiple local addresses -- a link-local address (autogenerated), as well as one or more global scope addresses, including both globally routeable addresses and maybe unique local addresses ("fd.."), both stateless/EUI-64 addresses and maybe DHCP assigned addresses, and with privacy extensions possibly also some additional rotating addresses.

For example, my local machine has 1x IPv4, and 7x IPv6 addresses.

Although Arduino/lwIP may not yet support all these types of addresses, then interface should prepare for varying numbers of addresses.

Generally when getting a single address, i.e. localIP() it is probably to display to the user (or in a log) as the address, useful for checking connections at the other end, etc.

Without any knowledge of the intended destination, I suggest ordering local IP addresses as if the destination is a global address, if possible, i.e. preference global addresses (globally routable or unique local addresses) over link-local, and IPv6 over IPv4 (see RFC 6724).

This won't always be what the user wants (e.g. in a dual stack network maybe they want to check the address in the logs of an IPv4 only destination), but if you only return a single address then you can't always know what will be needed.

sgryphon avatar Aug 24 '22 11:08 sgryphon

Warnings
:warning:

Some issues found for the commit messages in this PR:

  • the commit message "Update WiFiIPv6.ino":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update WiFiSTA.cpp":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update WiFiSTA.cpp":
    • summary looks empty
    • type/action looks empty
  • the commit message "Update WiFiSTA.h":
    • 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
  • 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).

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

👋 Hello fryefryefrye, 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 :no_entry_sign: dangerJS against 2b45fdf32f730e4d9a5d2906ff2d703ed5e639b2

github-actions[bot] avatar Dec 13 '23 09:12 github-actions[bot]

@sgryphon gave very good proposal. IPv6Address class will go into the history, now that IPAddress will support both v4 and v6. I will leave this open, just to reming me to add proper integration to the interfaces to get all assigned IPAddresses (both v4 and v6 flavors)

me-no-dev avatar Dec 13 '23 09:12 me-no-dev

Rather than have discussions across multiple issues and pull requests, I created a general IPv6 discussion that can be used: https://github.com/espressif/arduino-esp32/discussions/9009

sgryphon avatar Dec 17 '23 07:12 sgryphon