Add ability to set start and end ip for softAP #6422

mbm60 · 2019-08-14T21:51:06Z

Base on #3562 and discussed in #6031

JAndrassy · 2019-08-15T11:42:43Z

dhcp_start = local_ip;
dhcp_start[3] += 99;

someone could use .200 as local IP

d-a-v · 2019-08-15T20:30:50Z

libraries/ESP8266WiFi/src/ESP8266WiFiAP.cpp

+    uint32_t end_ip = htonl(dhcp_end.v4());
+    softap_ip >>= 8;
+    if (( start_ip >> 8 != softap_ip ) || ( end_ip >> 8 != softap_ip )) {
+        DEBUG_WIFI("[APConfig] dhcp_start or dhcp_end isn't in range of local_ip Address!\n");


This test is not right. v4 addresses need to be shifted by the number of zeroed bits in the netmask address. It is indeed 8 with 255.255.255.0 but since netmask is not hardcoded, "8" neither should be.
There are other attempts for a proper API for network calculations like #6196 .

You are right. But this part of the code compares the start and the end IP with the local IP to be in the same range. And I copied it from the esp-dhcpserver.c file. for example :
If the local IP is 192.168.4.1, the start IP and end IP must be in range 192.168.4 .
The comparison with the netmask you are looking for is done elsewhere.

@d-a-v comment is correct. What happens if the subnet mask is e. g. 255.255.0.0? Then this test is incorrect.
The calculation required is a bit more elaborate.

@d-a-v @devyte
I understand what you mean and you are absolutely right. But if the subnetmask is e.g 255.255.0.0 and local_ip is 192.168.1.1 and the dhcp_start or dhcp_end is e.g 192.168.5.10, the "[APConfig] wifi_set_ip_info failed" error occurs in debug. Because of the following lines in the "esp-dhcpserver.c" file :

Because you are right saying it is dependent on dhcp server,
I propose you test and exit with an error when netmask != IPAddress(255,255,255,0).

@d-a-v Thanks for your consideration.
Since the "DHCPS_MAX_LEASE" is limited to 100, we don't need more than 254 IPs in a range. And if we were to limit the "subnet mask" to 255.255.255.0, why would there be an option to set the "subnet mask"? And wouldn't it be better to implement "softAPConfig" without it as follows?

bool ESP8266WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress dhcp_start, IPAddress dhcp_end) { IPAddress subnet(255, 255, 255, 0); info.netmask.addr = subnet.v4();

mbm60 · 2019-08-16T20:12:34Z

@JAndrassy

someone could use .200 as local IP

This part of the code applies the default value and if someone needs another start and end IP, must be set wiith --> softAPConfig(local_ip, gateway, subnet, dhcp_start, dhcp_end)

JAndrassy · 2019-08-17T05:45:38Z

@JAndrassy

someone could use .200 as local IP

This part of the code applies the default value and if someone needs another start and end IP, must be set wiith --> softAPConfig(local_ip, gateway, subnet, dhcp_start, dhcp_end)

with .200 as IP of the AP, the default dhcp start IP would be .299

mbm60 · 2019-08-17T13:03:09Z

@JAndrassy

with .200 as IP of the AP, the default dhcp start IP would be .299

It was in the old code and I just used it again.

    IPAddress ip = local_ip;
    ip[3] += 99;
    dhcp_lease.start_ip.addr = ip.v4();

mbm60 · 2019-08-26T19:03:42Z

Maybe i should close this PR because of no feedback.

devyte · 2019-08-27T04:18:40Z

no feedback means some of us haven't gotten to it yet

devyte · 2019-08-27T04:24:35Z

libraries/ESP8266WiFi/src/ESP8266WiFiAP.cpp

+    uint32_t end_ip = htonl(dhcp_end.v4());
+    softap_ip >>= 8;
+    if (( start_ip >> 8 != softap_ip ) || ( end_ip >> 8 != softap_ip )) {
+        DEBUG_WIFI("[APConfig] dhcp_start or dhcp_end isn't in range of local_ip Address!\n");


@d-a-v comment is correct. What happens if the subnet mask is e. g. 255.255.0.0? Then this test is incorrect.
The calculation required is a bit more elaborate.

libraries/ESP8266WiFi/src/ESP8266WiFiAP.cpp

devyte · 2019-08-27T04:32:48Z

libraries/ESP8266WiFi/examples/StaticLease/StaticLease.ino

-     As it depend from your first address, and need to be done BEFORE any request from client,
-     this need to be specified after WiFi.softAPConfig() and before WiFi.softAP().
+  /* Start Access Point. You can remove the password parameter if you want the AP to be open. */
+  WiFi.softAP(ssid, password);


Starting the softAP before setting up the static leases allows a race condition, where a client could potentially connect as soon as the AP is up, but before the leases are set up. In that case, the lease setup could lead to an address clash. This is the reason behind the current call order.

Yes but I did test it and if it is before start softAP, "StaticLease" won't work

devyte · 2019-08-27T04:39:55Z

libraries/ESP8266WiFi/src/ESP8266WiFiAP.cpp

 */
-bool ESP8266WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress subnet) {
+bool ESP8266WiFiAPClass::softAPConfig(IPAddress local_ip, IPAddress gateway, IPAddress subnet, IPAddress dhcp_start, IPAddress dhcp_end) {


I believe that currently the dhcp server is always on, and there is no way to turn it off. I suggest the following:

Don't add additional parameters here. Instead, implement a method overload that has the additional parameters.

Change the current method to add a bool dhcpEnabled at the end with default true. When true, dhcp is enabled per current defaults. When false, dhcp setup is skipped altogether.

Refactor both methods to not have duplicate code.

JAndrassy · 2019-08-27T05:44:33Z

and the default start IP and end IP calculation? if the end IP calculation overflows the range will have start IP larger then end IP. is it OK for DHCP server?

Merge branch 'master' into mbm60-patch-1

mbm60 added 2 commits August 15, 2019 02:20

Added ability to set start and end ip for softAP

5ad6ff9

Base on #3562 and discussed in #6031

Update ESP8266WiFiAP.cpp

802b68b

mbm60 mentioned this pull request Aug 15, 2019

Fix APConfig DHCP IP start in debug #6419

Closed

mbm60 added 3 commits August 15, 2019 15:48

Fix Static DHCP Lease

9f1b343

Fix #6031

Update StaticLease.ino

eeeffbc

Update StaticLease.ino

5bc4914

d-a-v reviewed Aug 15, 2019

View reviewed changes

mbm60 changed the title ~~Added ability to set start and end ip for softAP~~ Add ability to set start and end ip for softAP Aug 16, 2019

Merge branch 'master' into mbm60-patch-1

2374085

devyte requested changes Aug 27, 2019

View reviewed changes

mbm60 added 3 commits September 1, 2019 00:15

Merge pull request #1 from esp8266/master

2b7acf0

Merge branch 'master' into mbm60-patch-1

Add some sainity check

47bf960

Fix some includes.

560460d

mbm60 closed this Sep 7, 2019

mbm60 deleted the mbm60-patch-1 branch September 7, 2019 20:49

mbm60 restored the mbm60-patch-1 branch September 20, 2019 16:48

mbm60 deleted the mbm60-patch-1 branch October 10, 2019 08:17

mbm60 restored the mbm60-patch-1 branch November 12, 2019 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to set start and end ip for softAP #6422

Add ability to set start and end ip for softAP #6422

mbm60 commented Aug 14, 2019

JAndrassy commented Aug 15, 2019

d-a-v Aug 15, 2019

mbm60 Aug 16, 2019

devyte Aug 27, 2019

mbm60 Aug 29, 2019 •

edited

Loading

d-a-v Aug 29, 2019

mbm60 Aug 31, 2019 •

edited

Loading

mbm60 commented Aug 16, 2019

JAndrassy commented Aug 17, 2019 •

edited

Loading

mbm60 commented Aug 17, 2019 •

edited

Loading

mbm60 commented Aug 26, 2019

devyte commented Aug 27, 2019

devyte Aug 27, 2019

devyte Aug 27, 2019

mbm60 Aug 29, 2019 •

edited

Loading

devyte Aug 27, 2019

JAndrassy commented Aug 27, 2019

Add ability to set start and end ip for softAP #6422

Add ability to set start and end ip for softAP #6422

Conversation

mbm60 commented Aug 14, 2019

JAndrassy commented Aug 15, 2019

d-a-v Aug 15, 2019

Choose a reason for hiding this comment

mbm60 Aug 16, 2019

Choose a reason for hiding this comment

devyte Aug 27, 2019

Choose a reason for hiding this comment

mbm60 Aug 29, 2019 • edited Loading

Choose a reason for hiding this comment

d-a-v Aug 29, 2019

Choose a reason for hiding this comment

mbm60 Aug 31, 2019 • edited Loading

Choose a reason for hiding this comment

mbm60 commented Aug 16, 2019

JAndrassy commented Aug 17, 2019 • edited Loading

mbm60 commented Aug 17, 2019 • edited Loading

mbm60 commented Aug 26, 2019

devyte commented Aug 27, 2019

devyte Aug 27, 2019

Choose a reason for hiding this comment

devyte Aug 27, 2019

Choose a reason for hiding this comment

mbm60 Aug 29, 2019 • edited Loading

Choose a reason for hiding this comment

devyte Aug 27, 2019

Choose a reason for hiding this comment

JAndrassy commented Aug 27, 2019

mbm60 Aug 29, 2019 •

edited

Loading

mbm60 Aug 31, 2019 •

edited

Loading

JAndrassy commented Aug 17, 2019 •

edited

Loading

mbm60 commented Aug 17, 2019 •

edited

Loading

mbm60 Aug 29, 2019 •

edited

Loading