[PATCH] modem: map modemmanager errors more detailed

[Thomas Bechtold <thomasbechtold jpberlin de>]

Hi,

attached is a patch to handle some ModemManager errors more detailed.
But i still have some questions about the code and i'm not sure if this
patch is the correct way to solve the problem that NM does not know the
exact reason why a modem connection fails.
Here are my questions:

- should NM have a NMDeviceStateReason for every MM error 
  or should some MM errors be combined to one NMDeviceStateReason?
- in NM/include/NetworkManager.h are some NMDeviceStateReason defined 
  which are never used. eg NM_DEVICE_STATE_REASON_MODEM_NO_DIAL_TONE, 
  NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER, 
  NM_DEVICE_STATE_REASON_MODEM_DIAL_FAILED, 
  NM_DEVICE_STATE_REASON_GSM_APN_FAILED, 
  NM_DEVICE_STATE_REASON_GSM_PIN_CHECK_FAILED
  maybe there are some more unused NMDeviceStateReason. 
  Why are these reason available?


Cheers,

Tom




* add 4 new NMDeviceStateReason to map ModemManager errors more detailed
* fix wrong error mapping for MM_MODEM_CONNECT_ERROR_NO_DIALTONE
---
 include/NetworkManager.h         |   12 +++++++
 src/modem-manager/nm-modem-gsm.c |   65 ++++++++++++++++++++++++++++---------
 src/nm-device.c                  |    8 +++++
 3 files changed, 69 insertions(+), 16 deletions(-)

diff --git a/include/NetworkManager.h b/include/NetworkManager.h
index 4755f30..eb72564 100644
--- a/include/NetworkManager.h
+++ b/include/NetworkManager.h
@@ -463,6 +463,18 @@ typedef enum {
 	/* The Bluetooth connection failed or timed out */
 	NM_DEVICE_STATE_REASON_BT_FAILED = 44,
 
+	/* GSM Modem's SIM Card not inserted */
+	NM_DEVICE_STATE_REASON_GSM_SIM_NOT_INSERTED = 45,
+
+	/* GSM Modem's SIM Pin required */
+	NM_DEVICE_STATE_REASON_GSM_SIM_PIN_REQUIRED = 46,
+
+	/* GSM Modem's SIM Puk required */
+	NM_DEVICE_STATE_REASON_GSM_SIM_PUK_REQUIRED = 47,
+
+	/* GSM Modem's SIM wrong */
+	NM_DEVICE_STATE_REASON_GSM_SIM_WRONG = 48,
+
 	/* Unused */
 	NM_DEVICE_STATE_REASON_LAST = 0xFFFF
 } NMDeviceStateReason;
diff --git a/src/modem-manager/nm-modem-gsm.c b/src/modem-manager/nm-modem-gsm.c
index 9a96ae9..777b2e2 100644
--- a/src/modem-manager/nm-modem-gsm.c
+++ b/src/modem-manager/nm-modem-gsm.c
@@ -140,20 +140,34 @@ translate_mm_error (GError *error)
 
 	if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_NO_CARRIER))
 		reason = NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER;
-	if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_NO_DIALTONE))
-		reason = NM_DEVICE_STATE_REASON_MODEM_DIAL_TIMEOUT;
-	if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_BUSY))
+	else if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_NO_DIALTONE))
+		reason = NM_DEVICE_STATE_REASON_MODEM_NO_DIAL_TONE;
+	else if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_BUSY))
 		reason = NM_DEVICE_STATE_REASON_MODEM_BUSY;
-	if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_NO_ANSWER))
+	else if (dbus_g_error_has_name (error, MM_MODEM_CONNECT_ERROR_NO_ANSWER))
 		reason = NM_DEVICE_STATE_REASON_MODEM_DIAL_TIMEOUT;
-	if (dbus_g_error_has_name (error, MM_MODEM_ERROR_NETWORK_NOT_ALLOWED))
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_NETWORK_NOT_ALLOWED))
 		reason = NM_DEVICE_STATE_REASON_GSM_REGISTRATION_DENIED;
-	if (dbus_g_error_has_name (error, MM_MODEM_ERROR_NETWORK_TIMEOUT))
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_NETWORK_TIMEOUT))
 		reason = NM_DEVICE_STATE_REASON_GSM_REGISTRATION_TIMEOUT;
-	if (dbus_g_error_has_name (error, MM_MODEM_ERROR_NO_NETWORK))
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_NO_NETWORK))
 		reason = NM_DEVICE_STATE_REASON_GSM_REGISTRATION_NOT_SEARCHING;
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_SIM_NOT_INSERTED))
+		reason = NM_DEVICE_STATE_REASON_GSM_SIM_NOT_INSERTED;
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_SIM_PIN))
+		reason = NM_DEVICE_STATE_REASON_GSM_SIM_PIN_REQUIRED;
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_SIM_PUK))
+		reason = NM_DEVICE_STATE_REASON_GSM_SIM_PUK_REQUIRED;
+	else if (dbus_g_error_has_name (error, MM_MODEM_ERROR_SIM_WRONG))
+		reason = NM_DEVICE_STATE_REASON_GSM_SIM_WRONG;
+	else
+	{
+		/* unable to map the ModemManager error to a NM_DEVICE_STATE_REASON */
+		nm_log_dbg (LOGD_MB, "unmapped dbus error detected: '%s'", dbus_g_error_get_name (error));
+		reason = NM_DEVICE_STATE_REASON_UNKNOWN;
+	}
 
-	/* FIXME: We have only GSM error messages here, and we have no idea which 
+	/* FIXME: We have only GSM error messages here, and we have no idea which
 	   activation state failed. Reasons like:
 	   NM_DEVICE_STATE_REASON_MODEM_DIAL_FAILED,
 	   NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED,
@@ -162,9 +176,6 @@ translate_mm_error (GError *error)
 	   NM_DEVICE_STATE_REASON_GSM_PIN_CHECK_FAILED
 	   are not used.
 	*/
-	else
-		reason = NM_DEVICE_STATE_REASON_UNKNOWN;
-
 	return reason;
 }
 
@@ -257,6 +268,7 @@ static void
 stage1_pin_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data)
 {
 	NMModemGsm *self = NM_MODEM_GSM (user_data);
+	NMDeviceStateReason reason;
 	GError *error = NULL;
 
 	if (dbus_g_proxy_end_call (proxy, call_id, &error, G_TYPE_INVALID)) {
@@ -266,9 +278,19 @@ stage1_pin_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data)
 		nm_log_warn (LOGD_MB, "GSM PIN unlock failed: (%d) %s",
 		             error ? error->code : -1,
 		             error && error->message ? error->message : "(unknown)");
-		g_error_free (error);
 
-		g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED);
+		/* try to translate the error reason */
+		reason = translate_mm_error (error);
+
+		if (reason == NM_DEVICE_STATE_REASON_UNKNOWN) {
+			/* use 'generic' reason NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED */
+			g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED);
+		} else {
+			/* use translated reason */
+			g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, reason);
+		}
+
+		g_error_free (error);
 	}
 }
 
@@ -301,6 +323,7 @@ static void
 stage1_enable_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_data)
 {
 	NMModemGsm *self = NM_MODEM_GSM (user_data);
+	NMDeviceStateReason reason;
 	GError *error = NULL;
 
 	if (dbus_g_proxy_end_call (proxy, call_id, &error, G_TYPE_INVALID))
@@ -310,10 +333,20 @@ stage1_enable_done (DBusGProxy *proxy, DBusGProxyCall *call_id, gpointer user_da
 		             error ? error->code : -1,
 		             error && error->message ? error->message : "(unknown)");
 
-		if (dbus_g_error_has_name (error, MM_MODEM_ERROR_SIM_PIN))
+		if (dbus_g_error_has_name (error, MM_MODEM_ERROR_SIM_PIN)) {
 			handle_enable_pin_required (self);
-		else
-			g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED);
+		} else {
+			/* try to translate the error reason */
+			reason = translate_mm_error (error);
+
+			if (reason == NM_DEVICE_STATE_REASON_UNKNOWN) {
+				/* use 'generic' reason NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED */
+				g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, NM_DEVICE_STATE_REASON_MODEM_INIT_FAILED);
+			} else {
+				/* use translated reason */
+				g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, reason);
+			}
+		}
 
 		g_error_free (error);
 	}
diff --git a/src/nm-device.c b/src/nm-device.c
index e285a57..53eece8 100644
--- a/src/nm-device.c
+++ b/src/nm-device.c
@@ -3863,6 +3863,14 @@ reason_to_string (NMDeviceStateReason reason)
 		return "modem-not-found";
 	case NM_DEVICE_STATE_REASON_BT_FAILED:
 		return "bluetooth-failed";
+	case NM_DEVICE_STATE_REASON_GSM_SIM_NOT_INSERTED:
+		return "gsm-sim-not-inserted";
+	case NM_DEVICE_STATE_REASON_GSM_SIM_PIN_REQUIRED:
+		return "gsm-sim-pin-required";
+	case NM_DEVICE_STATE_REASON_GSM_SIM_PUK_REQUIRED:
+		return "gsm-sim-puk-required";
+	case NM_DEVICE_STATE_REASON_GSM_SIM_WRONG:
+		return "gsm-sim-wrong";
 	default:
 		break;
 	}
-- 
1.7.5.4