Hi Denis, Den tors 15 aug. 2019 kl 22:47 skrev Denis Kenzior <[email protected]>:
> Hi Richard, > > On 8/14/19 1:24 PM, [email protected] wrote: > > From: Richard Röjfors <[email protected]> > > > > Since we have a different condition for the attach state > > when running on LTE, we should consider it in gprs_attached_update. > > Previously it's done in some instances. But for instance if > > the driver got detached from GPRS but now running on LTE with a > > context up, we would be deattached. > > So are you talking about a hand-over? Do we have logs to help visualize > this? > Sorry I have no logs. I think an example would be; Let say we are registered on non-LTE, and GPRS do attach. Then we flip to LTE and if GPRS gets de-attached. We would brutally exit the attached state, since LTE is currently not considered in that code path. > > > --- > > src/gprs.c | 52 +++++++++++++++++++++++++++++----------------------- > > 1 file changed, 29 insertions(+), 23 deletions(-) > > > > diff --git a/src/gprs.c b/src/gprs.c > > index bba482cb..94d13f82 100644 > > --- a/src/gprs.c > > +++ b/src/gprs.c > > @@ -1571,13 +1571,34 @@ static void release_active_contexts(struct > ofono_gprs *gprs) > > } > > } > > > > +static gboolean on_lte(struct ofono_gprs *gprs) > > +{ > > + if (ofono_netreg_get_technology(gprs->netreg) == > > + ACCESS_TECHNOLOGY_EUTRAN && > have_read_settings(gprs)) > > + return TRUE; > > + > > + return FALSE; > > +} > > + > > static void gprs_attached_update(struct ofono_gprs *gprs) > > { > > ofono_bool_t attached; > > + int status = gprs->status; > > > > - attached = gprs->driver_attached && > > - (gprs->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > > - gprs->status == > NETWORK_REGISTRATION_STATUS_ROAMING); > > + if (on_lte(gprs)) > > + /* > > + * For LTE we attached status reflects successful context > > + * activation. > > + * Since we in gprs_netreg_update not even try to attach > > + * to GPRS if we are running on LTE, we can on some modems > > + * expect the gprs status to be unknown. That must not > > + * result in detaching... > > + */ > > + attached = have_active_contexts(gprs); > > On LTE Attached should just be always True. Someone can correct me if > I'm wrong, but a default context is a pre-requisite for E-UTRAN attachment. > I guess this is about race conditions, since we read back settings of the context, so in theory I think we might not consider the context available yet. Should be a very short period of time. In netreg status we do not immediately change to attached in case of LTE, we require the context settings to have been read first. > > > + else > > + attached = gprs->driver_attached && > > + (status == NETWORK_REGISTRATION_STATUS_REGISTERED > || > > + status == > NETWORK_REGISTRATION_STATUS_ROAMING); > > > > if (attached == gprs->attached) > > return; > > @@ -1591,11 +1612,14 @@ static void gprs_attached_update(struct > ofono_gprs *gprs) > > if (attached == FALSE) { > > release_active_contexts(gprs); > > gprs->bearer = -1; > > - } else if (have_active_contexts(gprs) == TRUE) { > > + } else if (have_active_contexts(gprs) == TRUE && > > + !on_lte(gprs)) { > > Not our style. See item M4. Might want to see if it fits on the > previous line. > Will updat! > > > /* > > * Some times the context activates after a detach event > and > > * right before an attach. We close it to avoid unexpected > open > > * contexts. > > + * Skip that for LTE since the condition to be attached on > LTE > > + * is that a context gets activated > > */ > > release_active_contexts(gprs); > > gprs->flags |= GPRS_FLAG_ATTACHED_UPDATE; > > @@ -1660,15 +1684,6 @@ static void gprs_netreg_removed(struct ofono_gprs > *gprs) > > gprs_attached_update(gprs); > > } > > > > -static gboolean on_lte(struct ofono_gprs *gprs) > > -{ > > - if (ofono_netreg_get_technology(gprs->netreg) == > > - ACCESS_TECHNOLOGY_EUTRAN && > have_read_settings(gprs)) > > - return TRUE; > > - > > - return FALSE; > > -} > > - > > static void gprs_netreg_update(struct ofono_gprs *gprs) > > { > > ofono_bool_t attach; > > @@ -2573,16 +2588,7 @@ void ofono_gprs_status_notify(struct ofono_gprs > *gprs, int status) > > > > if (status != NETWORK_REGISTRATION_STATUS_REGISTERED && > > status != NETWORK_REGISTRATION_STATUS_ROAMING) { > > - /* > > - * For LTE we attached status reflects successful context > > - * activation. > > - * Since we in gprs_netreg_update not even try to attach > > - * to GPRS if we are running on LTE, we can on some modems > > - * expect the gprs status to be unknown. That must not > > - * result in detaching... > > - */ > > - if (!on_lte(gprs)) > > - gprs_attached_update(gprs); > > + gprs_attached_update(gprs); > > return; > > } > > > > > > Otherwise I'm fine moving the on_lte logic into gprs_attached_update > itself... > > Regards, > -Denis >
_______________________________________________ ofono mailing list [email protected] https://lists.ofono.org/mailman/listinfo/ofono
