[PATCH] altair-lte: prevent connect while processing sim refresh
Thieu Le
thieule at chromium.org
Wed May 28 13:57:45 PDT 2014
SGTM
On Wed, May 28, 2014 at 1:56 PM, Dan Williams <dcbw at redhat.com> wrote:
> On Wed, 2014-05-28 at 13:36 -0700, Thieu Le wrote:
>> I think I prefer having a more generic busy error code versus a
>> separate busy property due to the racy nature of properties:
>>
>> Eg.
>> 1. Client queries busy property.
>> 2. Modem indicates not busy.
>> 3. Client sends request.
>> 4. Modem became busy before receiving client request.
>
> You've convinced me :) It turns out we actually already have
> MM_CORE_ERROR/MM_CORE_ERROR_RETRY (Operation cannot yet be performed,
> retry later.) which I think would work well for this. Would you agree?
>
> Dan
>
>>
>> On Wed, May 28, 2014 at 1:00 PM, Dan Williams <dcbw at redhat.com> wrote:
>> > On Tue, 2014-05-27 at 14:27 -0700, Thieu Le wrote:
>> >> This patch modifies the Altair LTE plugin to fail connect requests while the
>> >> modem has detached from the network when processing sim refresh.
>> >
>> > So the patch looks fine to me, but I wonder if we shouldn't have a
>> > generic ModemManager error code for "try this again later". Or do
>> > people think SIM_BUSY is good enough for that? My thought is that if
>> > something else is blocking connect for some reason (like maybe a network
>> > scan is happening, and we need to wait for it to finish) that SIM_BUSY
>> > isn't appropriate for that reason, but a generic "wait" error would be.
>> > Just trying to make it easier to clients to handle this case, since they
>> > shouldn't really care that the SIM is busy vs. something else being
>> > busy.
>> >
>> > Alternatively, we could add another modem property to indicate
>> > busy/not-busy, that clients could use to know when they could send a new
>> > request?
>> >
>> > Dan
>> >
>> >> ---
>> >> plugins/altair/mm-broadband-bearer-altair-lte.c | 14 ++++++++++++++
>> >> plugins/altair/mm-broadband-modem-altair-lte.c | 19 ++++++++++++++++++-
>> >> plugins/altair/mm-broadband-modem-altair-lte.h | 2 ++
>> >> 3 files changed, 34 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/plugins/altair/mm-broadband-bearer-altair-lte.c b/plugins/altair/mm-broadband-bearer-altair-lte.c
>> >> index 4594d32..46e8da4 100644
>> >> --- a/plugins/altair/mm-broadband-bearer-altair-lte.c
>> >> +++ b/plugins/altair/mm-broadband-bearer-altair-lte.c
>> >> @@ -196,6 +196,20 @@ connect_3gpp (MMBroadbandBearer *self,
>> >> return;
>> >> }
>> >>
>> >> + /* Don't allow a connect while we detach from the network to process SIM
>> >> + * refresh.
>> >> + * */
>> >> + if (mm_broadband_modem_altair_lte_is_sim_refresh_detach_in_progress (modem)) {
>> >> + mm_dbg ("Detached from network to process SIM refresh, failing connect request");
>> >> + g_simple_async_report_error_in_idle (G_OBJECT (self),
>> >> + callback,
>> >> + user_data,
>> >> + MM_MOBILE_EQUIPMENT_ERROR,
>> >> + MM_MOBILE_EQUIPMENT_ERROR_SIM_BUSY,
>> >> + "Detached from network to process SIM refresh, can't connect.");
>> >> + return;
>> >> + }
>> >> +
>> >> data = mm_base_modem_peek_best_data_port (MM_BASE_MODEM (modem), MM_PORT_TYPE_NET);
>> >> if (!data) {
>> >> g_simple_async_report_error_in_idle (G_OBJECT (self),
>> >> diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c
>> >> index 77e2137..8240185 100644
>> >> --- a/plugins/altair/mm-broadband-modem-altair-lte.c
>> >> +++ b/plugins/altair/mm-broadband-modem-altair-lte.c
>> >> @@ -59,6 +59,10 @@ struct _MMBroadbandModemAltairLtePrivate {
>> >> * This indicates that there are no more SIM refreshes and we should
>> >> * reregister the device.*/
>> >> guint sim_refresh_timer_id;
>> >> + /* Flag indicating that we are detaching from the network to process SIM
>> >> + * refresh. This is used to prevent connect requests while we're in this
>> >> + * state.*/
>> >> + gboolean sim_refresh_detach_in_progress;
>> >> /* Regex for bearer related notifications */
>> >> GRegex *statcm_regex;
>> >> /* Regex for PCO notifications */
>> >> @@ -639,6 +643,7 @@ altair_reregister_ready (MMBaseModem *self,
>> >> } else {
>> >> mm_dbg ("Modem reregistered successfully");
>> >> }
>> >> + MM_BROADBAND_MODEM_ALTAIR_LTE (self)->priv->sim_refresh_detach_in_progress = FALSE;
>> >> }
>> >>
>> >> static void
>> >> @@ -648,6 +653,7 @@ altair_deregister_ready (MMBaseModem *self,
>> >> {
>> >> if (!mm_base_modem_at_command_finish (self, res, NULL)) {
>> >> mm_dbg ("Deregister modem failed");
>> >> + MM_BROADBAND_MODEM_ALTAIR_LTE (self)->priv->sim_refresh_detach_in_progress = FALSE;
>> >> return;
>> >> }
>> >>
>> >> @@ -681,6 +687,10 @@ altair_load_own_numbers_ready (MMIfaceModem *iface_modem,
>> >> g_strfreev (str_list);
>> >> }
>> >>
>> >> + /* Set this flag to prevent connect requests from being processed while we
>> >> + * detach from the network.*/
>> >> + self->priv->sim_refresh_detach_in_progress = TRUE;
>> >> +
>> >> /* Deregister */
>> >> mm_dbg ("Reregistering modem");
>> >> mm_base_modem_at_command (
>> >> @@ -703,8 +713,8 @@ altair_sim_refresh_timer_expired (MMBroadbandModemAltairLte *self)
>> >> MM_IFACE_MODEM (self),
>> >> (GAsyncReadyCallback)altair_load_own_numbers_ready,
>> >> self);
>> >> -
>> >> self->priv->sim_refresh_timer_id = 0;
>> >> +
>> >> return FALSE;
>> >> }
>> >>
>> >> @@ -1408,6 +1418,12 @@ mm_broadband_modem_altair_lte_new (const gchar *device,
>> >> NULL);
>> >> }
>> >>
>> >> +gboolean
>> >> +mm_broadband_modem_altair_lte_is_sim_refresh_detach_in_progress (MMBroadbandModem *self)
>> >> +{
>> >> + return MM_BROADBAND_MODEM_ALTAIR_LTE (self)->priv->sim_refresh_detach_in_progress;
>> >> +}
>> >> +
>> >> static void
>> >> mm_broadband_modem_altair_lte_init (MMBroadbandModemAltairLte *self)
>> >> {
>> >> @@ -1419,6 +1435,7 @@ mm_broadband_modem_altair_lte_init (MMBroadbandModemAltairLte *self)
>> >>
>> >> self->priv->sim_refresh_regex = g_regex_new ("\\r\\n\\%NOTIFYEV:\\s*SIMREFRESH,?(\\d*)\\r+\\n",
>> >> G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>> >> + self->priv->sim_refresh_detach_in_progress = FALSE;
>> >> self->priv->sim_refresh_timer_id = 0;
>> >> self->priv->statcm_regex = g_regex_new ("\\r\\n\\%STATCM:\\s*(\\d*),?(\\d*)\\r+\\n",
>> >> G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>> >> diff --git a/plugins/altair/mm-broadband-modem-altair-lte.h b/plugins/altair/mm-broadband-modem-altair-lte.h
>> >> index 50c9f3e..fc8d362 100644
>> >> --- a/plugins/altair/mm-broadband-modem-altair-lte.h
>> >> +++ b/plugins/altair/mm-broadband-modem-altair-lte.h
>> >> @@ -48,4 +48,6 @@ MMBroadbandModemAltairLte *mm_broadband_modem_altair_lte_new (const gchar *devic
>> >> guint16 vendor_id,
>> >> guint16 product_id);
>> >>
>> >> +gboolean mm_broadband_modem_altair_lte_is_sim_refresh_detach_in_progress (MMBroadbandModem *self);
>> >> +
>> >> #endif /* MM_BROADBAND_MODEM_ALTAIR_LTE_H */
>> >
>> >
>> _______________________________________________
>> ModemManager-devel mailing list
>> ModemManager-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
>
>
More information about the ModemManager-devel
mailing list