iface-modem: don't assume that setting bands is immediate

The modem may take the request to set bands and reply right away with
a success before actually changing the current bands in use.

Therefore, If we reload bands right after we receive the set response,
we may still see the old band mask instead of the new one. Try to
avoid this, by reloading current bands a couple of times after we have
set them if the band mask is not the expected one.

1 files changed, 101 insertions, 32 deletions

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 17b3c295..6c62f109 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -2257,9 +2257,14 @@ handle_set_current_capabilities (MmGdbusModem *skeleton,
 /*****************************************************************************/
 /* Current bands setting */
 
+#define AFTER_SET_LOAD_CURRENT_BANDS_RETRIES      5
+#define AFTER_SET_LOAD_CURRENT_BANDS_TIMEOUT_SECS 1
+
 typedef struct {
     MmGdbusModem *skeleton;
-    GArray *bands_array;
+    GArray       *bands_array;
+    GArray       *supported_bands_array; /* when ANY requested */
+    guint         retries;
 } SetCurrentBandsContext;
 
 static void
@@ -2269,6 +2274,8 @@ set_current_bands_context_free (SetCurrentBandsContext *ctx)
         g_object_unref (ctx->skeleton);
     if (ctx->bands_array)
         g_array_unref (ctx->bands_array);
+    if (ctx->supported_bands_array)
+        g_array_unref (ctx->supported_bands_array);
     g_slice_free (SetCurrentBandsContext, ctx);
 }
 
@@ -2289,48 +2296,116 @@ set_current_bands_complete_with_defaults (GTask *task)
 
     /* Never show just 'any' in the interface */
     if (ctx->bands_array->len == 1 && g_array_index (ctx->bands_array, MMModemBand, 0) == MM_MODEM_BAND_ANY) {
-        GArray *supported_bands;
-
-        supported_bands = (mm_common_bands_variant_to_garray (mm_gdbus_modem_get_supported_bands (ctx->skeleton)));
-        mm_common_bands_garray_sort (supported_bands);
-        mm_gdbus_modem_set_current_bands (ctx->skeleton, mm_common_bands_garray_to_variant (supported_bands));
-        g_array_unref (supported_bands);
-    } else {
-        mm_common_bands_garray_sort (ctx->bands_array);
-        mm_gdbus_modem_set_current_bands (ctx->skeleton, mm_common_bands_garray_to_variant (ctx->bands_array));
+        g_assert (ctx->supported_bands_array);
+        g_array_unref (ctx->bands_array);
+        ctx->bands_array = g_array_ref (ctx->supported_bands_array);
     }
 
+    mm_common_bands_garray_sort (ctx->bands_array);
+    mm_gdbus_modem_set_current_bands (ctx->skeleton, mm_common_bands_garray_to_variant (ctx->bands_array));
+
     g_task_return_boolean (task, TRUE);
     g_object_unref (task);
 }
 
+static void set_current_bands_reload_schedule (GTask *task);
+
 static void
 after_set_load_current_bands_ready (MMIfaceModem *self,
                                     GAsyncResult *res,
                                     GTask        *task)
 {
-    GError                 *error = NULL;
-    GArray                 *current_bands;
     SetCurrentBandsContext *ctx;
+    GArray                 *current_bands;
+    GError                 *error = NULL;
+    GArray                 *requested_bands = NULL;
 
     ctx = g_task_get_task_data (task);
 
     current_bands = MM_IFACE_MODEM_GET_INTERFACE (self)->load_current_bands_finish (self, res, &error);
     if (!current_bands) {
-        /* Errors when getting bands won't be critical */
+        /* If we can retry, do it */
+        if (ctx->retries > 0) {
+            mm_dbg ("couldn't load current bands: '%s' (will retry)", error->message);
+            g_clear_error (&error);
+            set_current_bands_reload_schedule (task);
+            goto out;
+        }
+
+        /* Errors when reloading bands won't be critical */
         mm_warn ("couldn't load current bands: '%s'", error->message);
-        g_error_free (error);
-        /* Default to the ones we requested */
+        g_clear_error (&error);
         set_current_bands_complete_with_defaults (task);
-        return;
+        goto out;
+    }
+
+    if ((ctx->bands_array->len == 1) && (g_array_index (ctx->bands_array, MMModemBand, 0) == MM_MODEM_BAND_ANY))
+        requested_bands = g_array_ref (ctx->supported_bands_array);
+    else
+        requested_bands = g_array_ref (ctx->bands_array);
+
+    /* Compare arrays */
+    if (!mm_common_bands_garray_cmp (current_bands, requested_bands)) {
+        gchar *requested_str;
+        gchar *current_str;
+
+        /* If we can retry, do it */
+        if (ctx->retries > 0) {
+            mm_dbg ("reloaded current bands different to the requested ones (will retry)");
+            set_current_bands_reload_schedule (task);
+            goto out;
+        }
+
+        requested_str = mm_common_build_bands_string ((MMModemBand *)requested_bands->data, requested_bands->len);
+        current_str   = mm_common_build_bands_string ((MMModemBand *)current_bands->data, current_bands->len);
+        error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                             "reloaded current bands (%s) different to the requested ones (%s)",
+                             current_str, requested_str);
+        g_free (requested_str);
+        g_free (current_str);
     }
 
+    /* Store as current the last loaded ones and set operation result */
     mm_common_bands_garray_sort (current_bands);
     mm_gdbus_modem_set_current_bands (ctx->skeleton, mm_common_bands_garray_to_variant (current_bands));
-    g_array_unref (current_bands);
 
-    g_task_return_boolean (task, TRUE);
+    if (error)
+        g_task_return_error (task, error);
+    else
+        g_task_return_boolean (task, TRUE);
     g_object_unref (task);
+
+ out:
+    g_array_unref (requested_bands);
+    g_array_unref (current_bands);
+}
+
+static gboolean
+set_current_bands_reload (GTask *task)
+{
+    MMIfaceModem           *self;
+    SetCurrentBandsContext *ctx;
+
+    self = g_task_get_source_object (task);
+    ctx  = g_task_get_task_data (task);
+
+    g_assert (ctx->retries > 0);
+    ctx->retries--;
+
+    MM_IFACE_MODEM_GET_INTERFACE (self)->load_current_bands (
+        self,
+        (GAsyncReadyCallback)after_set_load_current_bands_ready,
+        task);
+
+    return G_SOURCE_REMOVE;
+}
+
+static void
+set_current_bands_reload_schedule (GTask *task)
+{
+    g_timeout_add_seconds (AFTER_SET_LOAD_CURRENT_BANDS_TIMEOUT_SECS,
+                           (GSourceFunc) set_current_bands_reload,
+                           task);
 }
 
 static void
@@ -2348,10 +2423,7 @@ set_current_bands_ready (MMIfaceModem *self,
 
     if (MM_IFACE_MODEM_GET_INTERFACE (self)->load_current_bands &&
         MM_IFACE_MODEM_GET_INTERFACE (self)->load_current_bands_finish) {
-        MM_IFACE_MODEM_GET_INTERFACE (self)->load_current_bands (
-            self,
-            (GAsyncReadyCallback)after_set_load_current_bands_ready,
-            task);
+        set_current_bands_reload (task);
         return;
     }
 
@@ -2424,7 +2496,6 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
                                   gpointer user_data)
 {
     SetCurrentBandsContext *ctx;
-    GArray *supported_bands_array;
     GArray *current_bands_array;
     GError *error = NULL;
     gchar *bands_string;
@@ -2445,6 +2516,7 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
 
     /* Setup context */
     ctx = g_slice_new0 (SetCurrentBandsContext);
+    ctx->retries = AFTER_SET_LOAD_CURRENT_BANDS_RETRIES;
 
     task = g_task_new (self, NULL, callback, user_data);
     g_task_set_task_data (task, ctx, (GDestroyNotify)set_current_bands_context_free);
@@ -2465,8 +2537,8 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
                                                  bands_array->len);
 
     /* Get list of supported bands */
-    supported_bands_array = (mm_common_bands_variant_to_garray (
-                                 mm_gdbus_modem_get_supported_bands (ctx->skeleton)));
+    ctx->supported_bands_array = (mm_common_bands_variant_to_garray (
+                                      mm_gdbus_modem_get_supported_bands (ctx->skeleton)));
 
     /* Set ctx->bands_array to target list of bands before comparing with current list
      * of bands. If input list of bands contains only ANY, target list of bands is set
@@ -2475,8 +2547,8 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
         g_array_index (bands_array, MMModemBand, 0) == MM_MODEM_BAND_ANY) {
         guint i;
 
-        for (i = 0; i < supported_bands_array->len; i++) {
-            MMModemBand band = g_array_index (supported_bands_array, MMModemBand, i);
+        for (i = 0; i < ctx->supported_bands_array->len; i++) {
+            MMModemBand band = g_array_index (ctx->supported_bands_array, MMModemBand, i);
 
             if (band != MM_MODEM_BAND_ANY &&
                 band != MM_MODEM_BAND_UNKNOWN) {
@@ -2484,7 +2556,7 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
                     ctx->bands_array = g_array_sized_new (FALSE,
                                                           FALSE,
                                                           sizeof (MMModemBand),
-                                                          supported_bands_array->len);
+                                                          ctx->supported_bands_array->len);
                 g_array_append_val (ctx->bands_array, band);
             }
         }
@@ -2500,7 +2572,6 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
         mm_dbg ("Requested list of bands (%s) is equal to the current ones, skipping re-set",
                 bands_string);
         g_free (bands_string);
-        g_array_unref (supported_bands_array);
         g_array_unref (current_bands_array);
         g_task_return_boolean (task, TRUE);
         g_object_unref (task);
@@ -2515,13 +2586,12 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
     }
 
     /* Validate input list of bands */
-    if (!validate_bands (supported_bands_array,
+    if (!validate_bands (ctx->supported_bands_array,
                          ctx->bands_array,
                          &error)) {
         mm_dbg ("Requested list of bands (%s) cannot be handled",
                 bands_string);
         g_free (bands_string);
-        g_array_unref (supported_bands_array);
         g_array_unref (current_bands_array);
         g_task_return_error (task, error);
         g_object_unref (task);
@@ -2535,7 +2605,6 @@ mm_iface_modem_set_current_bands (MMIfaceModem *self,
         (GAsyncReadyCallback)set_current_bands_ready,
         task);
 
-    g_array_unref (supported_bands_array);
     g_array_unref (current_bands_array);
     g_free (bands_string);
 }