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

Inspect all networks to see if wifi is connected #9

Merged
merged 1 commit into from
May 30, 2019

Conversation

Albadon
Copy link
Contributor

@Albadon Albadon commented May 21, 2019

When connected to a VPN, ConnectivityManager.getActiveNetwork() returns the VPN's network which doesn't have the TRANSPORT_WIFI NetworkCapabilites flag set.
This causes the app to think there is no wifi connection (only "offline" pings are sent).

This solution uses WifiInfo.getNetworkId() which returns -1 if there is no active wifi network.
The alternative solution would be to iterate over all available networks to see if there is an active wifi network among them.

@@ -193,8 +193,7 @@ private boolean isConnectedToWiFi() {
return false;
}
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.M) {
NetworkCapabilities networkCapabilities = connectivityManager.getNetworkCapabilities(connectivityManager.getActiveNetwork());
return networkCapabilities != null && networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI);
return wifiManager != null && wifiManager.getConnectionInfo().getNetworkId() != -1;
Copy link
Owner

Choose a reason for hiding this comment

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

I think I'd prefer getBSSID(), because it seems to be able to distinguish between no network connected and permission to access wifi info missing (see doc ). Otherwise looks fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. What should happen if there is no permission?
Neither getNetworkId() nor getBSSID() seem to tell us if the device has a wifi connection when there is no permission to access location data:

State getNetworkId() getBSSID()
Wifi + permission X XX:XX:XX:XX:XX:XX
No wifi + permission -1 null
Wifi + no permission -1 02:00:00:00:00:00
No wifi + no permission -1 02:00:00:00:00:00

Since the purpose of this function is to determine if there is a wifi connection (regardless of permissions), I now actually prefer the alternative solution of iterating over all networks to see if it includes a wifi network.

Copy link
Owner

Choose a reason for hiding this comment

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

If you want to go with the iteration, that's totally fine with me, but the app already shows a pop-up if necessary permissions (location in this case) are missing and states that if the permissions are missing, the app will not recognize the connectet network. So at the point of this check, I think assuming the permissions are set should be ok too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All 3 methods (getNetworkId, getBSSID and network iteration) solve the issue I was having so I'll let you make the final call.
Let me know which one you prefer and I'll update the pull request accordingly.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, after thinking about it, I agree with you that the iteration is the most fitting method for what this check is supposed to do. Thanks for your efforts!

When connected to a VPN, ConnectivityManager.getActiveNetwork()
returns the VPN's network which doesn't have the TRANSPORT_WIFI
NetworkCapabilites flag set.
This causes the app to think there is no wifi connection.
@Albadon Albadon changed the title Use WifiManager to check if wifi is connected Inspect all networks to see if wifi is connected May 21, 2019
@ostrya ostrya merged commit ff7f9b0 into ostrya:master May 30, 2019
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