[connectivity] - add mobile network type to connection result #1945

bubunyo · 2019-08-05T14:45:52Z

Description

Currently, the ConnectionResult is an enum type with 3 values and no information on the type of mobile connection. With the PR, the ConnectionResult type is changed to an Object with two fields, type and subtype with type retaining previous ConnectionResult enum values and subtype providing information on the subtype of the connection if it is on mobile. (ie. EDGE, HSDPA and LTE.)

Related Issues

fix for flutter/flutter#37618

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

juliocbcotta · 2019-08-05T16:30:26Z

...s/connectivity/android/src/main/java/io/flutter/plugins/connectivity/ConnectivityPlugin.java

+      return "none";
+
+    switch (info.getSubtype()) {
+      case TelephonyManager.NETWORK_TYPE_1xRTT:


Is there a source for this info? How about a link to it?

i dont get you. Care to explain a bit more?

How someone is supposed to know the mapping you did here? Do you have an http link that could be used as documentation for this?

I suggest to fully expose each type as a enum to dart. So we don't lose any information here. Same for iOS.

I pushed some updates. I added reference links for the mobile broadband speeds.

cyanglaz · 2019-08-05T17:42:20Z

@bubunyo Seems like the same thing has been worked on #1727
Do you mind following up with this other PR? Maybe help the author of the other PR a bit to finish up the PR?

bubunyo · 2019-08-05T19:15:20Z

@cyanglaz sure. I will close the PR and merge this with his

cyanglaz · 2019-08-06T17:08:03Z

As discussed in #1727; we are going to work on this PR instead of that one.

cyanglaz

Thanks for the PR. Did a first round scan. Left some comments.
Basically I want to make 2 suggestions:

Could we keep the subtype more detailed by returning whatever we get from the platform, instead of translating them into different categories.
To avoid breaking change, let's keep the old API to just return the main type. And the new API to return the subtype.

...s/connectivity/android/src/main/java/io/flutter/plugins/connectivity/ConnectivityPlugin.java

cyanglaz · 2019-08-06T17:20:18Z

...s/connectivity/android/src/main/java/io/flutter/plugins/connectivity/ConnectivityPlugin.java

+      return "none";
+
+    switch (info.getSubtype()) {
+      case TelephonyManager.NETWORK_TYPE_1xRTT:


I suggest to fully expose each type as a enum to dart. So we don't lose any information here. Same for iOS.

packages/connectivity/lib/connectivity.dart

cyanglaz · 2019-08-06T17:26:56Z

packages/connectivity/lib/connectivity.dart

  ///
  /// Instead listen for connectivity changes via [onConnectivityChanged] stream.
-  Future<ConnectivityResult> checkConnectivity() async {
+  Future<ConnectivityResult> checkConnectivity(


checkSubtype seems not used. Also add some doc to link to the [getNetworkSubtype] for people who wants to check the subtype.

i added some docs. Please review and let me know if they are up to standard.

cyanglaz · 2019-08-06T17:27:17Z

packages/connectivity/lib/connectivity.dart

+  /// Return none if there is no connections
+  ///
+  /// Return unknown if it is connected but there is not connection subtype info. eg. Wifi
+  Future<ConnectionSubtype> getNetworkSubtype() async {


more doc to point users to the [checkConnectivity] for who wants to check the main type.

i added some docs. Please review and let me know if they are up to standard.

packages/connectivity/example/ios/Runner.xcodeproj/project.pbxproj

packages/connectivity/lib/connectivity.dart

…proj to a822896

sante85 · 2019-10-14T10:38:53Z

hi, any news?
when you able to merge?

Best regards

sante85 · 2019-10-14T10:39:23Z

@bubunyo you can review?

sante85 · 2019-10-14T14:06:22Z

we can definitively able to merge?

sante85 · 2019-10-14T14:07:00Z

we need for this pull request. thanks

sante85 · 2019-10-14T14:07:58Z

+1

sante85 · 2019-10-17T15:17:27Z

@cyanglaz @bubunyo please can merge and review?

best regards

sante85 · 2019-10-17T15:20:40Z

@cyanglaz why build-ipas fail?

cyanglaz · 2019-10-17T18:44:27Z

Seems there are some merge conflicts.

bubunyo · 2019-10-17T19:42:14Z

I am trying to resolve the conflict but a lot of other things have been added, it will take some time to for me to get this working properly.

sante85 · 2019-10-19T15:16:18Z

@cyanglaz @bubunyo when merge is scheduled?

sante85 · 2019-10-24T22:39:20Z

any news?

cyanglaz · 2019-11-07T20:09:58Z

@bubunyo It was on hold since we were migrating plugins to a new Android embedding. Now that the migrating is done with connectivity, I can help with merging this PR in. There are some conflicts with this PR, could you rebase and resolve them first?

bubunyo · 2019-11-07T20:46:13Z

Definitely, I can work on that tomorrow.

Migrate the cloud_firestore core plugin to use the platform_interface package. All the "method channel" implementation now lives in the platform_interface package. The plugin is now ready to support federated implementations. Co-authored-by: David Iglesias <[email protected]> Co-authored-by: Collin Jackson <[email protected]>

demsey2 · 2020-04-22T08:27:13Z

Any update on this? Thanks

canthelou-xyz · 2020-07-09T14:59:17Z

Did someone still work on this useful feature ? Thk

KohlsAdrian · 2020-07-18T19:52:43Z

Still wanting so bad this feature on the official plugin

Extazx2 · 2020-09-14T14:03:29Z

Hello here, any news on this feature ?

KohlsAdrian · 2020-10-29T19:44:44Z

Connectivity 2.0.0 with breaking changes released and this is still not yet approved?

stuartmorgan-g · 2021-02-04T15:45:34Z

We apologize for the long delay in following up on this PR. We’ve made the decision to no longer accept non-critical PRs for this plugin, as we hope that in time we’re able to transition users to the corresponding plugin in the Flutter Community Plus Plugins repository, so we encourage you to submit your PR there. I realize it's not ideal that this decision is happening after there has already had some initial review here, but hopefully that feedback will still be useful for a Flutter Community Plus Plugin version of the same change.

We’re in the process of overhauling our PR triage system to respond much more quickly to avoid this kind of delay going forward. (Transitioning plugins like this one to the Flutter Community group is in fact part of that effort.)

Bubunyo Nyavor added 5 commits August 5, 2019 13:42

added mobile connection subtype information(android,ios)

ebca508

refactored existing tests

137ed3b

refactored existing tests

6584174

test written for connection subtype

c3892d0

updated read me

72e6c48

bubunyo requested a review from cyanglaz as a code owner August 5, 2019 14:45

googlebot added the cla: yes label Aug 5, 2019

Bubunyo Nyavor added 4 commits August 5, 2019 14:59

Updated Changelog and pubsec

db287e7

overide == operator to prevent breaking changes

8a4d6fb

updated changelog

9ee6274

updated version in pubsec

f03a291

juliocbcotta reviewed Aug 5, 2019

View reviewed changes

Bubunyo Nyavor added 3 commits August 5, 2019 19:23

fix errors from CI

bbcbfa6

suppress unrelated_type_equality_check

47c9869

formatted files with flutter plugin tools

31e4b3b

cyanglaz mentioned this pull request Aug 6, 2019

[connectivity] - add network speed info #1727

Closed

4 tasks

cyanglaz reviewed Aug 6, 2019

View reviewed changes

cyanglaz added the in review label Aug 6, 2019

Bubunyo Nyavor added 9 commits August 6, 2019 17:56

added mobile broadband speed links

c548c27

break down mobile subtype into detailed category

7e1c52f

passing tests

f50263e

added more documenations to connectivity.dart

c59a587

revert change in connectivity/example/io/Runner.xcodeproj/project.pbs…

0fcc46f

…proj to a822896

new api and tests

26180ce

updates tests in connectivity driver tests

5716218

Readme updated with new apis]

f8a381c

formatter run

49c8174

Bubunyo Nyavor added 2 commits September 4, 2019 10:48

Update with Upstream

f9e3f78

update Changelog and bump connectivity package version

33a17a4

bubunyo requested a review from cyanglaz September 20, 2019 06:49

Resolve merge conflicts

57f45d6

Refactor Connectivity to match new android embedding

ea6f538

varunananddudeja approved these changes Dec 27, 2019

View reviewed changes

bubunyo requested a review from matthew-carroll as a code owner August 26, 2020 03:56

stuartmorgan-g added the p: connectivity label Jan 29, 2021

stuartmorgan-g closed this Feb 4, 2021

stuartmorgan-g added the plus-transition PR closed due to the goal of transitioning to Flutter Community Plus Plugins label Feb 4, 2021

[connectivity] - add mobile network type to connection result #1945

[connectivity] - add mobile network type to connection result #1945

Uh oh!

Conversation

bubunyo commented Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Checklist

Breaking Change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz commented Aug 5, 2019

Uh oh!

bubunyo commented Aug 5, 2019

Uh oh!

cyanglaz commented Aug 6, 2019

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sante85 commented Oct 14, 2019

Uh oh!

sante85 commented Oct 14, 2019

Uh oh!

sante85 commented Oct 14, 2019

Uh oh!

sante85 commented Oct 14, 2019

Uh oh!

sante85 commented Oct 14, 2019

Uh oh!

sante85 commented Oct 17, 2019

Uh oh!

sante85 commented Oct 17, 2019

Uh oh!

cyanglaz commented Oct 17, 2019

Uh oh!

bubunyo commented Oct 17, 2019

Uh oh!

sante85 commented Oct 19, 2019

Uh oh!

sante85 commented Oct 24, 2019

Uh oh!

cyanglaz commented Nov 7, 2019

Uh oh!

bubunyo commented Nov 7, 2019

Uh oh!

demsey2 commented Apr 22, 2020

Uh oh!

canthelou-xyz commented Jul 9, 2020

Uh oh!

KohlsAdrian commented Jul 18, 2020

Uh oh!

Extazx2 commented Sep 14, 2020

Uh oh!

KohlsAdrian commented Oct 29, 2020

bubunyo commented Aug 5, 2019 •

edited

Loading