[connectivity] - add network speed info #1727

sante85 · 2019-06-12T08:09:19Z

Description

[x ] I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
[x ] The analyzer (flutter analyze) does not report any problems on my PR.
[ x] I read and followed the Flutter Style Guide.
[ x] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [connectivity]
[ x] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
[ x] I updated CHANGELOG.md to add a description of the change.
[ x] I signed the CLA.
[ x] I am willing to follow-up on review comments in a timely manner.

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).
[ x] No, this is not a breaking change.

add network info speed

add network speed info

update version [email protected]

add version info

fix add parameter

sante85 · 2019-06-18T22:51:24Z

please help me on format

collinjackson · 2019-06-21T21:20:20Z

I reformatted and merged in the latest. I am not sure this PR is complete. It seems like you should be doing something on the Dart side to expose the various connectivity results now that the native side isn't just sending "mobile" for every network type.

https://github.com/sante85/plugins/blob/master/packages/connectivity/lib/connectivity.dart#L98

It might make sense to expose network type and subtype as separate properties on the Dart side.

collinjackson · 2019-07-08T14:27:23Z

@sante85 please let us know if you want to continue working on this PR or abandon it

sante85 · 2019-07-08T15:06:03Z

Please continue. Thanks you very much Il Lun 8 Lug 2019, 16:27 Collin Jackson <[email protected]> ha scritto:

…

@sante85 <https://github.com/sante85> please let us know if you want to continue working on this PR or abandon it — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTN7EB2KT357B7ZKVU2DP6NFGFA5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZNIEWI#issuecomment-509248089>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTN3CPIHGDEHHGIDCF6LP6NFGFANCNFSM4HXGMUBQ> .

sante85 · 2019-07-08T15:09:14Z

PR is complete, but i m not able to compile for osx. Can you finish and complete feature as you write? With new field Best regards Il Lun 8 Lug 2019, 17:05 Sante Stanisci <[email protected]> ha scritto:

…

Please continue. Thanks you very much Il Lun 8 Lug 2019, 16:27 Collin Jackson ***@***.***> ha scritto: > @sante85 <https://github.com/sante85> please let us know if you want to > continue working on this PR or abandon it > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1727?email_source=notifications&email_token=ADLHTN7EB2KT357B7ZKVU2DP6NFGFA5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZNIEWI#issuecomment-509248089>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ADLHTN3CPIHGDEHHGIDCF6LP6NFGFANCNFSM4HXGMUBQ> > . >

sante85 · 2019-07-08T15:16:48Z

I can do a new method for return specific mobile subtype?

collinjackson · 2019-07-08T16:43:21Z

New method sounds fine to me.

sante85 · 2019-07-08T18:15:12Z

Ok, I complete with a new method, but you can reformat and PR finish? Thanks Il giorno lun 8 lug 2019 alle ore 18:43 Collin Jackson < [email protected]> ha scritto:

…

New method sounds fine to me. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTN42NFPP2MA7DEPEEFDP6NVD7A5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZNVKLY#issuecomment-509302063>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTN357GJUPFHA22TKS63P6NVD7ANCNFSM4HXGMUBQ> .

add method getMobileConnectionType

add call at getNetworkSubType

add method getMobileConnectionType

sante85 · 2019-07-08T21:27:06Z

Please you can verify formatting. I have separate in a new method.

Best Regards

sante85 · 2019-07-09T12:59:34Z

How many time Is needed for merge? Thanks

sante85 · 2019-07-09T13:04:00Z

I have a question. Can you explain how to configure Mac for develop flutter plugins? Thanks

sante85

ok, thanks

sante85 · 2019-07-11T17:03:10Z

I not have understand if I approve your changes, or i make this changes.

Please can you specify?

Thanks

cyanglaz · 2019-07-11T17:12:27Z

I not have understand if I approve your changes, or i make this changes.

Please can you specify?

Thanks

If you'd like to make the change according to my comments, that'd be great. If you don't have time to work on this, giving permission, I can shepherd it from here as well.

sante85 · 2019-07-11T18:02:42Z

Yes you can change my code in any Time. In next time i won't add New method to calculate Internet Speed. Il Gio 11 Lug 2019, 19:12 Chris Yang <[email protected]> ha scritto:

…

I not have understand if I approve your changes, or i make this changes. Please can you specify? Thanks If you'd like to make the change according to my comments, that'd be great. If you don't have time to work on this, giving permission, I can shepherd it from here as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTN6IO6RIOS7P2ZSE52LP65SY7A5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZXLYGI#issuecomment-510573593>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTN6E7QUS44NDQG5B63DP65SY7ANCNFSM4HXGMUBQ> .

sante85 · 2019-07-12T18:46:15Z

how many time is required for merge?

best regards

cyanglaz · 2019-07-12T19:41:25Z

how many time is required for merge?

best regards

Did you mean how soon this can be merged? I probably don't have time to work on it anytime soon as we have other priorities. The general process is to prioritize the PR based on the priority of the related issue. see: initial PR review policy
If you'd like to have your PR prioritized, I encourage you to create an issue and link this PR to it. I am going to label it with "backlog" for now.

sante85 · 2019-07-15T20:50:42Z

please we can format and approve?

thanks

sante85 · 2019-07-16T05:55:23Z

@cyanglaz can you explain because build ipas raise in error?

thanks

sante85 · 2019-07-24T05:23:33Z

Ho, when we able to merge my pull request? Il Ven 12 Lug 2019, 21:41 Chris Yang <[email protected]> ha scritto:

…

how many time is required for merge? best regards Did you mean how soon this can be merged? I probably don't have time to work on it anytime soon as we have other priorities. The general process is to prioritize the PR based on the priority of the related issue. see: initial PR review policy <https://github.com/flutter/plugins/blob/master/CONTRIBUTING.md#the-review-process> If you'd like to have your PR prioritized, I encourage you to create an issue and link this PR to it. I am going to label it with "backlog" for now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTN5BOJQPMITYQEKDJPLP7DM73A5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ2WGXA#issuecomment-511009628>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTN7UMYXQR2WX3LMF4ULP7DM73ANCNFSM4HXGMUBQ> .

bubunyo · 2019-08-05T10:31:19Z

@sante85 what is the states of the changes you are to make? If you would like me to help with the reviews to get it approved, I will be happy to help.

sante85 · 2019-08-05T10:41:33Z

The state of changes Is finished If you want you can reformat and approve. I have a question. How to configure cirrus to compile and test for iOS? Because i not understood how to. Best Regards Il Lun 5 Ago 2019, 12:31 Bubunyo Nyavor <[email protected]> ha scritto:

…

@sante85 <https://github.com/sante85> what is the states of the changes you are to make? If you would like me to help with the reviews to get it approved, I will be happy to help. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTN7NMIJK3A7QT4KGAODQC76Q5A5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3RM53I#issuecomment-518180589>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTN6IIZXMGSQAU3DST5DQC76Q5ANCNFSM4HXGMUBQ> .

bubunyo · 2019-08-05T10:46:37Z

I will work on the reformat. on how to compile and test for iOS, I have no idea but I can look around and see if I can find anything out.

cyanglaz

Left some comments. It seems there are some formatting issues so a lot of extra diff shows up in the java implementation. It makes the code hard to review.
To format it correctly. You should first point your flutter to master.
and refer to https://github.com/flutter/plugin_tools
The command is pub global run flutter_plugin_tools format

packages/connectivity/CHANGELOG.md

cyanglaz · 2019-08-05T16:13:05Z

packages/connectivity/CHANGELOG.md

@@ -1,7 +1,11 @@
+## 0.4.3+5


need an extra line after the version number and before the description.

packages/connectivity/lib/connectivity.dart

cyanglaz · 2019-08-05T16:13:37Z

packages/connectivity/pubspec.yaml

  mobile/cellular) connectivity on Android and iOS.
 author: Flutter Team <[email protected]>
 homepage: https://github.com/flutter/plugins/tree/master/packages/connectivity
+


extra line here, could you remove it?

cyanglaz · 2019-08-05T16:13:47Z

packages/connectivity/pubspec.yaml

-version: 0.4.3+4
+
+version: 0.4.3+5
+


another extra line here.

sante85 · 2019-08-05T17:57:30Z

yes, remove Il giorno lun 5 ago 2019 alle ore 18:26 Chris Yang <[email protected]> ha scritto:

…

***@***.**** commented on this pull request. Left some comments. It seems there are some formatting issues so a lot of extra diff shows up in the java implementation. It makes the code hard to review. To format it correctly. You should first point your flutter to master. and refer to https://github.com/flutter/plugin_tools The command is pub global run flutter_plugin_tools format ------------------------------ In packages/connectivity/CHANGELOG.md <#1727 (comment)>: > @@ -2,6 +2,7 @@ * [Android] Updated logic to retrieve network info. + I am still seeing this extra line. Could you remove it? ------------------------------ In packages/connectivity/CHANGELOG.md <#1727 (comment)>: > @@ -1,7 +1,11 @@ +## 0.4.3+5 need an extra line after the version number and before the description. ------------------------------ In packages/connectivity/lib/connectivity.dart <#1727 (comment)>: > @@ -93,6 +93,10 @@ class Connectivity { Future<String> getWifiIP() async { return await methodChannel.invokeMethod<String>('wifiIPAddress'); } + + Future<String> getMobileConnectionType() async { dart doc is still missing. See https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#documentation-dartdocs-javadocs-etc for more details please. ------------------------------ In packages/connectivity/pubspec.yaml <#1727 (comment)>: > @@ -3,8 +3,10 @@ description: Flutter plugin for discovering the state of the network (WiFi & mobile/cellular) connectivity on Android and iOS. author: Flutter Team ***@***.***> homepage: https://github.com/flutter/plugins/tree/master/packages/connectivity + extra line here, could you remove it? ------------------------------ In packages/connectivity/pubspec.yaml <#1727 (comment)>: > @@ -3,7 +3,9 @@ description: Flutter plugin for discovering the state of the network (WiFi & mobile/cellular) connectivity on Android and iOS. author: Flutter Team ***@***.***> homepage: https://github.com/flutter/plugins/tree/master/packages/connectivity -version <https://github.com/flutter/plugins/tree/master/packages/connectivity-version>: 0.4.3+4 + +version: 0.4.3+5 + another extra line here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTNYOF7UAROJSSV6JT4TQDBICXA5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCASRWTI#pullrequestreview-270867277>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTN7HNWPETC24H53L4FDQDBICXANCNFSM4HXGMUBQ> .

bubunyo · 2019-08-05T19:31:37Z

@sante85 I fixed some issues, tracked upstream and did a few modifications. you can find the changes here. https://github.com/bubunyo/plugins

sante85 · 2019-08-05T20:19:18Z

ok, but what do I doing? Il giorno lun 5 ago 2019 alle ore 21:32 Bubunyo Nyavor < [email protected]> ha scritto:

…

@sante85 <https://github.com/sante85> I fixed some issues, tracked upstream and did a few modifications. you can find the changes here. https://github.com/bubunyo/plugins — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTNYJF57I3EM2WF2WPVLQDB53FA5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3S2X7Q#issuecomment-518368254>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTNYJ7LO2L64KUKETUMLQDB53FANCNFSM4HXGMUBQ> .

sante85 · 2019-08-05T20:20:45Z

i have readed modification and are ok. you go to merge. thank you Il giorno lun 5 ago 2019 alle ore 22:19 Sante Stanisci <[email protected]> ha scritto:

…

ok, but what do I doing? Il giorno lun 5 ago 2019 alle ore 21:32 Bubunyo Nyavor < ***@***.***> ha scritto: > @sante85 <https://github.com/sante85> I fixed some issues, tracked > upstream and did a few modifications. you can find the changes here. > https://github.com/bubunyo/plugins > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1727?email_source=notifications&email_token=ADLHTNYJF57I3EM2WF2WPVLQDB53FA5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3S2X7Q#issuecomment-518368254>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ADLHTNYJ7LO2L64KUKETUMLQDB53FANCNFSM4HXGMUBQ> > . >

bubunyo · 2019-08-05T20:24:16Z

I can't really tell because when I checked your code it was far back from the upstream, so I just rewrote it as I saw fit.

cyanglaz · 2019-08-05T20:38:25Z

@sante85 maybe you can check @bubunyo 's PR here see if you want to add anything to it? And we can all work on @bubunyo 's PR since his is newer and more update to date with the upstream?

sante85 · 2019-08-06T05:47:35Z

for me your PR is all ok, you can proceed to merge request Il giorno lun 5 ago 2019 alle ore 22:38 Chris Yang <[email protected]> ha scritto:

…

@sante85 <https://github.com/sante85> maybe you can check @bubunyo <https://github.com/bubunyo> 's PR here see if you want to add anything to it? And we can all work on @bubunyo <https://github.com/bubunyo> 's PR since his is newer and more update to date with the upstream? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1727?email_source=notifications&email_token=ADLHTNZAGKUSDE2EWM476WDQDCFVRA5CNFSM4HXGMUB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3TAEBA#issuecomment-518390276>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLHTNYU4U3OYLRT7D3WFEDQDCFVRANCNFSM4HXGMUBQ> .

cyanglaz · 2019-08-06T17:07:12Z

I am going to close this PR then. Let's all follow up and review @bubunyo 's PR here: #1945

sante85 and others added 4 commits June 12, 2019 09:43

Update ConnectivityPlugin.java

f08e866

add network info speed

Update ConnectivityPlugin.m

3e9b146

add network speed info

Update pubspec.yaml

68509b6

update version [email protected]

Update CHANGELOG.md

b222a5a

add version info

sante85 requested a review from cyanglaz as a code owner June 12, 2019 08:09

googlebot added the cla: yes label Jun 12, 2019

sante85 and others added 3 commits June 12, 2019 10:43

Update ConnectivityPlugin.java

c32222b

fix add parameter

format

9ccc51c

add m

16a9a71

sante85 mentioned this pull request Jun 19, 2019

[Connectivity][Android] Updated check network info logic #1761

Merged

13 tasks

collinjackson added 4 commits June 21, 2019 14:12

Update pubspec.yaml

596e5e4

Update CHANGELOG.md

8ff7d9c

Reformat

1525456

Merge remote-tracking branch 'origin/master' into sante85

df989a3

collinjackson self-requested a review June 21, 2019 21:17

sante85 added 3 commits July 8, 2019 23:16

Update connectivity.dart

97636ef

add method getMobileConnectionType

Update ConnectivityPlugin.java

8f0c1cd

add call at getNetworkSubType

Update ConnectivityPlugin.m

56dc958

add method getMobileConnectionType

Merge branch 'master' into master

ba74b91

sante85 commented Jul 11, 2019

View reviewed changes

cyanglaz added backlog feature labels Jul 12, 2019

sante85 added 4 commits July 15, 2019 22:17

Update pubspec.yaml

8ddf11f

Update CHANGELOG.md

843f316

Update ConnectivityPlugin.java

12e2a4b

Update ConnectivityPlugin.java

8f34cd8

cyanglaz removed their assignment Jul 23, 2019

cyanglaz reviewed Aug 5, 2019

View reviewed changes

cyanglaz mentioned this pull request Aug 5, 2019

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

Closed

13 tasks

cyanglaz closed this Aug 6, 2019

		version: 0.4.3+4

		version: 0.4.3+5

[connectivity] - add network speed info #1727

[connectivity] - add network speed info #1727

Uh oh!

Conversation

sante85 commented Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breaking Change

Uh oh!

sante85 commented Jun 18, 2019

Uh oh!

collinjackson commented Jun 21, 2019

Uh oh!

collinjackson commented Jul 8, 2019

Uh oh!

sante85 commented Jul 8, 2019 via email

Uh oh!

sante85 commented Jul 8, 2019 via email

Uh oh!

sante85 commented Jul 8, 2019

Uh oh!

collinjackson commented Jul 8, 2019

Uh oh!

sante85 commented Jul 8, 2019 via email

Uh oh!

sante85 commented Jul 8, 2019

Uh oh!

sante85 commented Jul 9, 2019

Uh oh!

sante85 commented Jul 9, 2019

Uh oh!

sante85 left a comment

Choose a reason for hiding this comment

Uh oh!

sante85 commented Jul 11, 2019

Uh oh!

cyanglaz commented Jul 11, 2019

Uh oh!

sante85 commented Jul 11, 2019 via email

Uh oh!

sante85 commented Jul 12, 2019

Uh oh!

cyanglaz commented Jul 12, 2019

Uh oh!

sante85 commented Jul 15, 2019

Uh oh!

sante85 commented Jul 16, 2019

Uh oh!

sante85 commented Jul 24, 2019 via email

Uh oh!

bubunyo commented Aug 5, 2019

Uh oh!

sante85 commented Aug 5, 2019 via email

Uh oh!

bubunyo commented Aug 5, 2019

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cyanglaz Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cyanglaz Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

cyanglaz Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

sante85 commented Aug 5, 2019 via email

Uh oh!

bubunyo commented Aug 5, 2019

Uh oh!

sante85 commented Aug 5, 2019 via email

Uh oh!

sante85 commented Aug 5, 2019 via email

Uh oh!

bubunyo commented Aug 5, 2019

Uh oh!

sante85 commented Jun 12, 2019 •

edited

Loading