option,hso: don't reset connection if cancelled, wait to get connected before

If we are requested to cancel the connection, we first need to wait for the
connection attempt to finish before issuing the disconnect command, as otherwise
the modem just returns an error saying that it cannot perform the operation and
at the end we end up with the modem connected but ModemManager thinking that it
isn't.

1 files changed, 101 insertions, 51 deletions

diff --git a/plugins/option/mm-broadband-bearer-hso.c b/plugins/option/mm-broadband-bearer-hso.c
index 8a12f645..0ea36b6f 100644
--- a/plugins/option/mm-broadband-bearer-hso.c
+++ b/plugins/option/mm-broadband-bearer-hso.c
@@ -241,6 +241,7 @@ typedef struct {
     GCancellable *cancellable;
     GSimpleAsyncResult *result;
     guint auth_idx;
+    GError *saved_error;
 } Dial3gppContext;
 
 static Dial3gppContext *
@@ -319,6 +320,37 @@ dial_3gpp_finish (MMBroadbandBearer *self,
     return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error);
 }
 
+static void
+connect_reset_ready (MMBaseModem *modem,
+                     GAsyncResult *res,
+                     Dial3gppContext *ctx)
+{
+    mm_base_modem_at_command_full_finish (modem, res, NULL);
+
+    /* error should have already been set in the simple async result */
+    dial_3gpp_context_complete_and_free (ctx);
+}
+
+static void
+connect_reset (Dial3gppContext *ctx)
+{
+    gchar *command;
+
+    /* Need to reset the connection attempt */
+    command = g_strdup_printf ("AT_OWANCALL=%d,0,1",
+                               ctx->cid);
+    mm_base_modem_at_command_full (ctx->modem,
+                                   ctx->primary,
+                                   command,
+                                   3,
+                                   FALSE,
+                                   FALSE, /* raw */
+                                   NULL, /* cancellable */
+                                   (GAsyncReadyCallback)connect_reset_ready,
+                                   ctx);
+    g_free (command);
+}
+
 void
 mm_broadband_bearer_hso_report_connection_status (MMBroadbandBearerHso *self,
                                                   MMBroadbandBearerHsoConnectionStatus status)
@@ -342,13 +374,26 @@ mm_broadband_bearer_hso_report_connection_status (MMBroadbandBearerHso *self,
 
     switch (status) {
     case MM_BROADBAND_BEARER_HSO_CONNECTION_STATUS_UNKNOWN:
-        g_warn_if_reached ();
         break;
 
     case MM_BROADBAND_BEARER_HSO_CONNECTION_STATUS_CONNECTED:
         if (!ctx)
+            /* We may get this if the timeout for the connection attempt is
+             * reached before the unsolicited response. We should probably
+             * keep the CID around to request explicit disconnection in this
+             * case. */
             break;
 
+        /* If we wanted to get cancelled before, do it now */
+        if (ctx->saved_error) {
+            /* Keep error */
+            g_simple_async_result_take_error (ctx->result, ctx->saved_error);
+            ctx->saved_error = NULL;
+            /* Cancel connection */
+            connect_reset (ctx);
+            return;
+        }
+
         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
         dial_3gpp_context_complete_and_free (ctx);
         return;
@@ -357,6 +402,15 @@ mm_broadband_bearer_hso_report_connection_status (MMBroadbandBearerHso *self,
         if (!ctx)
             break;
 
+        /* If we wanted to get cancelled before and now we couldn't connect,
+         * use the cancelled error and return */
+        if (ctx->saved_error) {
+            g_simple_async_result_take_error (ctx->result, ctx->saved_error);
+            ctx->saved_error = NULL;
+            dial_3gpp_context_complete_and_free (ctx);
+            return;
+        }
+
         g_simple_async_result_set_error (ctx->result,
                                          MM_CORE_ERROR,
                                          MM_CORE_ERROR_FAILED,
@@ -366,48 +420,29 @@ mm_broadband_bearer_hso_report_connection_status (MMBroadbandBearerHso *self,
 
     case MM_BROADBAND_BEARER_HSO_CONNECTION_STATUS_DISCONNECTED:
         if (ctx) {
+            /* If we wanted to get cancelled before and now we couldn't connect,
+             * use the cancelled error and return */
+            if (ctx->saved_error) {
+                g_simple_async_result_take_error (ctx->result, ctx->saved_error);
+                ctx->saved_error = NULL;
+                dial_3gpp_context_complete_and_free (ctx);
+                return;
+            }
+
             g_simple_async_result_set_error (ctx->result,
                                              MM_CORE_ERROR,
                                              MM_CORE_ERROR_FAILED,
                                              "Call setup failed");
             dial_3gpp_context_complete_and_free (ctx);
-        } else {
-            /* Just ensure we mark ourselves as being disconnected... */
-            mm_bearer_report_disconnection (MM_BEARER (self));
+            return;
         }
-        break;
-    }
-}
-
-static void
-connect_reset_ready (MMBaseModem *modem,
-                     GAsyncResult *res,
-                     Dial3gppContext *ctx)
-{
-    mm_base_modem_at_command_full_finish (modem, res, NULL);
-
-    /* error should have already been set in the simple async result */
-    dial_3gpp_context_complete_and_free (ctx);
-}
 
-static void
-connect_reset (Dial3gppContext *ctx)
-{
-    gchar *command;
+        /* Just ensure we mark ourselves as being disconnected... */
+        mm_bearer_report_disconnection (MM_BEARER (self));
+        return;
+    }
 
-    /* Need to reset the connection attempt */
-    command = g_strdup_printf ("AT_OWANCALL=%d,0,1",
-                               ctx->cid);
-    mm_base_modem_at_command_full (ctx->modem,
-                                   ctx->primary,
-                                   command,
-                                   3,
-                                   FALSE,
-                                   FALSE, /* raw */
-                                   NULL, /* cancellable */
-                                   (GAsyncReadyCallback)connect_reset_ready,
-                                   ctx);
-    g_free (command);
+    g_warn_if_reached ();
 }
 
 static gboolean
@@ -415,20 +450,31 @@ connect_timed_out_cb (MMBroadbandBearerHso *self)
 {
     Dial3gppContext *ctx;
 
-    /* Recover context and remove cancellation */
+    /* Recover context and remove it from the private info */
     ctx = self->priv->connect_pending;
+    self->priv->connect_pending = NULL;
 
-    g_cancellable_disconnect (ctx->cancellable,
-                              self->priv->connect_cancellable_id);
+    /* Remove cancellation, if found */
+    if (self->priv->connect_cancellable_id) {
+        g_cancellable_disconnect (ctx->cancellable,
+                                  self->priv->connect_cancellable_id);
+        self->priv->connect_cancellable_id = 0;
+    }
 
-    self->priv->connect_pending = NULL;
+    /* Cleanup timeout ID */
     self->priv->connect_pending_id = 0;
-    self->priv->connect_cancellable_id = 0;
 
-    g_simple_async_result_set_error (ctx->result,
-                                     MM_MOBILE_EQUIPMENT_ERROR,
-                                     MM_MOBILE_EQUIPMENT_ERROR_NETWORK_TIMEOUT,
-                                     "Connection attempt timed out");
+    /* If we were cancelled, prefer that error */
+    if (ctx->saved_error) {
+        g_simple_async_result_take_error (ctx->result, ctx->saved_error);
+        ctx->saved_error = NULL;
+    } else
+        g_simple_async_result_set_error (ctx->result,
+                                         MM_MOBILE_EQUIPMENT_ERROR,
+                                         MM_MOBILE_EQUIPMENT_ERROR_NETWORK_TIMEOUT,
+                                         "Connection attempt timed out");
+
+    /* It's probably pointless to try to reset this here, but anyway... */
     connect_reset (ctx);
 
     return FALSE;
@@ -438,22 +484,26 @@ static void
 connect_cancelled_cb (GCancellable *cancellable,
                       MMBroadbandBearerHso *self)
 {
-    GError *error = NULL;
     Dial3gppContext *ctx;
 
     /* Recover context and remove timeout */
     ctx = self->priv->connect_pending;
 
-    g_source_remove (self->priv->connect_pending_id);
+    /* Recover context but DON'T remove it from the private info */
+    ctx = self->priv->connect_pending;
 
-    self->priv->connect_pending = NULL;
-    self->priv->connect_pending_id = 0;
+    /* Remove the cancellable
+     * NOTE: we shouldn't remove the timeout yet. We still need to wait
+     * to get connected before running the explicit connection reset */
+    g_cancellable_disconnect (ctx->cancellable,
+                              self->priv->connect_cancellable_id);
     self->priv->connect_cancellable_id = 0;
 
-    g_assert (dial_3gpp_context_set_error_if_cancelled (ctx, &error));
+    /* Store cancelled error */
+    g_assert (dial_3gpp_context_set_error_if_cancelled (ctx, &ctx->saved_error));
 
-    g_simple_async_result_take_error (ctx->result, error);
-    connect_reset (ctx);
+    /* We cannot reset right here, we need to wait for the connection
+     * attempt to finish */
 }
 
 static void