shared-qmi,iface-modem : Do not reprobe on hotswap in inactive slot

Prior to this change, a hotswap on the inactive slot would result
in reprobe of the modem. With this change, the sim object would be
updated, but no reprobe will be performed. The advantage is
1) we do not drop connections on the active slot
2) Slot status on the inactive slot is susceptible to firmware bugs.
   We have seen scenarios where the inactive slot is reported as
   absent/ iccid={0}, followed by a correction. We should not
   reprobe the modem if our active slot hasn't changed.

3 files changed, 225 insertions, 11 deletions

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index c3c4106d..b3a301f7 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -134,6 +134,78 @@ mm_iface_modem_check_for_sim_swap (MMIfaceModem *self,
     g_object_unref (task);
 }
 
+static void
+sim_slot_free (MMBaseSim *sim)
+{
+    if (sim)
+        g_object_unref (sim);
+}
+
+void
+mm_iface_modem_modify_sim (MMIfaceModem  *self,
+                           guint          slot_index,
+                           MMBaseSim     *new_sim)
+{
+    MmGdbusModem *skeleton;
+    g_autoptr (GPtrArray) sim_slots_old = NULL;
+    g_autoptr (GPtrArray) sim_slots_new = NULL;
+    guint i;
+    GPtrArray *sim_slot_paths_array;
+    g_auto (GStrv) sim_slot_paths = NULL;
+
+    g_object_get (self,
+                  MM_IFACE_MODEM_SIM_SLOTS,
+                  &sim_slots_old,
+                  NULL);
+    if (!sim_slots_old) {
+        mm_obj_warn (self, "Failed to process SIM hot swap: couldn't load current list of SIM slots");
+        return;
+    }
+
+    sim_slot_paths_array = g_ptr_array_new ();
+    sim_slots_new        = g_ptr_array_new_with_free_func ((GDestroyNotify) sim_slot_free);
+    for (i = 0; i < sim_slots_old->len; i++) {
+        MMBaseSim   *sim;
+        const gchar *sim_path;
+
+        sim = MM_BASE_SIM (g_ptr_array_index (sim_slots_old, i));
+        if (i == slot_index) {
+            mm_obj_dbg (self, "Updating sim at slot %d", i + 1);
+            g_ptr_array_add (sim_slots_new, new_sim ? g_object_ref (new_sim) : NULL);
+            sim = new_sim;
+        } else {
+            g_ptr_array_add (sim_slots_new, g_object_ref (sim));
+        }
+
+        if (!sim) {
+            g_ptr_array_add (sim_slot_paths_array, g_strdup ("/"));
+            continue;
+        }
+
+        sim_path = mm_base_sim_get_path (sim);
+        g_ptr_array_add (sim_slot_paths_array, g_strdup (sim_path));
+    }
+
+    g_object_set (self,
+                  MM_IFACE_MODEM_SIM_SLOTS,
+                  sim_slots_new,
+                  NULL);
+
+    g_ptr_array_add (sim_slot_paths_array, NULL);
+    sim_slot_paths = (GStrv) g_ptr_array_free (sim_slot_paths_array, FALSE);
+
+    g_object_get (self,
+                  MM_IFACE_MODEM_DBUS_SKELETON,
+                  &skeleton,
+                  NULL);
+    if (!skeleton) {
+        mm_obj_warn (self, "Failed to get dbus skeleton in mm_iface_modem_modify_sim, will not update sim_slots");
+        return;
+    }
+    mm_gdbus_modem_set_sim_slots (skeleton, (const gchar *const *) sim_slot_paths);
+    g_object_unref (skeleton);
+}
+
 /*****************************************************************************/
 
 void
diff --git a/src/mm-iface-modem.h b/src/mm-iface-modem.h
index fa2b3dc8..34f3297b 100644
--- a/src/mm-iface-modem.h
+++ b/src/mm-iface-modem.h
@@ -609,4 +609,8 @@ gboolean mm_iface_modem_check_for_sim_swap_finish (MMIfaceModem *self,
                                                    GAsyncResult *res,
                                                    GError **error);
 
+void mm_iface_modem_modify_sim (MMIfaceModem *self,
+                                guint slot_index,
+                                MMBaseSim *new_sim);
+
 #endif /* MM_IFACE_MODEM_H */
diff --git a/src/mm-shared-qmi.c b/src/mm-shared-qmi.c
index 9edc595b..f82606e6 100644
--- a/src/mm-shared-qmi.c
+++ b/src/mm-shared-qmi.c
@@ -98,6 +98,7 @@ typedef struct {
     /* Slot status monitoring */
     QmiClient *uim_client;
     gulong     uim_slot_status_indication_id;
+    GArray    *slots_status;
     gulong     uim_refresh_indication_id;
     guint      uim_refresh_start_timeout_id;
 } Private;
@@ -121,6 +122,8 @@ private_free (Private *priv)
         g_object_unref (priv->loc_client);
     if (priv->uim_slot_status_indication_id)
         g_signal_handler_disconnect (priv->uim_client, priv->uim_slot_status_indication_id);
+    if (priv->slots_status)
+        g_array_unref (priv->slots_status);
     if (priv->uim_refresh_indication_id)
         g_signal_handler_disconnect (priv->uim_client, priv->uim_refresh_indication_id);
     if (priv->uim_client)
@@ -3326,15 +3329,17 @@ uim_get_slot_status_ready (QmiClientUim *client,
 {
     g_autoptr(QmiMessageUimGetSlotStatusOutput) output = NULL;
     LoadSimSlotsContext *ctx;
-    MMIfaceModem              *self;
-    GError                    *error = NULL;
-    GArray                    *physical_slots = NULL;
-    GArray                    *ext_information = NULL;
-    GArray                    *slot_eids = NULL;
-    guint                      i;
+    MMIfaceModem        *self;
+    Private             *priv;
+    GError              *error = NULL;
+    GArray              *physical_slots = NULL;
+    GArray              *ext_information = NULL;
+    GArray              *slot_eids = NULL;
+    guint                i;
 
     self = g_task_get_source_object (task);
     ctx  = g_task_get_task_data (task);
+    priv = get_private (MM_SHARED_QMI (self));
 
     output = qmi_client_uim_get_slot_status_finish (client, res, &error);
     if (!output ||
@@ -3345,6 +3350,12 @@ uim_get_slot_status_ready (QmiClientUim *client,
         return;
     }
 
+    /* Store the slot status before loading all sim slots.
+     * We will use this to check if a hotswap requires a reprobe. */
+    if (priv->slots_status)
+        g_array_unref (priv->slots_status);
+    priv->slots_status = g_array_ref (physical_slots);
+
     /* It's fine if we don't have EID information, but it should be well-formed if present. If it's malformed,
      * there is probably a modem firmware bug. */
     if (qmi_message_uim_get_slot_status_output_get_physical_slot_information (output, &ext_information, NULL) &&
@@ -3710,18 +3721,120 @@ uim_refresh_indication_cb (QmiClientUim                  *client,
     }
 }
 
+/* Modifies the sim at slot == index+1, based on the content of slot_status.
+ * Primarily used when a hotswap occurs on the inactive slot */
+static void
+update_sim_from_slot_status (MMSharedQmi               *self,
+                             QmiPhysicalSlotStatusSlot *slot_status,
+                             guint                      slot_index)
+{
+    g_autoptr(MMBaseSim)  sim = NULL;
+    g_autofree gchar     *raw_iccid = NULL;
+    g_autofree gchar     *iccid = NULL;
+    g_autoptr(GError)     inner_error = NULL;
+
+    mm_obj_dbg (self, "Updating sim at slot %d", slot_index + 1);
+    /* If sim is not present, ask mm_iface_modem to clear the sim object */
+    if (slot_status->physical_card_status != QMI_UIM_PHYSICAL_CARD_STATE_PRESENT) {
+        mm_iface_modem_modify_sim (MM_IFACE_MODEM (self), slot_index, NULL);
+        return;
+    }
+
+    raw_iccid = mm_utils_bin2hexstr ((const guint8 *)slot_status->iccid->data, slot_status->iccid->len);
+    if (!raw_iccid) {
+        mm_obj_warn (self, "not creating SIM object: failed to convert ICCID from BCD");
+        return;
+    }
+    iccid = mm_3gpp_parse_iccid (raw_iccid, &inner_error);
+    if (!iccid) {
+        mm_obj_warn (self, "not creating SIM object: couldn't parse SIM iccid: %s", inner_error->message);
+        return;
+    }
+    sim = mm_sim_qmi_new_initialized (MM_BASE_MODEM (self),
+                                      TRUE,                              /* consider DMS UIM deprecated if we're creating SIM slots */
+                                      slot_index + 1,                    /* slot number is the array index starting at 1 */
+                                      slot_status->physical_slot_status, /* is_active */
+                                      iccid,
+                                      NULL,  /* imsi unknown */
+                                      NULL,  /* eid unknown */
+                                      NULL,  /* operator id unknown */
+                                      NULL,  /* operator name unknown */
+                                      NULL); /* emergency numbers unknown */
+
+    mm_iface_modem_modify_sim (MM_IFACE_MODEM (self), slot_index, sim);
+}
+
+/* Checks for equality of two slots. */
+static gboolean
+slot_status_equal (QmiPhysicalSlotStatusSlot *slot_a,
+                   QmiPhysicalSlotStatusSlot *slot_b)
+{
+    guint j;
+
+    if (slot_a->physical_slot_status != slot_b->physical_slot_status)
+        return FALSE;
+    if (slot_a->physical_card_status != slot_b->physical_card_status)
+        return FALSE;
+    if (slot_a->iccid->len != slot_b->iccid->len)
+        return FALSE;
+    for (j = 0; j < slot_a->iccid->len; j++) {
+        if (g_array_index (slot_a->iccid, guint8, j) != g_array_index (slot_b->iccid, guint8, j))
+            return FALSE;
+    }
+    return TRUE;
+}
+
+/* Checks for equality of QmiPhysicalSlotStatusSlot arrays.
+ * The number of elements in each array is expected to be the number of sim slots in the modem.*/
+static gboolean
+slot_array_status_equal (GArray   *slots_status1,
+                         GArray   *slots_status2,
+                         gboolean  check_active_slots_only)
+{
+    guint i;
+
+    if (!slots_status1 && !slots_status2)
+        return TRUE;
+    if (!slots_status1 || !slots_status2 || slots_status1->len != slots_status2->len)
+        return FALSE;
+    for (i = 0; i < slots_status1->len; i++) {
+        /* Compare slot at index i from slots_status1 and slots_status2 */
+        QmiPhysicalSlotStatusSlot *slot_a;
+        QmiPhysicalSlotStatusSlot *slot_b;
+
+        slot_a = &g_array_index (slots_status1, QmiPhysicalSlotStatusSlot, i);
+        slot_b = &g_array_index (slots_status2, QmiPhysicalSlotStatusSlot, i);
+
+        /* Check that slot_a and slot_b have the same slot status (i.e. active or inactive) */
+        if (slot_a->physical_slot_status != slot_b->physical_slot_status)
+            return FALSE;
+        /* Once slot_a and slot_b are confirmed to have the same physical slot status,
+         * we will ignore inactive slots if check_active_slots_only is set. */
+        if (check_active_slots_only && slot_a->physical_slot_status != QMI_UIM_SLOT_STATE_ACTIVE) {
+            g_assert (slot_a->physical_slot_status == slot_b->physical_slot_status);
+            continue;
+        }
+        if (!slot_status_equal (slot_a, slot_b))
+            return FALSE;
+    }
+    return TRUE;
+}
+
 static void
 uim_slot_status_indication_cb (QmiClientUim                     *client,
                                QmiIndicationUimSlotStatusOutput *output,
                                MMSharedQmi                      *self)
 {
-    GArray            *physical_slots = NULL;
+    GArray            *new_slots_status = NULL;
+    Private           *priv;
+    guint              i;
     g_autoptr(GError)  error = NULL;
 
+    priv = get_private (self);
     mm_obj_dbg (self, "received slot status indication");
 
     if (!qmi_indication_uim_slot_status_output_get_physical_slot_status (output,
-                                                                         &physical_slots,
+                                                                         &new_slots_status,
                                                                          &error)) {
         mm_obj_warn (self, "could not process slot status indication: %s", error->message);
         return;
@@ -3731,9 +3844,34 @@ uim_slot_status_indication_cb (QmiClientUim                     *client,
      * 1) The physical slot to logical slot mapping has changed as a
      * result of switching the slot. or,
      * 2) A card has been removed from, or inserted to, the physical slot. or,
-     * 3) A physical slot is powered up or down. or,
-     * In all these cases, we must reprobe the modem to keep SIM objects updated */
-    mm_base_modem_process_sim_event (MM_BASE_MODEM (self));
+     * 3) A physical slot is powered up or down. */
+
+    /* Reprobe if the active slot changed or the information in an
+     * active slot's status changed */
+    if (!slot_array_status_equal (priv->slots_status, new_slots_status, TRUE)) {
+        mm_obj_dbg (self, "An active slot had a status change, will reprobe the modem");
+        mm_base_modem_process_sim_event (MM_BASE_MODEM (self));
+        return;
+    }
+
+    /* An inactive slot changed, we won't be reprobing the modem.
+     * Instead, we will ask mm_iface_modem to update the sim object.
+     * Iterate over each slot to identify the slots that changed state.*/
+    for (i = 0; i < priv->slots_status->len; i++) {
+        QmiPhysicalSlotStatusSlot *old_slot;
+        QmiPhysicalSlotStatusSlot *new_slot;
+
+        old_slot = &g_array_index (priv->slots_status, QmiPhysicalSlotStatusSlot, i);
+        new_slot = &g_array_index (new_slots_status, QmiPhysicalSlotStatusSlot, i);
+
+        if (!slot_status_equal (old_slot, new_slot)) {
+            mm_obj_dbg (self, "Slot %d (inactive) had a status change. Will update sims, but not reprobe", i + 1);
+            update_sim_from_slot_status (self, new_slot, i);
+        }
+    }
+
+    g_clear_pointer (&priv->slots_status, g_array_unref);
+    priv->slots_status = g_array_ref (new_slots_status);
 }
 
 static void