>> > >> 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 _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel