cinterion: modification to fetching unlock retries

[Aleksander Morgado]

Hey Colin!

>> >> > > Hi, I just wondered if there was any appetite for re-visiting this issue. The latest 'GTask' rework to the plugin means I'll need to redo accordingly the patch I've been using - but thought I'd enquire before embarking on that.
>> >> > You're talking about the parallel enables issue, or using AT^SPIC to
>> >> > fetch unlock retries?
>> >> > What patch are you currently using, could you share it somewhere?
>> >>
>> >> Sorry - pasted the wrong URL! See instead:
>> >> https://lists.freedesktop.org/archives/modemmanager-devel/2017-April/004485.html
>> >> I didn't get to a point where I thought my patch would be mainstream-acceptable (it just 'works for me'), but I can post in entirety if necessary. It's essentially as per my original post in the thread.
>> >
>> >
>> > I've modified my patch for a later 'gtask oriented' Master version of MM (66dce6dacc440d8bfe4270562ef5a840c87e5a04 - Dec 5), which I'd like to re-offer for review/comment? (and inclusion?)
>> >
>> >
>> > As I write, it's almost working except two of the queries are sometimes returning "reference data not found" errors:
>> > AT+CSIM=10,"0020000100" -> 63C3
>> > AT+CSIM=10,"002C000100" -> 63CA
>> > AT+CSIM=10,"0020008100" -> 6A88
>> > AT+CSIM=10,"002C008100" -> 6A88
>> >
>> > It seems to be some sort of timing issue (on the Cinterion EHS5) - sometimes I get all four responses successfully, sometimes one/other/both of the last two fail.
>> > I'll try to seek out any telling differences in the logs, but any thoughts as to what sequence/timing might cause this in the modem....? (varying registration time? SIM not fully initialized?)
>>
>> Forgot to attach the patch? :)
>>
>
> Ah well, yes - was waiting to see if there was any interest :)
> Attached here.
>

Looks good!

> To try to build in some method of flexibility, I've added a second 'map' of commands, using CSIM.
> There's a couple of #define's to allow for changing whether SPIC or CSIM is tried first.
> If the *first* step of the first map fails, then it switches over and tries the second instead.
>
> I realise my approach may not be to everyone's/anyone's liking. It would be simpler to just add the CSIM commands to the existing map and let it roll through them all, hopefully assembling all values correctly, but I didn't want to prejudge (e.g. if the response to one method contradicts the response to the other!) and possibly open up other problems. However that *would* allow for modems which only support a *mix* of methods, if there are any.

I think we should keep both maps separate, and only try CSIM if SPIC
fails. Actually, see below.

> What are people's thoughts? I'd like to have a fix of some sort in the mainstream code (since I need it, and have been caught out once by my private patch becoming outdated over time by other changes)
>

Yes, let's provide a common solution for this.

> In any case, the helper function is needed for parsing the CSIM responses - I think this is still the same as in the telit plugin. Could it be a generic function?
>

Yes, definitely, and we should actually have a generic method based on
CSIM to load unlock retries. I believe this is what we should be
doing:

 1) Write a generic load_unlock_retries() method in MMBroadbandModem,
based on CSIM only (i.e. just the CSIM map with the 4 commands and
iterate over them to build the resulting MMUnlockRetries). The parser
helper would also be generic, i.e. in mm-modem-helpers.[c|h]. All
AT-based modems would try this method to load unlock retries.
 2) Make the Cinterion implementation run its SPIC based logic, and if
you detect errors with that logic, the plugin falls back to run the
"parent" object implementation (see e.g. load_supported_modes() in the
same Cinterion plugin on how to call the parent method). This would
make the Cinterion plugin use SPIC if supported and otherwise run the
parent CSIM-based logic implemented in step 1.
 3) The Telit implementation is a bit more complex than the generic
one. In Telit we support CSIM locking/unlocking operations, before
running the CSIM sequence of commands, we run CSIM=1 to lock and once
finished, we run CSIM=0 to unlock. The sequence itself is the same as
with the generic implementation, though. So, we could also reuse the
generic one, but in this case, we do the CSIM locking (if supported)
then we run the parent implementation, then we run CSIM unlocking.

Doing this we would end up with one single CSIM based implementation
in the generic modem, and the cinterion and telit plugins would use
that same implementation as needed.

What do you think? Does this make sense?

-- 
Aleksander
https://aleksander.es