Simplify handing of activate; return activate errors

plugins/modemmgr.c

Index: plugins/modemmgr.c
diff --git a/plugins/modemmgr.c b/plugins/modemmgr.c
index e544823d37314a4e09d8cec8deb8c0f6afa5a344..35798c75cf10d1c173d0c31a405c4206f1959717 100644
--- a/plugins/modemmgr.c
+++ b/plugins/modemmgr.c
@@ -145,8 +145,6 @@ enum modem_event {
 	ME_REGISTRATION_STATE_CHANGED,
 	ME_ENABLE_SUCCEEDED,
 	ME_ENABLE_FAILED,
-	ME_ACTIVATE_SUCCEEDED,
-	ME_ACTIVATE_FAILED,
 	ME_REGISTER_SUCCEEDED,
 	ME_REGISTER_FAILED,
 	ME_DISABLE_SUCCEEDED,
@@ -327,7 +325,6 @@ static int modem_task_run_new(struct modem_data *modem,
 
 static void modem_enable_task(struct modem_data *);
 static void modem_get_identifiers_task(struct modem_data *);
-static void modem_activate_task(struct modem_data *);
 static void modem_disable_task(struct modem_data *);
 static void modem_get_status_task(struct modem_data *);
 static void modem_get_info_task(struct modem_data *);
@@ -354,12 +351,6 @@ static struct modem_task_work enable_cdma_modem_work = {
 		modem_get_registration_state_task,
 		0}};
 
-static struct modem_task_work activate_modem_work = {
-	"activate", {
-		modem_activate_task,
-		modem_get_registration_state_task,
-		0}};
-
 static struct modem_task_work enable_gsm_modem_work = {
 	"enable_gsm", {
 		modem_enable_task,
@@ -1120,7 +1111,6 @@ static void modem_update_activation_state(struct modem_data *modem,
 	}
 }
 
-
 /**
  * modem_activate_reply:
  * @modem: modem object
@@ -1134,54 +1124,59 @@ static void modem_update_activation_state(struct modem_data *modem,
 static void modem_activate_reply(DBusPendingCall *call, void *user_data)
 {
 	struct modem_data *modem = (struct modem_data *)user_data;
+	uint32_t status = -1;
+	DBusMessage *reply;
+	DBusError error;
 
-	_DBG_MODEMMGR("activate_reply modem %s", modem->dbus_path);
-	modem_dbus_handle_reply(__func__, call, modem,
-				ME_ACTIVATE_SUCCEEDED, ME_ACTIVATE_FAILED,
- 				MM_MODEM_ACTIVATED_ERROR,
-				DBUS_TYPE_INVALID /* no return values */);
-}
-
-
-/**
- * modem_activate_task:
- * @modem: modem object
- * @task: modem task object
- *
- * Send a DBUS message to activate a modem.  As a task, all failures
- * must be reported via modem_handle_event.
- */
-static void modem_activate_task(struct modem_data *modem)
-{
-	const char *carrier = "";	/* Use modem's default carrier */
-	DBusMessage *message;
-
-	_DBG_MODEMMGR("activate modem %s", modem->dbus_path);
+	dbus_error_init(&error);
 
-	if (modem_dbus_build_message(modem->owner, MM_MODEM_CDMA_INTERFACE,
-				     modem->dbus_path, &message,
-				     MM_MODEM_CDMA_METHOD_ACTIVATE) < 0) {
-		_DBG_MODEMMGR("activate modem %s", modem->dbus_path);
-		modem_handle_event(modem, ME_ACTIVATE_FAILED);
-		return;
+	reply = dbus_pending_call_steal_reply(call);
+	if (reply == NULL) {
+		CONNMAN_ERROR("%s: Failed steal reply", __func__);
+		goto done_no_reply;
 	}
 
-	dbus_message_append_args(message,
-				 DBUS_TYPE_STRING, &carrier,
-				 DBUS_TYPE_INVALID);
-
-	modem_update_activation_state(modem,
-		CONNMAN_NETWORK_ACTIVATION_STATE_ACTIVATING,
-		CONNMAN_ELEMENT_ERROR_UNKNOWN);
+	if (dbus_set_error_from_message(&error, reply)) {
+		if (g_strcmp0(error.name, MM_MODEM_INITIATED_ERROR) == 0) {
+			/*
+			 * Activation can take a long while.  We rely
+			 * on a signal to indicate a future state
+			 * change of the modem, or for the modem to
+			 * disappear.  Nothing to do.
+			 */
+			dbus_message_unref(reply);
+			return;
+		}
+		CONNMAN_ERROR("DBus Error: %s:%s",
+			      error.name, error.message);
+		goto done;
+	}
 
-	if (modem_dbus_send_with_reply(message,
-				       modem,
-				       modem_activate_reply,
-				       ACTIVATE_TIMEOUT_MS) < 0) {
-		_DBG_MODEMMGR("activate modem %s", modem->dbus_path);
-		modem_handle_event(modem, ME_ACTIVATE_FAILED);
-		return;
+	if (dbus_message_get_args(reply, &error,
+				  DBUS_TYPE_UINT32,
+				  &status,
+				  DBUS_TYPE_INVALID) == FALSE) {
+		if (dbus_error_is_set(&error) == TRUE) {
+			CONNMAN_ERROR("Error retrieving reply argument: %s:%s",
+				      error.name, error.message);
+		} else {
+			CONNMAN_ERROR("Reply argument type is not UINT32");
+		}
+	}
+done:
+	dbus_message_unref(reply);
+done_no_reply:
+	if (status != 0) {
+		modem_update_activation_state(modem,
+		     /* FIXME(jglasgow): need to leave state unchanged */
+		     CONNMAN_NETWORK_ACTIVATION_STATE_NOT_ACTIVATED,
+		     convert_activation_error(status));
 	}
+	/*
+	 * For the case of status == 0, we rely either on an explicit
+	 * signal of the new modem state, or we expect the modem to
+	 * disappear, reset itself and come back
+	 */
 }
 
 
@@ -2453,16 +2448,6 @@ static void modem_handle_event(struct modem_data *modem,
 			modem_set_powered(modem, TRUE);
 			modem_task_completed(&modem);
 			return;
-		case ME_ACTIVATE_SUCCEEDED:
-			/* dbus signal will update activation state later */
-			modem_task_run(modem);
-			return;
-		case ME_ACTIVATE_FAILED:
-			modem_update_activation_state(modem,
-				CONNMAN_NETWORK_ACTIVATION_STATE_NOT_ACTIVATED,
-				CONNMAN_ELEMENT_ERROR_ACTIVATION_FAILED);
-			modem_task_run(modem);
-			return;
 		case ME_REGISTER_SUCCEEDED:
 			/*
 			 * Do not change state yet,
@@ -2559,16 +2544,6 @@ static void modem_handle_event(struct modem_data *modem,
 		case ME_GET_REGISTRATION_STATE_FAILED:
 			modem_task_completed(&modem);
 			return;
-		case ME_ACTIVATE_SUCCEEDED:
-			/* dbus signal will update activation state later */
-			modem_task_run(modem);
-			return;
-		case ME_ACTIVATE_FAILED:
-			modem_update_activation_state(modem,
-				CONNMAN_NETWORK_ACTIVATION_STATE_NOT_ACTIVATED,
-				CONNMAN_ELEMENT_ERROR_ACTIVATION_FAILED);
-			modem_task_run(modem);
-			return;
 		case ME_GET_SIGNALQUALITY_FAILED:
 			/* N.B. We can continue even without signal quality */
 			return;
@@ -2653,8 +2628,6 @@ static const char *eventToString(enum modem_event event)
 	case ME_REGISTRATION_STATE_CHANGED: return "REGISTRATION_STATE_CHANGED";
 	case ME_ENABLE_SUCCEEDED: return "ENABLE_SUCCEEDED";
 	case ME_ENABLE_FAILED: return "ENABLE_FAILED";
-	case ME_ACTIVATE_SUCCEEDED: return "ACTIVATE_SUCCEEDED";
-	case ME_ACTIVATE_FAILED: return "ACTIVATE_FAILED";
 	case ME_REGISTER_SUCCEEDED: return "REGISTER_SUCCEEDED";
 	case ME_REGISTER_FAILED: return "REGISTER_FAILED";
 	case ME_DISABLE_SUCCEEDED: return "DISABLE_SUCCEEDED";
@@ -2943,12 +2916,12 @@ static void network_remove(struct connman_network *network)
 /**
  * Activate the modem on the network.
  *
- * We may need to bring in some upstream changes.
  */
 static int modem_network_activate(struct connman_network *network,
 	const char *carrier)
 {
 	struct modem_data *modem;
+	DBusMessage *message;
 
 	modem = network_get_modem(network, __func__);
 	if (modem == NULL) {
@@ -2967,7 +2940,31 @@ static int modem_network_activate(struct connman_network *network,
 			      stateToString(modem->state));
 		return -EIO;
 	}
-	return modem_task_run_new(modem, &activate_modem_work);
+
+	if (modem_dbus_build_message(modem->owner, MM_MODEM_CDMA_INTERFACE,
+				     modem->dbus_path, &message,
+				     MM_MODEM_CDMA_METHOD_ACTIVATE) < 0) {
+		_DBG_MODEMMGR("activate modem %s", modem->dbus_path);
+		return -EIO;
+	}
+
+	_DBG_MODEMMGR("activate modem %s", modem->dbus_path);
+	dbus_message_append_args(message,
+				 DBUS_TYPE_STRING, &carrier,
+				 DBUS_TYPE_INVALID);
+
+	if (modem_dbus_send_with_reply(message,
+				       modem,
+				       modem_activate_reply,
+				       ACTIVATE_TIMEOUT_MS) < 0) {
+		_DBG_MODEMMGR("activate modem %s", modem->dbus_path);
+		return -EIO;
+	}
+	modem_update_activation_state(modem,
+		CONNMAN_NETWORK_ACTIVATION_STATE_ACTIVATING,
+		CONNMAN_ELEMENT_ERROR_UNKNOWN);
+
+	return 0;
 }
 
 static struct connman_network_driver network_driver = {
@@ -3264,17 +3261,6 @@ static void modem_activation_state_changed(struct modem_data *modem,
 	modem_update_activation_state(modem,
 				      convert_activation_state(mm_state),
 				      convert_activation_error(mm_error));
-	/*
-	 * If the activate modem task is running, then the receipt of
-	 * the modem activation state changed signal means that we are
-	 * done with this step of the task.  Run the next step.
-	 *
-	 * In the case of a non flimflam process communicating
-	 * directly with the modem it is possible that no modemmgr
-	 * task is running when the signal is received.
-	 */
-	if (modem->pending_task && modem->task.work == &activate_modem_work)
-		modem_task_run(modem);
 }
 
 static void mm_properties_changed(struct modem_data *modem,