<div dir="ltr">> So why is current_sms_mem1_storage UNKNOWN? At which point did it > fail, if it did? Did it fail doing AT+CPMS?<div> </div><div>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</div>> storage, but defaulting to "SM" isn't a good approach either. [...]<div>> If it is UNKNOWN because init_current_storages() failed, we should > also error out when trying to set_default_storage() <div> </div><div>I understand that. So, set_default_storage should return error directly when it finds that mem1 is UNKNOWN. Is that right?</div></div></div> <div class="gmail_quote"><div dir="ltr">On Thu, 23 Mar 2017 at 17:16 Aleksander Morgado <<a href="mailto:aleksander@aleksander.es">aleksander@aleksander.es</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, Mar 23, 2017 at 2:29 PM, Carlo Lobrano <<a href="mailto:c.lobrano@gmail.com" class="gmail_msg" target="_blank">c.lobrano@gmail.com</a>> 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 > <a href="mailto:ModemManager-devel@lists.freedesktop.org" class="gmail_msg" target="_blank">ModemManager-devel@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel" rel="noreferrer" class="gmail_msg" target="_blank">https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel</a> -- Aleksander <a href="https://aleksander.es" rel="noreferrer" class="gmail_msg" target="_blank">https://aleksander.es</a> </blockquote></div>