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_basic): plugin added #201

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from
Draft

feat(wifi_basic): plugin added #201

wants to merge 31 commits into from

Conversation

daadu
Copy link
Member

@daadu daadu commented Nov 21, 2021

Related to #187

@daadu daadu changed the title feat(wifi_basic) plugin added feat(wifi_basic): plugin added Nov 21, 2021
…led`

- [wifi_basic,test] `WiFiBasic.setEnabled`: arg name changed `state` -> `enabled`; returning if successful or not
- [example,android] necessary permissions added to manifest file
- [example] proper example code added
… removed `shouldOpenSettings` arg

- [wifi_basic,android,test] `WiFiBasic`, `WifiBasicPlugin`: `openSettings` method added, implemented and test for it added
- [wifi_basic,example] smaller example with just buttons
import Flutter
import UIKit

public class SwiftWifiBasicPlugin: NSObject, FlutterPlugin {
Copy link
Member Author

@daadu daadu Nov 28, 2021

Choose a reason for hiding this comment

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

@DominikStarke Can you review this file? esp. handling of isEnabled (lifted code from SO!) - tested with iOS - iOS 15 simlulator (returns true), and iOS 12 device (it is working), will soon test on iOS 15 too - will let you know.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like it always returns true in iOS 15.

So may be wont go ahead with this unofficial API.

@DominikStarke Do you have any views on what best-case API to call if iOS does not allow it? Currently in wifi_iot - it checks if currentSSID != null - other forums suggest using SCNetworkReachability (as used by connectivity_plus plugin).

Both suggest that the mobile is "connected" to a WifiNetwork and not if the hw/service is "enabled" - but looks like this is the best we can achieve. My views are we should use SCNetworkReachability as it does not require any additional permission from user. Le me know your view.

- [wifi_basic] `WiFiBasic`: `getGeneration` method added; memoizing `isSupported` and `getGeneration` results
- [wifi_basic,test] aaded `getGeneration` to basic test
- [wifi_basic,example] added `getGeneration` to example app
- [wifi_basic,ios] added `getGeneration` [incomplete implementation]; filled in info in podspec file
- [wifi_basic,android] added `getGeneration` [incomplete implementation]
- [wifi_basic,test] fix - returning int instead of enum
…Basic.getCurrentInfo` method added

- [wifi_basic,extension] `ToEnumExtension` extension added and integrated with `WiFiBasic`
- [wifi_basic,example] `getCurrentInfo` added to example app
- [wifi_basic,test] added proper unit tests
@fernando-s97
Copy link

fernando-s97 commented Dec 11, 2021

I took a look at the flutter code of this PR and I have some proposals:

a) wifi_basic.dart

  1. Split the wifi_basic.dart file into multiple ones for better organization.

In case of WiFiInfo, that needs the private _fromMap, we could make use of the part and part of keywords.

  1. Make WiFiInfo a proper data class, implementing at least == and hashCode.

  2. Probably cast every property of WiFiInfo._fromMap to its expected type (for clarity).

  3. Probably make WiFiInfo.toString also show its name. Ex.: "WiFiInfo(...properties...)".

  4. WiFiBasic._channel name should be prefixed by the plugin domain, as recommended per flutter.

  5. invokeMethod casts

Maybe, instead of _channel.invokeMethod("getGeneration") as int?, _channel.invokeMethod<int?>("getGeneration") should be preferred, since it's already part of the method.

  1. Make WiFiBasic.getCurrentInfo return null if _channel.invokeMapMethod return null

b) extensions.dart

Changing the current code to the below one would make much more clear which values are mapped to which enum

extension WiFiGenerationsExtension on WiFiGenerations {
  static WiFiGenerations fromInt(int? value) {
    switch (value) {
      case 0:
      case 1:
      case 2:
      case 3:
        return WiFiGenerations.legacy;
      case 4:
        return WiFiGenerations.wifi4;
      case 5:
        return WiFiGenerations.wifi5;
      case 6:
        return WiFiGenerations.wifi6;
      default:
        return WiFiGenerations.unknown;
    }
  }
}

extension WiFiNetworkSecurityExtension on WiFiNetworkSecurity {
  static WiFiNetworkSecurity fromInt(int? value) {
    switch (value) {
      case 0:
        return WiFiNetworkSecurity.unknown;
      case 1:
        return WiFiNetworkSecurity.none;
      case 2:
        return WiFiNetworkSecurity.wep;
      case 3:
        return WiFiNetworkSecurity.wpa;
      case 4:
        return WiFiNetworkSecurity.wpa2;
      case 5:
        return WiFiNetworkSecurity.wpa3;
      default:
        return WiFiNetworkSecurity.unknown;
    }
  }
}

OBS: I can make those changes if necessary

@daadu
Copy link
Member Author

daadu commented Dec 11, 2021

@fernando-s97 Looks like good suggestions - Except point 5. plugin name is unique enough to avoid name clashes + generated plugin scaffold does not add org name as prefix - will keep as it is.

If you got time then can you file new PR with this changes against this branch?

@daadu
Copy link
Member Author

daadu commented Jan 9, 2022

I have been first completing wifi_scan looked like simpler to complete - can you guys review it at #205.

@daadu daadu force-pushed the master branch 5 times, most recently from 397aaa3 to a527cb8 Compare November 5, 2022 10:43
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.

3 participants