[MM] [PATCH] sim: retry SIM operations during initialization

[Ben Chan <benchan chromium org>]

This patch modifies the MMSim interface to validate the IMSI value
reported by AT+CIMI and retry the command upon an error. It also
modifies other SIM operations during the interface initialization to
retry upon an error.
---
 src/mm-sim.c |  124 ++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/src/mm-sim.c b/src/mm-sim.c
index 9cd31cc..eb0bb00 100644
--- a/src/mm-sim.c
+++ b/src/mm-sim.c
@@ -1066,15 +1066,48 @@ load_sim_identifier (MMSim *self,
 /* IMSI */
 
 static gchar *
+parse_imsi (const gchar *response,
+            GError **error)
+{
+    const gchar *s;
+    int len;
+    gboolean success;
+
+    if (response) {
+        success = TRUE;
+        for (s = response, len = 0; *s; ++s, ++len) {
+            /* IMSI is a number with 15 or less decimal digits. */
+            if (!isdigit (*s) || len > 15) {
+                success = FALSE;
+                break;
+            }
+        }
+        if (success)
+            return g_strdup (response);
+    }
+
+    g_set_error (error,
+                 MM_CORE_ERROR,
+                 MM_CORE_ERROR_FAILED,
+                 "Invalid +CIMI response '%s'", response ? response : "<null>");
+    return NULL;
+}
+
+static gchar *
 load_imsi_finish (MMSim *self,
                   GAsyncResult *res,
                   GError **error)
 {
+    const gchar *result;
     gchar *imsi;
 
     if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))
         return NULL;
-    imsi = g_strdup (g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res)));
+    result = g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res));
+
+    imsi = parse_imsi (result, error);
+    if (!imsi)
+        return NULL;
 
     mm_dbg ("loaded IMSI: %s", imsi);
     return imsi;
@@ -1093,7 +1126,7 @@ load_imsi (MMSim *self,
         MM_BASE_MODEM (self->priv->modem),
         "+CIMI",
         3,
-        TRUE,
+        FALSE,
         (GAsyncReadyCallback)load_imsi_command_ready,
         g_simple_async_result_new (G_OBJECT (self),
                                    callback,
@@ -1368,7 +1401,7 @@ struct _InitAsyncContext {
     GCancellable *cancellable;
     MMSim *self;
     InitializationStep step;
-    guint sim_identifier_tries;
+    guint step_tries;
 };
 
 static void
@@ -1381,6 +1414,21 @@ init_async_context_free (InitAsyncContext *ctx)
     g_free (ctx);
 }
 
+static void
+init_async_context_next_step (InitAsyncContext *ctx)
+{
+    ctx->step++;
+    ctx->step_tries = 0;
+}
+
+static gboolean
+interface_initialization_step_again (InitAsyncContext *ctx)
+{
+    interface_initialization_step (ctx);
+
+    return FALSE;
+}
+
 MMSim *
 mm_sim_new_finish (GAsyncResult  *res,
                    GError       **error)
@@ -1409,41 +1457,6 @@ initable_init_finish (GAsyncInitable  *initable,
     return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (result), error);
 }
 
-static void
-load_sim_identifier_ready (MMSim *self,
-                           GAsyncResult *res,
-                           InitAsyncContext *ctx)
-{
-    GError *error = NULL;
-    gchar *simid;
-
-    simid  = MM_SIM_GET_CLASS (ctx->self)->load_sim_identifier_finish (self, res, &error);
-    if (!simid) {
-        /* TODO: make the retries gobi-specific? */
-
-        /* Try one more time... Gobi 1K cards may reply to the first
-         * request with '+CRSM: 106,134,""' which is bogus because
-         * subsequent requests work fine.
-         */
-        if (++ctx->sim_identifier_tries < 2) {
-            g_clear_error (&error);
-            interface_initialization_step (ctx);
-            return;
-        }
-
-        mm_warn ("couldn't load SIM identifier: '%s'",
-                 error ? error->message : "unknown error");
-        g_clear_error (&error);
-    }
-
-    mm_gdbus_sim_set_sim_identifier (MM_GDBUS_SIM (self), simid);
-    g_free (simid);
-
-    /* Go on to next step */
-    ctx->step++;
-    interface_initialization_step (ctx);
-}
-
 #undef STR_REPLY_READY_FN
 #define STR_REPLY_READY_FN(NAME,DISPLAY)                                \
     static void                                                         \
@@ -1455,19 +1468,30 @@ load_sim_identifier_ready (MMSim *self,
         gchar *val;                                                     \
                                                                         \
         val = MM_SIM_GET_CLASS (ctx->self)->load_##NAME##_finish (self, res, &error); \
-        mm_gdbus_sim_set_##NAME (MM_GDBUS_SIM (self), val);             \
-        g_free (val);                                                   \
+        if (!val) {                                                     \
+            if (++ctx->step_tries < 3) {                                \
+                g_clear_error (&error);                                 \
+                g_timeout_add_seconds (1,                               \
+                                       (GSourceFunc)interface_initialization_step_again, \
+                                       ctx);                            \
+                return;                                                 \
+            }                                                           \
                                                                         \
-        if (error) {                                                    \
-            mm_warn ("couldn't load %s: '%s'", DISPLAY, error->message); \
-            g_error_free (error);                                       \
+            mm_warn ("couldn't load %s: '%s'",                          \
+                     DISPLAY,                                           \
+                     error ? error->message : "unknown error");         \
+            g_clear_error (&error);                                     \
         }                                                               \
                                                                         \
+        mm_gdbus_sim_set_##NAME (MM_GDBUS_SIM (self), val);             \
+        g_free (val);                                                   \
+                                                                        \
         /* Go on to next step */                                        \
-        ctx->step++;                                                    \
+        init_async_context_next_step (ctx);                             \
         interface_initialization_step (ctx);                            \
     }
 
+STR_REPLY_READY_FN (sim_identifier, "SIM identifier")
 STR_REPLY_READY_FN (imsi, "IMSI")
 STR_REPLY_READY_FN (operator_identifier, "Operator identifier")
 STR_REPLY_READY_FN (operator_name, "Operator name")
@@ -1478,7 +1502,7 @@ interface_initialization_step (InitAsyncContext *ctx)
     switch (ctx->step) {
     case INITIALIZATION_STEP_FIRST:
         /* Fall down to next step */
-        ctx->step++;
+        init_async_context_next_step (ctx);
 
     case INITIALIZATION_STEP_SIM_IDENTIFIER:
         /* SIM ID is meant to be loaded only once during the whole
@@ -1494,7 +1518,7 @@ interface_initialization_step (InitAsyncContext *ctx)
             return;
         }
         /* Fall down to next step */
-        ctx->step++;
+        init_async_context_next_step (ctx);
 
     case INITIALIZATION_STEP_IMSI:
         /* IMSI is meant to be loaded only once during the whole
@@ -1510,7 +1534,7 @@ interface_initialization_step (InitAsyncContext *ctx)
             return;
         }
         /* Fall down to next step */
-        ctx->step++;
+        init_async_context_next_step (ctx);
 
     case INITIALIZATION_STEP_OPERATOR_ID:
         /* Operator ID is meant to be loaded only once during the whole
@@ -1526,7 +1550,7 @@ interface_initialization_step (InitAsyncContext *ctx)
             return;
         }
         /* Fall down to next step */
-        ctx->step++;
+        init_async_context_next_step (ctx);
 
     case INITIALIZATION_STEP_OPERATOR_NAME:
         /* Operator Name is meant to be loaded only once during the whole
@@ -1542,7 +1566,7 @@ interface_initialization_step (InitAsyncContext *ctx)
             return;
         }
         /* Fall down to next step */
-        ctx->step++;
+        init_async_context_next_step (ctx);
 
     case INITIALIZATION_STEP_LAST:
         /* We are done without errors! */
@@ -1575,7 +1599,7 @@ common_init_async (GAsyncInitable *initable,
                         g_object_ref (cancellable) :
                         NULL);
     ctx->step = INITIALIZATION_STEP_FIRST;
-    ctx->sim_identifier_tries = 0;
+    ctx->step_tries = 0;
 
     interface_initialization_step (ctx);
 }
-- 
1.7.7.3