[PATCH] Fixed wrong MEM1 value passed to +CPMS

[Carlo Lobrano]

> So why is current_sms_mem1_storage UNKNOWN? At which point did it
> fail, if it did? Did it fail doing AT+CPMS?

Correct. AT+CPMS? returns with SIM Busy. It's something that will be better
addressed when "delay till #QSS: 3" will be implemented.

> I agree that we shouldn't call any CPMS command passing "unknown" as
> storage, but defaulting to "SM" isn't a good approach either. [...]
> If it is UNKNOWN because init_current_storages() failed, we should
> also error out when trying to set_default_storage()

I understand that. So, set_default_storage should return error directly
when it finds that mem1 is UNKNOWN. Is that right?

On Thu, 23 Mar 2017 at 17:16 Aleksander Morgado <aleksander at aleksander.es>
wrote:

> On Thu, Mar 23, 2017 at 2:29 PM, Carlo Lobrano <c.lobrano at gmail.com>
> wrote:
> > Let mem1_str defaulting to NULL when current_sms_mem1_storage is
> > MM_SMS_STORAGE_UNKNOWN
> >
> > modem_messaging_set_default_storage considers mem1 storage
> > string valid if *not NULL*, but, previously, when we could not get
> > current_sms_mem1_storage value right, mem1_str defaulted to "unknown"
> > and that string was used in +CPMS command, resulting in an ERROR.
> >
>
> The current_sms_mem1_storage shouldn't be UNKNOWN unless:
>   * init_current_storages() hasn't been run yet
>   * init_current_storages () failed.
>
> So why is current_sms_mem1_storage UNKNOWN? At which point did it
> fail, if it did? Did it fail doing AT+CPMS? in
> modem_messaging_init_current_storages()? Or is it that
> set_default_storage() was called before init_current_storages() was
> executed?
>
> I agree that we shouldn't call any CPMS command passing "unknown" as
> storage, but defaulting to "SM" isn't a good approach either. We
> actually test which are the supported storages at some point
> (modem_messaging_load_supported_storages()) to avoid assuming any of
> them is always available.
>
> If it is UNKNOWN because init_current_storages() failed, we should
> also error out when trying to set_default_storage()
>
> > ---
> >  src/mm-broadband-modem.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> > index 17b3253..776e6d1 100644
> > --- a/src/mm-broadband-modem.c
> > +++ b/src/mm-broadband-modem.c
> > @@ -5587,7 +5587,7 @@ modem_messaging_set_default_storage
> (MMIfaceModemMessaging *_self,
> >      MMBroadbandModem *self = MM_BROADBAND_MODEM (_self);
> >      gchar *cmd;
> >      GSimpleAsyncResult *result;
> > -    gchar *mem1_str;
> > +    gchar *mem1_str = NULL;
> >      gchar *mem_str;
> >
> >      result = g_simple_async_result_new (G_OBJECT (self),
> > @@ -5599,11 +5599,14 @@ modem_messaging_set_default_storage
> (MMIfaceModemMessaging *_self,
> >      self->priv->current_sms_mem2_storage = storage;
> >
> >      /* We provide the current sms storage for mem1 if not UNKNOWN */
> > -    mem1_str = g_ascii_strup (mm_sms_storage_get_string
> (self->priv->current_sms_mem1_storage), -1);
> > +    if (self->priv->current_sms_mem1_storage != MM_SMS_STORAGE_UNKNOWN)
> {
> > +        mem1_str = g_ascii_strup (mm_sms_storage_get_string
> (self->priv->current_sms_mem1_storage), -1);
> > +    }
> >
> >      mem_str = g_ascii_strup (mm_sms_storage_get_string (storage), -1);
> >      cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\",\"%s\"",
> > -                           mem1_str ? mem1_str : "",
> > +                           /* Empty value is not accepted and SM is the
> default one */
> > +                           mem1_str ? mem1_str : "SM",
> >                             mem_str,
> >                             mem_str);
> >      mm_base_modem_at_command (MM_BASE_MODEM (self),
> > --
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170323/200fd865/attachment-0001.html>