fix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) #294

KhatibFX · 2022-06-21T06:22:58Z

Fixed an issue on Android < Q where if the user already successfully connected to the network before, a new connection request with a wrong password would succeed. (Tested on Android 8.1 OnePlus 5T)

Changed the WIFI behavior when disconnecting from an app-created network on Android < Q to be loosely similar to observed behvaior on Android 11 (OnePlus 7 Pro) where the device will try to reconnect to the previously connected network.

This branch was created because the previous contribution from @lmmfranco didn't work for me. I hope it doesn't affect his work either and it can work for everyone who faced this issue.

The code might be a little messy, feel free to edit it out to fix inconsistencies.

…connected to the network before, a new connection request with a wrong password would succeed. (Tested on Android 8.1 OnePlus 5T) Changed the WIFI behavior when disconnecting from an app-created network on Android < Q to be loosely similar to observed behvaior on Android 11 (OnePlus 7 Pro) where the device will try to reconnect to the previously connected network.

daadu · 2022-06-21T08:26:39Z

@lmmfranco Can you review these changes as well?

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

diegotori · 2022-08-25T15:43:27Z

@daadu if these changes look good, then I suggest moving forward with them. So far, I don't see anything egregious with the changes, except for minor nits.

diegotori · 2022-08-25T18:45:28Z

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

+            break;
+          }
+
+          enabled = moWiFi.enableNetwork(updateNetwork, true);


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?

WiFiFlutter/packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

Lines 1586 to 1596 in 796f784

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?

WiFiFlutter/packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

Lines 1586 to 1596 in 796f784

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.

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.

daadu

@KhatibFX Sorry, for delaying the review.

I have attached some comments, please check them and give your views on it.

daadu · 2022-08-28T15:33:25Z

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

+            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++) {


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

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.

daadu · 2022-08-28T15:34:39Z

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

+            for (int i = 0; i < 20; i++) {
+              int randomInteger = random.nextInt(10000);
+              //create a valid SSID with max length of 32
+              String ssidRandomized = ssid + randomInteger;


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

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

daadu · 2022-08-28T15:46:42Z

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

+            break;
+          }
+
+          enabled = moWiFi.enableNetwork(updateNetwork, true);


WiFiFlutter/packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

Lines 1586 to 1596 in 796f784

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?

daadu · 2022-08-28T15:47:19Z

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java

@@ -1510,24 +1578,38 @@ private Boolean connectToDeprecated(
    if (!enabled) return false;

    boolean connected = false;
+    int networkId = -1;


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

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

KhatibFX · 2022-08-28T17:17:18Z

Hi, sorry I have been busy lately and haven't gotten a chance to check or follow up on the comments. I will try to get back to them within the next week or so.

And thanks for the great library, of course!

daadu · 2022-09-05T05:53:44Z

@KhatibFX Any update?

KhatibFX · 2022-10-13T07:27:40Z

@daadu Hey, Sorry for the late response, I just responded to the comments, let me know if I can help further. Thanks.

KhatibFX changed the title ~~ix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended)~~ fix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) Jun 21, 2022

diegotori suggested changes Aug 25, 2022

View reviewed changes

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java Outdated Show resolved Hide resolved

diegotori suggested changes Aug 25, 2022

View reviewed changes

packages/wifi_iot/android/src/main/java/com/alternadom/wifiiot/WifiIotPlugin.java Outdated Show resolved Hide resolved

diegotori suggested changes Aug 25, 2022

View reviewed changes

daadu added 2 commits August 28, 2022 20:44

minor changes

d023eaa

formatting

6338aa8

daadu reviewed Aug 28, 2022

View reviewed changes

daadu force-pushed the master branch 5 times, most recently from 397aaa3 to a527cb8 Compare November 5, 2022 10:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) #294

fix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) #294

KhatibFX commented Jun 21, 2022

daadu commented Jun 21, 2022

diegotori commented Aug 25, 2022

diegotori Aug 25, 2022

daadu Aug 28, 2022 •

edited

Loading

daadu Aug 28, 2022

KhatibFX Oct 13, 2022

daadu left a comment

daadu Aug 28, 2022

KhatibFX Oct 13, 2022

daadu Aug 28, 2022

KhatibFX Oct 13, 2022

daadu Aug 28, 2022 •

edited

Loading

daadu Aug 28, 2022

KhatibFX Oct 13, 2022

KhatibFX commented Aug 28, 2022

daadu commented Sep 5, 2022

KhatibFX commented Oct 13, 2022

	if(connected == true) {
	break;
	} else {
	disconnect = moWiFi.disconnect();
	if (!disconnect) {
	break;
	}

	enabled = moWiFi.enableNetwork(updateNetwork, true);
	break;
	}

fix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) #294

Are you sure you want to change the base?

fix(wifi_iot): Fix connection to registered network on Android < 10 (Q) (Extended) #294

Conversation

KhatibFX commented Jun 21, 2022

daadu commented Jun 21, 2022

diegotori commented Aug 25, 2022

Choose a reason for hiding this comment

daadu Aug 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daadu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daadu Aug 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KhatibFX commented Aug 28, 2022

daadu commented Sep 5, 2022

KhatibFX commented Oct 13, 2022

daadu Aug 28, 2022 •

edited

Loading

daadu Aug 28, 2022 •

edited

Loading