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

Add capability to have light static DHCP lease #5594

Merged
merged 14 commits into from Jan 25, 2019

Conversation

SmartBlug
Copy link
Contributor

Just add a small function that allow to have static lease for a specific mac when ESP8266 is used as AP.

This is really usefull to easily retrieve device content based on their IP.
Each time wifi_softap_add_dhcps_lease(mac) is called, it reserv the next IP address for this mac, starting to the first addr = dhcps_lease.start_ip.addr and with last possible allocation = dhcps_lease.end_ip.addr

Small feature but really usefull for my point of view

@devyte devyte requested a review from d-a-v January 7, 2019 03:31
@devyte
Copy link
Collaborator

devyte commented Jan 7, 2019

Maybe small, but it's actually one of the things I wanted to look into at some point in the future for my own use case.
However, you added a function that follows sdk naming to lwip, then exported it through user_interface.h. That doesn't look right to me.

@d-a-v assuming that lwip doesn't have this already buried behind some config macro or something, this might make a good proposal for them. In that case, the function name would need to change to follow lwip convention, though.
If we don't want to propose it to lwip, then we should put the function elsewhere. One option would be to add it to our lwip glue.
Whether it's adopted by lwip, or we put it in the lwip glue, or elsewhere, we can then have a wrapper on our side with the proposed name that follows the sdk naming.
What do you think?

@SmartBlug
Copy link
Contributor Author

I think it's a small change that could easily be part of lwip if they agree. Any suggestions about naming and where it should be exposed ?
I put it there because this was similar to wifi_softap_add_dhcps_lease

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 7, 2019

Your PR updates the espressif dhcpserver in lwIP-1.4.

For historical reason, we try to not modify lwIP-v1.4 because espressif might change these source files anytime (last time was 1 year ago).
Could you add a comment next to your new declaration so we can understand when we update the sdk (user_interface.h) ?
Also for the same reason, is it easy to move your new function into another .c file ?

dhcpserver in lwIP-v2 is an updated copy of dhcpserver from lwIP-v1.4 (the one you modified).
It you want to update dhcpserver in lwip2 (implementing lwIP-v2), you'll have to update tools/sdk/lwip2/builder/glue-lwip/esp-dhcpserver.c. Upstream repository is https://github.com/d-a-v/esp82xx-nonos-linklayer.

I think it's a small change that could easily be part of lwip if they agree. Any suggestions about naming and where it should be exposed ?

There is no dhcp server in upstream lwIP.
I don't know the original author of this dhcp server.
There is no license.

@SmartBlug
Copy link
Contributor Author

Is it better if I put it in a new file tools/sdk/lwip2/builder/glue-lwip/esp-dhcplease.c?

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 8, 2019 via email

@SmartBlug
Copy link
Contributor Author

Ok, so I'll update tools/sdk/lwip2/builder/glue-lwip/esp-dhcpserver.c

@SmartBlug SmartBlug force-pushed the master branch 3 times, most recently from af2bbcc to 5d9bfe3 Compare January 12, 2019 11:21
@d-a-v
Copy link
Collaborator

d-a-v commented Jan 12, 2019

You should PR on lwip2 repo first

edit:
GH-Fork the repo, then in tools/sdk/lwip2/builder, do

git remote add SmartBlug https://github.com/SmartBlug/esp82xx-nonos-linklayer.git
git push SmartBlug master

Then press the PR button on github

@SmartBlug
Copy link
Contributor Author

I tryed to do it in one time but no way...
I've just PR the first part on lwip2

d-a-v added a commit to d-a-v/Arduino that referenced this pull request Jan 13, 2019
@d-a-v
Copy link
Collaborator

d-a-v commented Jan 13, 2019

#5613 is proposed as PR in return, with your add-on,
please

  • remove your addition in .gitmodules
  • add a comment to your addition in user_interface.h

@SmartBlug
Copy link
Contributor Author

Done

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 15, 2019

add static lease on the list, this will be the next available address

@SmartBlug Can you please give more details?
After re-reading, I understand that you give a mac address to dhcp-server for it to reserve in advance an (unpredictable and unknown before a dhcp-request, but stable) IP address for future dhcp-client incoming requests.
I'm not a heavy user of AP mode and dhcp server feature, nor fully aware of this dhcp server logic.
I assume that a dhcp client renewing its lease gets the same IP address as from the precedent renewal or initial attribution.
Does this PR

  • mean that currently a rebooting client sometimes does not get the same IP address than before ?
  • will ensure that the initially assigned address will stay across client rebooting ?

@devyte
Copy link
Collaborator

devyte commented Jan 15, 2019

From my side: I assume that after having the ESP reboot the client connected to the softap get different IP addresses?

@SmartBlug
Copy link
Contributor Author

Your right, this not clear enough.
Here how it's working before I re-write the comment :
First you configure your dhcp address or dhcp lease range with current functions
Second, you start to add static leases and you asign leases starting to the first one of your lease range
As you haven't started yet, no leases have been delivered (list is empty) and is only fill with your static leases.
When a dhcp request is done, it is ACK if same or NACK if not the static you specified
if NACK, device do a Discover anr received an offer corresponding to the static lease.

Ex: you specify Static IP address for your DHCP Server :

IPAddress apIP(192, 168, 0, 1);

void setup() {
  Serial.begin(115200);
  Serial.println("\r\nStarting...");
  WiFi.mode(WIFI_AP);
  // Configure SoftAP with 192.168.0.1
  WiFi.softAPConfig(apIP, apIP, IPAddress(255, 255, 255, 0));
  WiFi.softAP(AP_ssid,AP_password);
  Serial.print("\r\nServer IP address: ");
  Serial.println(WiFi.softAPIP());

  // As SoftAP is 192.168.0.1 and no other specific indication 
  // ... so first lease will be 192.168.0.100

  uint8 mac_CAM[6] = { 0x00,0x0C,0x43,0x01,0x60,0x15 };
  uint8 mac_PC[6] = { 0xb4,0x52,0x7e,0x9a,0x19,0xa5 };

  wifi_softap_add_dhcps_lease(mac_CAM);  // always 192.168.0.100
  wifi_softap_add_dhcps_lease(mac_PC);   // always 192.168.0.101
}

If unknown device is connected, it will be 192.168.0.102
if PC is connected, it will always be 192.168.0.101
if CAM is connected, it will always be 192.168.0.100

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 15, 2019

Nice explanation and use case.
If you could add a full sketch with the above example into libraries/ESP8266WiFi/examples/StaticLease/StaticLease.ino that would be good.
One could regret we cannot specify the IP address, but it is useful as is, and can be later improved if necessary.

Please

  • update the comment in user_interface.h with a reference to the example to come
  • add a newline in the end of .gitmodules
  • the example

Hint: before committing the example, use tests/examples_restyle.sh to avoid endless issues with travis-CI.

@devyte
Copy link
Collaborator

devyte commented Jan 15, 2019

A question: WiFi.softAP() is called first, then the lease setup api is called. What happens if there are clients that connect in between? isn't that a race condition?
My suspicion, and this is just a guess, is that it will work as long as there is no yield() or delay() in between. I don't know if that is in fact the case at the moment.

d-a-v added a commit that referenced this pull request Jan 16, 2019
@SmartBlug
Copy link
Contributor Author

Nice explanation and use case.
If you could add a full sketch with the above example into libraries/ESP8266WiFi/examples/StaticLease/StaticLease.ino that would be good.
One could regret we cannot specify the IP address, but it is useful as is, and can be later improved if necessary.

Please

  • update the comment in user_interface.h with a reference to the example to come
  • add a newline in the end of .gitmodules
  • the example

Hint: before committing the example, use tests/examples_restyle.sh to avoid endless issues with travis-CI.

I first started to specify mac and ip @ but it need more changes in the current code and also require more memory.
The way the core of the code is right now (not my code, the existing one) :

  • every time a new DHCP request is received, it check if it's already in the list and if not, add a lease.
  • a lease consist of a struc in memory. This struct is created dynamicaly (memory used only when needed) and this is a chained list with struct->next pointing to the next one.
  • there is no IP @ in the struct (to avoid memory usage)
  • To get the IP, the code start with ip = dhcp_lease_first and compare to the memory... and continue until comparison match or when struct->next = none, every time, @ip = @ip +1 until it's found or it require a new one

Adding IP @ need to change this logic (no pb) but also need to add IP @ in memory (then use more memory)

That's the reason why I changed my mind and get back on just the mac @ ;-)

@SmartBlug
Copy link
Contributor Author

A question: WiFi.softAP() is called first, then the lease setup api is called. What happens if there are clients that connect in between? isn't that a race condition?
My suspicion, and this is just a guess, is that it will work as long as there is no yield() or delay() in between. I don't know if that is in fact the case at the moment.

You were right @devyte,

call to wifi_softap_add_dhcps_lease need to be done between WiFi.softAPConfig and WiFi.softAP otherwise there is a chance @ 100 is delivered to the wrong device in my previous sample !

I'll update the sample when I'll include the sample as suggested by @d-a-v.
Not possible today but I'll do it ASAP

@SmartBlug
Copy link
Contributor Author

I found also a case where leases are reset when using WiFi.persistant. This is important that WiFi.persistent is set to false otherwise softAP may re-call softAPConfig after wifi_softap_add_dhcps_lease and reset the lease chained list (this is included in the sample I just commit).
Honnestly, I don't think at all that softAP should be allowed to reset that list if the list exist...

@SmartBlug
Copy link
Contributor Author

@d-a-v : Do I need to update something else or it's ok to validate and close this PR ?

@d-a-v d-a-v self-assigned this Jan 23, 2019
Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will merge asap

@earlephilhower earlephilhower merged commit 5e4c2e9 into esp8266:master Jan 25, 2019
@mbm60
Copy link
Contributor

mbm60 commented Apr 29, 2019

If unknown device is connected, it will be 192.168.0.102
if PC is connected, it will always be 192.168.0.101
if CAM is connected, it will always be 192.168.0.100

Hi.
I tested this example, but DHCP Lease not working because the unknown device get ip 192.168.0.100 , not 102

@devyte
Copy link
Collaborator

devyte commented Apr 29, 2019

@mbm60 did you test the above code or the official staticlease.ino example?

@mbm60
Copy link
Contributor

mbm60 commented Apr 29, 2019

yes i tested libraries/ESP8266WiFi/examples/StaticLease/StaticLease.ino with version 2.5 and git version.

@devyte
Copy link
Collaborator

devyte commented Apr 30, 2019

Did you clear wifi settings? Use persistent(false)? You may be encountering the case described above.

@mbm60
Copy link
Contributor

mbm60 commented Apr 30, 2019

yes i used exactly above example, just add some debug code and persistent still false and erase all flash content before uploading.
Sometimes an unknown device get IP 192.168.0.100 and sometimes get 192.168.0.2

Platform

  • Hardware: ESP-12
  • Core Version: 2.5.0
  • Development Env: Arduino IDE
  • Operating System: Windows

Settings in IDE

  • Module: Generic ESP8266 Module
  • Flash Mode: Dout
  • Flash Size: 4MB
  • lwip Variant: v2 Lower Memory
  • Reset Method: ck
  • Flash Frequency: 40Mhz
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 115200

MCVE Sketch

#include <lwip/init.h>

#if LWIP_VERSION_MAJOR == 1

void setup() {
  Serial.begin(115200);
  Serial.println("wifi_softap_add_dhcps_lease() is not implemented with lwIP-v1");
}

void loop() {
}

#else


#include <ESP8266WiFi.h>

const char *ssid = "ESPap";
const char *password = "thereisnospoon";

IPAddress apIP(192, 168, 0, 1);

void setup() {
  struct station_info *stat_info;
  String address;
  uint8_t i;

  uint8 mac_CAM[6] = { 0x00, 0x0C, 0x43, 0x01, 0x60, 0x15 };
  uint8 mac_PC[6] = { 0xb4, 0x52, 0x7e, 0x9a, 0x19, 0xa5 };

  Serial.begin(74880);
  Serial.println();
  Serial.println("Configuring access point...");

  WiFi.persistent(false);
  WiFi.mode(WIFI_AP);
  WiFi.softAPConfig(apIP, apIP, IPAddress(255, 255, 255, 0));

  wifi_softap_add_dhcps_lease(mac_CAM);  // always 192.168.0.100
  wifi_softap_add_dhcps_lease(mac_PC);   // always 192.168.0.101

  WiFi.softAP(ssid, password);
  
  Serial.print("AP IP address: ");
  Serial.println(WiFi.softAPIP());
  Serial.println("HTTP server started");
  
  while ( WiFi.softAPgetStationNum() == 0 ) {
    delay ( 500 ); Serial.print ( "." );
  }

  stat_info = wifi_softap_get_station_info();
  while (stat_info != NULL) {
    address = IPAddress(stat_info->ip).toString();
    Serial.print("client= ");
    Serial.print(i);
    Serial.print(" IP adress is = ");
    Serial.print((address));
    Serial.print("  Mac : ");
    Serial.printf("%02X:%02X:%02X:%02X:%02X:%02X", MAC2STR(stat_info->bssid));
    stat_info = STAILQ_NEXT(stat_info, next);
    i++;
    Serial.println();
  }
}

void loop() {
}

#endif // lwIP-v2

Debug Messages

ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 1384, room 16 
tail 8
chksum 0x2d
csum 0x2d
v951aeffa
~ld

SDK:3.0.0-dev(c0f7b44)/Core:2.5.0=20500000/lwIP:STABLE-2_1_2_RELEASE/glue:1.1/BearSSL:6778687

Configuring access point...
[APConfig] local_ip: 192.168.0.1 gateway: 192.168.0.1 subnet: 255.255.255.0
[APConfig] DHCP IP start: 192.168.0.100
[APConfig] DHCP IP end: 192.168.0.200
bcn 0
del if1
usl
add if1
dhcp server start:(ip:192.168.0.1,mask:255.255.255.0,gw:192.168.0.1)
bcn 100
AP IP address: 192.168.0.1
HTTP server started
..........
wifi evt: 7
add 1
aid 1
station: 40:4a:03:57:a0:de join, AID = 1
wifi evt: 5
wifi evt: 9
client= 0 IP adress is = 192.168.0.2  Mac : 40:4A:03:57:A0:DE

@devyte
Copy link
Collaborator

devyte commented Apr 30, 2019

Please open a NEW issue. This is a PR that is already merged.

@mbm60
Copy link
Contributor

mbm60 commented Apr 30, 2019

ok

@devyte devyte mentioned this pull request Apr 30, 2019
6 tasks
@SmartBlug
Copy link
Contributor Author

This seems to be related to the comment I made on 17 Jan :
Someone reset the lease list after the allocation... Certainly another call that has been added since the merge
I'm investigating but the best option is to avoid to reset the list if the list exist as suggested
I'll see to propose something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants