fix(wifi_iot): iOS isConnected always true even connection is failed

JooYoo commented 9 months ago

BugReproduce:

On iOS device
Location Permission is enabled and granted. Performing final connectedSSID = await WiFiForIoTPlugin.getSSID() should see the current connectedSSID.
iOS device is already connected to any available wifi.
provide an unavailable SSID then perform final isConnected = await WiFiForIoTPlugin.connect(wrongSSID); -> iOS System dialog is showing to indicate the connection is failed -> expecting isConnected is [false] but get [true]

RootCause

Where: packages/wifi_iot/ios/Classes/SwiftWifiIotPlugin.swift: private func connect(call: FlutterMethodCall, result: @escaping FlutterResult)
How: The way to determine isConnected is that, after connect(targetSSID) is performed, then getSSID to get connectedSSID, checking if targetSSID is equal to connectedSSID.
What: In the original code base, because of the confused naming, it only unwrapped the connectedSSID ( original sSSID, after unwrapping ssid ) and returned as result by comparing itself ( original result(ssid == sSSID) ). In this case, once the device is already connected to a ssid, getSSID() works correctly, then getSSID() will return the connectedSSID, unwrapping connectedSSID, comparing itself as the result, it will be always true. It leads connect(targetSSID) returns true, even if the connection is actually failed.

Solution

Renaming the parameter of getSSID closure: sSSID -> connectedSSID
Check if targetSSID is equal to connectedSSID: result(sSSID == connectedSSID)

JooYoo commented 8 months ago

hey @daadu

A friendly question. This PR is already staying here for 3 weeks. Who can I contact to review this PR? 😉

JooYoo commented 8 months ago

hey @mavyfaby

A friendly question. This PR is already staying here for a month. Who can I contact to review this PR? 😉

milomai commented 1 month ago

I have same issue

JooYoo commented 1 month ago

I have same issue

I know. If this PR is merged, the problem can be solved. But nobody review and merge this PR in this repo. I created this PR in February, and have waited 8 months.

mavyfaby commented 1 month ago

hello @JooYoo, let's wait for @daadu for last review. I have no permission for merging PRs.

cc: @daadu

JooYoo commented 1 month ago

Hi @mavyfaby thanks for the supporting.😉 Then let's wait for @daadu .

daadu commented 1 month ago

@JooYoo extremely sorry for being unresponsive.

The changes looks good to go.

I'll merge this currently, unfortunately I'm traveling with limited connectivity - will release this to pub.dev in a day or two

daadu commented 1 month ago

for some reason the CI checks are not trigger in PRs - need to look into that before merging this.

daadu commented 1 month ago

@JooYoo Can you sync the fork and branch once.

I am unable to do it:

❯ git push pr-371 pr-371:fix/wifi-iot-ios-is-connected 
Enter passphrase for key '/Users/harsh/.ssh/id_rsa': 
Enumerating objects: 10, done.
Counting objects: 100% (10/10), done.
Delta compression using up to 4 threads
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 438 bytes | 146.00 KiB/s, done.
Total 4 (delta 2), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (2/2), completed with 2 local objects.
To github.com:JooYoo/fork-flutter_wifi_flutter.git
 ! [remote rejected] pr-371 -> fix/wifi-iot-ios-is-connected (permission denied)
error: failed to push some refs to 'github.com:JooYoo/fork-flutter_wifi_flutter.git'

JooYoo commented 1 month ago

Hi @daadu welcome back!

I synced the branch. This branch is too out of date. My local branch and the remote branch have diverged. I had to sync the master, and then merge the changes.

Now this PR contains 17 files changing. But only packages/wifi_iot/ios/Classes/SwiftWifiIotPlugin.swift is the fix. All other changes should come from the main, I compared the files one by one.

Let me know, if I need to do something to support this PR. Thanks for your work. 👍🏼

JooYoo commented 1 month ago

Hi @daadu

If you feel too risky to merge this PR. I opened another PR. It branched from the latest master. It's clean and only contains the necessary changes in one file.

Clean PR: https://github.com/flutternetwork/WiFiFlutter/pull/407

Once the merge is successful just delete the rest. Sorry to cause the trouble. 😖

daadu commented 1 month ago

Yes will go with the cleaner PR if the changes are identical

flutternetwork / WiFiFlutter

fix(wifi_iot): iOS isConnected always true even connection is failed #371