Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,15 @@

package dev.flutter.plugins.connectivity;

import android.content.Context;
import android.net.ConnectivityManager;
import android.net.wifi.WifiManager;
import io.flutter.embedding.engine.plugins.FlutterPlugin;
import io.flutter.plugin.common.EventChannel;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugins.connectivity.BroadcastReceiverRegistrarImpl;
import io.flutter.plugins.connectivity.Connectivity;
import io.flutter.plugins.connectivity.ConnectivityEventChannelHandler;
import io.flutter.plugins.connectivity.ConnectivityMethodChannelHandler;

/**
Expand All @@ -24,12 +30,30 @@ public void onAttachedToEngine(FlutterPluginBinding binding) {
final EventChannel eventChannel =
new EventChannel(
binding.getFlutterEngine().getDartExecutor(), "plugins.flutter.io/connectivity_status");
ConnectivityManager connectivityManager =
(ConnectivityManager)
binding
.getApplicationContext()
.getApplicationContext()
.getSystemService(Context.CONNECTIVITY_SERVICE);
WifiManager wifiManager =
(WifiManager)
binding
.getApplicationContext()
.getApplicationContext()
.getSystemService(Context.WIFI_SERVICE);

ConnectivityMethodChannelHandler handler =
new ConnectivityMethodChannelHandler(binding.getApplicationContext());
Connectivity connectivity = new Connectivity(connectivityManager, wifiManager);

channel.setMethodCallHandler(handler);
eventChannel.setStreamHandler(handler);
ConnectivityMethodChannelHandler methodChannelHandler =
new ConnectivityMethodChannelHandler(connectivity);
channel.setMethodCallHandler(methodChannelHandler);

BroadcastReceiverRegistrarImpl receiverRegistrar =
new BroadcastReceiverRegistrarImpl(binding.getApplicationContext(), connectivity);
ConnectivityEventChannelHandler eventChannelHandler =
new ConnectivityEventChannelHandler(receiverRegistrar);
eventChannel.setStreamHandler(eventChannelHandler);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.flutter.plugins.connectivity;

import android.content.BroadcastReceiver;
import androidx.annotation.NonNull;
import io.flutter.plugin.common.EventChannel;

/** Responsible for constructing a BroadcastReceiver as well as registering and unregistering it. */
Copy link
Contributor

Choose a reason for hiding this comment

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

While this statement is true, it may not be useful for developers. When a developer reads this doc, what kinds of questions might that developer be attempting to answer?

Flutter style guide: Writing prompts for good documentation
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#writing-prompts-for-good-documentation

public interface BroadcastReceiverRegistrar {

/**
* Triggered when it is ready for the BroadcastReceiver to be registered. Register the receiver in
Copy link
Contributor

Choose a reason for hiding this comment

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

Triggered when what is ready? This method probably involves a broader context. Can you fill in the reader on what is happening when this method is called, or why it exists? @mklim might have some suggestions here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a little unclear because it's phrased using passive voice, so it's not obvious who is doing what. https://developers.google.com/style/voice

I'd rephrase the first sentence to <Object> triggers this when it's ready for the BroadcastReceiver to be registered. With how it is now I think the worry is that you the developer need to trigger it somehow in some other place. I think the second sentence is clear as-is.

* this method body.
*
* @param receiver the receiver is going to be registered.
*/
void readyToRegisterBroadcastReceiver(@NonNull BroadcastReceiver receiver);

/**
* Triggered when it is ready for the BroadcastReceiver to be unregistered. Unregister the
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

* receiver in this method body.
*
* @param receiver the receiver is going to be unregistered.
*/
void readyToUnregisterBroadcastReceiver(@NonNull BroadcastReceiver receiver);

/**
* Creates a Broadcast receiver.
Copy link
Contributor

Choose a reason for hiding this comment

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

In it's current form, this probably qualifies as "useless documentation"

https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-useless-documentation

*
* @param events The events helps the broadcast receiver to dump information to the event channel.
* @return A BroadcastReceiver.
*/
@NonNull
BroadcastReceiver createReceiver(@NonNull final EventChannel.EventSink events);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.flutter.plugins.connectivity;

import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.net.ConnectivityManager;
import androidx.annotation.NonNull;
import io.flutter.plugin.common.EventChannel;

/** The BroadcastReceiverRegistrar used for the plugin. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comment here about effective prompts and useless documentation.

public class BroadcastReceiverRegistrarImpl implements BroadcastReceiverRegistrar {
private Context context;
private Connectivity connectivity;

/**
* @param context used to register and unregister the broadcastReceiver.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably useless documentation. @mklim do you think this is worth documenting?

Copy link
Contributor

Choose a reason for hiding this comment

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

I could see it go either way. This seems fairly straightforward to me but having the explanation about what context is used for is also kind of useful.

* @param connectivity used to check connectivity information.
*/
public BroadcastReceiverRegistrarImpl(
@NonNull Context context, @NonNull Connectivity connectivity) {
this.context = context;
this.connectivity = connectivity;
}

@Override
public void readyToRegisterBroadcastReceiver(BroadcastReceiver receiver) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing this implementation leads me to believe that this method should be called registerBroadcastReceiver instead of readyToRegisterBroadcastReceiver. It doesn't appear to be an event, but rather a command.

Same for the unregister method.

context.registerReceiver(receiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
}

@Override
public void readyToUnregisterBroadcastReceiver(BroadcastReceiver receiver) {
context.unregisterReceiver(receiver);
}

@Override
public BroadcastReceiver createReceiver(final EventChannel.EventSink events) {
return new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
events.success(connectivity.checkNetworkType());
}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package io.flutter.plugins.connectivity;

import android.net.ConnectivityManager;
import android.net.Network;
import android.net.NetworkCapabilities;
import android.net.NetworkInfo;
import android.net.wifi.WifiInfo;
import android.net.wifi.WifiManager;
import android.os.Build;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;


/** Responsible for checking connectivity information. */
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can do better here. This class doesn't seem to do much checking of anything. It appears to be far more concerned with reporting wifi properties and status.

public class Connectivity {
private ConnectivityManager connectivityManager;
private WifiManager wifiManager;

/**
* Constructs a ConnectivityChecker
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably another candidate for "useless documentation"

*
* @param connectivityManager used to check connectivity information.
* @param wifiManager used to check wifi information.
*/
public Connectivity(ConnectivityManager connectivityManager, WifiManager wifiManager) {
this.connectivityManager = connectivityManager;
this.wifiManager = wifiManager;
}

@NonNull
String checkNetworkType() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this method "checking"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Any update on this? Does this method need to exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No It doesn't. It was copied from the old code. I'll remove this method

return getNetworkType();
}

@Nullable
String getWifiName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all of these methods being with "/* package */"? @mklim for style input

Copy link
Contributor

Choose a reason for hiding this comment

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

It's one of those totally subjective style things, like annotating primitive arguments with /*name=*/. It's fine either way IMHO. From what I can tell the existing code doesn't use it, so it probably makes more sense to leave it off to be consistent.

WifiInfo wifiInfo = getWifiInfo();
String ssid = null;
if (wifiInfo != null) ssid = wifiInfo.getSSID();
if (ssid != null) ssid = ssid.replaceAll("\"", ""); // Android returns "SSID"
return ssid;
}

@Nullable
String getWifiBSSID() {
WifiInfo wifiInfo = getWifiInfo();
String bssid = null;
if (wifiInfo != null) {
bssid = wifiInfo.getBSSID();
}
return bssid;
}

@Nullable
String getWifiIPAddress() {
WifiInfo wifiInfo = null;
if (wifiManager != null) wifiInfo = wifiManager.getConnectionInfo();

String ip = null;
int i_ip = 0;
if (wifiInfo != null) i_ip = wifiInfo.getIpAddress();

if (i_ip != 0)
ip =
String.format(
"%d.%d.%d.%d",
(i_ip & 0xff), (i_ip >> 8 & 0xff), (i_ip >> 16 & 0xff), (i_ip >> 24 & 0xff));

return ip;
}

@Nullable
private WifiInfo getWifiInfo() {
return wifiManager == null ? null : wifiManager.getConnectionInfo();
}

private String getNetworkType() {
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Network network = connectivityManager.getActiveNetwork();
NetworkCapabilities capabilities = connectivityManager.getNetworkCapabilities(network);
if (capabilities == null) {
return "none";
}
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
|| capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)) {
return "wifi";
}
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
return "mobile";
}
}

return getNetworkTypeLegacy();
}

@SuppressWarnings("deprecation")
private String getNetworkTypeLegacy() {
// handle type for Android versions less than Android 9
NetworkInfo info = connectivityManager.getActiveNetworkInfo();
if (info == null || !info.isConnected()) {
return "none";
}
int type = info.getType();
switch (type) {
case ConnectivityManager.TYPE_ETHERNET:
case ConnectivityManager.TYPE_WIFI:
case ConnectivityManager.TYPE_WIMAX:
return "wifi";
case ConnectivityManager.TYPE_MOBILE:
case ConnectivityManager.TYPE_MOBILE_DUN:
case ConnectivityManager.TYPE_MOBILE_HIPRI:
return "mobile";
default:
return "none";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.flutter.plugins.connectivity;

import android.content.BroadcastReceiver;
import androidx.annotation.NonNull;
import io.flutter.plugin.common.EventChannel;

/** Handles the event channel for the plugin. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Another candidate for "writing prompts for good documentation"

What does it mean to "handle"? What is "the event channel"?

public class ConnectivityEventChannelHandler implements EventChannel.StreamHandler {
private final BroadcastReceiverRegistrar broadcastReceiverRegistrar;
private BroadcastReceiver broadcastReceiver;

/**
* Constructs a ConnectivityEventChannelHandler
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably "useless documentation"

*
* @param broadcastReceiverRegistrar handling registration of the broadcastReceiver.
*/
public ConnectivityEventChannelHandler(
@NonNull BroadcastReceiverRegistrar broadcastReceiverRegistrar) {
this.broadcastReceiverRegistrar = broadcastReceiverRegistrar;
}

@Override
public void onListen(Object arguments, EventChannel.EventSink events) {
broadcastReceiver = broadcastReceiverRegistrar.createReceiver(events);
broadcastReceiverRegistrar.readyToRegisterBroadcastReceiver(broadcastReceiver);
}

@Override
public void onCancel(Object arguments) {
broadcastReceiverRegistrar.readyToUnregisterBroadcastReceiver(broadcastReceiver);
broadcastReceiver = null;
}
}
Loading