- Notifications
You must be signed in to change notification settings - Fork13.3k
Add DNS forwarder to DNSServer#7237
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Reduced size of a chunk of HTML string.Added maxlength to HTML input fields for credentials.
Added print statements for monitoring size of chunks etc.Novice improvents to HTML and variable names.
to avoid large allocations to buffer the send.
…to pr-dns-forwarder
…to pr-dns-forwarder
Plus 1 for new complier catch!
d-a-v commentedJul 17, 2020
Is it expensive in RAM when it is enabled ? Those questions because I wonder what is against enabling it by default, and its impact on sketch that are not using STA. |
mhightower83 commentedJul 17, 2020
The forwarder queue size is a power of 2. It is currently at 8 items, total size of 96 bytes. I don't think an STA configured, enters directly into the DNSForwarder resource requirements. As you indicated it isn't very useful outside of using NAPT. From time to time I see complaints about the slowly increasing of resources with each release, so I tried to make it resource flexible. Also as it is written, it is easy to increase or decrease the queue size. It is a simple constant in the .h. However, for me, 8 items seemed to work well with one host browsing. With a smaller value, I saw more dropped requests. |
mhightower83 commentedJul 17, 2020
Sorry, I wasn't thinking that all the way through. W/O STA there is no need for the forwarder. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| @@ -0,0 +1,330 @@ | |||
| #ifndefWIFIHTTP_H_ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why not .ino? just to avoid confusion with static things in headers and conditional typos
configHead, configHeadMin, and etc.
(although, I am coming from the external language server experience, perhaps it is easier in the current arduino IDE environment to work with files like this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
...and also, why not a single .ino entrypoint, with .cpp actually implementing things?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
...and also, why not a single .ino entrypoint, with .cpp actually implementing things?
I don't think I follow this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
...and also, why not a single .ino entrypoint, with .cpp actually implementing things?
I don't think I follow this point.
Only keeping NAPTCaptivePortal.ino for setup() & loop(), for the sole purpose of avoiding ide concatenation of .ino files. Just a consideration, as I noted I may be out-of-loop and this is actually easier to manage within IDE
removed typedefChanged run_CI_locally.sh to run ci/style_check.sh.Copied resulting style changes from /tmp to PR
mcspr left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks!
Just tried with a couple of devices, all seems to work like intended
libraries/DNSServer/examples/NAPTCaptivePortal/NAPTCaptivePortal.ino OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
libraries/DNSServer/examples/NAPTCaptivePortal/NAPTCaptivePortal.ino OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| #endif | ||
| DNSReplyCode _errorReplyCode; | ||
| bool _forwarder; | ||
| unsignedchar _resolvedIP[4]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
btw shouldn't this be IPAddress?
libraries/DNSServer/examples/NAPTCaptivePortal/NAPTCaptivePortal.ino OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
libraries/DNSServer/examples/NAPTCaptivePortal/NAPTCaptivePortal.ino OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
…al.inoCo-authored-by: Max Prokhorov <prokhorov.max@outlook.com>
…al.inoCo-authored-by: Max Prokhorov <prokhorov.max@outlook.com>
The key functions added are:`bool enableForwarder(const String &domainName=emptyString, const IPAddress &dns=uint32_t)0)`If specified, `enableForwarder` will update the `domainName` that is used to match DNS request to this AP's IP Address. A non-matching request will be forwarded to the DNS server specified by `dns`. Returns `true` on success.Returns `false`, * when forwarding `dns` is not set, or * unable to allocate resources for managing the DNS forward function.`void disableForwarder(const String &domainName=emptyString, bool freeResources=false)``disableForwarder` will stop forwarding DNS requests. If specified, updates the `domainName` that is matched for returning this AP's IP Address.Optionally, resources used for the DNS forward function can be freed.
Uh oh!
There was an error while loading.Please reload this page.
The key functions added are:
bool enableForwarder(const String &domainName=emptyString, const IPAddress &dns=uint32_t)0)If specified,
enableForwarderwill update thedomainNamethat is used to match DNS request to this AP's IP Address. A non-matching request will be forwarded to the DNS server specified bydns.Returns
trueon success.Returns
false,dnsis not set, orvoid disableForwarder(const String &domainName=emptyString, bool freeResources=false)disableForwarderwill stop forwarding DNS requests. If specified, updates thedomainNamethat is matched for returning this AP's IP Address.Optionally, resources used for the DNS forward function can be freed.