feat(connectivity)!: support multiple ConnectivityResult values at the same time

[suquant]

trafficstars

Description

This PR updates the connectivity_plus plugin to handle multiple connectivity types simultaneously. Previously, checkConnectivity and onConnectivityChanged methods returned a single ConnectivityResult type, which could be either wifi, cellular, ethernet, bluetooth, vpn, or none. Due to this, on iOS/macOS platforms, the types other & vpn were never returned because the presence of wifi or cellular took precedence. This update changes the return type of these methods to a list of ConnectivityResult, allowing for a more accurate representation of the device's connectivity status, especially in scenarios where multiple types of connections are available or in use simultaneously.

Related Issues

• Related #2522
• Related #2262
• Related #2142
• Related #2041

Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I titled the PR using Conventional Commits.
• [x] I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
• [x] All existing and new tests are passing.
• [x] The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

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

• [x] Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
• [ ] No, this is not a breaking change.

Users of the connectivity_plus plugin will need to update their apps to handle the new list of ConnectivityResult types returned by checkConnectivity and onConnectivityChanged methods. This change allows for a more comprehensive understanding of the device's connectivity status, particularly in environments where multiple connectivity options are available or in use.

[suquant]

@miquelbeltran

Regarding the code, and before I go and do a more deep review, is there any reason why is preferred to handle the list of connectivity items as a coma-separated string rather than a List of Strings? e.g. in Connectivity.java, the MethodChannel, etc.

Fair note, agreed!

[miquelbeltran]

I'll try to run some tests this week and do a deeper code review. Thanks!

[miquelbeltran]

Tested Android (also with VPN), iOS, MacOS, Linux and Chrome. I could not test Windows. Seems to work as expected, except in Linux and Web it did not report VPN correctly, but that's another issue.

[vbuberen]

I see that the latest podspec comment is addressed as well. So with that change all looks good to me. Let's merge it.

Thanks a lot for this contribution, George.

We will need to do some updates to our docs, I believe, but let's do not in this PR.

[suquant]

If all is good, I'm gonna squash all commits into a single one for better tracking and well organized. Thanks for review guys @miquelbeltran @vbuberen

[vbuberen]

If all is good, I'm gonna squash all commits into a single one for better tracking and well organized. Thanks for review guys @miquelbeltran @vbuberen

Don't do it. We squash it ourselves.

[pamafe1976]

Hi, I just tested this update in Android, and the behavior does not seem to be as intended. I always receive a List with 1 ConnectivityResult.

If there is neither wifi or mobile I get [ConnectivityResult.none] Then I turn on mobile and I get [ConnectivityResult.mobile] =>OnConnectionChanged called 3 times Then I turn on wifi and I get [ConnectivityResult.wifi] =>OnConnectionChanged called 4 times If at this point I turn off mobile, onConnectionChanged is not called. Then I turn on wifi and I get [ConnectivityResult.none] =>OnConnectionChanged called 1 time

What I expected was

If there is neither wifi or mobile I get [ConnectivityResult.none] Then I turn on mobile and I get [ConnectivityResult.mobile] Then I turn on wifi and I get [ConnectivityResult.wifi, ConnectivityResult.mobile] Then turn off mobile and get [ConnectivityResult.wifi] Then turn off wifiand get [ConnectivityResult.none]

Of course the callback should be called only 1 time on each change

[vbuberen]

You are on Android, right?

If so, check the API documentation as it is documented that on Android having both wifi and mobile returns only wifi (or, to be correct, the one that is used for network activity). For example, if you do the same test with VPN turned on you would see 2 results instead.

As to number of times you get the event - on Android we listen to onCapabilitiesChanged callback from NetworkManager as well to catch all events from network state changes. So it is how this callback works. It is up to you to distinct changes in the stream.

With that being said feel free to open a new issue instead of discussing here, but from what you described it is expected behavior from the plugin and part about currently active states on Android is described in the API documentation as well.

[janniklind]

Is there some documentation around for why CONNECTIVITY_NONE is returned in the list?

I would prefer that in case of CONNECTIVITY_NONE then an empty list is returned, i.e. no available connections. It would be a cleaner API, that is more explicit.

Today when CONNECTIVITY_NONE is returned in the list, is it then guaranteed that it will always be the only item in the list?

I am using it like the following now, and I think many users would probably do the same:

bool _isConnected(List<ConnectivityResult> result) {
    return result.where((e) => e != ConnectivityResult.none).isNotEmpty;
}

[miquelbeltran]

Today when CONNECTIVITY_NONE is returned in the list, is it then guaranteed that it will always be the only item in the list?

Yes, and that is documented (not sure if this has been published, but it is in main)

/// The returned list is never empty. In case of no connectivity, the list contains
/// a single element of [ConnectivityResult.none]. Note also that this is the only
/// case where [ConnectivityResult.none] is present.

You should be able to safely do result.first != ConnectivityResult.none or similar.

[janniklind]

Thanks for pointing to the documentation :) May I ask what is the reasoning behind never returning an empty list?

[miquelbeltran]

Since the plugin returned a ConnectivityResult.none originally, we kept that.

We could add a convenience extension method that returns a single value and wraps the response with a .first, and/or one that just tells if there's connectivity or not.