Awsome! Thank you <div class="gmail_quote"><div dir="ltr">Il mer 10 ago 2016 09:53 Aleksander Morgado <<a href="mailto:aleksander@aleksander.es">aleksander@aleksander.es</a>> ha scritto: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Aug 8, 2016 at 3:47 PM, Carlo Lobrano <<a href="mailto:c.lobrano@gmail.com" target="_blank">c.lobrano@gmail.com</a>> wrote: > BaseModem > added reprobe property. > > MMDevice > added logic to recreate the modem if it is set invalid and "to reprobe" > > MMBroadbandModem > * added initialization step for SIM hot swap: > 1. keep dedicated ports open to listen to modem's unsolicited > 2. dedicated error management in case of initialization failure due to SIM missing > * added function to be called in order to act upon SIM insertion/removal: > 1. close dedicated ports > 2. set the modem to be reprobed > 3. disable modem > * added SIM HOT SWAP boolean property > > MMIfaceModem > * added initialization step for SIM hot swap, if supported by the plugin > * dedicated error management in case of initialization failure due to SIM missing > > --- > > Hi Aleksander, > > hope this time the patch works fine :) > The previous one had issues probably because I was keeping two different branches, one for the changes to the core and one for the plugin. > > The main changes are in patch 1/2, according to your last code review. > Pushed to git master. I did some minor follow up coding style changes in a separate commit, btw. Thanks! > Best regards, > Carlo > > --- > src/mm-base-modem.c | 26 ++++++++++ > src/mm-base-modem.h | 4 ++ > src/mm-broadband-modem.c | 120 +++++++++++++++++++++++++++++++++++++++++++++-- > src/mm-broadband-modem.h | 4 ++ > src/mm-device.c | 15 ++++++ > src/mm-iface-modem.c | 56 ++++++++++++++++++++++ > src/mm-iface-modem.h | 17 +++++-- > 7 files changed, 233 insertions(+), 9 deletions(-) > > diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c > index 9cd7c5f..ede014b 100644 > --- a/src/mm-base-modem.c > +++ b/src/mm-base-modem.c > @@ -47,6 +47,7 @@ enum { > PROP_VENDOR_ID, > PROP_PRODUCT_ID, > PROP_CONNECTION, > + PROP_REPROBE, > PROP_LAST > }; > > @@ -70,6 +71,7 @@ struct _MMBaseModemPrivate { > > gboolean hotplugged; > gboolean valid; > + gboolean reprobe; > > guint max_timeouts; > > @@ -392,6 +394,24 @@ mm_base_modem_set_valid (MMBaseModem *self, > } > } > > +void > +mm_base_modem_set_reprobe (MMBaseModem *self, > + gboolean reprobe) > +{ > + g_return_if_fail (MM_IS_BASE_MODEM (self)); > + > + self->priv->reprobe = reprobe; > +} > + > +gboolean > +mm_base_modem_get_reprobe (MMBaseModem *self) > +{ > + g_return_val_if_fail (MM_IS_BASE_MODEM (self), FALSE); > + > + return self->priv->reprobe; > +} > + > + > gboolean > mm_base_modem_get_valid (MMBaseModem *self) > { > @@ -1295,6 +1315,9 @@ set_property (GObject *object, > case PROP_VALID: > mm_base_modem_set_valid (self, g_value_get_boolean (value)); > break; > + case PROP_REPROBE: > + mm_base_modem_set_reprobe (self, g_value_get_boolean (value)); > + break; > case PROP_MAX_TIMEOUTS: > self->priv->max_timeouts = g_value_get_uint (value); > break; > @@ -1338,6 +1361,9 @@ get_property (GObject *object, > case PROP_VALID: > g_value_set_boolean (value, self->priv->valid); > break; > + case PROP_REPROBE: > + g_value_set_boolean (value, self->priv->reprobe); > + break; > case PROP_MAX_TIMEOUTS: > g_value_set_uint (value, self->priv->max_timeouts); > break; > diff --git a/src/mm-base-modem.h b/src/mm-base-modem.h > index 3c0d16f..c4bcdce 100644 > --- a/src/mm-base-modem.h > +++ b/src/mm-base-modem.h > @@ -166,6 +166,10 @@ void mm_base_modem_set_valid (MMBaseModem *self, > gboolean valid); > gboolean mm_base_modem_get_valid (MMBaseModem *self); > > +void mm_base_modem_set_reprobe (MMBaseModem *self, > + gboolean reprobe); > +gboolean mm_base_modem_get_reprobe (MMBaseModem *self); > + > const gchar *mm_base_modem_get_device (MMBaseModem *self); > const gchar **mm_base_modem_get_drivers (MMBaseModem *self); > const gchar *mm_base_modem_get_plugin (MMBaseModem *self); > diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c > index 04c716d..3ebe8c5 100644 > --- a/src/mm-broadband-modem.c > +++ b/src/mm-broadband-modem.c > @@ -114,6 +114,7 @@ enum { > PROP_MODEM_MESSAGING_SMS_DEFAULT_STORAGE, > PROP_MODEM_VOICE_CALL_LIST, > PROP_MODEM_SIMPLE_STATUS, > + PROP_MODEM_SIM_HOT_SWAP_SUPPORTED, > PROP_LAST > }; > > @@ -126,7 +127,9 @@ typedef struct _PortsContext PortsContext; > struct _MMBroadbandModemPrivate { > /* Broadband modem specific implementation */ > PortsContext *enabled_ports_ctx; > + PortsContext *sim_hot_swap_ports_ctx; > gboolean modem_init_run; > + gboolean sim_hot_swap_supported; > > /*<--- Modem interface --->*/ > /* Properties */ > @@ -8549,6 +8552,11 @@ disabling_stopped (MMBroadbandModem *self, > ports_context_unref (self->priv->enabled_ports_ctx); > self->priv->enabled_ports_ctx = NULL; > } > + > + if (self->priv->sim_hot_swap_ports_ctx) { > + ports_context_unref (self->priv->sim_hot_swap_ports_ctx); > + self->priv->sim_hot_swap_ports_ctx = NULL; > + } > return TRUE; > } > > @@ -9667,6 +9675,7 @@ typedef enum { > INITIALIZE_STEP_IFACE_SIGNAL, > INITIALIZE_STEP_IFACE_OMA, > INITIALIZE_STEP_IFACE_FIRMWARE, > + INITIALIZE_STEP_SIM_HOT_SWAP, > INITIALIZE_STEP_IFACE_SIMPLE, > INITIALIZE_STEP_LAST, > } InitializeStep; > @@ -10008,6 +10017,38 @@ initialize_step (InitializeContext *ctx) > ctx); > return; > > + case INITIALIZE_STEP_SIM_HOT_SWAP: > + { > + gboolean is_sim_hot_swap_supported = FALSE; > + > + g_object_get (ctx->self, > + MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, > + &is_sim_hot_swap_supported, > + NULL); > + > + if (!is_sim_hot_swap_supported) { > + ctx->self->priv->sim_hot_swap_ports_ctx = NULL; > + } else { > + PortsContext *ports; > + GError *error = NULL; > + > + mm_dbg ("Creating PortsContext for SIM hot swap."); > + > + ports = g_new0 (PortsContext, 1); > + ports->ref_count = 1; > + > + 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); > + } > + } > + /* Fall down to next step */ > + ctx->step++; > + > case INITIALIZE_STEP_IFACE_SIMPLE: > if (ctx->self->priv->modem_state != MM_MODEM_STATE_FAILED) > mm_iface_modem_simple_initialize (MM_IFACE_MODEM_SIMPLE (ctx->self)); > @@ -10028,11 +10069,35 @@ initialize_step (InitializeContext *ctx) > "cannot fully initialize"); > } else { > /* Fatal SIM, firmware, or modem failure :-( */ > - g_simple_async_result_set_error (ctx->result, > - MM_CORE_ERROR, > - MM_CORE_ERROR_WRONG_STATE, > - "Modem is unusable, " > - "cannot fully initialize"); > + gboolean is_sim_hot_swap_supported = 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..."); > + 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"); > + } > + > /* 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. > @@ -10045,6 +10110,7 @@ initialize_step (InitializeContext *ctx) > mm_iface_modem_voice_shutdown (MM_IFACE_MODEM_VOICE (ctx->self)); > mm_iface_modem_time_shutdown (MM_IFACE_MODEM_TIME (ctx->self)); > mm_iface_modem_simple_shutdown (MM_IFACE_MODEM_SIMPLE (ctx->self)); > + > } > initialize_context_complete_and_free (ctx); > return; > @@ -10192,6 +10258,39 @@ mm_broadband_modem_create_device_identifier (MMBroadbandModem *self, > MM_GDBUS_MODEM (MM_BROADBAND_MODEM (self)->priv->modem_dbus_skeleton)))); > } > > + > +/*****************************************************************************/ > +static void > +after_hotswap_event_disable_ready (MMBaseModem *self, > + GAsyncResult *res, > + gpointer user_data) > +{ > + GError *error = NULL; > + mm_base_modem_disable_finish (self, res, &error); > + if (error) { > + mm_err ("Disable modem error: %s", error->message); > + g_error_free (error); > + } else { > + mm_base_modem_set_valid (MM_BASE_MODEM (self), FALSE); > + } > +} > + > + > +void > +mm_broadband_modem_update_sim_hot_swap_detected (MMBroadbandModem *self) > +{ > + if (self->priv->sim_hot_swap_ports_ctx) { > + mm_dbg ("Releasing SIM hot swap ports context"); > + ports_context_unref (self->priv->sim_hot_swap_ports_ctx); > + self->priv->sim_hot_swap_ports_ctx = NULL; > + } > + > + mm_base_modem_set_reprobe (MM_BASE_MODEM (self), TRUE); > + mm_base_modem_disable (MM_BASE_MODEM (self), > + (GAsyncReadyCallback) after_hotswap_event_disable_ready, > + NULL); > +} > + > /*****************************************************************************/ > > MMBroadbandModem * > @@ -10323,6 +10422,9 @@ set_property (GObject *object, > g_clear_object (&self->priv->modem_simple_status); > self->priv->modem_simple_status = g_value_dup_object (value); > break; > + case PROP_MODEM_SIM_HOT_SWAP_SUPPORTED: > + self->priv->sim_hot_swap_supported = g_value_get_boolean (value); > + break; > default: > G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); > break; > @@ -10425,6 +10527,9 @@ get_property (GObject *object, > case PROP_MODEM_SIMPLE_STATUS: > g_value_set_object (value, self->priv->modem_simple_status); > break; > + case PROP_MODEM_SIM_HOT_SWAP_SUPPORTED: > + g_value_set_boolean (value, self->priv->sim_hot_swap_supported); > + break; > default: > G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); > break; > @@ -10453,6 +10558,7 @@ mm_broadband_modem_init (MMBroadbandModem *self) > self->priv->modem_messaging_sms_default_storage = MM_SMS_STORAGE_UNKNOWN; > self->priv->current_sms_mem1_storage = MM_SMS_STORAGE_UNKNOWN; > self->priv->current_sms_mem2_storage = MM_SMS_STORAGE_UNKNOWN; > + self->priv->sim_hot_swap_supported = FALSE; > } > > static void > @@ -10907,4 +11013,8 @@ mm_broadband_modem_class_init (MMBroadbandModemClass *klass) > g_object_class_override_property (object_class, > PROP_MODEM_SIMPLE_STATUS, > MM_IFACE_MODEM_SIMPLE_STATUS); > + > + g_object_class_override_property (object_class, > + PROP_MODEM_SIM_HOT_SWAP_SUPPORTED, > + MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED); > } > diff --git a/src/mm-broadband-modem.h b/src/mm-broadband-modem.h > index 93ffeb5..9cb3494 100644 > --- a/src/mm-broadband-modem.h > +++ b/src/mm-broadband-modem.h > @@ -127,5 +127,9 @@ gboolean mm_broadband_modem_lock_sms_storages_finish (MMBroadbandModem *self, > void mm_broadband_modem_unlock_sms_storages (MMBroadbandModem *self, > gboolean mem1, > gboolean mem2); > +/* Helper to update SIM hot swap */ > +void mm_broadband_modem_update_sim_hot_swap_detected (MMBroadbandModem *self); > + > + > > #endif /* MM_BROADBAND_MODEM_H */ > diff --git a/src/mm-device.c b/src/mm-device.c > index 2456cc8..8435cf1 100644 > --- a/src/mm-device.c > +++ b/src/mm-device.c > @@ -489,8 +489,23 @@ modem_valid (MMBaseModem *modem, > MMDevice *self) > { > if (!mm_base_modem_get_valid (modem)) { > + GDBusObjectManagerServer *object_manager = self->priv->object_manager; > + > /* Modem no longer valid */ > mm_device_remove_modem (self); > + > + if (mm_base_modem_get_reprobe (modem)) { > + GError *error = NULL; > + > + if (!mm_device_create_modem (self, object_manager, &error)) { > + mm_warn ("Could not recreate modem for device at '%s': %s", > + mm_device_get_path (self), > + error ? error->message : "unknown"); > + g_error_free (error); > + } else { > + mm_dbg ("Modem recreated for device '%s'", mm_device_get_path (self)); > + } > + } > } else { > /* Modem now valid, export it, but only if we really have it around. > * It may happen that the initialization sequence fails because the > diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c > index 30fe20d..c942e7b 100644 > --- a/src/mm-iface-modem.c > +++ b/src/mm-iface-modem.c > @@ -3747,6 +3747,7 @@ typedef enum { > INITIALIZATION_STEP_SUPPORTED_BANDS, > INITIALIZATION_STEP_SUPPORTED_IP_FAMILIES, > INITIALIZATION_STEP_POWER_STATE, > + INITIALIZATION_STEP_SIM_HOT_SWAP, > INITIALIZATION_STEP_UNLOCK_REQUIRED, > INITIALIZATION_STEP_SIM, > INITIALIZATION_STEP_OWN_NUMBERS, > @@ -4203,6 +4204,27 @@ load_current_bands_ready (MMIfaceModem *self, > interface_initialization_step (ctx); > } > > +/*****************************************************************************/ > +/* Setup SIM hot swap (Modem interface) */ > +static void > +setup_sim_hot_swap_ready (MMIfaceModem *self, > + GAsyncResult *res, > + InitializationContext *ctx) { > + GError *error = NULL; > + > + MM_IFACE_MODEM_GET_INTERFACE (self)->setup_sim_hot_swap_finish (self, res, &error); > + if (error) { > + mm_warn ("Iface modem: SIM hot swap setup failed: '%s'", error->message); > + g_error_free (error); > + } else { > + mm_dbg ("Iface modem: SIM hot swap setup succeded"); > + } > + > + /* Go on to next step */ > + ctx->step++; > + interface_initialization_step (ctx); > +} > + > static void > interface_initialization_step (InitializationContext *ctx) > { > @@ -4544,6 +4566,18 @@ interface_initialization_step (InitializationContext *ctx) > /* Fall down to next step */ > ctx->step++; > > + case INITIALIZATION_STEP_SIM_HOT_SWAP: > + if (MM_IFACE_MODEM_GET_INTERFACE (ctx->self)->setup_sim_hot_swap && > + MM_IFACE_MODEM_GET_INTERFACE (ctx->self)->setup_sim_hot_swap_finish) { > + MM_IFACE_MODEM_GET_INTERFACE (ctx->self)->setup_sim_hot_swap ( > + MM_IFACE_MODEM (ctx->self), > + (GAsyncReadyCallback) setup_sim_hot_swap_ready, > + ctx); > + return; > + } > + /* Fall down to next step */ > + ctx->step++; > + > case INITIALIZATION_STEP_UNLOCK_REQUIRED: > /* Only check unlock required if we were previously not unlocked */ > if (mm_gdbus_modem_get_unlock_required (ctx->skeleton) != MM_MODEM_LOCK_NONE) { > @@ -4707,6 +4741,21 @@ interface_initialization_step (InitializationContext *ctx) > ctx->self); > > if (ctx->fatal_error) { > + if (g_error_matches (ctx->fatal_error, > + MM_MOBILE_EQUIPMENT_ERROR, > + MM_MOBILE_EQUIPMENT_ERROR_SIM_NOT_INSERTED)) { > + gboolean is_sim_hot_swap_supported = FALSE; > + > + g_object_get (ctx->self, > + MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, > + &is_sim_hot_swap_supported, > + NULL); > + > + if (is_sim_hot_swap_supported) { > + mm_iface_modem_update_failed_state (ctx->self, > + MM_MODEM_STATE_FAILED_REASON_SIM_MISSING); > + } > + } > g_simple_async_result_take_error (ctx->result, ctx->fatal_error); > ctx->fatal_error = NULL; > } else { > @@ -5119,6 +5168,13 @@ 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, > + "Sim Hot Swap Supported", > + "Whether the modem supports sim hot swap or not.", > + FALSE, > + G_PARAM_READWRITE)); > > initialized = TRUE; > } > diff --git a/src/mm-iface-modem.h b/src/mm-iface-modem.h > index 74ea9f9..517c651 100644 > --- a/src/mm-iface-modem.h > +++ b/src/mm-iface-modem.h > @@ -32,10 +32,11 @@ > #define MM_IS_IFACE_MODEM(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), MM_TYPE_IFACE_MODEM)) > #define MM_IFACE_MODEM_GET_INTERFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE ((obj), MM_TYPE_IFACE_MODEM, MMIfaceModem)) > > -#define MM_IFACE_MODEM_DBUS_SKELETON "iface-modem-dbus-skeleton" > -#define MM_IFACE_MODEM_STATE "iface-modem-state" > -#define MM_IFACE_MODEM_SIM "iface-modem-sim" > -#define MM_IFACE_MODEM_BEARER_LIST "iface-modem-bearer-list" > +#define MM_IFACE_MODEM_DBUS_SKELETON "iface-modem-dbus-skeleton" > +#define MM_IFACE_MODEM_STATE "iface-modem-state" > +#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" > > typedef struct _MMIfaceModem MMIfaceModem; > > @@ -327,6 +328,14 @@ struct _MMIfaceModem { > MMBaseBearer * (*create_bearer_finish) (MMIfaceModem *self, > GAsyncResult *res, > GError **error); > + /* Setup SIM hot swap */ > + void (*setup_sim_hot_swap) (MMIfaceModem *self, > + GAsyncReadyCallback callback, > + gpointer user_data); > + > + gboolean (*setup_sim_hot_swap_finish) (MMIfaceModem *self, > + GAsyncResult *res, > + GError **error); > }; > > GType mm_iface_modem_get_type (void); > -- > 2.7.4 > > _______________________________________________ > ModemManager-devel mailing list > <a href="mailto:ModemManager-devel@lists.freedesktop.org" target="_blank">ModemManager-devel@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel</a> -- Aleksander <a href="https://aleksander.es" rel="noreferrer" target="_blank">https://aleksander.es</a> </blockquote></div>