sim hot swap: improved error management

Currently, when SIM hot swap fails in either mm-iface or plugin, the
ModemManager still opens ports context and prints a message saying that
SIM hot swap is supported and that it's waiting for SIM insertion,
instead of clearly saying that SIM hot swap is not working.

This patch:

1. introduces a new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED
   which is FALSE by default and set to TRUE only when
   setup_sim_hot_swap_finish() succeded.
2. subordinates the completion of SIM hot swap setup (in
   mm-broadband-modem) and the related messages to the the value of
   MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED

Finally, this patch replaces the MBIM's sim_hot_swap_on private property
with the new property MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, since they have the
same meaning.

5 files changed, 93 insertions, 38 deletions

diff --git a/plugins/telit/mm-broadband-modem-telit.c b/plugins/telit/mm-broadband-modem-telit.c
index e7650c05..edd9093b 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -1377,6 +1377,7 @@ mm_broadband_modem_telit_new (const gchar *device,
                          MM_BASE_MODEM_VENDOR_ID, vendor_id,
                          MM_BASE_MODEM_PRODUCT_ID, product_id,
                          MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
+                         MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
                          NULL);
 }
 
diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
index c69893f6..7f5d2fc8 100644
--- a/src/mm-broadband-modem-mbim.c
+++ b/src/mm-broadband-modem-mbim.c
@@ -89,8 +89,6 @@ struct _MMBroadbandModemMbimPrivate {
     MbimDataClass available_data_classes;
     MbimDataClass highest_available_data_class;
 
-    /* For checking whether the SIM has been hot swapped */
-    gboolean sim_hot_swap_on;
     MbimSubscriberReadyState last_ready_state;
 };
 
@@ -1990,6 +1988,7 @@ basic_connect_notification_subscriber_ready_status (MMBroadbandModemMbim *self,
         (self->priv->last_ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
                ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED)) {
         /* SIM has been removed or reinserted, re-probe to ensure correct interfaces are exposed */
+        mm_dbg ("SIM hot swap detected");
         mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM (self));
     }
 
@@ -2286,10 +2285,15 @@ cleanup_unsolicited_events_3gpp (MMIfaceModem3gpp *_self,
                                  gpointer user_data)
 {
     MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
+    gboolean is_sim_hot_swap_configured = FALSE;
+
+    g_object_get (self,
+                  MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, &is_sim_hot_swap_configured,
+                  NULL);
 
     self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SIGNAL_QUALITY;
     self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_CONNECT;
-    if (self->priv->sim_hot_swap_on)
+    if (is_sim_hot_swap_configured)
         self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
     self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_PACKET_SERVICE;
     common_setup_cleanup_unsolicited_events (self, FALSE, callback, user_data);
@@ -2500,7 +2504,6 @@ enable_subscriber_info_unsolicited_events_ready (MMBroadbandModemMbim *self,
 
     if (!common_enable_disable_unsolicited_events_finish (self, res, &error)) {
         mm_dbg ("Failed to enable subscriber info events: %s", error->message);
-        self->priv->sim_hot_swap_on = FALSE;
         g_task_return_error (task, error);
         g_object_unref (task);
         return;
@@ -2519,7 +2522,6 @@ setup_subscriber_info_unsolicited_events_ready (MMBroadbandModemMbim *self,
 
     if (!common_setup_cleanup_unsolicited_events_finish (self, res, &error)) {
         mm_dbg ("Failed to set up subscriber info events: %s", error->message);
-        self->priv->sim_hot_swap_on = FALSE;
         g_task_return_error (task, error);
         g_object_unref (task);
         return;
@@ -2541,7 +2543,6 @@ modem_setup_sim_hot_swap (MMIfaceModem *_self,
 
     task = g_task_new (self, NULL, callback, user_data);
 
-    self->priv->sim_hot_swap_on = TRUE;
     self->priv->setup_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
     common_setup_cleanup_unsolicited_events (self,
                                              TRUE,
@@ -2566,10 +2567,15 @@ modem_3gpp_disable_unsolicited_events (MMIfaceModem3gpp *_self,
                                        gpointer user_data)
 {
     MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
+    gboolean is_sim_hot_swap_configured = FALSE;
+
+    g_object_get (self,
+                  MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, &is_sim_hot_swap_configured,
+                  NULL);
 
     self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_SIGNAL_QUALITY;
     self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_CONNECT;
-    if (!self->priv->sim_hot_swap_on)
+    if (is_sim_hot_swap_configured)
         self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
     self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_PACKET_SERVICE;
     common_enable_disable_unsolicited_events (self, callback, user_data);
@@ -3154,6 +3160,7 @@ mm_broadband_modem_mbim_new (const gchar *device,
                          MM_BASE_MODEM_VENDOR_ID, vendor_id,
                          MM_BASE_MODEM_PRODUCT_ID, product_id,
                          MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
+                         MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, FALSE,
                          NULL);
 }
 
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index d3d349e9..2d2f6509 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -116,6 +116,7 @@ enum {
     PROP_MODEM_VOICE_CALL_LIST,
     PROP_MODEM_SIMPLE_STATUS,
     PROP_MODEM_SIM_HOT_SWAP_SUPPORTED,
+    PROP_MODEM_SIM_HOT_SWAP_CONFIGURED,
     PROP_FLOW_CONTROL,
     PROP_LAST
 };
@@ -134,6 +135,7 @@ struct _MMBroadbandModemPrivate {
     PortsContext *sim_hot_swap_ports_ctx;
     gboolean modem_init_run;
     gboolean sim_hot_swap_supported;
+    gboolean sim_hot_swap_configured;
 
     /*<--- Modem interface --->*/
     /* Properties */
@@ -10098,27 +10100,38 @@ initialize_step (GTask *task)
          * (we may be re-running the initialization step after SIM-PIN unlock) */
         if (!ctx->self->priv->sim_hot_swap_ports_ctx) {
             gboolean is_sim_hot_swap_supported = FALSE;
+            gboolean is_sim_hot_swap_configured = FALSE;
 
             g_object_get (ctx->self,
                           MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, &is_sim_hot_swap_supported,
                           NULL);
 
+            g_object_get (ctx->self,
+                          MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, &is_sim_hot_swap_configured,
+                          NULL);
+
             if (is_sim_hot_swap_supported) {
-                PortsContext *ports;
-                GError *error = NULL;
 
-                mm_dbg ("Creating ports context for SIM hot swap");
+                if (!is_sim_hot_swap_configured) {
+                    mm_warn ("SIM hot swap supported but not configured. Skipping opening ports");
+                } else {
+                    PortsContext *ports;
+                    GError *error = NULL;
 
-                ports = g_new0 (PortsContext, 1);
-                ports->ref_count = 1;
+                    mm_dbg ("Creating ports context for SIM hot swap");
 
-                if (!open_ports_enabling (ctx->self, ports, FALSE, &error)) {
-                    mm_warn ("Couldn't open ports during Modem SIM hot swap enabling: %s", error? error->message : "unknown reason");
-                    g_error_free (error);
-                } else
-                    ctx->self->priv->sim_hot_swap_ports_ctx = ports_context_ref (ports);
+                    ports = g_new0 (PortsContext, 1);
+                    ports->ref_count = 1;
 
-                ports_context_unref (ports);
+                    if (!open_ports_enabling (ctx->self, ports, FALSE, &error)) {
+                        mm_warn ("Couldn't open ports during Modem SIM hot swap enabling: %s", error? error->message : "unknown reason");
+                        g_error_free (error);
+                    } else {
+                        ctx->self->priv->sim_hot_swap_ports_ctx = ports_context_ref (ports);
+                    }
+
+                    ports_context_unref (ports);
+                }
             }
         } else
             mm_dbg ("Ports context for SIM hot swap already available");
@@ -10146,32 +10159,44 @@ initialize_step (GTask *task)
             } else {
                 /* Fatal SIM, firmware, or modem failure :-( */
                 gboolean is_sim_hot_swap_supported = FALSE;
+                gboolean is_sim_hot_swap_configured = FALSE;
+
                 MMModemStateFailedReason reason =
                     mm_gdbus_modem_get_state_failed_reason (
                         (MmGdbusModem*)ctx->self->priv->modem_dbus_skeleton);
 
                 g_object_get (ctx->self,
-                             MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED,
-                             &is_sim_hot_swap_supported,
-                             NULL);
-
-                if (reason == MM_MODEM_STATE_FAILED_REASON_SIM_MISSING &&
-                    is_sim_hot_swap_supported &&
-                    ctx->self->priv->sim_hot_swap_ports_ctx) {
-                    mm_info ("SIM is missing, but the modem supports SIM hot swap. Waiting for SIM...");
-                    error = g_error_new (MM_CORE_ERROR,
-                                         MM_CORE_ERROR_WRONG_STATE,
-                                         "Modem is unusable due to SIM missing, "
-                                         "cannot fully initialize, "
-                                         "waiting for SIM insertion.");
-                } else {
-                    mm_dbg ("SIM is missing and Modem does not support SIM Hot Swap");
-                    error = g_error_new (MM_CORE_ERROR,
-                                         MM_CORE_ERROR_WRONG_STATE,
-                                         "Modem is unusable, "
-                                         "cannot fully initialize");
+                              MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED,
+                              &is_sim_hot_swap_supported,
+                              NULL);
+
+                g_object_get (ctx->self,
+                              MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED,
+                              &is_sim_hot_swap_configured,
+                              NULL);
+
+                if (reason == MM_MODEM_STATE_FAILED_REASON_SIM_MISSING) {
+                    if (!is_sim_hot_swap_supported) {
+                        mm_dbg ("SIM is missing, but this modem does not support SIM hot swap.");
+                    } else if (!is_sim_hot_swap_configured) {
+                        mm_warn ("SIM is missing, but SIM hot swap could not be configured.");
+                    } else if (!ctx->self->priv->sim_hot_swap_ports_ctx) {
+                        mm_err ("SIM is missing and SIM hot swap is configured, but ports are not opened.");
+                    } else {
+                        mm_dbg ("SIM is missing, but SIM hot swap is enabled. Waiting for SIM...");
+                        error = g_error_new (MM_CORE_ERROR,
+                                             MM_CORE_ERROR_WRONG_STATE,
+                                             "Modem is unusable due to SIM missing, "
+                                             "cannot fully initialize, waiting for SIM insertion.");
+                        goto sim_hot_swap_enabled;
+                    }
                 }
 
+                error = g_error_new (MM_CORE_ERROR,
+                                     MM_CORE_ERROR_WRONG_STATE,
+                                     "Modem is unusable, "
+                                     "cannot fully initialize");
+sim_hot_swap_enabled:
                 /* Ensure we only leave the Modem, OMA, and Firmware interfaces
                  * around.  A failure could be caused by firmware issues, which
                  * a firmware update, switch, or provisioning could fix.
@@ -10498,6 +10523,9 @@ set_property (GObject *object,
     case PROP_MODEM_SIM_HOT_SWAP_SUPPORTED:
         self->priv->sim_hot_swap_supported = g_value_get_boolean (value);
         break;
+    case PROP_MODEM_SIM_HOT_SWAP_CONFIGURED:
+        self->priv->sim_hot_swap_configured = g_value_get_boolean (value);
+        break;
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
         break;
@@ -10603,6 +10631,9 @@ get_property (GObject *object,
     case PROP_MODEM_SIM_HOT_SWAP_SUPPORTED:
         g_value_set_boolean (value, self->priv->sim_hot_swap_supported);
         break;
+    case PROP_MODEM_SIM_HOT_SWAP_CONFIGURED:
+        g_value_set_boolean (value, self->priv->sim_hot_swap_configured);
+        break;
     case PROP_FLOW_CONTROL:
         g_value_set_flags (value, self->priv->flow_control);
         break;
@@ -11102,6 +11133,9 @@ mm_broadband_modem_class_init (MMBroadbandModemClass *klass)
                                       PROP_MODEM_SIM_HOT_SWAP_SUPPORTED,
                                       MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED);
 
+    g_object_class_override_property (object_class,
+                                      PROP_MODEM_SIM_HOT_SWAP_CONFIGURED,
+                                      MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED);
     properties[PROP_FLOW_CONTROL] =
         g_param_spec_flags (MM_BROADBAND_MODEM_FLOW_CONTROL,
                             "Flow control",
diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index d59303a8..ec453a8f 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -4335,6 +4335,9 @@ setup_sim_hot_swap_ready (MMIfaceModem *self,
         g_error_free (error);
     } else {
         mm_dbg ("Iface modem: SIM hot swap setup succeded");
+        g_object_set (self,
+                      MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED, TRUE,
+                      NULL);
     }
 
     /* Go on to next step */
@@ -4880,8 +4883,9 @@ interface_initialization_step (GTask *task)
                               MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, &is_sim_hot_swap_supported,
                               NULL);
 
-                if (is_sim_hot_swap_supported)
+                if (is_sim_hot_swap_supported) {
                     mm_iface_modem_update_failed_state (self, MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
+                }
             }
         } else {
             /* We are done without errors!
@@ -5284,6 +5288,7 @@ iface_modem_init (gpointer g_iface)
                               "List of bearers handled by the modem",
                               MM_TYPE_BEARER_LIST,
                               G_PARAM_READWRITE));
+
     g_object_interface_install_property
         (g_iface,
          g_param_spec_boolean (MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED,
@@ -5292,6 +5297,13 @@ iface_modem_init (gpointer g_iface)
                                FALSE,
                                G_PARAM_READWRITE));
 
+    g_object_interface_install_property
+        (g_iface,
+         g_param_spec_boolean (MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED,
+                               "Sim Hot Swap Configured",
+                               "Whether the sim hot swap support is configured correctly.",
+                               FALSE,
+                               G_PARAM_READWRITE));
     initialized = TRUE;
 }
 
diff --git a/src/mm-iface-modem.h b/src/mm-iface-modem.h
index 17ded5bc..711d2f29 100644
--- a/src/mm-iface-modem.h
+++ b/src/mm-iface-modem.h
@@ -37,6 +37,7 @@
 #define MM_IFACE_MODEM_SIM                     "iface-modem-sim"
 #define MM_IFACE_MODEM_BEARER_LIST             "iface-modem-bearer-list"
 #define MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED  "iface-modem-sim-hot-swap-supported"
+#define MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED "iface-modem-sim-hot-swap-configured"
 
 typedef struct _MMIfaceModem MMIfaceModem;