Re: [PATCH] atmodem: implement standard PIN retries

[Marcel Holtmann <marcel@holtmann.org>]

[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]

Hi Remi,

> > >  drivers/atmodem/sim.c |   48
> > >  ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 48
> > >  insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
> > > index 0938998..dfd40f8 100644
> > > --- a/drivers/atmodem/sim.c
> > > +++ b/drivers/atmodem/sim.c
> > > @@ -57,6 +57,7 @@ static const char *cpin_prefix[] = { "+CPIN:", NULL };
> > > 
> > >  static const char *clck_prefix[] = { "+CLCK:", NULL };
> > >  static const char *huawei_cpin_prefix[] = { "^CPIN:", NULL };
> > >  static const char *xpincnt_prefix[] = { "+XPINCNT:", NULL };
> > > 
> > > +static const char *cpinr_prefixes[] = { "+CPINR:", "+CPINRE:", NULL };
> > 
> > if you are not processing +CPINRE, why bother consuming it in the first
> > place?
> 
> So that they're not misinterpreted as unsolicited responses.

they would be. Do you care?

> > >  static const char *none_prefix[] = { NULL };
> > >  
> > >  static void at_crsm_info_cb(gboolean ok, GAtResult *result, gpointer
> > >  user_data)
> > > 
> 
> > > @@ -577,6 +622,9 @@ static void at_pin_retries_query(struct ofono_sim
> > > *sim,
> > > 
> > >  		break;
> > >  	
> > >  	default:
> > > +		if (g_at_chat_send(sd->chat, "AT+CPINR", cpinr_prefixes,
> > > +					at_cpinr_cb, cbd, g_free) > 0)
> > > +			return;
> > 
> > since this command is pretty new, don't we need to check if it is
> > supported? Is there any impact not checking it?
> 
> This will just defer the (failure) callback until the ERROR response.

Okay. If there is no impact here I am fine with it.

Regards

Marcel