> On 01 January 2018 at 11:27 Colin Helliwell wrote: > > > > On 01 January 2018 at 10:48 Aleksander Morgado wrote: > > > > > > On Sat, Dec 30, 2017 at 4:30 PM, Colin Helliwell > > wrote: > > > > > >> On 30 December 2017 at 15:07 Colin Helliwell wrote: > > >> > > >> > > >> 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 :) _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel