jamesmontemagno / Xamarin.Plugins

Cross-platform Native API Access from Shared Code!
MIT License
1.3k stars 380 forks source link

Changing IsConnected getter to directly use System.Net.NetworkInforma… #292

Closed biste5 closed 7 years ago

biste5 commented 8 years ago

Please take a moment to fill out the following (change to preview to check or place x in []):

Fixes #244 .

Changes Proposed in this pull request:

This fixes/implements:

Which plugin does this impact:

which I experienced works correctly on devices as Lumia 635, 925 when in airplane mode.

jamesmontemagno commented 8 years ago

I don't believe it is very accurate: https://msdn.microsoft.com/en-us/library/system.net.networkinformation.networkinterface.getisnetworkavailable(v=vs.95).aspx

Remarks A network connection is considered to be available if any network interface is marked "up" and is not a loopback or tunnel interface. There are many cases in which a device or computer is not connected to a useful network but is still considered available and GetIsNetworkAvailable will return true. For example, if the device running the application is connected to a wireless network that requires a proxy, but the proxy is not set, GetIsNetworkAvailable will return true. Another example of when GetIsNetworkAvailable will return true is if the application is running on a computer that is connected to a hub or router where the hub or router has lost the upstream connection.

biste5 commented 8 years ago

Thanks for your reply James. I understand your points, and I agree but it doesn't seem the case. If you see my comments added to #244 you'll see in this moment it always gives a false positive when devices are in airplane mode.

In my experience, this is definitively more reliable (in real life)

What do you think?

jamesmontemagno commented 8 years ago

I should be able to pull this in.

I have a question though since you can test on device is if you can tell me what

profile.GetNetworkConnectivityLevel()

returns as it should return "None" in airplane mode...

Also, I think this code is shared with WinRT 8.1 desktop and wondering how it impacts it.

Let me know, James

biste5 commented 8 years ago

I have tried once more with a couple of physical devices, and I do see a different and more correct behavior with the current version for the plugin.

On the same Lumia 925 and on a Lumia 620 I tried to reproduce the issue, the plugin returns IsConnected = false if devices are in airplane mode. To reply to your query specifically, profile is null in case of airplane mode, which seems as expected in code.

On the emulator still returns IsConnected = true when in airplane mode, but that's less interesting to have correct.

I am sure about the tests I have performed at the time I raised the issue, as these I did recently, so I can't explain why these differences. I'm also sure devices haven't been updated meanwhile, and that the issue was encountered also with our published app on the store.

I will have to try with a Lumia 635 which was the first device where I've seen this kind of issue also before using your plugins, but that will take a bit because I don't have it with me in these days.

If you have any idea on what could be the reason of different behavior, again also with same device where OS wasn't updated meanwhile. The machine and tools where the App is built is instead different.

biste5 commented 7 years ago

Just tested on Lumia 635 again, with latest Plugin version 2.2.12 and the issue doesn't occur anymore there either. I will then close this PR for now because not necessary