[PATCH v2] Fixed wrong MEM1 value passed to +CPMS

Carlo Lobrano c.lobrano at gmail.com
Fri Mar 24 11:48:19 UTC 2017


Let modem_messaging_set_default_storage returns with error
if current_sms_mem1_storage is MM_SMS_STORAGE_UNKNOWN

---

I updated the patch according the code review, moreover I'd like
to note a little difference respect the previous version:

currently, even if +CPMS fails because of the wrong value for
mem1, we do store "storage" as the current_sms_mem2_storage value.

     self->priv->current_sms_mem2_storage = storage;

In this patch I preferred to change this behaviour and setting default
storage for SMS mem2 only if we are actually going to set
this value on the modem.

What do you think about it?

---
 src/mm-broadband-modem.c       | 29 +++++++++++++++++++----------
 src/mm-iface-modem-messaging.c |  2 +-
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 17b3253..0bbf626 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -5590,22 +5590,31 @@ modem_messaging_set_default_storage (MMIfaceModemMessaging *_self,
     gchar *mem1_str;
     gchar *mem_str;
 
-    result = g_simple_async_result_new (G_OBJECT (self),
-                                        callback,
-                                        user_data,
-                                        modem_messaging_set_default_storage);
+    /* We provide the current sms storage for mem1 if not UNKNOWN */
+    if (self->priv->current_sms_mem1_storage == MM_SMS_STORAGE_UNKNOWN) {
+        g_simple_async_report_error_in_idle (G_OBJECT (self),
+                                             callback,
+                                             user_data,
+                                             MM_CORE_ERROR,
+                                             MM_CORE_ERROR_INVALID_ARGS,
+                                             "Current SMS MEM1 storage is UNKNOWN (SIM busy?)");
+        return;
+    }
 
     /* Set defaults as current */
     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);
-
     mem_str = g_ascii_strup (mm_sms_storage_get_string (storage), -1);
-    cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\",\"%s\"",
-                           mem1_str ? mem1_str : "",
-                           mem_str,
-                           mem_str);
+
+    cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\",\"%s\"", mem1_str, mem_str, mem_str);
+
+    result = g_simple_async_result_new (G_OBJECT (self),
+                                        callback,
+                                        user_data,
+                                        modem_messaging_set_default_storage);
+
+
     mm_base_modem_at_command (MM_BASE_MODEM (self),
                               cmd,
                               3,
diff --git a/src/mm-iface-modem-messaging.c b/src/mm-iface-modem-messaging.c
index 0cff1f2..01a30f0 100644
--- a/src/mm-iface-modem-messaging.c
+++ b/src/mm-iface-modem-messaging.c
@@ -789,7 +789,7 @@ set_default_storage_ready (MMIfaceModemMessaging *self,
     GError *error = NULL;
 
     if (!MM_IFACE_MODEM_MESSAGING_GET_INTERFACE (self)->set_default_storage_finish (self, res, &error)) {
-        mm_dbg ("Couldn't set default storage: '%s'", error->message);
+        mm_warn ("Could not set default storage: '%s'", error->message);
         g_error_free (error);
     }
 
-- 
2.9.3



More information about the ModemManager-devel mailing list