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

WiFi: ARP gratuitous API for wifi station mode #6889

Open
wants to merge 13 commits into
base: master
from

Conversation

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 7, 2019

Based on multiple reports and examples

fixes #6886
fixes #5998 (together with #6484, hopefully)

d-a-v added 3 commits Dec 7, 2019
@d-a-v d-a-v mentioned this pull request Dec 7, 2019
4 of 6 tasks complete
@TD-er

This comment has been minimized.

Copy link
Contributor

TD-er commented Dec 7, 2019

Just looking at the code and I think this is probably not optimal to use.

bool ESP8266WiFiSTAClass::stationKeepAliveEnabled ()
{
    return _keepStationAliveUs != 0;
}

void ESP8266WiFiSTAClass::stationKeepAliveStop ()
{
    _keepStationAliveUs = -1;
    // will be set to 0 at recurrent call
}

bool ESP8266WiFiSTAClass::stationKeepAliveSetupMs (int ms)
{
    if (_keepStationAliveUs != 0 || ms <= 0)
        return false;
[...]

So on other words, you can only change the value by calling stop and then wait for the next scheduled interval before you can even set a new value.
Worst case scenario is to wait for as long as the previous set interval.
It would make sense to set the interval to anything > 0 and if you want to stop it, call the stop function.
Now you just have to poll as long as you receive false to set a new interval.

Also I do miss to send (or restart the scheduler) when a wifi connection to the AP is established.
That's the most important one to send these ARP packets. If it was set to for example 10 seconds and you make a connection right after the scheduler tried sending these ARP packets, you may be unable to make a connection to another host, for about 10 seconds after successful wifi connection was established.

@d-a-v

This comment has been minimized.

Copy link
Collaborator Author

d-a-v commented Dec 8, 2019

@TD-er please review the API change

d-a-v added 3 commits Dec 8, 2019
@d-a-v

This comment has been minimized.

Copy link
Collaborator Author

d-a-v commented Dec 8, 2019

@TD-er the API has changed, based on your use case. Setting the interval will:

  • cancel everything that was planned
  • immediately send a packet
  • schedule the next ones based on the new interval
@d-a-v d-a-v requested a review from devyte Dec 8, 2019
@Jason2866 Jason2866 mentioned this pull request Dec 9, 2019
12 of 15 tasks complete
&& interface->num == STATION_IF
&& (!ip4_addr_isany_val(*netif_ip4_addr(interface)))
#endif
)

This comment has been minimized.

Copy link
@TD-er

TD-er Dec 10, 2019

Contributor

Just curious.
A lot of code examples (e.g. in AddrList.h) iterate over the addrList to see if one got a valid IP-address.
The code here does iterate over the interfaces.

What is the better approach?
I got the feeling one of my functions using the examples in AddrList.h does sometimes yield a false positive result when looping over the addrList, so that's why I am asking.

This comment has been minimized.

Copy link
@d-a-v

d-a-v Dec 10, 2019

Author Collaborator

AddrList is meant to iterate over IP addresses (multiple per lwIP interfaces with IPv6), implicitly over all interfaces. But there's always only one IPv4 address per interface.

Here we only need to iterate over the (and once per) interface/s to announce the mac address which is unique per lwIP interface. Checking IPv4 is sufficient to know whether the interface needs ARP announce.

When IPv6 is not enabled, looping with AddrList is like walking through netif_list.

@marrold

This comment has been minimized.

Copy link

marrold commented Dec 12, 2019

Novice question, is there a simple way I can test this branch in platformio?

@d-a-v

This comment has been minimized.

Copy link
Collaborator Author

d-a-v commented Dec 12, 2019

If you are using git for this repository in your platformIO setup, then yes.
You first need to checkout this PR (help with this) then rebuild using PIO.

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.