cinterion: modification to fetching unlock retries

[Colin Helliwell]

> On 28 December 2017 at 17:00 Aleksander Morgado wrote:
> 
> 
> 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?
> 

Sounds fine to me. I hadn't realised that only Cinterion plugin uses ^SPIC - are these/equivalent not fetched at all for modems other than Cinterion/Telit? (or with a different cmd?)