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

Fix the race condition that happens when in LOAD_AND_RESTORE mode and a user tries to configure a new network. #135

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mtetreault
Copy link

The DNS server is, in some use cases, unwanted. I added the possibility to disable it as required. By default it is enable, so it doesn't change the actual behavior.

My use case is the following. I have a mobile application that configure the wifi and do the pairing with Google IOT. Having the dns server was not useless.

I also moved assets and web files to folders.

@mtetreault mtetreault changed the title Allows users to enable or disable the DNS server Fix the race condition that happens when in LOAD_AND_RESTORE mode and a user tries to configure a new network. Jun 1, 2021
@mtetreault
Copy link
Author

This PR should fix:
Issues: #118 & #109

@hqawasmi & @MPC-BlackBox: Could you please try this. It should fix your issue. Let me know if you still have some issues.

@mtetreault mtetreault closed this Jun 2, 2021
@mtetreault mtetreault reopened this Jun 2, 2021
@mtetreault
Copy link
Author

@tonyp7: Are you still maintaining this project? Would you have the time to review this?

@tonyp7
Copy link
Owner

tonyp7 commented Jun 6, 2021

Hello Matthieu,

I wish I could tell you I was, but as you can clearly see nothing has moved for quite a while. I'm a new dad and moved place so I no longer have my electronics labs for now...

If you would like to be part of the maintenance and auto-accept your PR let me know!



#include "json.h"
#include "dns_server.h"
#include "nvs_sync.h"
#include "wifi_manager.h"


Copy link
Owner

Choose a reason for hiding this comment

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

I'm not too sure about this. It adds nothing useful. The build system will always define CONFIG_WIFI_MANAGER_DNS_SERVER_ENABLE

Copy link
Owner

Choose a reason for hiding this comment

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

You should define the build time variables as per wifi_manager.h which is best practice. No need to define a new global byte.

Copy link
Author

Choose a reason for hiding this comment

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

I'll have to validate, from what I remember when set to 'n' `CONFIG_WIFI_MANAGER_DNS_SERVER_ENABLE' was simply undefined. That why I check if it's defined or not.

You're right though, this should be in wifi_manager.h.

@mtetreault
Copy link
Author

Hello Matthieu,

I wish I could tell you I was, but as you can clearly see nothing has moved for quite a while. I'm a new dad and moved place so I no longer have my electronics labs for now...

If you would like to be part of the maintenance and auto-accept your PR let me know!

Congrats on you new born, if I extrapolate, he/she is about a year old right?

If you don't mind, I would love to help you maintain this repository:)

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.

2 participants