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

Changes to support multiple WiFi networks. #60

Closed
wants to merge 2 commits into from

Conversation

srt19170
Copy link
Contributor

These changes allow you to specify multiple WiFi networks in config.cpp, and the code will try to connect to them in order of signal strength until it succeeds. If you specify only one network, the behavior is the same as the original code.

This may be useful in the case where the weather app will be moved around to different parts of a house where there are different networks, or between different houses, etc.

@lmarzen
Copy link
Owner

lmarzen commented Oct 23, 2023

This is a great idea. I'll check it out when I have more time. Looks like it has some compile errors, though.

@srt19170
Copy link
Contributor Author

src/client_utils.cpp:97:32: error: invalid application of 'sizeof' to incomplete type 'const String [][2]'
   int length = sizeof(WIFI_INFO) / sizeof(WIFI_INFO[0]);

Yeah, that's my bad. I was trying to avoid having to specify the length of the array holding the WiFi information. That may not be possible. I'll think about it.

@lmarzen
Copy link
Owner

lmarzen commented Oct 23, 2023

I think the issue is that it is extern'ed. The reason I used a separate .cpp and extern everything rather than putting everything in the header is to reduce compile times when people are modifying settings and want to make a small configuration change and reupload. Any changes to the config header will require a bunch of files to be recompiled since it is included in quite a few places. This makes it the job of the linker to find and replace across translation units. I've considered moving everything to the header file, now that there is a collection of preprocessor macro settings that must be in the header. There are probably better configuration schemes that I should look into.

@srt19170
Copy link
Contributor Author

Your fix seems to work, so thumbs-up from me!

@lmarzen
Copy link
Owner

lmarzen commented Oct 23, 2023

I haven't gotten to look to closely at it yet. Can you tell me, how do these changes affect time spent awake?

@srt19170
Copy link
Contributor Author

There's no impact if you only list one network.

If you list more than one network, there's an added "scan" to identify the available networks. You should then connect to the strongest of your listed networks.

If the strongest of your listed networks fails to connect (e.g., you've listed an incorrect password) you will fall through to the next available network, and so on. So in the worst case, you'll try all the networks you've listed and get them all wrong, resulting in as many connection attempts as you have listed networks.

@lmarzen
Copy link
Owner

lmarzen commented Oct 23, 2023

Thanks, this seems acceptable. I'll review it this week.

@lmarzen
Copy link
Owner

lmarzen commented Oct 30, 2023

I have been very busy with school work. Still haven't gotten a chance to look at this more closely. Thanks for your patience. It might be another week or two before I have time to check this out.

@lmarzen
Copy link
Owner

lmarzen commented Nov 12, 2023

Just following up to let you know that I haven't forgotten about this. It may be another week yet until I get to it.

@lmarzen
Copy link
Owner

lmarzen commented Apr 29, 2024

Going to close this PR. I am overhauling the project's configuration to enable configuration from a phone (default options will still be able set at compile time). I will add multiple networks as an option at that point. So, this feature is planned (for this summer) but will be implemented when the new setup process has been implemented.

@lmarzen lmarzen closed this Apr 29, 2024
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