[PATCH] sim hot swap: improved error management
Aleksander Morgado
aleksander at aleksander.es
Wed Jul 12 13:37:22 UTC 2017
+ Eric for comments
On Wed, Jul 12, 2017 at 2:28 PM, Carlo Lobrano <c.lobrano at gmail.com> wrote:
> 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
It looks to me that the "sim_hot_swap_on" in mm-broadband-modem-mbim.c
would serve the same purpose of this new
MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED property, isn't that right?
i.e. if SIM how swap enabling fails, both those booleans get set to
FALSE, so that the code afterwards skips steps as sim hot swap isn't
configured. What do you guys think? Maybe we should also update the
MBIM implementation to use this new
MM_IFACE_MODEM_SIM_HOT_SWAP_CONFIGURED property?
> ---
> plugins/telit/mm-broadband-modem-telit.c | 1 +
> src/mm-broadband-modem.c | 91 ++++++++++++++++++++++----------
> src/mm-iface-modem.c | 14 ++++-
> src/mm-iface-modem.h | 1 +
> 4 files changed, 78 insertions(+), 29 deletions(-)
>
> diff --git a/plugins/telit/mm-broadband-modem-telit.c b/plugins/telit/mm-broadband-modem-telit.c
> index a8ee886..9f5b588 100644
> --- a/plugins/telit/mm-broadband-modem-telit.c
> +++ b/plugins/telit/mm-broadband-modem-telit.c
> @@ -1373,6 +1373,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.c b/src/mm-broadband-modem.c
> index 08aa23a..9f842c9 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 */
> @@ -10242,27 +10244,38 @@ initialize_step (InitializeContext *ctx)
> * (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");
> @@ -10291,34 +10304,47 @@ initialize_step (InitializeContext *ctx)
> } 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...");
> + 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...");
> g_simple_async_result_set_error (ctx->result,
> 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");
> - g_simple_async_result_set_error (ctx->result,
> - MM_CORE_ERROR,
> - MM_CORE_ERROR_WRONG_STATE,
> - "Modem is unusable, "
> - "cannot fully initialize");
> + "cannot fully initialize, waiting for SIM insertion.");
> + goto sim_hot_swap_enabled;
> + }
> +
> }
>
> + g_simple_async_result_set_error (ctx->result,
> + 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.
> @@ -10644,6 +10670,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;
> @@ -10749,6 +10778,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;
> @@ -11248,6 +11280,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 e79e55c..73183c0 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 17ded5b..711d2f2 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;
>
> --
> 2.9.3
>
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
--
Aleksander
https://aleksander.es
More information about the ModemManager-devel
mailing list