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.

Sign up for GitHub

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

Jump to bottom

fix: Verify connection after enabling network #31

Merged
merged 3 commits into from
Jan 21, 2020
Merged

fix: Verify connection after enabling network #31

merged 3 commits into from
Jan 21, 2020

Conversation

Rapsssito
Copy link
Collaborator

@Rapsssito Rapsssito commented Jan 12, 2020 •
edited
Loading

Bug

Android

There was an issue when connecting to an already failed connection where enableNetwork() returned always true because of its non-blocking nature.

iOS

There was an issue where NEHotspotConfigurationManager returned no error if the device could connect to another SSID quickly enough. (#23)

Solution

Android

Implemented a BroadcastReceiver to verify that connection has been successfully established.

iOS

Implemented a check for the current SSID after NEHotspotConfigurationManager returns. (Fixes: #23).

@Rapsssito Rapsssito changed the title fix(Android): Verify connection after enableNetwork() fix: Verify connection after enabling network Jan 13, 2020
@Rapsssito Rapsssito mentioned this pull request Jan 13, 2020
Closed
@JuanSeBestia
Copy link
Owner

How can I test it?

@Rapsssito
Copy link
Collaborator Author

Rapsssito commented Jan 14, 2020 •
edited
Loading

@JuanSeBestia
Connect your device to two WiFi networks, so they are saved in the device settings. Then, use the example app to connect to one of them, but with the wrong password.

  • Without fix: the promise should resolve as if everything went okay.
  • With fix: the promise should reject an error.

@eliaslecomte
Copy link
Collaborator

eliaslecomte commented Jan 15, 2020 •
edited
Loading

Nice work @Rapsssito but I was thinking we should leverage the work to https://github.com/ThanosFisherman/WifiUtils instead of doing everything native here. This lib already solved all those issues and is also ready for Android 10 (once my PR is merged; ThanosFisherman/WifiUtils#46).

@Rapsssito
Copy link
Collaborator Author

Rapsssito commented Jan 15, 2020 •
edited
Loading

@eliaslecomte, I agree with using an external dependency for the Android part.
However, as this is a cross-platform fix, I think this PR could be merged now and then a new PR should be created for the WifiUtils implementation.

@eliaslecomte
Copy link
Collaborator

@eliaslecomte, I agree with using an external dependency for the Android part.
However, as this is a cross-platform fix, I think this PR could be merged now and then a new PR should be created for the WifiUtils implementation.

Yes indeed definitely should!

@JuanSeBestia
Copy link
Owner

Testing...

@JuanSeBestia
Copy link
Owner

In my Xoami not work properly... but not this functionality, all connect not work.

I will trust

@JuanSeBestia JuanSeBestia merged commit 9fa86ee into JuanSeBestia:master Jan 21, 2020
JuanSeBestia pushed a commit that referenced this pull request Jan 21, 2020
@semantic-release-bot
chore(release): 2.3.2 [skip ci]
1468ae0 
## [2.3.2](v2.3.1...v2.3.2) (2020-01-21)

### Bug Fixes

* **android:** Verify connection after enabling network ([#31](#31)) ([9fa86ee](9fa86ee))
@JuanSeBestia
Copy link
Owner

🎉 This PR is included in version 2.3.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Rapsssito Rapsssito deleted the fix-verifyConnection branch January 22, 2020 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[iOS] connectToProtectedSSID NEHotspotConfigurationManager completionHandler always returns nil
3 participants
@Rapsssito @JuanSeBestia @eliaslecomte