Add DNS forwarder to DNSServer by mhightower83 · Pull Request #7237 · 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

Conversation17 Commits39 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 }})

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.

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.

Plus 1 for new complier catch!

Is it expensive in RAM when it is enabled ?
Is it always the same price, even when STA is not enabled ?
I guess it isn't useful when NAPT is not enabled.

Those questions because I wonder what is against enabling it by default, and its impact on sketch that are not using STA.

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.

I don't think an STA configured, enters directly into the DNSForwarder resource requirements.

Sorry, I wasn't thinking that all the way through. W/O STA there is no need for the forwarder.

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)

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?

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.

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 typedef

Changed run_CI_locally.sh to run ci/style_check.sh. Copied resulting style changes from /tmp to PR

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

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?

removed stale #define add const remove struct prefix

…al.ino

Co-authored-by: Max Prokhorov prokhorov.max@outlook.com

…al.ino

Co-authored-by: Max Prokhorov prokhorov.max@outlook.com

mcspr added this to the 3.1 milestone

May 8, 2022

hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request

Nov 18, 2024

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.

Labels

alpha

included in alpha release