Merge lp:~phablet-team/network-manager/ofono-rm-unused-code into lp:~network-manager/network-manager/ubuntu

[Tony Espy]

This merge proposal is a general cleanup of the NM ofono code ( src/devices/wwan/nm-modem-ofono.c ), getting rid of unused and/or code that was commented out.

[lp:~phablet-team/network-manager/ofono-rm-unused-code]

958. By Tony Espy on 2015-04-06

Remove unused nm-modem-ofono.c function: do_context_prepare()

[Ricardo Salveti (rsalveti)]

Looks good, makes it easier to fix bugs and maintain the code as we go (and also for possible vivid specific issues), so good to land soon if possible.

review: Approve

[Mathieu Trudel-Lapierre (cyphermox)]

Changeset is fine, but given that the unused/commented code is obvious and straightforward, I'd rather avoid making cosmetic changes two weeks away from Vivid release, even when bundled with bugfixes. I don't see why it couldn't wait two more weeks and land in the next release cycle.

Call me extra careful, but it's *very* late in the release so I'd defer the decision of landing this despite it being cosmetic changes to the release team. Could you please bring it up to them?

review: Abstain

[Ricardo Salveti (rsalveti)]

The thing about not waiting for two more extra weeks (for next release) is because we're still going to maintain this code base (vivid based) for RTM for a few months.

[Tony Espy (awe)]

Also this code has *nothing* to do with any code run on the desktop.

The only way any of the deleted static functions could possibly be used is if they were referenced in a function table, which none of them are.

The rest of the changes are super straight-forward, private variables that aren't ever used after being initialized, and code that's commented out, or ifdef'd out.

=== modified file 'debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch'
--- debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch	2015-04-06 22:03:27 +0000
+++ debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch	2015-04-06 22:03:27 +0000
@@ -304,7 +304,7 @@
 ===================================================================
 --- /dev/null
 +++ network-manager-0.9.10.0/src/devices/wwan/nm-modem-ofono.c
-@@ -0,0 +1,1395 @@
+@@ -0,0 +1,1144 @@
 +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
 +/* NetworkManager -- Network link manager
 + *
@@ -343,47 +343,6 @@
 +#include "nm-dbus-manager.h"
 +#include "NetworkManagerUtils.h"
 +
-+typedef enum {
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_ANY = 0,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_GPRS,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_EDGE,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_UMTS,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_HSDPA,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_2G_PREFERRED,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_3G_PREFERRED,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_2G_ONLY,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_3G_ONLY,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_HSUPA,
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_HSPA,
-+
-+    MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_LAST = MM_MODEM_GSM_NETWORK_DEPRECATED_MODE_HSPA
-+} MMModemDeprecatedMode;
-+
-+typedef enum {
-+    MM_MODEM_GSM_ALLOWED_MODE_ANY          = 0,
-+    MM_MODEM_GSM_ALLOWED_MODE_2G_PREFERRED = 1,
-+    MM_MODEM_GSM_ALLOWED_MODE_3G_PREFERRED = 2,
-+    MM_MODEM_GSM_ALLOWED_MODE_2G_ONLY      = 3,
-+    MM_MODEM_GSM_ALLOWED_MODE_3G_ONLY      = 4,
-+    MM_MODEM_GSM_ALLOWED_MODE_4G_PREFERRED = 5,
-+    MM_MODEM_GSM_ALLOWED_MODE_4G_ONLY      = 6,
-+
-+    MM_MODEM_GSM_ALLOWED_MODE_LAST = MM_MODEM_GSM_ALLOWED_MODE_4G_ONLY
-+} MMModemGsmAllowedMode;
-+
-+typedef enum {
-+	MM_MODEM_GSM_ALLOWED_AUTH_UNKNOWN  = 0x0000,
-+    /* bits 0..4 order match Ericsson device bitmap */
-+    MM_MODEM_GSM_ALLOWED_AUTH_NONE     = 0x0001,
-+    MM_MODEM_GSM_ALLOWED_AUTH_PAP      = 0x0002,
-+    MM_MODEM_GSM_ALLOWED_AUTH_CHAP     = 0x0004,
-+    MM_MODEM_GSM_ALLOWED_AUTH_MSCHAP   = 0x0008,
-+    MM_MODEM_GSM_ALLOWED_AUTH_MSCHAPV2 = 0x0010,
-+    MM_MODEM_GSM_ALLOWED_AUTH_EAP      = 0x0020,
-+
-+    MM_MODEM_GSM_ALLOWED_AUTH_LAST = MM_MODEM_GSM_ALLOWED_AUTH_EAP
-+} MMModemGsmAllowedAuth;
-+
 +G_DEFINE_TYPE (NMModemOfono, nm_modem_ofono, NM_TYPE_MODEM)
 +
 +#define NM_MODEM_OFONO_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_MODEM_OFONO, NMModemOfonoPrivate))
@@ -398,13 +357,8 @@
 +	DBusGProxy *context_proxy;
 +	DBusGProxy *simmanager_proxy;
 +
-+	DBusGProxyCall *call;
-+
 +	GError *property_error;
 +
-+	guint connman_iface_source;
-+	guint connman_iface_retries;
-+
 +	char **interfaces;
 +	char *context_path;
 +
@@ -551,11 +505,7 @@
 +	} else {
 +		new_state = NM_MODEM_STATE_SEARCHING;
 +		reason = NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER;
-+		//nm_modem_device_state_changed (NM_MODEM (self),
-+		//                               NM_DEVICE_STATE_UNAVAILABLE,
-+		//                               NM_DEVICE_STATE_UNKNOWN,
-+		//                               reason);
-+        }
++	}
 +
 +	nm_modem_set_state (NM_MODEM (self),
 +	                    new_state,
@@ -864,8 +814,6 @@
 +
 +	nm_log_dbg (LOGD_MB, "in %s", __func__);
 +
-+	priv->call = NULL;
-+
 +	if (priv->connect_properties) {
 +		g_hash_table_destroy (priv->connect_properties);
 +		priv->connect_properties = NULL;
@@ -1129,153 +1077,6 @@
 +	}
 +}
 +
-+static void
-+do_context_prepare (NMModemOfono *self, char *context_path)
-+{
-+	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
-+
-+	g_return_val_if_fail (self != NULL, FALSE);
-+	g_return_val_if_fail (NM_IS_MODEM_OFONO (self), FALSE);
-+
-+	nm_log_dbg (LOGD_MB, "in %s", __func__);
-+
-+	if (priv->context_proxy)
-+		g_object_unref (priv->context_proxy);
-+
-+	priv->context_proxy = get_ofono_proxy (self,
-+	                                       context_path,
-+	                                       OFONO_DBUS_INTERFACE_CONNECTION_CONTEXT);
-+
-+	if (priv->context_proxy) {
-+		priv->property_error = NULL;
-+		g_hash_table_foreach (priv->connect_properties,
-+		                      context_set_property,
-+		                      (gpointer) self);
-+		do_context_activate (self, context_path);
-+	}
-+}
-+
-+static void
-+create_new_context_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data)
-+{
-+	NMModemOfono *self = NM_MODEM_OFONO (user_data);
-+	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
-+	GError *error = NULL;
-+	char *context_path = NULL;
-+	gboolean ret = FALSE;
-+
-+	nm_log_dbg (LOGD_MB, "in %s", __func__);
-+
-+        ret = dbus_g_proxy_end_call (proxy,
-+                                     call_id,
-+	                             &error,
-+	                             DBUS_TYPE_G_OBJECT_PATH, &context_path,
-+	                             G_TYPE_INVALID);
-+
-+	nm_log_dbg (LOGD_MB, "%s: context path: %s", __func__, context_path);
-+
-+	if (ret)
-+                do_context_prepare (self, context_path);
-+        else {
-+                nm_log_warn (LOGD_MB, "Ofono modem context creation failed: (%d) %s",
-+                             error ? error->code : -1,
-+                             error && error->message ? error->message : "(unknown)");
-+
-+		g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED);
-+
-+		g_error_free (error);
-+        }
-+}
-+
-+static void
-+do_create_new_context (NMModemOfono *self)
-+{
-+	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
-+	DBusGProxy *proxy;
-+
-+	g_return_val_if_fail (self != NULL, FALSE);
-+	g_return_val_if_fail (NM_IS_MODEM_OFONO (self), FALSE);
-+
-+	nm_log_dbg (LOGD_MB, "in %s", __func__);
-+
-+	if (priv->connman_proxy) {
-+		dbus_g_proxy_begin_call_with_timeout (priv->connman_proxy,
-+	                                              "AddContext", create_new_context_done,
-+	                                              self, NULL, 20000,
-+	                                              G_TYPE_STRING, "internet",
-+	                                              G_TYPE_INVALID);
-+	}
-+	else {
-+		nm_log_err (LOGD_MB, "could not bring up connection manager proxy "
-+		                     "to add a new context");
-+	}
-+}
-+
-+static gboolean
-+try_create_new_context (NMModemOfono *self)
-+{
-+	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
-+	GHashTable *properties;
-+	char **interfaces;
-+	GError *error = NULL;
-+	gboolean found = FALSE;
-+	int i;
-+
-+	nm_log_dbg (LOGD_MB, "in %s", __func__);
-+
-+	/* Only retry up to 20 times */
-+	if (priv->connman_iface_retries < 20) {
-+		dbus_g_proxy_call_with_timeout (priv->modem_proxy,
-+	                                        "GetProperties",
-+	                                        250, &error,
-+	                                        G_TYPE_INVALID,
-+	                                        DBUS_TYPE_G_MAP_OF_VARIANT, &properties,
-+	                                        G_TYPE_INVALID);
-+
-+		if (!error) {
-+			interfaces = (char **) g_value_get_boxed (g_hash_table_lookup (properties, "Interfaces"));
-+
-+			for (i = 0; interfaces[i]; i++) {
-+				nm_log_dbg (LOGD_MB, "%s ?? %s",
-+				            interfaces[i],
-+				            OFONO_DBUS_INTERFACE_CONNECTION_MANAGER);
-+				if (!g_strcmp0 (interfaces[i],
-+				                OFONO_DBUS_INTERFACE_CONNECTION_MANAGER)) {
-+					found = TRUE;
-+					break;
-+				}
-+			}
-+		}
-+		else {
-+			nm_log_dbg (LOGD_MB, "failed test for properties: %s",
-+			            error && error->message ? error->message : "(unknown)");
-+		}
-+		priv->connman_iface_retries++;
-+	}
-+	else {
-+		if (priv->connman_iface_source != 0)
-+			g_source_remove (priv->connman_iface_source);
-+
-+		priv->connman_iface_source = 0;
-+		priv->connman_iface_retries = 0;
-+
-+		g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, 0);
-+
-+		return FALSE;
-+	}
-+
-+	if (found) {
-+		if (priv->connman_iface_source != 0)
-+			g_source_remove (priv->connman_iface_source);
-+
-+		priv->connman_iface_source = 0;
-+		priv->connman_iface_retries = 0;
-+		do_create_new_context (self);
-+	}
-+
-+	return !found;
-+}
-+
 +static void stage1_enable_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data);
 +
 +static void
@@ -1288,12 +1089,6 @@
 +	nm_log_dbg (LOGD_MB, "in %s", __func__);
 +
 +	if (dbus_g_proxy_end_call (proxy, call_id, &error, G_TYPE_INVALID)) {
-+#if 0
-+		/* Try once every 5 seconds to see if we've got the right interfaces */
-+		priv->connman_iface_retries = 0;
-+		priv->connman_iface_source
-+			= g_timeout_add (500, (GSourceFunc) try_create_new_context, self);
-+#endif
 +		if (priv->context_path)
 +			do_context_activate (self, priv->context_path);
 +		else
@@ -1514,44 +1309,6 @@
 +	                         G_TYPE_INVALID);
 +}
 +
-+#if 0
-+static void
-+set_ofono_enabled (NMModem *self, gboolean enabled)
-+{
-+	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
-+	GValue value = G_VALUE_INIT;
-+	gboolean ret;
-+	GError *error = NULL;
-+
-+	g_return_if_fail (self != NULL);
-+	g_return_if_fail (NM_IS_MODEM_OFONO (self));
-+	g_return_if_fail (priv != NULL);
-+	g_return_if_fail (priv->connman_proxy != NULL);
-+
-+	nm_log_dbg (LOGD_MB, "in %s", __func__);
-+
-+	g_value_init (&value, G_TYPE_BOOLEAN);
-+	g_value_set_boolean (&value, enabled);
-+
-+	ret = dbus_g_proxy_call_with_timeout (priv->connman_proxy,
-+	                                      "SetProperty",
-+	                                      20000,
-+	                                      &error,
-+	                                      G_TYPE_STRING, "Powered",
-+	                                      G_TYPE_VALUE, &value,
-+	                                      G_TYPE_INVALID,
-+	                                      G_TYPE_INVALID);
-+
-+	if (!ret) {
-+		nm_log_warn (LOGD_MB, "OFONO modem set enabled failed: (%d) %s",
-+		             error ? error->code : -1,
-+		             error && error->message ? error->message : "(unknown)");
-+	}
-+	else {
-+		get_ofono_conn_manager_properties (self);
-+	}
-+}
-+#endif
 +static void
 +set_ofono_enabled (NMModem *self, gboolean enabled)
 +{
@@ -1600,9 +1357,6 @@
 +	priv->context_proxy = NULL;
 +	priv->simmanager_proxy = NULL;
 +
-+	priv->connman_iface_source = 0;
-+	priv->connman_iface_retries = 0;
-+
 +	priv->modem_online = FALSE;
 +	priv->gprs_powered = FALSE;
 +	priv->gprs_attached = FALSE;
@@ -1690,14 +1444,9 @@
 +	modem_class->set_mm_enabled = set_ofono_enabled;
 +        modem_class->disconnect = disconnect;
 +	modem_class->deactivate = deactivate;
-+	//modem_class->get_user_pass = get_user_pass;
-+	//modem_class->get_setting_name = get_setting_name;
-+	//modem_class->complete_connection = complete_connection;
 +	modem_class->check_connection_compatible = check_connection_compatible;
 +	modem_class->act_stage1_prepare = act_stage1_prepare;
 +        modem_class->static_stage3_ip4_config_start = static_stage3_ip4_config_start;
-+
-+	//dbus_g_error_domain_register (NM_OFONO_ERROR, NULL, NM_TYPE_OFONO_ERROR);
 +}
 +
 Index: network-manager-0.9.10.0/src/devices/wwan/nm-modem-ofono.h