Observed issue:

    With Telit Modem HE910, all commands like `AT+CPMS="",...` fail as "Operation Not Supported"::

    <debug> [1448531013.833175] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): --> 'AT+CPMS="","ME","ME"<CR>'
    <debug> [1448531013.902282] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): <-- '<CR><LF>+CMS ERROR: 303<CR><LF>'
    <debug> [1448531013.933761] [mm-serial-parsers.c:364] mm_serial_parser_v1_parse(): Got failure code 303: Operation not supported
    <debug> [1448531013.967401] [mm-iface-modem-messaging.c:792] set_default_storage_ready(): Couldn't set default storage: 'Operation not supported'



Following is my analisys and a possible patch:

ModemManager tries to set default storages for MEM2 and MEM3 only with `+CPMS` providing an empty string for MEM1 argument::

    <debug> [1448531013.833175] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): --> 'AT+CPMS="","ME","ME"<CR>'
    <debug> [1448531013.902282] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): <-- '<CR><LF>+CMS ERROR: 303<CR><LF>'
    <debug> [1448531013.933761] [mm-serial-parsers.c:364] mm_serial_parser_v1_parse(): Got failure code 303: Operation not supported
    <debug> [1448531013.967401] [mm-iface-modem-messaging.c:792] set_default_storage_ready(): Couldn't set default storage: 'Operation not supported'

Some modems do support NULL arguments (considering that as a request to do not change the correspondant value), but passing an empty string should be considered invalid, since it is not a "supported value"::

    <debug> [1448531007.342611] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): --> 'AT+CPMS=?<CR>'
    <debug> [1448531007.389244] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): <-- '<CR><LF>'
    <debug> [1448531007.418215] [mm-port-serial-at.c:440] debug_log(): (ttyACM0): <-- '+CPMS: ("SM","ME"),("SM","ME"),("SM","ME")<CR><LF><CR><LF>OK<CR><LF>'

According to the previous logs, only SM and ME are supported strings, so the command should at most be `AT+CPMS=,"ME","ME"`

However, HE910 does not support NULL argument either, so I made the following changes in order to provide always the current value of MEM1, if no other value is provided,
this way the code works fine with HE910 and should work with the other modems too, since it's conservative::

    diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
    index af24248..34407d4 100644
    --- a/src/mm-broadband-modem.c
    +++ b/src/mm-broadband-modem.c
    @@ -5361,8 +5361,9 @@ mm_broadband_modem_lock_sms_storages (MMBroadbandModem *self,
             ctx->previous_mem1 = self->priv->current_sms_mem1_storage;
             self->priv->mem1_storage_locked = TRUE;
             self->priv->current_sms_mem1_storage = mem1;
    -        mem1_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem1_storage), -1);
         }
    +    mem1_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem1_storage), -1);
    +    g_assert(mem1_str != NULL);
     
         if (mem2 != MM_SMS_STORAGE_UNKNOWN) {
             ctx->mem2_locked = TRUE;
    @@ -5380,7 +5381,7 @@ mm_broadband_modem_lock_sms_storages (MMBroadbandModem *self,
     
         if (mem2_str)
             cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\"",
    -                               mem1_str ? mem1_str : "",
    +                               mem1_str,
                                    mem2_str);
         else if (mem1_str)
             cmd = g_strdup_printf ("+CPMS=\"%s\"", mem1_str);
    @@ -5434,25 +5435,32 @@ modem_messaging_set_default_storage (MMIfaceModemMessaging *_self,
         MMBroadbandModem *self = MM_BROADBAND_MODEM (_self);
         gchar *cmd;
         GSimpleAsyncResult *result;
    -    gchar *mem_str;
    +    gchar *mem1_str;
    +    gchar *mem2_str;
    +    MMSmsStorage current_mem1;
    +
     
         result = g_simple_async_result_new (G_OBJECT (self),
                                             callback,
                                             user_data,
                                             modem_messaging_set_default_storage);
     
    +    current_mem1 = self->priv->current_sms_mem1_storage? self->priv->current_sms_mem1_storage : storage;
         /* Set defaults as current */
         self->priv->current_sms_mem2_storage = storage;
     
    -    mem_str = g_ascii_strup (mm_sms_storage_get_string (storage), -1);
    -    cmd = g_strdup_printf ("+CPMS=\"\",\"%s\",\"%s\"", mem_str, mem_str);
    +    mem1_str = g_ascii_strup (mm_sms_storage_get_string (current_mem1), -1);
    +    mem2_str = g_ascii_strup (mm_sms_storage_get_string (storage), -1);
    +
    +    cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\",\"%s\"", mem1_str, mem2_str, mem2_str);
         mm_base_modem_at_command (MM_BASE_MODEM (self),
                                   cmd,
                                   3,
                                   FALSE,
                                   (GAsyncReadyCallback)cpms_set_ready,
                                   result);
    -    g_free (mem_str);
    +    g_free (mem1_str);
    +    g_free (mem2_str);
         g_free (cmd);
     }


Let me know if I misunderstood something and/or I can improve this patch somehow.

Best regards,
Carlo