iface-modem-firmware: make the interface always available

We no longer "check for support" of this interface, it will always be
available for all modem objects.

The only implementation that used this interface was the QMI one, and
only for a very small subset of devices (those supporting the "QMI DMS
List Stored Images" operation). The logic was changed so that the list
of stored images wasn't preloaded during the support check; it will
now instead be preloaded during the first call to List().

3 files changed, 87 insertions, 156 deletions

diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-qmi.c
index 9f1927c3..4f294a7c 100644
--- a/src/mm-broadband-modem-qmi.c
+++ b/src/mm-broadband-modem-qmi.c
@@ -123,6 +123,7 @@ struct _MMBroadbandModemQmiPrivate {
     guint oma_event_report_indication_id;
 
     /* Firmware helpers */
+    gboolean firmware_list_preloaded;
     GList *firmware_list;
     MMFirmwareProperties *current_firmware;
 
@@ -7021,10 +7022,10 @@ oma_enable_unsolicited_events (MMIfaceModemOma *self,
 /* Check firmware support (Firmware interface) */
 
 typedef struct {
-    gchar *build_id;
-    GArray *modem_unique_id;
-    GArray *pri_unique_id;
-    gboolean current;
+    gchar    *build_id;
+    GArray   *modem_unique_id;
+    GArray   *pri_unique_id;
+    gboolean  current;
 } FirmwarePair;
 
 static void
@@ -7040,20 +7041,20 @@ typedef struct {
     QmiClientDms *client;
     GList *pairs;
     GList *l;
-} FirmwareCheckSupportContext;
+} FirmwareListPreloadContext;
 
 static void
-firmware_check_support_context_free (FirmwareCheckSupportContext *ctx)
+firmware_list_preload_context_free (FirmwareListPreloadContext *ctx)
 {
     g_list_free_full (ctx->pairs, (GDestroyNotify)firmware_pair_free);
     g_object_unref (ctx->client);
-    g_slice_free (FirmwareCheckSupportContext, ctx);
+    g_slice_free (FirmwareListPreloadContext, ctx);
 }
 
 static gboolean
-firmware_check_support_finish (MMIfaceModemFirmware *self,
-                               GAsyncResult *res,
-                               GError **error)
+firmware_list_preload_finish (MMBroadbandModemQmi  *self,
+                              GAsyncResult         *res,
+                              GError              **error)
 {
     return g_task_propagate_boolean (G_TASK (res), error);
 }
@@ -7066,7 +7067,7 @@ get_pri_image_info_ready (QmiClientDms *client,
                           GTask *task)
 {
     MMBroadbandModemQmi *self;
-    FirmwareCheckSupportContext *ctx;
+    FirmwareListPreloadContext *ctx;
     QmiMessageDmsGetStoredImageInfoOutput *output;
     GError *error = NULL;
     FirmwarePair *current;
@@ -7165,7 +7166,7 @@ static void
 get_next_image_info (GTask *task)
 {
     MMBroadbandModemQmi *self;
-    FirmwareCheckSupportContext *ctx;
+    FirmwareListPreloadContext *ctx;
     QmiMessageDmsGetStoredImageInfoInputImage image_id;
     QmiMessageDmsGetStoredImageInfoInput *input;
     FirmwarePair *current;
@@ -7231,7 +7232,7 @@ list_stored_images_ready (QmiClientDms *client,
                           GAsyncResult *res,
                           GTask *task)
 {
-    FirmwareCheckSupportContext *ctx;
+    FirmwareListPreloadContext *ctx;
     GArray *array;
     gint pri_id;
     gint modem_id;
@@ -7286,10 +7287,9 @@ list_stored_images_ready (QmiClientDms *client,
     }
 
     if (pri_id < 0 || modem_id < 0) {
-        mm_warn ("We need both PRI (%s) and MODEM (%s) images. "
-                 "Assuming firmware unsupported.",
-                 pri_id < 0 ? "not found" : "found",
-                 modem_id < 0 ? "not found" : "found");
+        mm_dbg ("We need both PRI (%s) and MODEM (%s) images",
+                pri_id < 0 ? "not found" : "found",
+                modem_id < 0 ? "not found" : "found");
         g_task_return_boolean (task, FALSE);
         g_object_unref (task);
         qmi_message_dms_list_stored_images_output_unref (output);
@@ -7338,8 +7338,7 @@ list_stored_images_ready (QmiClientDms *client,
     }
 
     if (!ctx->pairs) {
-        mm_warn ("No valid PRI+MODEM pairs found. "
-                 "Assuming firmware unsupported.");
+        mm_dbg ("No valid PRI+MODEM pairs found");
         g_task_return_boolean (task, FALSE);
         g_object_unref (task);
         qmi_message_dms_list_stored_images_output_unref (output);
@@ -7354,26 +7353,24 @@ list_stored_images_ready (QmiClientDms *client,
 }
 
 static void
-firmware_check_support (MMIfaceModemFirmware *self,
-                        GAsyncReadyCallback callback,
-                        gpointer user_data)
+firmware_list_preload (MMBroadbandModemQmi *self,
+                       GAsyncReadyCallback  callback,
+                       gpointer             user_data)
 {
-    FirmwareCheckSupportContext *ctx;
-    GTask *task;
-    QmiClient *client = NULL;
+    FirmwareListPreloadContext *ctx;
+    GTask                      *task;
+    QmiClient                  *client = NULL;
 
     if (!mm_shared_qmi_ensure_client (MM_SHARED_QMI (self),
                                       QMI_SERVICE_DMS, &client,
                                       callback, user_data))
         return;
 
-    ctx = g_slice_new0 (FirmwareCheckSupportContext);
+    ctx = g_slice_new0 (FirmwareListPreloadContext);
     ctx->client = g_object_ref (client);
 
     task = g_task_new (self, NULL, callback, user_data);
-    g_task_set_task_data (task,
-                          ctx,
-                          (GDestroyNotify)firmware_check_support_context_free);
+    g_task_set_task_data (task, ctx, (GDestroyNotify)firmware_list_preload_context_free);
 
     mm_dbg ("loading firmware images...");
     qmi_client_dms_list_stored_images (QMI_CLIENT_DMS (client),
@@ -7402,18 +7399,15 @@ firmware_load_list_finish (MMIfaceModemFirmware *self,
 }
 
 static void
-firmware_load_list (MMIfaceModemFirmware *_self,
-                    GAsyncReadyCallback callback,
-                    gpointer user_data)
+firmware_load_list_preloaded (GTask *task)
 {
-    MMBroadbandModemQmi *self = MM_BROADBAND_MODEM_QMI (_self);
-    GList *dup;
-    GTask *task;
+    MMBroadbandModemQmi *self;
+    GList               *dup;
 
-    /* We'll return the new list of new references we create here */
-    dup = g_list_copy_deep (self->priv->firmware_list, (GCopyFunc)g_object_ref, NULL);
+    self = g_task_get_source_object (task);
+    g_assert (self->priv->firmware_list_preloaded);
 
-    task = g_task_new (self, NULL, callback, user_data);
+    dup = g_list_copy_deep (self->priv->firmware_list, (GCopyFunc)g_object_ref, NULL);
     if (dup)
         g_task_return_pointer (task, dup, (GDestroyNotify)firmware_list_free);
     else
@@ -7422,6 +7416,43 @@ firmware_load_list (MMIfaceModemFirmware *_self,
     g_object_unref (task);
 }
 
+static void
+firmware_list_preload_ready (MMBroadbandModemQmi *self,
+                             GAsyncResult        *res,
+                             GTask               *task)
+{
+    GError *error = NULL;
+
+    if (!firmware_list_preload_finish (self, res, &error)) {
+        mm_dbg ("firmware list loading failed: %s", error ? error->message : "unsupported");
+        g_clear_error (&error);
+    }
+
+    firmware_load_list_preloaded (task);
+}
+
+static void
+firmware_load_list (MMIfaceModemFirmware *_self,
+                    GAsyncReadyCallback callback,
+                    gpointer user_data)
+{
+    MMBroadbandModemQmi *self = MM_BROADBAND_MODEM_QMI (_self);
+
+    GTask *task;
+
+    task = g_task_new (self, NULL, callback, user_data);
+
+    if (self->priv->firmware_list_preloaded) {
+        firmware_load_list_preloaded (task);
+        return;
+    }
+
+    self->priv->firmware_list_preloaded = TRUE;
+    firmware_list_preload (self,
+                           (GAsyncReadyCallback)firmware_list_preload_ready,
+                           task);
+}
+
 /*****************************************************************************/
 /* Load current firmware (Firmware interface) */
 
@@ -8766,8 +8797,6 @@ iface_modem_oma_init (MMIfaceModemOma *iface)
 static void
 iface_modem_firmware_init (MMIfaceModemFirmware *iface)
 {
-    iface->check_support = firmware_check_support;
-    iface->check_support_finish = firmware_check_support_finish;
     iface->load_list = firmware_load_list;
     iface->load_list_finish = firmware_load_list_finish;
     iface->load_current = firmware_load_current;
diff --git a/src/mm-iface-modem-firmware.c b/src/mm-iface-modem-firmware.c
index bce5e294..06f07754 100644
--- a/src/mm-iface-modem-firmware.c
+++ b/src/mm-iface-modem-firmware.c
@@ -21,12 +21,6 @@
 #include "mm-iface-modem-firmware.h"
 #include "mm-log.h"
 
-#define SUPPORT_CHECKED_TAG "firmware-support-checked-tag"
-#define SUPPORTED_TAG       "firmware-supported-tag"
-
-static GQuark support_checked_quark;
-static GQuark supported_quark;
-
 /*****************************************************************************/
 
 void
@@ -249,15 +243,13 @@ static void interface_initialization_step (GTask *task);
 
 typedef enum {
     INITIALIZATION_STEP_FIRST,
-    INITIALIZATION_STEP_CHECK_SUPPORT,
-    INITIALIZATION_STEP_FAIL_IF_UNSUPPORTED,
     INITIALIZATION_STEP_UPDATE_SETTINGS,
     INITIALIZATION_STEP_LAST
 } InitializationStep;
 
 struct _InitializationContext {
     MmGdbusModemFirmware *skeleton;
-    InitializationStep step;
+    InitializationStep    step;
 };
 
 static void
@@ -267,6 +259,14 @@ initialization_context_free (InitializationContext *ctx)
     g_free (ctx);
 }
 
+gboolean
+mm_iface_modem_firmware_initialize_finish (MMIfaceModemFirmware  *self,
+                                           GAsyncResult          *res,
+                                           GError               **error)
+{
+    return g_task_propagate_boolean (G_TASK (res), error);
+}
+
 static void
 load_update_settings_ready (MMIfaceModemFirmware *self,
                             GAsyncResult         *res,
@@ -299,35 +299,6 @@ load_update_settings_ready (MMIfaceModemFirmware *self,
 }
 
 static void
-check_support_ready (MMIfaceModemFirmware *self,
-                     GAsyncResult *res,
-                     GTask *task)
-{
-    InitializationContext *ctx;
-    GError *error = NULL;
-
-    if (!MM_IFACE_MODEM_FIRMWARE_GET_INTERFACE (self)->check_support_finish (self,
-                                                                             res,
-                                                                             &error)) {
-        if (error) {
-            /* This error shouldn't be treated as critical */
-            mm_dbg ("Firmware support check failed: '%s'", error->message);
-            g_error_free (error);
-        }
-    } else {
-        /* Firmware is supported! */
-        g_object_set_qdata (G_OBJECT (self),
-                            supported_quark,
-                            GUINT_TO_POINTER (TRUE));
-    }
-
-    /* Go on to next step */
-    ctx = g_task_get_task_data (task);
-    ctx->step++;
-    interface_initialization_step (task);
-}
-
-static void
 interface_initialization_step (GTask *task)
 {
     MMIfaceModemFirmware *self;
@@ -344,54 +315,6 @@ interface_initialization_step (GTask *task)
 
     switch (ctx->step) {
     case INITIALIZATION_STEP_FIRST:
-        /* Setup quarks if we didn't do it before */
-        if (G_UNLIKELY (!support_checked_quark))
-            support_checked_quark = (g_quark_from_static_string (
-                                         SUPPORT_CHECKED_TAG));
-        if (G_UNLIKELY (!supported_quark))
-            supported_quark = (g_quark_from_static_string (
-                                   SUPPORTED_TAG));
-
-        /* Fall down to next step */
-        ctx->step++;
-
-    case INITIALIZATION_STEP_CHECK_SUPPORT:
-        if (!GPOINTER_TO_UINT (g_object_get_qdata (G_OBJECT (self),
-                                                   support_checked_quark))) {
-            /* Set the checked flag so that we don't run it again */
-            g_object_set_qdata (G_OBJECT (self),
-                                support_checked_quark,
-                                GUINT_TO_POINTER (TRUE));
-            /* Initially, assume we don't support it */
-            g_object_set_qdata (G_OBJECT (self),
-                                supported_quark,
-                                GUINT_TO_POINTER (FALSE));
-
-            if (MM_IFACE_MODEM_FIRMWARE_GET_INTERFACE (self)->check_support &&
-                MM_IFACE_MODEM_FIRMWARE_GET_INTERFACE (self)->check_support_finish) {
-                MM_IFACE_MODEM_FIRMWARE_GET_INTERFACE (self)->check_support (
-                    self,
-                    (GAsyncReadyCallback)check_support_ready,
-                    task);
-                return;
-            }
-
-            /* If there is no implementation to check support, assume we DON'T
-             * support it. */
-        }
-        /* Fall down to next step */
-        ctx->step++;
-
-    case INITIALIZATION_STEP_FAIL_IF_UNSUPPORTED:
-        if (!GPOINTER_TO_UINT (g_object_get_qdata (G_OBJECT (self),
-                                                   supported_quark))) {
-            g_task_return_new_error (task,
-                                     MM_CORE_ERROR,
-                                     MM_CORE_ERROR_UNSUPPORTED,
-                                     "Firmware interface not available");
-            g_object_unref (task);
-            return;
-        }
         /* Fall down to next step */
         ctx->step++;
 
@@ -411,14 +334,10 @@ interface_initialization_step (GTask *task)
         /* We are done without errors! */
 
         /* Handle method invocations */
-        g_signal_connect (ctx->skeleton,
-                          "handle-list",
-                          G_CALLBACK (handle_list),
-                          self);
-        g_signal_connect (ctx->skeleton,
-                          "handle-select",
-                          G_CALLBACK (handle_select),
-                          self);
+        g_object_connect (ctx->skeleton,
+                          "signal::handle-list",   G_CALLBACK (handle_list),   self,
+                          "signal::handle-select", G_CALLBACK (handle_select), self,
+                          NULL);
 
         /* Finally, export the new interface */
         mm_gdbus_object_skeleton_set_modem_firmware (MM_GDBUS_OBJECT_SKELETON (self),
@@ -432,23 +351,15 @@ interface_initialization_step (GTask *task)
     g_assert_not_reached ();
 }
 
-gboolean
-mm_iface_modem_firmware_initialize_finish (MMIfaceModemFirmware *self,
-                                           GAsyncResult *res,
-                                           GError **error)
-{
-    return g_task_propagate_boolean (G_TASK (res), error);
-}
-
 void
 mm_iface_modem_firmware_initialize (MMIfaceModemFirmware *self,
-                                    GCancellable *cancellable,
-                                    GAsyncReadyCallback callback,
-                                    gpointer user_data)
+                                    GCancellable         *cancellable,
+                                    GAsyncReadyCallback   callback,
+                                    gpointer              user_data)
 {
     InitializationContext *ctx;
-    MmGdbusModemFirmware *skeleton = NULL;
-    GTask *task;
+    MmGdbusModemFirmware  *skeleton = NULL;
+    GTask                 *task;
 
     /* Did we already create it? */
     g_object_get (self,
@@ -462,7 +373,6 @@ mm_iface_modem_firmware_initialize (MMIfaceModemFirmware *self,
     }
 
     /* Perform async initialization here */
-
     ctx = g_new0 (InitializationContext, 1);
     ctx->step = INITIALIZATION_STEP_FIRST;
     ctx->skeleton = skeleton;
diff --git a/src/mm-iface-modem-firmware.h b/src/mm-iface-modem-firmware.h
index 2fbfb07c..15052bce 100644
--- a/src/mm-iface-modem-firmware.h
+++ b/src/mm-iface-modem-firmware.h
@@ -34,14 +34,6 @@ typedef struct _MMIfaceModemFirmware MMIfaceModemFirmware;
 struct _MMIfaceModemFirmware {
     GTypeInterface g_iface;
 
-    /* Check for Firmware support (async) */
-    void (* check_support) (MMIfaceModemFirmware *self,
-                            GAsyncReadyCallback callback,
-                            gpointer user_data);
-    gboolean (* check_support_finish) (MMIfaceModemFirmware *self,
-                                       GAsyncResult *res,
-                                       GError **error);
-
     /* Get update settings (async) */
     void                       (* load_update_settings)        (MMIfaceModemFirmware  *self,
                                                                 GAsyncReadyCallback    callback,