wwan: use modem states instead of enabled/connected properties

Determining when the NMDeviceModem is available and when different
connections are available is easier if the modem's state is tracked,
instead of using the separate Enabled and Connected properties.
These properties could not accurately represent the SIM lock state
and prevented NetworkManager from making the modem available for
auto-activation when locked, even if a PIN was available.

In this new scheme, the NMDeviceModem is UNAVAILABLE when the
ModemManager modem state is FAILED, UNKNOWN, or INITIALIZING.  It
transitions to the NM DISCONNECTED state when the modem has finished
initializing and has not failed.

Once the NMDeviceModem is in DISCONNECTED state it can be activated
even if the SIM is locked and a PIN is required; the PIN will be
requested when starting activation, either from the connection itself
or via a secrets request.  This makes auto-activation of WWAN
connections possible.

This also allows us to consolidate code dealing with modem enable/disable
into the base NMModem class using the modem state, and to log more modem
information for debugging purposes.

9 files changed, 391 insertions, 224 deletions

diff --git a/include/NetworkManager.h b/include/NetworkManager.h
index 89b42aeb82..e0df39be4c 100644
--- a/include/NetworkManager.h
+++ b/include/NetworkManager.h
@@ -552,6 +552,12 @@ typedef enum {
 	/* teamd control failed */
 	NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED = 56,
 
+	/* Modem failed or no longer available */
+	NM_DEVICE_STATE_REASON_MODEM_FAILED = 57,
+
+	/* Modem now ready and available */
+	NM_DEVICE_STATE_REASON_MODEM_AVAILABLE = 58,
+
 	/* Unused */
 	NM_DEVICE_STATE_REASON_LAST = 0xFFFF
 } NMDeviceStateReason;
diff --git a/introspection/nm-device.xml b/introspection/nm-device.xml
index f9fbc61d07..34afcfc2a8 100644
--- a/introspection/nm-device.xml
+++ b/introspection/nm-device.xml
@@ -612,6 +612,16 @@
           teamd control failed.
         </tp:docstring>
       </tp:enumvalue>
+      <tp:enumvalue suffix="MODEM_FAILED" value="57">
+        <tp:docstring>
+          Modem failed or no longer available.
+        </tp:docstring>
+      </tp:enumvalue>
+      <tp:enumvalue suffix="MODEM_AVAILABLE" value="58">
+        <tp:docstring>
+          Modem now ready and available.
+        </tp:docstring>
+      </tp:enumvalue>
     </tp:enum>
 
     <tp:struct name="NM_DEVICE_STATE_REASON_STRUCT">
diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c
index 6998bb1e20..35c44763c4 100644
--- a/src/devices/bluetooth/nm-device-bt.c
+++ b/src/devices/bluetooth/nm-device-bt.c
@@ -572,6 +572,38 @@ modem_cleanup (NMDeviceBt *self)
 }
 
 static void
+modem_state_cb (NMModem *modem,
+                NMModemState new_state,
+                NMModemState old_state,
+                gpointer user_data)
+{
+	NMDevice *device = NM_DEVICE (user_data);
+	NMDeviceState dev_state = nm_device_get_state (device);
+
+	if (new_state <= NM_MODEM_STATE_DISABLING && old_state > NM_MODEM_STATE_DISABLING) {
+		/* Will be called whenever something external to NM disables the
+		 * modem directly through ModemManager.
+		 */
+		if (nm_device_is_activating (device) || dev_state == NM_DEVICE_STATE_ACTIVATED) {
+			nm_device_state_changed (device,
+			                         NM_DEVICE_STATE_DISCONNECTED,
+			                         NM_DEVICE_STATE_REASON_USER_REQUESTED);
+			return;
+		}
+	}
+
+	if (new_state < NM_MODEM_STATE_CONNECTING &&
+	    old_state >= NM_MODEM_STATE_CONNECTING &&
+	    dev_state >= NM_DEVICE_STATE_NEED_AUTH &&
+	    dev_state <= NM_DEVICE_STATE_ACTIVATED) {
+		/* Fail the device if the modem disconnects unexpectedly while the
+		 * device is activating/activated. */
+		nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER);
+		return;
+	}
+}
+
+static void
 modem_removed_cb (NMModem *modem, gpointer user_data)
 {
 	NMDeviceBt *self = NM_DEVICE_BT (user_data);
@@ -652,6 +684,7 @@ component_added (NMDevice *device, GObject *component)
 	g_signal_connect (modem, NM_MODEM_IP4_CONFIG_RESULT, G_CALLBACK (modem_ip4_config_result), self);
 	g_signal_connect (modem, NM_MODEM_AUTH_REQUESTED, G_CALLBACK (modem_auth_requested), self);
 	g_signal_connect (modem, NM_MODEM_AUTH_RESULT, G_CALLBACK (modem_auth_result), self);
+	g_signal_connect (modem, NM_MODEM_STATE_CHANGED, G_CALLBACK (modem_state_cb), self);
 	g_signal_connect (modem, NM_MODEM_REMOVED, G_CALLBACK (modem_removed_cb), self);
 
 	/* In the old ModemManager the data port is known from the very beginning;
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 7a48fe75c2..cd1de57f55 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -6573,6 +6573,10 @@ reason_to_string (NMDeviceStateReason reason)
 		return "DCB-FCoE-failed";
 	case NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED:
 		return "teamd-control-failed";
+	case NM_DEVICE_STATE_REASON_MODEM_FAILED:
+		return "modem-failed";
+	case NM_DEVICE_STATE_REASON_MODEM_AVAILABLE:
+		return "modem-available";
 	default:
 		break;
 	}
diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c
index 68619fde3f..7bf2560c93 100644
--- a/src/devices/wwan/nm-device-modem.c
+++ b/src/devices/wwan/nm-device-modem.c
@@ -165,38 +165,59 @@ data_port_changed_cb (NMModem *modem, GParamSpec *pspec, gpointer user_data)
 }
 
 static void
-modem_enabled_cb (NMModem *modem, GParamSpec *pspec, gpointer user_data)
+modem_state_cb (NMModem *modem,
+                NMModemState new_state,
+                NMModemState old_state,
+                gpointer user_data)
 {
 	NMDevice *device = NM_DEVICE (user_data);
 	NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device);
-	NMDeviceState state;
-
-	/* Called when the ModemManager modem enabled state is changed externally
-	 * to NetworkManager (eg something using MM's D-Bus API directly).
-	 */
-
-	if (priv->rf_enabled && nm_modem_get_mm_enabled (modem) == FALSE) {
-		state = nm_device_get_state (device);
-		if (nm_device_is_activating (device) || state == NM_DEVICE_STATE_ACTIVATED) {
+	NMDeviceState dev_state = nm_device_get_state (device);
+
+	if (new_state <= NM_MODEM_STATE_DISABLING &&
+	    old_state > NM_MODEM_STATE_DISABLING &&
+	    priv->rf_enabled) {
+		/* Called when the ModemManager modem enabled state is changed externally
+		 * to NetworkManager (eg something using MM's D-Bus API directly).
+		 */
+		if (nm_device_is_activating (device) || dev_state == NM_DEVICE_STATE_ACTIVATED) {
 			/* user-initiated action, hence DISCONNECTED not FAILED */
 			nm_device_state_changed (device,
 			                         NM_DEVICE_STATE_DISCONNECTED,
 			                         NM_DEVICE_STATE_REASON_USER_REQUESTED);
+			return;
 		}
 	}
-	nm_device_emit_recheck_auto_activate (device);
-}
 
-static void
-modem_connected_cb (NMModem *modem, GParamSpec *pspec, gpointer user_data)
-{
-	NMDeviceModem *self = NM_DEVICE_MODEM (user_data);
-	NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (self);
+	if (new_state < NM_MODEM_STATE_CONNECTING &&
+	    old_state >= NM_MODEM_STATE_CONNECTING &&
+	    dev_state >= NM_DEVICE_STATE_NEED_AUTH &&
+	    dev_state <= NM_DEVICE_STATE_ACTIVATED) {
+		/* Fail the device if the modem disconnects unexpectedly while the
+		 * device is activating/activated. */
+		nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER);
+		return;
+	}
+
+	if (new_state > NM_MODEM_STATE_LOCKED && old_state == NM_MODEM_STATE_LOCKED) {
+		/* If the modem is now unlocked, enable/disable it according to the
+		 * device's enabled/disabled state.
+		 */
+		nm_modem_set_mm_enabled (priv->modem, priv->rf_enabled);
+	}
 
-	if (   nm_device_get_state (NM_DEVICE (self)) == NM_DEVICE_STATE_ACTIVATED
-	    && !nm_modem_get_mm_connected (priv->modem)) {
-		/* Fail the device if the modem disconnects unexpectedly */
-		nm_device_state_changed (NM_DEVICE (self), NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER);
+	if ((dev_state >= NM_DEVICE_STATE_DISCONNECTED) && !nm_device_is_available (device)) {
+		nm_device_state_changed (device,
+		                         NM_DEVICE_STATE_UNAVAILABLE,
+		                         NM_DEVICE_STATE_REASON_MODEM_FAILED);
+		return;
+	}
+
+	if ((dev_state == NM_DEVICE_STATE_UNAVAILABLE) && nm_device_is_available (device)) {
+		nm_device_state_changed (device,
+		                         NM_DEVICE_STATE_DISCONNECTED,
+		                         NM_DEVICE_STATE_REASON_MODEM_AVAILABLE);
+		return;
 	}
 }
 
@@ -233,10 +254,19 @@ device_state_changed (NMDevice *device,
                       NMDeviceState old_state,
                       NMDeviceStateReason reason)
 {
-	nm_modem_device_state_changed (NM_DEVICE_MODEM_GET_PRIVATE (device)->modem,
-	                               new_state,
-	                               old_state,
-	                               reason);
+	NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device);
+
+	g_assert (priv->modem);
+
+	if (new_state == NM_DEVICE_STATE_UNAVAILABLE &&
+	    old_state < NM_DEVICE_STATE_UNAVAILABLE) {
+		/* Log initial modem state */
+		nm_log_info (LOGD_MB, "(%s): modem state '%s'",
+		             nm_device_get_iface (device),
+		             nm_modem_state_to_string (nm_modem_get_state (priv->modem)));
+	}
+
+	nm_modem_device_state_changed (priv->modem, new_state, old_state, reason);
 }
 
 static guint
@@ -259,6 +289,33 @@ check_connection_compatible (NMDevice *device,
 }
 
 static gboolean
+check_connection_available (NMDevice *device,
+                            NMConnection *connection,
+                            const char *specific_object)
+{
+	NMDeviceModem *self = NM_DEVICE_MODEM (device);
+	NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (self);
+	NMModemState state;
+
+	if (!priv->rf_enabled || !priv->modem)
+		return FALSE;
+
+	state = nm_modem_get_state (priv->modem);
+	if (state <= NM_MODEM_STATE_INITIALIZING)
+		return FALSE;
+
+	if (state == NM_MODEM_STATE_LOCKED) {
+		NMSettingGsm *s_gsm = nm_connection_get_setting_gsm (connection);
+
+		/* Can't use a connection without a PIN if the modem is locked */
+		if (!s_gsm || !nm_setting_gsm_get_pin (s_gsm))
+			return FALSE;
+	}
+
+	return TRUE;
+}
+
+static gboolean
 complete_connection (NMDevice *device,
                      NMConnection *connection,
                      const char *specific_object,
@@ -337,8 +394,9 @@ static gboolean
 get_enabled (NMDevice *device)
 {
 	NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (device);
+	NMModemState modem_state = nm_modem_get_state (priv->modem);
 
-	return priv->rf_enabled && nm_modem_get_mm_enabled (priv->modem);
+	return priv->rf_enabled && (modem_state >= NM_MODEM_STATE_LOCKED);
 }
 
 static void
@@ -368,14 +426,24 @@ static gboolean
 is_available (NMDevice *dev)
 {
 	NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE (dev);
+	NMModemState modem_state;
 
-	/* The device is available whenever it's not rfkilled */
 	if (!priv->rf_enabled) {
 		nm_log_dbg (LOGD_MB, "(%s): not available because WWAN airplane mode is on",
 		            nm_device_get_iface (dev));
+		return FALSE;
+	}
+
+	g_assert (priv->modem);
+	modem_state = nm_modem_get_state (priv->modem);
+	if (modem_state <= NM_MODEM_STATE_INITIALIZING) {
+		nm_log_dbg (LOGD_MB, "(%s): not available because modem is not ready (%s)",
+		            nm_device_get_iface (dev),
+		            nm_modem_state_to_string (modem_state));
+		return FALSE;
 	}
 
-	return priv->rf_enabled;
+	return TRUE;
 }
 
 /*****************************************************************************/
@@ -432,8 +500,7 @@ set_modem (NMDeviceModem *self, NMModem *modem)
 	g_signal_connect (modem, NM_MODEM_IP4_CONFIG_RESULT, G_CALLBACK (modem_ip4_config_result), self);
 	g_signal_connect (modem, NM_MODEM_AUTH_REQUESTED, G_CALLBACK (modem_auth_requested), self);
 	g_signal_connect (modem, NM_MODEM_AUTH_RESULT, G_CALLBACK (modem_auth_result), self);
-	g_signal_connect (modem, "notify::" NM_MODEM_ENABLED, G_CALLBACK (modem_enabled_cb), self);
-	g_signal_connect (modem, "notify::" NM_MODEM_CONNECTED, G_CALLBACK (modem_connected_cb), self);
+	g_signal_connect (modem, NM_MODEM_STATE_CHANGED, G_CALLBACK (modem_state_cb), self);
 	g_signal_connect (modem, NM_MODEM_REMOVED, G_CALLBACK (modem_removed_cb), self);
 
 	/* In the old ModemManager the data port is known from the very beginning;
@@ -514,6 +581,7 @@ nm_device_modem_class_init (NMDeviceModemClass *mclass)
 
 	device_class->get_hw_address_length = get_hw_address_length;
 	device_class->check_connection_compatible = check_connection_compatible;
+	device_class->check_connection_available = check_connection_available;
 	device_class->complete_connection = complete_connection;
 	device_class->deactivate = deactivate;
 	device_class->act_stage1_prepare = act_stage1_prepare;
diff --git a/src/devices/wwan/nm-modem-broadband.c b/src/devices/wwan/nm-modem-broadband.c
index 40cf32c772..83bf4a2657 100644
--- a/src/devices/wwan/nm-modem-broadband.c
+++ b/src/devices/wwan/nm-modem-broadband.c
@@ -531,17 +531,6 @@ get_user_pass (NMModem *modem,
 /* Query/Update enabled state */
 
 static void
-update_mm_enabled (NMModem *self,
-                   gboolean new_enabled)
-{
-	if (nm_modem_get_mm_enabled (self) != new_enabled) {
-		g_object_set (self,
-		              NM_MODEM_ENABLED, new_enabled,
-		              NULL);
-	}
-}
-
-static void
 modem_disable_ready (MMModem *modem_iface,
                      GAsyncResult *res,
                      NMModemBroadband *self)
@@ -552,10 +541,9 @@ modem_disable_ready (MMModem *modem_iface,
 		nm_log_warn (LOGD_MB, "(%s) failed to disable modem: %s",
 		             nm_modem_get_uid (NM_MODEM (self)),
 		             error && error->message ? error->message : "(unknown)");
+		nm_modem_set_prev_state (NM_MODEM (self), "disable failed");
 		g_clear_error (&error);
-	} else
-		/* Update enabled/disabled state again */
-		update_mm_enabled (NM_MODEM (self), FALSE);
+	}
 
 	/* Balance refcount */
 	g_object_unref (self);
@@ -572,9 +560,9 @@ modem_enable_ready (MMModem *modem_iface,
 		nm_log_warn (LOGD_MB, "(%s) failed to enable modem: %s",
 		             nm_modem_get_uid (NM_MODEM (self)),
 		             error && error->message ? error->message : "(unknown)");
+		nm_modem_set_prev_state (NM_MODEM (self), "enable failed");
 		g_clear_error (&error);
-	} else
-		update_mm_enabled (NM_MODEM (self), TRUE);
+	}
 
 	/* Balance refcount */
 	g_object_unref (self);
@@ -587,14 +575,6 @@ set_mm_enabled (NMModem *_self,
 	NMModemBroadband *self = NM_MODEM_BROADBAND (_self);
 
 	if (enabled) {
-		/* Don't even try to enable if we're known to be already locked */
-		if (mm_modem_get_state (self->priv->modem_iface) == MM_MODEM_STATE_LOCKED) {
-			nm_log_warn (LOGD_MB, "(%s) cannot enable modem: locked",
-			             nm_modem_get_uid (NM_MODEM (self)));
-			g_signal_emit_by_name (self, NM_MODEM_AUTH_REQUESTED, 0);
-			return;
-		}
-
 		mm_modem_enable (self->priv->modem_iface,
 		                 NULL, /* cancellable */
 		                 (GAsyncReadyCallback)modem_enable_ready,
@@ -604,9 +584,6 @@ set_mm_enabled (NMModem *_self,
 		                  NULL, /* cancellable */
 		                  (GAsyncReadyCallback)modem_disable_ready,
 		                  g_object_ref (self));
-
-		/* When disabling don't say we're enabled */
-		update_mm_enabled (NM_MODEM (self), enabled);
 	}
 }
 
@@ -782,6 +759,29 @@ deactivate (NMModem *_self, NMDevice *device)
 
 /*****************************************************************************/
 
+#define MAP_STATE(name) case MM_MODEM_STATE_##name: return NM_MODEM_STATE_##name;
+
+static NMModemState
+mm_state_to_nm (MMModemState mm_state)
+{
+	switch (mm_state) {
+	MAP_STATE(UNKNOWN)
+	MAP_STATE(FAILED)
+	MAP_STATE(INITIALIZING)
+	MAP_STATE(LOCKED)
+	MAP_STATE(DISABLED)
+	MAP_STATE(DISABLING)
+	MAP_STATE(ENABLING)
+	MAP_STATE(ENABLED)
+	MAP_STATE(SEARCHING)
+	MAP_STATE(REGISTERED)
+	MAP_STATE(DISCONNECTING)
+	MAP_STATE(CONNECTING)
+	MAP_STATE(CONNECTED)
+	}
+	return NM_MODEM_STATE_UNKNOWN;
+}
+
 static void
 modem_state_changed (MMModem *modem,
                      MMModemState old_state,
@@ -789,28 +789,17 @@ modem_state_changed (MMModem *modem,
                      MMModemStateChangeReason reason,
                      NMModemBroadband *self)
 {
-	gboolean old;
-	gboolean new;
-
-	nm_log_info (LOGD_MB, "(%s) modem state changed, '%s' --> '%s' (reason: %s)\n",
-	             nm_modem_get_uid (NM_MODEM (self)),
-	             mm_modem_state_get_string (old_state),
-	             mm_modem_state_get_string (new_state),
-	             mm_modem_state_change_reason_get_string (reason));
-
-	old = nm_modem_get_mm_enabled (NM_MODEM (self));
-	new = (mm_modem_get_state (self->priv->modem_iface) >= MM_MODEM_STATE_ENABLED);
-	if (old != new)
-		g_object_set (self,
-		              NM_MODEM_ENABLED, new,
-		              NULL);
 
-	old = nm_modem_get_mm_connected (NM_MODEM (self));
-	new = (mm_modem_get_state (self->priv->modem_iface) >= MM_MODEM_STATE_CONNECTED);
-	if (old != new)
-		g_object_set (self,
-		              NM_MODEM_CONNECTED, new,
-		              NULL);
+	/* After the SIM is unlocked MM1 will move the device to INITIALIZING which
+	 * is an unavailable state.  That makes state handling confusing here, so
+	 * suppress this state change and let the modem move from LOCKED to DISABLED.
+	 */
+	if (new_state == MM_MODEM_STATE_INITIALIZING && old_state == MM_MODEM_STATE_LOCKED)
+		return;
+
+	nm_modem_set_state (NM_MODEM (self),
+	                    mm_state_to_nm (new_state),
+	                    mm_modem_state_change_reason_get_string (reason));
 }
 
 /*****************************************************************************/
@@ -831,16 +820,6 @@ nm_modem_broadband_new (GObject *object, GError **error)
 	g_return_val_if_fail (!!modem_iface, NULL);
 	g_return_val_if_fail (!!mm_modem_get_primary_port (modem_iface), NULL);
 
-	/* If the modem is in 'FAILED' state we cannot do anything with it.
-	 * This happens when a severe error happened when trying to initialize it,
-	 * like missing SIM. */
-	if (mm_modem_get_state (modem_iface) == MM_MODEM_STATE_FAILED) {
-		g_set_error (error, NM_MODEM_ERROR, NM_MODEM_ERROR_INITIALIZATION_FAILED,
-		             "(%s): unusable modem detected",
-		             mm_modem_get_primary_port (modem_iface));
-		return NULL;
-	}
-
 	/* Build a single string with all drivers listed */
 	drivers = g_strjoinv (", ", (gchar **)mm_modem_get_drivers (modem_iface));
 
@@ -849,6 +828,7 @@ nm_modem_broadband_new (GObject *object, GError **error)
 	                      NM_MODEM_UID, mm_modem_get_primary_port (modem_iface),
 	                      NM_MODEM_CONTROL_PORT, mm_modem_get_primary_port (modem_iface),
 	                      NM_MODEM_DATA_PORT, NULL, /* We don't know it until bearer created */
+	                      NM_MODEM_STATE, mm_state_to_nm (mm_modem_get_state (modem_iface)),
 	                      NM_MODEM_BROADBAND_MODEM, modem_object,
 	                      NM_MODEM_DRIVER, drivers,
 	                      NULL);
@@ -883,11 +863,6 @@ set_property (GObject *object,
                           G_CALLBACK (modem_state_changed),
                           self);
 
-		g_object_set (object,
-		              NM_MODEM_ENABLED, (mm_modem_get_state (self->priv->modem_iface) >= MM_MODEM_STATE_ENABLED),
-		              NM_MODEM_CONNECTED, (mm_modem_get_state (self->priv->modem_iface) >= MM_MODEM_STATE_CONNECTED),
-		              NULL);
-
 		/* Note: don't grab the Simple iface here; the Modem interface is the
 		 * only one assumed to be always valid and available */
 		break;
diff --git a/src/devices/wwan/nm-modem-old.c b/src/devices/wwan/nm-modem-old.c
index f8bbcebb85..708d318357 100644
--- a/src/devices/wwan/nm-modem-old.c
+++ b/src/devices/wwan/nm-modem-old.c
@@ -43,6 +43,7 @@ typedef struct {
 
 	MMOldModemState state;
 	NMDeviceModemCapabilities caps;
+	char *unlock_required;
 
 	DBusGProxyCall *call;
 	GHashTable *connect_properties;
@@ -141,6 +142,29 @@ translate_mm_error (GError *error)
 	return reason;
 }
 
+#define MAP_STATE(name) case MM_OLD_MODEM_STATE_##name: return NM_MODEM_STATE_##name;
+
+static NMModemState
+mm_state_to_nm (MMOldModemState mm_state, const char *unlock_required)
+{
+	if (unlock_required && *unlock_required)
+		return NM_MODEM_STATE_LOCKED;
+
+	switch (mm_state) {
+	MAP_STATE(UNKNOWN)
+	MAP_STATE(DISABLED)
+	MAP_STATE(DISABLING)
+	MAP_STATE(ENABLING)
+	MAP_STATE(ENABLED)
+	MAP_STATE(SEARCHING)
+	MAP_STATE(REGISTERED)
+	MAP_STATE(DISCONNECTING)
+	MAP_STATE(CONNECTING)
+	MAP_STATE(CONNECTED)
+	}
+	return NM_MODEM_STATE_UNKNOWN;
+};
+
 /*****************************************************************************/
 
 DBusGProxy *
@@ -170,52 +194,6 @@ nm_modem_old_get_proxy (NMModemOld *self, const char *interface)
 /* Query/Update enabled state */
 
 static void
-update_mm_enabled (NMModem *self,
-                   gboolean new_enabled)
-{
-	if (nm_modem_get_mm_enabled (self) != new_enabled) {
-		g_object_set (self,
-		              NM_MODEM_ENABLED, new_enabled,
-		              NULL);
-	}
-}
-
-static void
-get_mm_enabled_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data)
-{
-	NMModem *self = NM_MODEM (user_data);
-	GError *error = NULL;
-	GValue value = G_VALUE_INIT;
-
-	if (!dbus_g_proxy_end_call (proxy, call_id, &error,
-	                            G_TYPE_VALUE, &value,
-	                            G_TYPE_INVALID)) {
-		nm_log_warn (LOGD_MB, "failed get modem enabled state: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
-		return;
-	}
-
-	if (G_VALUE_HOLDS_BOOLEAN (&value)) {
-		update_mm_enabled (self, g_value_get_boolean (&value));
-	} else
-		nm_log_warn (LOGD_MB, "failed get modem enabled state: unexpected reply type");
-
-	g_value_unset (&value);
-}
-
-static void
-query_mm_enabled (NMModemOld *self)
-{
-	dbus_g_proxy_begin_call (NM_MODEM_OLD_GET_PRIVATE (self)->props_proxy,
-	                         "Get", get_mm_enabled_done,
-	                         self, NULL,
-	                         G_TYPE_STRING, MM_OLD_DBUS_INTERFACE_MODEM,
-	                         G_TYPE_STRING, "Enabled",
-	                         G_TYPE_INVALID);
-}
-
-static void
 set_mm_enabled_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data)
 {
 	GError *error = NULL;
@@ -224,28 +202,19 @@ set_mm_enabled_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_d
 		nm_log_warn (LOGD_MB, "failed to enable/disable modem: (%d) %s",
 		             error ? error->code : -1,
 		             error && error->message ? error->message : "(unknown)");
+		nm_modem_set_prev_state (NM_MODEM (user_data), "enable/disable failed");
 	}
-
-	/* Update enabled/disabled state again */
-	query_mm_enabled (NM_MODEM_OLD (user_data));
+	/* Wait for the state change signal to indicate enabled state changed */
 }
 
 static void
 set_mm_enabled (NMModem *self, gboolean enabled)
 {
-	/* FIXME: For now this just toggles the ModemManager enabled state.  In the
-	 * future we want to tie this into rfkill state instead so that the user can
-	 * toggle rfkill status of the WWAN modem.
-	 */
-	dbus_g_proxy_begin_call (nm_modem_old_get_proxy (NM_MODEM_OLD (self),
-	                                                     MM_OLD_DBUS_INTERFACE_MODEM),
+	dbus_g_proxy_begin_call (nm_modem_old_get_proxy (NM_MODEM_OLD (self), MM_OLD_DBUS_INTERFACE_MODEM),
 	                         "Enable", set_mm_enabled_done,
-	                         self, NULL,
+	                         g_object_ref (self), g_object_unref,
 	                         G_TYPE_BOOLEAN, enabled,
 	                         G_TYPE_INVALID);
-	/* If we are disabling the modem, stop saying that it's enabled. */
-	if (!enabled)
-		update_mm_enabled (self, enabled);
 }
 
 /*****************************************************************************/
@@ -543,13 +512,12 @@ act_stage1_prepare (NMModem *modem,
 {
 	NMModemOld *self = NM_MODEM_OLD (modem);
 	NMModemOldPrivate *priv = NM_MODEM_OLD_GET_PRIVATE (self);
-	gboolean enabled = nm_modem_get_mm_enabled (modem);
 
 	if (priv->connect_properties)
 		g_hash_table_destroy (priv->connect_properties);
 	priv->connect_properties = create_connect_properties (connection);
 
-	if (enabled)
+	if (nm_modem_get_state (modem) >= NM_MODEM_STATE_ENABLING)
 		do_connect (self);
 	else
 		do_enable (self);
@@ -718,18 +686,11 @@ modem_properties_changed (DBusGProxy *proxy,
 	NMModemOld *self = NM_MODEM_OLD (user_data);
 	NMModemOldPrivate *priv = NM_MODEM_OLD_GET_PRIVATE (self);
 	GValue *value;
-	MMOldModemState new_state;
+	gboolean update_state = FALSE;
 
 	if (strcmp (interface, MM_OLD_DBUS_INTERFACE_MODEM))
 		return;
 
-	value = g_hash_table_lookup (props, "Enabled");
-	if (value && G_VALUE_HOLDS_BOOLEAN (value)) {
-		g_object_set (self,
-		              NM_MODEM_ENABLED, g_value_get_boolean (value),
-		              NULL);
-	}
-
 	value = g_hash_table_lookup (props, "IpMethod");
 	if (value && G_VALUE_HOLDS_UINT (value)) {
 		g_object_set (self,
@@ -737,20 +698,23 @@ modem_properties_changed (DBusGProxy *proxy,
 		              NULL);
 	}
 
+	value = g_hash_table_lookup (props, "UnlockRequired");
+	if (value && G_VALUE_HOLDS_STRING (value)) {
+		g_free (priv->unlock_required);
+		priv->unlock_required = g_value_dup_string (value);
+		update_state = TRUE;
+	}
+
 	value = g_hash_table_lookup (props, "State");
 	if (value && G_VALUE_HOLDS_UINT (value)) {
-		new_state = g_value_get_uint (value);
-		if (new_state != priv->state) {
-			if (new_state == MM_OLD_MODEM_STATE_CONNECTED)
-				g_object_set (self,
-				              NM_MODEM_CONNECTED, TRUE,
-				              NULL);
-			else if (priv->state == MM_OLD_MODEM_STATE_CONNECTED)
-				g_object_set (self,
-				              NM_MODEM_CONNECTED, FALSE,
-				              NULL);
-			priv->state = new_state;
-		}
+		priv->state = g_value_get_uint (value);
+		update_state = TRUE;
+	}
+
+	if (update_state) {
+		nm_modem_set_state (NM_MODEM (self),
+		                    mm_state_to_nm (priv->state, priv->unlock_required),
+		                    NULL);
 	}
 }
 
@@ -970,6 +934,7 @@ nm_modem_old_new (const char *path, GHashTable *properties, GError **error)
 	const char *data_device = NULL;
 	const char *driver = NULL;
 	const char *master_device = NULL;
+	const char *unlock_required = NULL;
 	guint32 modem_type = MM_OLD_MODEM_TYPE_UNKNOWN;
 	guint32 ip_method = MM_MODEM_IP_METHOD_PPP;
 	guint32 ip_timeout = 0;
@@ -994,6 +959,8 @@ nm_modem_old_new (const char *path, GHashTable *properties, GError **error)
 			ip_timeout = g_value_get_uint (value);
 		else if (g_strcmp0 (prop, "State") == 0)
 			state = g_value_get_uint (value);
+		else if (g_strcmp0 (prop, "UnlockRequired") == 0)
+			unlock_required = g_value_get_string (value);
 	}
 
 	if (modem_type == MM_OLD_MODEM_TYPE_UNKNOWN) {
@@ -1028,7 +995,7 @@ nm_modem_old_new (const char *path, GHashTable *properties, GError **error)
 	                                    NM_MODEM_DATA_PORT, data_device,
 	                                    NM_MODEM_IP_METHOD, ip_method,
 	                                    NM_MODEM_IP_TIMEOUT, ip_timeout,
-	                                    NM_MODEM_CONNECTED, (state == MM_OLD_MODEM_STATE_CONNECTED),
+	                                    NM_MODEM_STATE, mm_state_to_nm (state, unlock_required),
 	                                    NULL);
 	if (self) {
 		if (modem_type == MM_OLD_MODEM_TYPE_CDMA)
@@ -1037,6 +1004,8 @@ nm_modem_old_new (const char *path, GHashTable *properties, GError **error)
 			caps |= NM_DEVICE_MODEM_CAPABILITY_GSM_UMTS;
 
 		NM_MODEM_OLD_GET_PRIVATE (self)->caps = caps;
+		NM_MODEM_OLD_GET_PRIVATE (self)->state = state;
+		NM_MODEM_OLD_GET_PRIVATE (self)->unlock_required = g_strdup (unlock_required);
 	}
 
 	return (NMModem *) self;
@@ -1084,8 +1053,6 @@ constructor (GType type,
 	                             object,
 	                             NULL);
 
-	query_mm_enabled (NM_MODEM_OLD (object));
-
 	return object;
 }
 
@@ -1114,6 +1081,9 @@ dispose (GObject *object)
 		priv->enable_delay_id = 0;
 	}
 
+	g_free (priv->unlock_required);
+	priv->unlock_required = NULL;
+
 	G_OBJECT_CLASS (nm_modem_old_parent_class)->dispose (object);
 }
 
diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c
index 2ff0200894..494f85e641 100644
--- a/src/devices/wwan/nm-modem.c
+++ b/src/devices/wwan/nm-modem.c
@@ -15,7 +15,7 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
- * Copyright (C) 2009 - 2011 Red Hat, Inc.
+ * Copyright (C) 2009 - 2014 Red Hat, Inc.
  * Copyright (C) 2009 Novell, Inc.
  */
 
@@ -44,8 +44,7 @@ enum {
 	PROP_DRIVER,
 	PROP_IP_METHOD,
 	PROP_IP_TIMEOUT,
-	PROP_ENABLED,
-	PROP_CONNECTED,
+	PROP_STATE,
 
 	LAST_PROP
 };
@@ -58,6 +57,8 @@ typedef struct {
 	char *data_port;
 	char *ppp_iface;
 	guint32 ip_method;
+	NMModemState state;
+	NMModemState prev_state;  /* revert to this state if enable/disable fails */
 
 	NMPPPManager *ppp_manager;
 
@@ -65,9 +66,7 @@ typedef struct {
 	guint32 secrets_tries;
 	guint32 secrets_id;
 
-	gboolean mm_enabled;
 	guint32 mm_ip_timeout;
-	gboolean mm_connected;
 
 	/* PPP stats */
 	guint32 in_bytes;
@@ -82,6 +81,7 @@ enum {
 	AUTH_REQUESTED,
 	AUTH_RESULT,
 	REMOVED,
+	STATE_CHANGED,
 
 	LAST_SIGNAL
 };
@@ -101,30 +101,111 @@ nm_modem_error_quark (void)
 }
 
 /*****************************************************************************/
-/* Get/Set enabled/connected */
+/* State/enabled/connected */
+
+static const char *state_table[] = {
+	[NM_MODEM_STATE_UNKNOWN]       = "unknown",
+	[NM_MODEM_STATE_FAILED]        = "failed",
+	[NM_MODEM_STATE_INITIALIZING]  = "initializing",
+	[NM_MODEM_STATE_LOCKED]        = "locked",
+	[NM_MODEM_STATE_DISABLED]      = "disabled",
+	[NM_MODEM_STATE_DISABLING]     = "disabling",
+	[NM_MODEM_STATE_ENABLING]      = "enabling",
+	[NM_MODEM_STATE_ENABLED]       = "enabled",
+	[NM_MODEM_STATE_SEARCHING]     = "searching",
+	[NM_MODEM_STATE_REGISTERED]    = "registered",
+	[NM_MODEM_STATE_DISCONNECTING] = "disconnecting",
+	[NM_MODEM_STATE_CONNECTING]    = "connecting",
+	[NM_MODEM_STATE_CONNECTED]     = "connected",
+};
 
-gboolean
-nm_modem_get_mm_enabled (NMModem *self)
+const char *
+nm_modem_state_to_string (NMModemState state)
+{
+	if (state >= 0 && state < G_N_ELEMENTS (state_table))
+		return state_table[state];
+	return NULL;
+}
+
+NMModemState
+nm_modem_get_state (NMModem *self)
 {
-	return NM_MODEM_GET_PRIVATE (self)->mm_enabled;
+	return NM_MODEM_GET_PRIVATE (self)->state;
 }
 
 void
-nm_modem_set_mm_enabled (NMModem *self,
-                         gboolean enabled)
+nm_modem_set_state (NMModem *self,
+                    NMModemState new_state,
+                    const char *reason)
 {
-	NMModemPrivate *priv;
+	NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
+	NMModemState old_state = priv->state;
 
-	priv = NM_MODEM_GET_PRIVATE (self);
+	priv->prev_state = NM_MODEM_STATE_UNKNOWN;
+
+	if (new_state != old_state) {
+		nm_log_info (LOGD_MB, "(%s): modem state changed, '%s' --> '%s' (reason: %s)\n",
+		             nm_modem_get_uid (self),
+		             nm_modem_state_to_string (old_state),
+		             nm_modem_state_to_string (new_state),
+		             reason ? reason : "none");
 
-	if (priv->mm_enabled != enabled)
-		NM_MODEM_GET_CLASS (self)->set_mm_enabled (self, enabled);
+		priv->state = new_state;
+		g_object_notify (G_OBJECT (self), NM_MODEM_STATE);
+		g_signal_emit (self, signals[STATE_CHANGED], 0, new_state, old_state, reason);
+	}
 }
 
-gboolean
-nm_modem_get_mm_connected (NMModem *self)
+void
+nm_modem_set_prev_state (NMModem *self, const char *reason)
+{
+	NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
+
+	/* Reset modem to previous state if the state hasn't already changed */
+	if (priv->prev_state != NM_MODEM_STATE_UNKNOWN)
+		nm_modem_set_state (self, priv->prev_state, reason);
+}
+
+void
+nm_modem_set_mm_enabled (NMModem *self,
+                         gboolean enabled)
 {
-	return NM_MODEM_GET_PRIVATE (self)->mm_connected;
+	NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
+	NMModemState prev_state = priv->state;
+
+	if (enabled && priv->state >= NM_MODEM_STATE_ENABLING) {
+		nm_log_dbg (LOGD_MB, "(%s) cannot enable modem: already enabled",
+		            nm_modem_get_uid (self));
+		return;
+	}
+	if (!enabled && priv->state <= NM_MODEM_STATE_DISABLING) {
+		nm_log_dbg (LOGD_MB, "(%s) cannot disable modem: already disabled",
+		            nm_modem_get_uid (self));
+		return;
+	}
+
+	if (priv->state <= NM_MODEM_STATE_INITIALIZING) {
+		nm_log_dbg (LOGD_MB, "(%s) cannot enable/disable modem: initializing or failed",
+		            nm_modem_get_uid (self));
+		return;
+	} else if (priv->state == NM_MODEM_STATE_LOCKED) {
+		/* Don't try to enable if the modem is locked since that will fail */
+		nm_log_warn (LOGD_MB, "(%s) cannot enable/disable modem: locked",
+		             nm_modem_get_uid (self));
+
+		/* Try to unlock the modem if it's being enabled */
+		if (enabled)
+			g_signal_emit_by_name (self, NM_MODEM_AUTH_REQUESTED, 0);
+		return;
+	}
+
+	NM_MODEM_GET_CLASS (self)->set_mm_enabled (self, enabled);
+
+	/* Pre-empt the state change signal */
+	nm_modem_set_state (self,
+	                    enabled ? NM_MODEM_STATE_ENABLING : NM_MODEM_STATE_DISABLING,
+	                    "user preference");
+	priv->prev_state = prev_state;
 }
 
 void
@@ -769,11 +850,8 @@ get_property (GObject *object, guint prop_id,
 	case PROP_IP_TIMEOUT:
 		g_value_set_uint (value, priv->mm_ip_timeout);
 		break;
-	case PROP_ENABLED:
-		g_value_set_boolean (value, priv->mm_enabled);
-		break;
-	case PROP_CONNECTED:
-		g_value_set_boolean (value, priv->mm_connected);
+	case PROP_STATE:
+		g_value_set_enum (value, priv->state);
 		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
@@ -812,11 +890,8 @@ set_property (GObject *object, guint prop_id,
 	case PROP_IP_TIMEOUT:
 		priv->mm_ip_timeout = g_value_get_uint (value);
 		break;
-	case PROP_ENABLED:
-		priv->mm_enabled = g_value_get_boolean (value);
-		break;
-	case PROP_CONNECTED:
-		priv->mm_connected = g_value_get_boolean (value);
+	case PROP_STATE:
+		priv->state = g_value_get_enum (value);
 		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
@@ -929,20 +1004,13 @@ nm_modem_class_init (NMModemClass *klass)
 		                    G_PARAM_READWRITE));
 
 	g_object_class_install_property
-		(object_class, PROP_ENABLED,
-		 g_param_spec_boolean (NM_MODEM_ENABLED,
-		                       "Enabled",
-		                       "Enabled",
-		                       TRUE,
-		                       G_PARAM_READWRITE));
-
-	g_object_class_install_property
-		(object_class, PROP_CONNECTED,
-		 g_param_spec_boolean (NM_MODEM_CONNECTED,
-		                       "Connected",
-		                       "Connected",
-		                       TRUE,
-		                       G_PARAM_READWRITE));
+		(object_class, PROP_STATE,
+		 g_param_spec_enum (NM_MODEM_STATE,
+		                   "State",
+		                   "State",
+		                   NM_TYPE_MODEM_STATE,
+		                   NM_MODEM_STATE_UNKNOWN,
+		                   G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
 
 	/* Signals */
 
@@ -1003,6 +1071,14 @@ nm_modem_class_init (NMModemClass *klass)
 		              NULL, NULL, NULL,
 		              G_TYPE_NONE, 0);
 
+	signals[STATE_CHANGED] =
+		g_signal_new (NM_MODEM_STATE_CHANGED,
+		              G_OBJECT_CLASS_TYPE (object_class),
+		              G_SIGNAL_RUN_FIRST,
+		              G_STRUCT_OFFSET (NMModemClass, state_changed),
+		              NULL, NULL, NULL,
+		              G_TYPE_NONE, 2, NM_TYPE_MODEM_STATE, NM_TYPE_MODEM_STATE);
+
 	dbus_g_error_domain_register (NM_MODEM_ERROR,
 	                              NM_DBUS_INTERFACE_DEVICE_MODEM,
 	                              NM_TYPE_MODEM_ERROR);
diff --git a/src/devices/wwan/nm-modem.h b/src/devices/wwan/nm-modem.h
index f15be26769..7bb0b8802e 100644
--- a/src/devices/wwan/nm-modem.h
+++ b/src/devices/wwan/nm-modem.h
@@ -36,6 +36,7 @@ G_BEGIN_DECLS
 #define NM_IS_MODEM_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass),  NM_TYPE_MODEM))
 #define NM_MODEM_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj),  NM_TYPE_MODEM, NMModemClass))
 
+/* Properties */
 #define NM_MODEM_UID          "uid"
 #define NM_MODEM_PATH         "path"
 #define NM_MODEM_DRIVER       "driver"
@@ -43,9 +44,9 @@ G_BEGIN_DECLS
 #define NM_MODEM_DATA_PORT    "data-port"
 #define NM_MODEM_IP_METHOD    "ip-method"
 #define NM_MODEM_IP_TIMEOUT   "ip-timeout"
-#define NM_MODEM_ENABLED      "enabled"
-#define NM_MODEM_CONNECTED    "connected"
+#define NM_MODEM_STATE        "state"
 
+/* Signals */
 #define NM_MODEM_PPP_STATS         "ppp-stats"
 #define NM_MODEM_PPP_FAILED        "ppp-failed"
 #define NM_MODEM_PREPARE_RESULT    "prepare-result"
@@ -53,6 +54,7 @@ G_BEGIN_DECLS
 #define NM_MODEM_AUTH_REQUESTED    "auth-requested"
 #define NM_MODEM_AUTH_RESULT       "auth-result"
 #define NM_MODEM_REMOVED           "removed"
+#define NM_MODEM_STATE_CHANGED     "state-changed"
 
 #define MM_MODEM_IP_METHOD_PPP    0
 #define MM_MODEM_IP_METHOD_STATIC 1
@@ -66,6 +68,22 @@ typedef enum {
 	NM_MODEM_ERROR_INITIALIZATION_FAILED,   /*< nick=InitializationFailed >*/
 } NMModemError;
 
+typedef enum {  /*< underscore_name=nm_modem_state >*/
+	NM_MODEM_STATE_UNKNOWN       = 0,
+	NM_MODEM_STATE_FAILED        = 1,
+	NM_MODEM_STATE_INITIALIZING  = 2,
+	NM_MODEM_STATE_LOCKED        = 3,
+	NM_MODEM_STATE_DISABLED      = 4,
+	NM_MODEM_STATE_DISABLING     = 5,
+	NM_MODEM_STATE_ENABLING      = 6,
+	NM_MODEM_STATE_ENABLED       = 7,
+	NM_MODEM_STATE_SEARCHING     = 8,
+	NM_MODEM_STATE_REGISTERED    = 9,
+	NM_MODEM_STATE_DISCONNECTING = 10,
+	NM_MODEM_STATE_CONNECTING    = 11,
+	NM_MODEM_STATE_CONNECTED     = 12,
+} NMModemState;
+
 #define NM_MODEM_ERROR (nm_modem_error_quark ())
 GQuark nm_modem_error_quark (void);
 
@@ -121,6 +139,10 @@ typedef struct {
 	void (*auth_requested)    (NMModem *self);
 	void (*auth_result)       (NMModem *self, GError *error);
 
+	void (*state_changed)     (NMModem *self,
+	                           NMModemState new_state,
+	                           NMModemState old_state);
+
 	void (*removed)           (NMModem *self);
 } NMModemClass;
 
@@ -179,11 +201,14 @@ void nm_modem_device_state_changed (NMModem *modem,
                                     NMDeviceState old_state,
                                     NMDeviceStateReason reason);
 
-gboolean      nm_modem_get_mm_enabled (NMModem *self);
-
 void          nm_modem_set_mm_enabled (NMModem *self, gboolean enabled);
 
-gboolean      nm_modem_get_mm_connected (NMModem *self);
+NMModemState  nm_modem_get_state (NMModem *self);
+void          nm_modem_set_state (NMModem *self,
+                                  NMModemState new_state,
+                                  const char *reason);
+void          nm_modem_set_prev_state (NMModem *self, const char *reason);
+const char *  nm_modem_state_to_string (NMModemState state);
 
 /* For the modem-manager only */
 void          nm_modem_emit_removed (NMModem *self);