search

sudomesh / disaster-radio

A (paused) work-in-progress long-range, low-bandwidth wireless disaster recovery mesh network powered by the sun.
https://disaster.radio
1.05k stars 109 forks source link

Improve Wifi client mode #97

Closed grisu48 closed 3 years ago

grisu48 commented 3 years ago

This commit makes connecting to an existing WiFi more robust and adds the missing secrets.h file for WiFi configuration.

It fixes issue #98 and restores the secrets.h template from an an old commit. It also adds a section to the README.md in firmware, so that it's documented on how to integrate their disaster radio node into their existing Wifi network

paidforby commented 3 years ago

Thanks for the PR! Looks good, makes Two suggestions, comment out the #define WIFI_SSID "WIFI_SSID" and #define WIFI_PASSWORD "WIFI_PASSWORD" so a node doesn't try connecting to the placeholder WIFI_SSID (also to match your description in the README). And it would be wise to add firmware/secrets.h to the .gitignore so a developer doesn't accidentally commit their private wifi network info.

grisu48 commented 3 years ago

Hi @paidforby! Thanks for the comments. I've commented out the two #defines (should have done that already before). The file secret.h is already in the global .gitignore. The only way I found to exclude changes is to not have the file in git. So I renamed the file secret.h to secret.H. The user is supposed to do

cp secret.H secret.h

And then edit the secret.h, which is not tracked by git anymore. I also updated the README accordingly.

Please let me know if this solution would work for you or if you have a better idea.

paidforby commented 3 years ago

@grisu48 I think the problem you are seeing is that you can't do it all in one commit. i.e. you need to make one commit where you add secret.h (and remove it from the global .gitignore) and then another commit where you add secret.h back to the .gitignore.

Although your solution works also, as long as people follow the instructions in the README :P

paidforby commented 3 years ago

Went ahead and merged this. I haven't tested it. And if I get inspired I make the change I suggested in my last commit, i.e. to rename secrets.H to secrets.h, so it is actually in the the gitignore.

grisu48 commented 3 years ago

Thanks for the merge. I didn't had much time to react on your comment, but consider this as input for a future improvement ;-)