> 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 <[email protected]> wrote: > On Thu, Mar 23, 2017 at 2:29 PM, Carlo Lobrano <[email protected]> > 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 > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel > > > > -- > Aleksander > https://aleksander.es >
_______________________________________________ ModemManager-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
