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): iOS isConnected always true even connection is failed #371

Closed

Conversation

JooYoo
Copy link
Contributor

@JooYoo JooYoo commented Feb 25, 2024

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
Copy link
Contributor Author

JooYoo commented Mar 20, 2024

hey @daadu

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

@JooYoo
Copy link
Contributor Author

JooYoo commented Mar 28, 2024

hey @mavyfaby

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

UnluckyY1 and others added 3 commits August 27, 2024 13:40
…er 3.24 (flutternetwork#398)

* fix:Update compileSdkVersion to fix Android release build with flutter 3.24

* wifi_iot(android): upgrade compleiSDKVersion to 34

* fix dart analyzer issues

---------

Co-authored-by: Harsh Bhikadia <harsh@bhikadia.com>
@milomai
Copy link

milomai commented Oct 12, 2024

I have same issue

@JooYoo
Copy link
Contributor Author

JooYoo commented Oct 13, 2024

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
Copy link
Contributor

mavyfaby commented Oct 14, 2024

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

cc: @daadu

@JooYoo
Copy link
Contributor Author

JooYoo commented Oct 15, 2024

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

@daadu
Copy link
Member

daadu commented Oct 15, 2024

@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
Copy link
Member

daadu commented Oct 15, 2024

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

@daadu
Copy link
Member

daadu commented Oct 15, 2024

@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
Copy link
Contributor Author

JooYoo commented Oct 16, 2024

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
Copy link
Contributor Author

JooYoo commented Oct 16, 2024

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: #407

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

@daadu
Copy link
Member

daadu commented Oct 17, 2024

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

@daadu daadu closed this Oct 17, 2024
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.

5 participants