Hi Denis, Den fre 2 aug. 2019 kl 13:04 skrev Denis Kenzior <[email protected]>:
> Hi Richard, > > On 8/1/19 6:48 AM, [email protected] wrote: > > From: Richard Röjfors <[email protected]> > > > > Previously the valid "unknown" registration status was set > > during startup, but its a bit problematic for gprs. > > There might be cases where a LTE context is activated > > before netreg is finished updating its status. > > And then gprs could make faulty actions. > > Instead we set the status to -1 when the drivers supports > > fetching the status manually, which is done during startup. > > During the time the status is -1, gprs postpones actions until > > the status is valid (>= 0). > > --- > > src/gprs.c | 12 ++++++++++-- > > src/network.c | 5 ++++- > > 2 files changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/src/gprs.c b/src/gprs.c > > index bdb8c8a2..07802c36 100644 > > --- a/src/gprs.c > > +++ b/src/gprs.c > > @@ -1661,6 +1661,14 @@ static void gprs_netreg_update(struct ofono_gprs > *gprs) > > { > > ofono_bool_t attach; > > > > + /* This function can get called by other reasons than netreg > > + * updating its status. So check if we have a valid netreg status > yet. > > + * The only reason for not having a valid status is basically > during > > + * startup while the netreg atom is fetching the status. > > + */ > > Note doc/coding-style.txt item M2 for our commenting style... > Will fix the comment! > > > + if (gprs->netreg_status < 0) > > + return; > > + > > attach = gprs->netreg_status == > NETWORK_REGISTRATION_STATUS_REGISTERED; > > > > attach = attach || (gprs->roaming_allowed && > > @@ -3083,8 +3091,8 @@ struct ofono_gprs *ofono_gprs_create(struct > ofono_modem *modem, > > break; > > } > > > > - gprs->status = NETWORK_REGISTRATION_STATUS_UNKNOWN; > > - gprs->netreg_status = NETWORK_REGISTRATION_STATUS_UNKNOWN; > > + gprs->status = -1; > > Why do you need to mess with this? It seems to be not used > This was just of convenience reasons so it would look the same for gprs and netreg status. > > > + gprs->netreg_status = -1; > > gprs->used_pids = l_uintset_new(MAX_CONTEXTS); > > > > Wouldn't changes to gprs.c be sufficient, without touching netreg.c? > No unfortunately not. gprs reads the netreg status when it attaches to the atom. Which would be unknown if we do not touch netreg.c. .. and unknown is a valid status which can not be distinguished from "not yet updated". > > > return gprs; > > diff --git a/src/network.c b/src/network.c > > index 2824eae6..1db01bcf 100644 > > --- a/src/network.c > > +++ b/src/network.c > > @@ -1874,7 +1874,7 @@ struct ofono_netreg *ofono_netreg_create(struct > ofono_modem *modem, > > if (netreg == NULL) > > return NULL; > > > > - netreg->status = NETWORK_REGISTRATION_STATUS_UNKNOWN; > > + netreg->status = -1; > > Okay, but a minor problem with this is that there now exists a brief > moment in type where the Status property has a weird value... > I looked at this, and most statuses, even some defined results as unknown. Any unhandled values as negative would do that too. If you are refering to the D-Bus property? > > > netreg->location = -1; > > netreg->cellid = -1; > > netreg->technology = -1; > > @@ -1893,6 +1893,9 @@ struct ofono_netreg *ofono_netreg_create(struct > ofono_modem *modem, > > continue; > > > > netreg->driver = drv; > > + /* Check if we can init the status properly using the > driver */ > > + if (netreg->driver->registration_status == NULL) > > + netreg->status = > NETWORK_REGISTRATION_STATUS_UNKNOWN; > > break; > > } > > > > > --Richard
_______________________________________________ ofono mailing list [email protected] https://lists.ofono.org/mailman/listinfo/ofono
