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(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) #294

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import io.flutter.view.FlutterNativeView;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand Down Expand Up @@ -73,6 +74,8 @@ public class WifiIotPlugin
private List<WifiNetworkSuggestion> networkSuggestions;
private List<String> ssidsToBeRemovedOnExit = new ArrayList<String>();
private List<WifiNetworkSuggestion> suggestionsToBeRemovedOnExit = new ArrayList<>();
//last connected network ID from outside the app
private int lastConnectedNetworkId = -1;

// Permission request management
private boolean requestingPermission = false;
Expand All @@ -98,20 +101,12 @@ private void initWithActivity(Activity activity) {

// cleanup
private void cleanup() {
if (!ssidsToBeRemovedOnExit.isEmpty()) {
List<WifiConfiguration> wifiConfigList = moWiFi.getConfiguredNetworks();
for (String ssid : ssidsToBeRemovedOnExit) {
for (WifiConfiguration wifiConfig : wifiConfigList) {
if (wifiConfig.SSID.equals(ssid)) {
moWiFi.removeNetwork(wifiConfig.networkId);
}
}
}
}
removeAddedNetworks();
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q && !suggestionsToBeRemovedOnExit.isEmpty()) {
moWiFi.removeNetworkSuggestions(suggestionsToBeRemovedOnExit);
}
// setting all members to null to avoid memory leaks
lastConnectedNetworkId = -1;
channel = null;
eventChannel = null;
moActivity = null;
Expand All @@ -120,6 +115,20 @@ private void cleanup() {
moWiFiAPManager = null;
}

private void removeAddedNetworks() {
if (!ssidsToBeRemovedOnExit.isEmpty()) {
List<WifiConfiguration> wifiConfigList = moWiFi.getConfiguredNetworks();
for (String ssid : ssidsToBeRemovedOnExit) {
for (WifiConfiguration wifiConfig : wifiConfigList) {
if (wifiConfig.SSID.equals(ssid)) {
moWiFi.removeNetwork(wifiConfig.networkId);
}
}
}
}
ssidsToBeRemovedOnExit.clear();
}

/** Plugin registration. This is used for registering with v1 Android embedding. */
public static void registerWith(Registrar registrar) {
final MethodChannel channel = new MethodChannel(registrar.messenger(), "wifi_iot");
Expand Down Expand Up @@ -968,7 +977,7 @@ private void registerWifiNetwork(final MethodCall poCall, final Result poResult)
android.net.wifi.WifiConfiguration conf =
generateConfiguration(ssid, bssid, password, security, isHidden);

int updateNetwork = registerWifiNetworkDeprecated(conf);
int updateNetwork = registerWifiNetworkDeprecated(conf, false);

if (updateNetwork == -1) {
poResult.error("Error", "Error updating network configuration", "");
Expand Down Expand Up @@ -1107,6 +1116,10 @@ private void disconnect(Result poResult) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
//noinspection deprecation
disconnected = moWiFi.disconnect();
if (lastConnectedNetworkId != -1) {
//android 8.1 won't automatically reconnect to the previous network if it shares the same SSID
moWiFi.enableNetwork(lastConnectedNetworkId, true);
}
} else {
if (networkCallback != null) {
final ConnectivityManager connectivityManager =
Expand Down Expand Up @@ -1186,14 +1199,25 @@ private void removeWifiNetwork(MethodCall poCall, Result poResult) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
List<android.net.wifi.WifiConfiguration> mWifiConfigList = moWiFi.getConfiguredNetworks();
for (android.net.wifi.WifiConfiguration wifiConfig : mWifiConfigList) {
String comparableSSID = ('"' + prefix_ssid); //Add quotes because wifiConfig.SSID has them
if (wifiConfig.SSID.startsWith(comparableSSID)) {
moWiFi.removeNetwork(wifiConfig.networkId);
moWiFi.saveConfiguration();
removed = true;
break;
String comparableSSID =
('"' + prefix_ssid + '"'); //Add quotes because wifiConfig.SSID has them
if (wifiConfig.SSID.equals(comparableSSID)) {
Boolean isRemoved = moWiFi.removeNetwork(wifiConfig.networkId);
if (isRemoved) {
moWiFi.saveConfiguration();
removed = true;
//if the last connected network was our app's network, reset the last connected network
if (wifiConfig.networkId == lastConnectedNetworkId) {
lastConnectedNetworkId = -1;
}
}
//multiple networks with the same SSID could be removed
}
}
if (lastConnectedNetworkId != -1) {
//android 8.1 won't automatically reconnect to the previous network if it shares the same SSID
moWiFi.enableNetwork(lastConnectedNetworkId, true);
}
}

// remove network suggestion
Expand Down Expand Up @@ -1399,7 +1423,8 @@ public void onUnavailable() {
}

@SuppressWarnings("deprecation")
private int registerWifiNetworkDeprecated(android.net.wifi.WifiConfiguration conf) {
private int registerWifiNetworkDeprecated(
android.net.wifi.WifiConfiguration conf, Boolean joinOnce) {
int updateNetwork = -1;
int registeredNetwork = -1;

Expand All @@ -1414,14 +1439,54 @@ private int registerWifiNetworkDeprecated(android.net.wifi.WifiConfiguration con
|| wifiConfig.BSSID.equals(conf.BSSID))) {
conf.networkId = wifiConfig.networkId;
registeredNetwork = wifiConfig.networkId;
updateNetwork = moWiFi.updateNetwork(conf);
//only try to update the configuration if joinOnce is false
//otherwise use the new add/update method
if (joinOnce != null && !joinOnce.booleanValue()) {
updateNetwork = moWiFi.updateNetwork(conf);
//required for pre API 26
moWiFi.saveConfiguration();
}
//Android 6.0 and higher no longer allows you to update a network that wasn't created
//from our app, nor delete it, nor add a network with the same SSID
//See https://developer.android.com/about/versions/marshmallow/android-6.0-changes.html#behavior-network
if (updateNetwork == -1) {
//we add some random number to the conf SSID, add that network, then change the SSID
// back to circumvent the issue
String ssid = conf.SSID;
Random random = new Random(System.currentTimeMillis());
//loop in the rare case that the generated ssid already exists
for (int i = 0; i < 20; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this retry loop necessary? shouldn't failing/abort make more sense after one attempt.

Copy link
Author

Choose a reason for hiding this comment

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

From what I remember, I sometimes had it fail on me without explicit reason, but it has been a while since I tried, so I'm not sure why. The loop doesn't take that long in that failure case.

int randomInteger = random.nextInt(10000);
//create a valid SSID with max length of 32
String ssidRandomized = ssid + randomInteger;
Copy link
Member

Choose a reason for hiding this comment

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

ssid will be wrapped around quote (") - that should be trimmed, right?

Copy link
Author

Choose a reason for hiding this comment

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

It could be, but it isn't necessary, since we mostly care about the length anyway. It would be "cleaner" to trim it, yes.

int ssidRandomizedExtraLength = ssidRandomized.length() - 32;
if (ssidRandomizedExtraLength > 0) {
ssidRandomized =
ssid.substring(0, ssid.length() - ssidRandomizedExtraLength) + randomInteger;
}
conf.SSID = "\"" + ssidRandomized + "\"";
updateNetwork = moWiFi.addNetwork(conf); // Add my wifi with another name
conf.SSID = ssid;
conf.networkId = updateNetwork;
updateNetwork =
moWiFi.updateNetwork(
conf); // After my wifi is added with another name, I change it to the desired name
moWiFi.saveConfiguration();
if (updateNetwork != -1) {
break;
}
}
}
//no need to continue looping
break;
}
}
}

/// If network not already in configured networks add new network
if (updateNetwork == -1) {
updateNetwork = moWiFi.addNetwork(conf);
conf.networkId = updateNetwork;
moWiFi.saveConfiguration();
}

Expand Down Expand Up @@ -1491,7 +1556,7 @@ private Boolean connectToDeprecated(
android.net.wifi.WifiConfiguration conf =
generateConfiguration(ssid, bssid, password, security, isHidden);

int updateNetwork = registerWifiNetworkDeprecated(conf);
int updateNetwork = registerWifiNetworkDeprecated(conf, joinOnce);

if (updateNetwork == -1) {
return false;
Expand All @@ -1500,6 +1565,9 @@ private Boolean connectToDeprecated(
if (joinOnce != null && joinOnce.booleanValue()) {
ssidsToBeRemovedOnExit.add(conf.SSID);
}
if (lastConnectedNetworkId == -1) {
lastConnectedNetworkId = moWiFi.getConnectionInfo().getNetworkId();
}

boolean disconnect = moWiFi.disconnect();
if (!disconnect) {
Expand All @@ -1510,24 +1578,38 @@ private Boolean connectToDeprecated(
if (!enabled) return false;

boolean connected = false;
int networkId = -1;
Copy link
Member

Choose a reason for hiding this comment

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

This change looks unnecessary, it is not being used outside the for loop scope.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah the declaration can be done inside the for loop instead.

for (int i = 0; i < 20; i++) {
WifiInfo currentNet = moWiFi.getConnectionInfo();
int networkId = currentNet.getNetworkId();
networkId = currentNet.getNetworkId();
SupplicantState netState = currentNet.getSupplicantState();

// Wait for connection to reach state completed
// to discard false positives like auth error
if (networkId != -1 && netState == SupplicantState.COMPLETED) {
connected = networkId == updateNetwork;
break;
if (connected) {
break;
} else {
disconnect = moWiFi.disconnect();
if (!disconnect) {
break;
}

enabled = moWiFi.enableNetwork(updateNetwork, true);

Choose a reason for hiding this comment

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

Looks like this updated enabled value isn't getting used after updating it here. Is it even necessary to assign it to a value if it won't get read after this line?

Copy link
Member

@daadu daadu Aug 28, 2022

Choose a reason for hiding this comment

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

if(connected == true) {
break;
} else {
disconnect = moWiFi.disconnect();
if (!disconnect) {
break;
}
enabled = moWiFi.enableNetwork(updateNetwork, true);
break;
}

@KhatibFX Can you explain this change block? why is it necessary?

Copy link
Member

Choose a reason for hiding this comment

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

if(connected == true) {
break;
} else {
disconnect = moWiFi.disconnect();
if (!disconnect) {
break;
}
enabled = moWiFi.enableNetwork(updateNetwork, true);
break;
}

@KhatibFX Can you explain this change block? why is it necessary?

So after netState == SupplicantState.COMPLETED - we check if connected, if not then disconnect and "enable network". I am not sure what the reason behind it.

Also, can the code be simplified? - avoiding break in each if-else branch.

Copy link
Author

Choose a reason for hiding this comment

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

So the reason here is that sometimes after supplicantstate.completed, it turns out the connection failed. On newer android versions, the phone will automatically revert back to the WiFi connection that was being used before trying to connect to this new network. On older Android versions, it will just remain disconnected and won't attempt to reconnect to the previously connected network. This part of the code attempts to do that reconnection step, to make the experience similar to newer Android versions. From my testing, it wasn't enough to just ask to enableNetwork of the previous network. I had to fully reset the connection by disconnecting and then reenabling the created network.

There may be a better way to rewrite this block better, but I am not amazing in Java so I'm not sure.

break;
}
}
try {
Thread.sleep(500);
Thread.sleep(1000);
} catch (InterruptedException ignored) {
break;
}
}

if (!connected && lastConnectedNetworkId != -1) {
//android 8.1 won't automatically reconnect to the previous network if it shares the same SSID
moWiFi.enableNetwork(lastConnectedNetworkId, true);
}
return connected;
}
}