Den fre 2 aug. 2019 kl 13:04 skrev Denis Kenzior <denkenz(a)gmail.com>:
On 8/1/19 6:48 AM, richard.rojfors(a)gmail.com wrote:
> From: Richard Röjfors <richard(a)puffinpack.se>
> 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
> 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
> + * The only reason for not having a valid status is basically
> + * 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 ==
> attach = attach || (gprs->roaming_allowed &&
> @@ -3083,8 +3091,8 @@ struct ofono_gprs *ofono_gprs_create(struct
> - 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
> + 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
> 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
> 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
> netreg->driver = drv;
> + /* Check if we can init the status properly using the
> + if (netreg->driver->registration_status == NULL)
> + netreg->status =