Cinterion plugin - unsupported ^SIND="simstatus"

[Aleksander Morgado]

>> > >> The Cinterion plugin tries 'AT^SIND="simstatus",2' in after_sim_unlock(). I have two Cinterion modems, neither of which - according to their AT Command Set spec - support the simstatus indicator on this command, and so instead return "+CME ERROR: 21" (invalid index).
>> > >> Nonetheless, the operation continues with all 15 of its retries. Should there perhaps be a 'if (!response)' trap in simstatus_check_ready() so that it bombs out?
>> > >> I ask for comment, since I'm not sure (a) exactly what the retries are waiting for; and, in particular (b) what the response would be while its waiting. (I don't have a modem which does support the indicator).
>> > >>
>> > >
>> > > I imagine not even getting a good response can be treated as unsupported, let alone needing to parse it for ^SIND content . Hence
>> > >
>> > >
>> > >  plugins/cinterion/mm-broadband-modem-cinterion.c | 99 ++++++++++++++++++++----
>> > >  1 file changed, 6 insertions(+), 1 deletions(-)
>> > >
>> > > diff --git a/plugins/cinterion/mm-broadband-modem-cinterion.c b/plugins/cinterion/mm-broadband-modem-cinterion.c
>> > > index 64c5e08..1034d5c 100644
>> > > --- a/plugins/cinterion/mm-broadband-modem-cinterion.c
>> > > +++ b/plugins/cinterion/mm-broadband-modem-cinterion.c
>> > > @@ -1590,7 +1629,12 @@ simstatus_check_ready (MMBaseModem *self,
>> > >  const gchar *response;
>> > >
>> > >  response = mm_base_modem_at_command_finish (self, res, NULL);
>> > >
>> > >
>> > > * if (response) {
>> > >
>> > >
>> > > + if (!response) {
>> > > + /* "simstatus" not supported, go on anyway */
>> > > + g_task_return_boolean (task, TRUE);
>> > > + g_object_unref (task);
>> > > + return;
>> > > + } else {
>> > >  gchar *descr = NULL;
>> > >  guint val = 0;
>> > Does the modem support AT^SIND at all? Or just the "simstatus"
>> > indication isn't supported in the command?
>>
>> Just that simstatus isn't supported
>> > As with other things, the best way to maintain this upstream is to try
>> > to query first for the support of the specific feature and, if
>> > unsupported, ignore it from then on. In this case the support checks
>> > would be for either the whole command (e.g. AT^SIND=?) or for the
>> > actual "simstatus" indication (e.g. looking for "simstatus" in
>> > AT^SIND=? if that makes sense).
>>
>
> I would though put a case that: this command is only used in the Cinterion plugin, and no later knowledge of its outcome is currently needed - it seems to be just a 'pause step' in a sequence of other operations. 'psinfo' is also looked for elsewhwere, but again a fairly simple known/not-known operation.
> So your suggestion, whilst for sure its strictly correct and best, could perhaps be - at this time, at least - an unnecessary complication/overhead...?
> Respectfully yours etc :)

Well, I'm not sure. I do see in the logic that getting an error
currently triggers a 1s timeout before retrying the same command; with
that logic you're suggesting the timeout and retries would not be
done.

What kind of error do you get when running the AT^SIND command? Maybe
we could use the type of error to see whether we do the retries or
treat it as unsupported.

-- 
Aleksander
https://aleksander.es