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

feat(wifi_iot): proper response to methods when using LocalOnlyHostspot #268

Merged
merged 27 commits into from
May 17, 2022
Merged

feat(wifi_iot): proper response to methods when using LocalOnlyHostspot #268

merged 27 commits into from
May 17, 2022

Conversation

PoloLacoste
Copy link
Contributor

fix #134

@daadu
Copy link
Member

daadu commented Apr 12, 2022

@PoloLacoste Thanks for the contribution.

I haven't reviewed and tested it properly, but can you also make necessary changes in plugin README as well.

I think this features are for >=29? right? while the "hidden AP" API is used only for <=26 - therefore for 27-28 version - there is no API for get these details? right?

@daadu
Copy link
Member

daadu commented Apr 12, 2022

  • need to fix formatting, with - melos run format
  • revert the version bump - will bump before release

@PoloLacoste
Copy link
Contributor Author

PoloLacoste commented Apr 12, 2022

On this link the documentation says its only from API level >= 30 and up.
However we can still use getWifiConfiguration from the LocalOnlyHotspotReservation until API 29.

@PoloLacoste
Copy link
Contributor Author

PoloLacoste commented Apr 12, 2022

Yes, to get if the SSID is hidden there is no API to get this information for API 27 to 29 :/
The method getWifiConfiguration returning the object WifiConfiguration doesn't contain any information related to the visibility of the LocalOnlyHotspot.

@daadu daadu changed the title fix(wifi_iot): proper response to methods when using LocalOnlyHostspot feat(wifi_iot): proper response to methods when using LocalOnlyHostspot May 14, 2022
Copy link
Member

@daadu daadu left a comment

Choose a reason for hiding this comment

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

Can we pollyfill getWiFiAPState as well? - that returns one of these 3 - WIFI_AP_STATE_DISABLED, WIFI_AP_STATE_ENABLED, WIFI_AP_STATE_FAILED (failed if the last attempted setWiFiAPEnabled(true) failed - we can cache these in some var in LocalOnlyHotspotCallback.onFailed callback - reset these var back before new attempt to start AP)

@daadu
Copy link
Member

daadu commented May 14, 2022

@PoloLacoste I have reviewed the PR, Thanks for the contribution.

I have a few ideas (mentioned above) that could still improve it, let me know what you think. It would be great if included in this PR or if you are done/tired with this then I could create feature request issues regarding it so that could be picked up in future. Just let me know - waiting for your response!

Regards.

- return true for isSSIDHidden for SDK 27 to 29 (with a warning log)
packages/wifi_iot/README.md Outdated Show resolved Hide resolved
Copy link
Member

@daadu daadu left a comment

Choose a reason for hiding this comment

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

@PoloLacoste add null check for apReservation.getWifiConfiguration() (all places), and return appropriate error.

Returns the WifiConfiguration of the current Local Only Hotspot (LOHS). May be null if hotspot enabled and security type is not WifiConfiguration.KeyMgmt.None or WifiConfiguration.KeyMgmt.WPA2_PSK.

@daadu
Copy link
Member

daadu commented May 16, 2022

@PoloLacoste Can you fix the build fail in CI

make sure to return only one error or one success
Copy link
Member

@daadu daadu left a comment

Choose a reason for hiding this comment

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

LGTM.

@daadu
Copy link
Member

daadu commented May 17, 2022

@PoloLacoste Thanks for your contribution on it.

@daadu daadu merged commit eda67ca into flutternetwork:master May 17, 2022
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.

[ap,android] proper response to methods when using LocalOnlyHostspot
2 participants