So the logic required is just this?

AT+CSCA?
  “xxxxxxxxxxxx”,145
  OK
AT+CSCA=xxxxxxxxxxxx
  OK

It would be good to confirm whether this is needed before each SMS sent or just once on boot.

If it's required just once on boot, we could override "check_support" command in the Messaging interface in the Huawei plugin, so that it first runs the generic (parent) check_support, and if supported, we run this extra sequence required.

An example of the check_support override in the Messaging interface can be found in the "messaging_check_support" function in the cinterion plugin (mm-broadband-modem-cinterion.c). In this case it's not calling the parent check first, but there are several other examples of how that would be done with other parent interfaces in the same file, e.g. iface_modem_parent->load_supported_modes() is calling the parent load_supported_modes() implementation.

Actually, it's even simpler than that.  From Minicom (with ">" and "<" to represent input/output"):

> AT+CSCA?
< +CSCA: "+xxxxxxxxxxx",145

< OK
> AT+CSCA="+xxxxxxxxxxx",145
< OK

If I then subsequently start ModemManager, SMS messages can be sent without supplying an SMSC, and the modem seems to behave properly until the next device disconnect/reconnect.  As such, I think your suggestion about running these lines in "check_support" should work.  I will take a look at the code you mention and see if I can get something running!

Created attachment 132727 [details] [review]
Proposed patch for SMSC issue

Here's a first attempt at a patch for this issue.  Any feedback on how I can improve it is of course most welcome!

As noted in the comments, I encountered a case where blindly setting the SMSC value returned by "AT+CSCA?" causes issues: in some cases the modem responds with something like the following:

--> 'AT+CSCA?<CR>'
<-- '<CR><LF>+CSCA: "002B00350035003500350035003500350035003500350035",145<CR><LF><CR><LF>OK<CR><LF>'

The "002B00350035003500350035003500350035003500350035" part is a hex-encoding of "+55555555555" (I've substituted the real string I see for an equivalent one).  If modemmanager executes 'AT+CSCA="002B00350035003500350035003500350035003500350035",145' the modem will freeze/crash, so this is to be avoided.

I think the code could be changed to parse the string, get the actual SMSC as text, and then use that in a "AT+CSCA=" call, but this appears unnecessary, as it seems that whenever the SMSC is reported in this weird format, the bug is no longer present -- i.e. the SMSC is only ever reported in this format after an "AT+CSCA=..." command has already been executed.  Given this, in the patch I haven't tried to parse the hex strings when I see them.

The only other issue I can think of with this is that the check its doing to look for a hex string is the presence of a "0" at the start of the number.  I reason for this is that I wanted to cover both the case of a 129 and 145 address type (based on http://www.developershome.com/sms/cscaCommand.asp).  My understanding is that neither of these can start with zero, but I'm not 100% sure on this.

To be clear, the 002B00350035003500350035003500350035003500350035" (equivalent) string returned by the modem always parses to the default SMSC as set on the modem previously, so it's not some random number or anything.

That hex is the UCS2 encoded SMSC address, and the modem will report it if the default configured encoding is UCS2. Unfortunately, we load/set the current charset a bit afterwards, during the Modem interface *enabling* step (see initializing vs enabling here https://www.freedesktop.org/software/ModemManager/api/latest/ref-overview-modem-state-machine.html).

So, I'm thinking that instead of doing the setup during the "check_support" async method in the initialization phase, we could do it in the enabling phase of the Messaging interface, with its own callback, e.g. after "setup_sms_format" is executed for example. In that way, when you read the string from the AT+CSCA? response, you can use mm_broadband_modem_take_and_convert_to_utf8() to convert the string to UTF-8 (takes into account the configured charset in the modem) and then when setting it you can always default to using UTF-8, as that will likely always work being the CSCA number just alpha+numeric characters.

What do you think?

Another option would be to run this hack just before the first time we try to send an SMS with the modem... And this may actually be a much better idea, although it involves creating a MMSmsHuawei object and subclassing the send() method there (i.e. check if the CSCA setting has already been done, and if not, do it just before sending the SMS).

Also, I wouldn't make this hack E220-only.

Ah, OK thanks for the explanation!

I'm happy to try either of the proposed approaches.  It sounds like you favour the latter approach - creating a subclassed SMS send method - so I will give that a go.  Is there an existing precedent in one of the other plugins that I might usefully use as a starting point for this?

(In reply to Alastair Pharo from comment #7)
> Ah, OK thanks for the explanation!
> 
> I'm happy to try either of the proposed approaches.  It sounds like you
> favour the latter approach - creating a subclassed SMS send method - so I
> will give that a go.  Is there an existing precedent in one of the other
> plugins that I might usefully use as a starting point for this?

Not really, I don't think any other plugin subclass-es SMS objects; but they do subclass e.g. SIM objects, which is really the same idea.

You also have the QMI and MBIM SMS subclasses (in src/) that you may find useful.

In this specific case, we just want a subclass that leaves all parent methods untouched except for the send() method, for which we do the CSCA check and then we call the parent send().

> In this specific case, we just want a subclass that leaves all parent
> methods untouched except for the send() method, for which we do the CSCA
> check and then we call the parent send().

BTW; you'll reach a point in the logic where you will need to have the modem object "store" somehow whether the CSCA hack has already been done or not, and you won't be able to e.g. have a boolean in the private info of the modem as the logic you're implementing is in the SMS object, out of the modem object. 

Instead, you can use g_object_set_data() and g_object_get_data() to set/get a boolean tag (with GBOOLEAN_TO_POINTER() and GPOINTER_TO_BOOLEAN) named e.g. "TAG_CSCA_CHECK_DONE" or something like that.

Created attachment 135582 [details] [review]
Subclassing SMS objects

Hi,

I finally got around to trying this subclassing method out.  Everything seems to work insofar at the "CSCA?" and "CSCA=..." commands get executed at the right time, and the UCS2 is parsed by mm_broadband_modem_take_and_convert_to_utf8 correctly.  I'm hoping you can take a look at what I've done so far, and let me know if there's anything obvious I've missed out in terms of doing this correctly.

For whatever reason, the "CSCA=+......,145" call always seems to result in an error from the modem, so I don't think this patch as it stands actually solves the issue (I need to do more testing to confirm this though).

I am wondering if perhaps I should try switching the modem out of UCS2 charset mode, performing the hack, and then switching it back.  In the intervening months I've been using my previous patch, and that seems to work in all cases (i.e. setting before switching to UCS2)

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mobile-broadband/ModemManager/issues/20.