Hi Denis,

Den fre 16 aug. 2019 kl 17:55 skrev Denis Kenzior <denkenz@gmail.com>:
Hi Richard,

On 8/16/19 5:44 AM, Richard Röjfors wrote:
> Hi Denis,
>
> Den tors 15 aug. 2019 kl 22:47 skrev Denis Kenzior <denkenz@gmail.com
> <mailto:denkenz@gmail.com>>:
>
>     Hi Richard,
>
>     On 8/14/19 1:24 PM, richard.rojfors@gmail.com
>     <mailto:richard.rojfors@gmail.com> wrote:
>      > From: Richard Röjfors <richard@puffinpack.se
>     <mailto:richard@puffinpack.se>>
>      >
>      > 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.

Okay, so handover.  It would be helpful to have logs to help visualize
what is happening.

As I said, I'm afraid I have no logs for this. I tried to visualize how it could happen.

But anyway I think it makes more sense to "calculate" the attached state in one place, 
(gprs_attached_update) and one of the places this gets called from is when GPRS detaches.
 

>
>
>      > ---
>      >   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.

I think it makes sense to signal Attached right away in the case that we
know we are on LTE.  But I can accept your argument of not signaling
Attached until the first context is active.  That is likely more driven
by ConMan not being updated for LTE, which likely won't work in the end.

And I would like to add, that this how things are today, its nothing I have or
inteded to change.
I just want to make sure we don't get deattached
because we miss to check for LTE in some places in the code.