DHCP custom option by mcspr · Pull Request #8582 · esp8266/Arduino (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation12 Commits19 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
To the risk of being pedantic, If there a way to retrieve user option from flash ?
Sure, but what is the use-case anyway?
Playing around with the code here... We could have static opts as static vars, sure, but then we can't have anything dynamic that way. e.g. based on the netif IP, or some other service info we would want to add.
Suppose, we have #8451 with a pointer attached to the instance instead
dhcpSoftAP.custom_offer_options = [](auto& options, const DhcpServer& server) {
options.add(/* code = */ 43, /* data = */ {0xca, 0xfe, 0xca, 0xfe, 0xfe});
char clientId[32];
... somehow generate it from server._netif data ...
options.add(93, reinterpret_cast<const uint8_t*>(&clientId[0]), len);
};Where auto& options is the magic handler with size + code checks from above.
My fingers always hurt when storing an array in a source code, which is copied on boot from flash to ram, then by code from ram to ram. Data are there three times, so this was my question. Of course some dhcp configuration blocks are dynamic, so it should not be prevented to deal with them.
I was thinking of something like memcpy_P(optptr, &option.data.begin(), option.data.length()) instead of this loop
for (auto it = option.data.begin(); it != option.data.end(); ++it) {
*optptr++ = *it;
But yes, dhcp data blocks are often dynamic and among the static ones not many are long enough to get a real ram gain.
So yes, it was a pedantic question 😛
With the above, add() could accept u8 pointer then and do memcpy.
Let me fight the clang-format out first... It does some weird stuff with the built-in options when processing them as function arguments.
Updated. Since it is also a showcase of the existing options helpers, size of changes... increased (sry).
Arrays could be anywhere. Initializer list, stack, heap, etc.
Overloading add() was sure fun, but idk how obvious it is without code completion and syntax helpers. add_<type> might be more self-explanatory.
/to d-a-v it has automatic storage, here it's the same stack based one (just one less line for us)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one !
Hello,
In 'LwipDhcpServer.cpp' line 444 I found:std::memcpy((u8_t*)q->payload, reinterpret_cast<u8_t*>(&m), q->len);
shouldn't that be:std::memcpy((u8_t*)q->payload, (u8_t*)m, q->len);
(use 'm' instead of '&m')?
Also, in 'ESP8266WiFiAP.cpp', the call to wifi_softap_dhcps_stop() in ESP8266WiFiAPClass::softAPConfig() has been removed. Shouldn't that be kept there so that the DHCPS from the SDK is kept turned off?
In previous cores, I can use custom IP such as '192.168.0.XXX' using DHCP in access point mode, but if wifi_softap_dhcps_stop() is removed then that custom IP will not work.
Could you verify those two? Thank you!
shouldn't that be:
std::memcpy((u8_t*)q->payload, (u8_t*)m, q->len);
it should. just a bad c/p
Also, in 'ESP8266WiFiAP.cpp', the call to wifi_softap_dhcps_stop() in ESP8266WiFiAPClass::softAPConfig() has been removed. Shouldn't that be kept there so that the DHCPS from the SDK is kept turned off?
probably? iirc, there's no difference between server.end() and wifi_softap_dhcps_stop() besides wifi_softap_dhcps_status() getting changed. it ends up calling dhcps_stop() which is proxied to server.end() anyway, while also doing some extraneous checks we don't really care about
yeah, it also should. on a second look, it won't set our network info without it
mcspr added a commit to mcspr/esp8266-Arduino that referenced this pull request
mcspr added a commit to mcspr/esp8266-Arduino that referenced this pull request
As noticed in esp8266#8582 (comment)
Can't really use server.begin() and server.end() directly, only
default static IP is applied to the interface since DHCP server is
deemed 'running' (see wifi_softap_dhcps_status() return value)
mcspr added a commit that referenced this pull request
- Fix sending NACK, use helper function to fill pbuf
As noticed in #8582 (comment)
Plus, handle the case when pbuf->len is less than struct size
- Make sure to call SDK functions to start and stop DHCP server
As noticed in #8582 (comment)
Can't really use server.begin() and server.end() directly, only
default static IP is applied to the interface since DHCP server is
deemed 'running' (see wifi_softap_dhcps_status() return value)
- s
Co-authored-by: david gauchard gauchard@laas.fr
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request
works
fixup! works
back to callbacks
names
daisy chain
seconds
less inline
fix dns setter
might as well keep using initlist
/to d-a-v it has automatic storage, here it's the same stack based one (just one less line for us)
shift blame
naming
fix impl
revert to ip4 dns
merge fix
restyle
masking done wrong
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request
- Fix sending NACK, use helper function to fill pbuf
As noticed in esp8266#8582 (comment)
Plus, handle the case when pbuf->len is less than struct size
- Make sure to call SDK functions to start and stop DHCP server
As noticed in esp8266#8582 (comment)
Can't really use server.begin() and server.end() directly, only
default static IP is applied to the interface since DHCP server is
deemed 'running' (see wifi_softap_dhcps_status() return value)
- s
Co-authored-by: david gauchard gauchard@laas.fr