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

Robustness improvements to buendia-wifi-watchdog #249

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

Conversation

schuyler
Copy link
Member

@schuyler schuyler commented Dec 3, 2019

Some improvements to buendia-wifi-watchdog made while testing #247 in the hopes of making it more robust:

  • skip running if no wifi interface is configured
  • use iwconfig rather than wpa_cli to ensure that what we're looking at is network association, and ensure that wireless-tools is required by the package
  • use ip to check that we actually have an IP address

Tested on the NUC by watching buendia-wifi-watchdog.log for several minutes, both with the network unconfigured and subsequently configured, to ensure the desired behavior. Also tested with and without a wifi interface defined. Performed same manual tests across multiple boots for confirmation.

@zestyping
Copy link
Contributor

@schuyler Quick question here—why is iwconfig better than wpa_cli?

@schuyler
Copy link
Member Author

schuyler commented Dec 4, 2019

@schuyler Quick question here—why is iwconfig better than wpa_cli?

The only good answer I have is that it works when wpa_supplicant isn't running, e.g. when the Wi-Fi network is open and doesn't require WPA security. This is not a use case the project currently has, at least not that I know of, but it was impossible for me to test the networking stack otherwise last week when the only Wi-Fi network I could use was an open one.

A less good answer is that iwconfig has been around longer and is more mature? But I wouldn't be sold on that myself per se.

Happy to take out this particular commit out of the PR if we're concerned about this choice.

@schuyler
Copy link
Member Author

@zestyping bump for review?

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