On Mon, Mar 7, 2016 at 10:07 AM, Carlo Lobrano <[email protected]> wrote: > I am working on a fix for Bug 93135 - Telit HE910: `+CPMS="",...` fails as > "Operation Not Supported". > The problem is that Telit modems does not support empty strings as +CPMS > parameter, > while in MM the empty parameter is used in order to keep the current memory > configuration for > that field (like AT+CPMS="","ME") > > I followed the hint given by Aleksander in a previous e-mail in order to > have mem1 set to the current > supported storage, if not set otherwise, in place of being unknown > >> [...] what I would do, is to provide in MMBroadbandModemClass a new >> async virtual method to "load_current_sms_mem_storage()" (or something >> like that). The logic in MMBroadbandModem which checks which are the >> supported storages could then have a new step to query which is the >> current storage (which we only need to do once), all this in the >> MMBroadbandModem class. > > however the same kind of AT command is emitted in sms_store function > (mm-sms-base.c), where > mm_broadband_modem_lock_sms_storages is called with mem1 explicitly set to > UNKNOWN: > > mm_broadband_modem_lock_sms_storages ( > MM_BROADBAND_MODEM (self->priv->modem), > MM_SMS_STORAGE_UNKNOWN, /* none required for mem1 */ > ctx->storage, > (GAsyncReadyCallback)store_lock_sms_storages_ready, > ctx); > > I do not get why none is required for mem1 in this case and I would like to > know if can be considered > safe to use the value of "current_sms_mem1_storage" when the local value of > mem1 in > mm_broadband_modem_lock_sms_storages is UNKNOWN
"mem1" is for reading/listing/deleting messages; "mem2" is for storing/sending. In different operations we may want to lock different mems. In your specific question, none is required for mem1 in sms_store() because we're storing only, so we don't need to select explicitly a given storage for mem1 (which is why we give the empty field in mem1). If Telit modems don't support the mem1 field being empty, what you need to provide instead of an empty string is the current mem1 value, so that it effectively doesn't get changed. That's, IIRC, why I was suggesting the load_current_sms_mem_storage(): you run it once at the beginning (e.g. once we have listed the supported memory storages) and from then on we keep track of which one is selected in all the next operations we do, so that "current_sms_mem1_storage" has always the correct value (and so that you can use "current_sms_mem1_storage" instead of an empty string). Currently the only moment in which "current_sms_mem1_storage" may not have the correct value is upon boot, before mm_broadband_modem_lock_sms_storages() is called with a non-UNKNOWN "mem1" parameter. E.g. we currently do this: current_sms_mem1_storage = UNKNOWN mm_broadband_modem_lock_sms_storages (mem1=SM) current_sms_mem1_storage = SM mm_broadband_modem_lock_sms_storages (mem1=ME) current_sms_mem1_storage = ME ... If we never called mm_broadband_modem_lock_sms_storages() with a mem1 parameter different to UNKNOWN, then current_sms_mem1_storage would always be UNKNOWN, and you don't want that. What I think we want is: current_sms_mem1_storage = UNKNOWN load_current_sms_mem_storage() current_sms_mem1_storage = SM <--- as read from the modem mm_broadband_modem_lock_sms_storages (mem1=SM) current_sms_mem1_storage = SM mm_broadband_modem_lock_sms_storages (mem1=ME) current_sms_mem1_storage = ME ... And now that I see it... this may not be an issue after all, because we do try to list all SMS messages during the Messaging interface enabling step, and that means that we're automatically calling mm_broadband_modem_lock_sms_storages() with different mem1 values... Could you validate that that is what's happening? Have you tried to implement the fix by just re-using the "current_sms_mem1_storage" value instead of an empty string in AT+CPMS? Did it work? -- Aleksander https://aleksander.es _______________________________________________ ModemManager-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
