12:58 a.m.
From: Richard Röjfors <richard(a)puffinpack.se>
There are cases where the gprs status might updated to for instance
"unknown" while LTE is the bearer.
In that case we should not set the attach state to FALSE,
since then running LTE the conext activation reflects the attached
state.
---
src/gprs.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 9cb69d14..1a9b234d 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1660,6 +1660,15 @@ static void gprs_netreg_removed(struct ofono_gprs *gprs)
gprs_attached_update(gprs);
}
+static gboolean expect_lte_context_auto_activation(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;
@@ -1682,8 +1691,7 @@ static void gprs_netreg_update(struct ofono_gprs *gprs)
DBG("attach: %u, driver_attached: %u", attach, gprs->driver_attached);
- if (ofono_netreg_get_technology(gprs->netreg) ==
- ACCESS_TECHNOLOGY_EUTRAN && have_read_settings(gprs))
+ if (expect_lte_context_auto_activation(gprs))
/*
* For LTE we set attached status only on successful
* context activation.
@@ -2560,7 +2568,12 @@ void ofono_gprs_status_notify(struct ofono_gprs *gprs, int status)
if (status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
status != NETWORK_REGISTRATION_STATUS_ROAMING) {
- gprs_attached_update(gprs);
+ /*
+ * For LTE we attached status reflects successful context
+ * activation.
+ */
+ if (!expect_lte_context_auto_activation(gprs))
+ gprs_attached_update(gprs);
return;
}
--
2.20.1
12:57 a.m.
Hi Richard,
On 8/11/19 9:58 AM, richard.rojfors(a)gmail.com wrote:
From: Richard Röjfors <richard(a)puffinpack.se>
There are cases where the gprs status might updated to for instance
"unknown" while LTE is the bearer.
In that case we should not set the attach state to FALSE,
since then running LTE the conext activation reflects the attached
state.
It might be nice to include a log snippet of how the current behavior
fails...
---
src/gprs.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 9cb69d14..1a9b234d 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1660,6 +1660,15 @@ static void gprs_netreg_removed(struct ofono_gprs *gprs)
gprs_attached_update(gprs);
}
+static gboolean expect_lte_context_auto_activation(struct ofono_gprs *gprs)
I'm not thrilled about the name? Isn't this just 'on_lte?' or something
like that?
+{
+ 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;
@@ -1682,8 +1691,7 @@ static void gprs_netreg_update(struct ofono_gprs *gprs)
DBG("attach: %u, driver_attached: %u", attach, gprs->driver_attached);
- if (ofono_netreg_get_technology(gprs->netreg) ==
- ACCESS_TECHNOLOGY_EUTRAN && have_read_settings(gprs))
+ if (expect_lte_context_auto_activation(gprs))
/*
* For LTE we set attached status only on successful
* context activation.
@@ -2560,7 +2568,12 @@ void ofono_gprs_status_notify(struct ofono_gprs *gprs, int
status)
if (status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
status != NETWORK_REGISTRATION_STATUS_ROAMING) {
- gprs_attached_update(gprs);
+ /*
+ * For LTE we attached status reflects successful context
+ * activation.
+ */
So I don't get this. We have CREG telling us: "we're on LTE" and CGREG
telling us "Oh we don't know what our status is". Why do you trust one
and not the other? Are you sure this doesn't belong in a driver quirk
somewhere?
+ if (!expect_lte_context_auto_activation(gprs))
+ gprs_attached_update(gprs);
return;
}
Regards,
-Denis
4:37 a.m.
Hi Denis,
Den mån 12 aug. 2019 kl 18:58 skrev Denis Kenzior <denkenz(a)gmail.com>:
Hi Richard,
On 8/11/19 9:58 AM, richard.rojfors(a)gmail.com wrote:
> From: Richard Röjfors <richard(a)puffinpack.se>
>
> There are cases where the gprs status might updated to for instance
> "unknown" while LTE is the bearer.
> In that case we should not set the attach state to FALSE,
> since then running LTE the conext activation reflects the attached
> state.
It might be nice to include a log snippet of how the current behavior
fails...
I'm not sure I can dig one up right now.
The case is that we are running LTE a context gets auto activated,
we don't bother to attach to GPRS (since we are on LTE), we get notified
that the
status is unknown -> BAM, we get detached, eventhough we are on LTE and the
context is up.
> ---
> src/gprs.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/src/gprs.c b/src/gprs.c
> index 9cb69d14..1a9b234d 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -1660,6 +1660,15 @@ static void gprs_netreg_removed(struct ofono_gprs
*gprs)
> gprs_attached_update(gprs);
> }
>
> +static gboolean expect_lte_context_auto_activation(struct ofono_gprs
*gprs)
I'm not thrilled about the name? Isn't this just 'on_lte?' or something
like that?
Sounds better.
> +{
> + 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;
> @@ -1682,8 +1691,7 @@ static void gprs_netreg_update(struct ofono_gprs
*gprs)
>
> DBG("attach: %u, driver_attached: %u", attach,
gprs->driver_attached);
>
> - if (ofono_netreg_get_technology(gprs->netreg) ==
> - ACCESS_TECHNOLOGY_EUTRAN &&
have_read_settings(gprs))
> + if (expect_lte_context_auto_activation(gprs))
> /*
> * For LTE we set attached status only on successful
> * context activation.
> @@ -2560,7 +2568,12 @@ void ofono_gprs_status_notify(struct ofono_gprs
*gprs, int status)
>
> if (status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
> status != NETWORK_REGISTRATION_STATUS_ROAMING) {
> - gprs_attached_update(gprs);
> + /*
> + * For LTE we attached status reflects successful context
> + * activation.
> + */
So I don't get this. We have CREG telling us: "we're on LTE" and
CGREG
telling us "Oh we don't know what our status is". Why do you trust one
and not the other? Are you sure this doesn't belong in a driver quirk
somewhere?
When CREG tells us we are on LTE, we will probably not even try attach to
gprs (CGATT),
refer to gprs_netreg_update.
In my understanding that means that its natural that CGREG reports unknown
or similar.
CEREG would be the interesting one... but since the code is built around
using activated context as condition when running on LTE I think we should
use the same condition here.
> + if (!expect_lte_context_auto_activation(gprs))
> + gprs_attached_update(gprs);
> return;
> }
>
>
Regards,
-Denis
5:57 a.m.
Hi Richard,
So I don't get this. We have CREG telling us:
"we're on LTE" and CGREG
telling us "Oh we don't know what our status is". Why do you trust
one
and not the other? Are you sure this doesn't belong in a driver quirk
somewhere?
When CREG tells us we are on LTE, we will probably not even try attach
to gprs (CGATT),
refer to gprs_netreg_update.
In my understanding that means that its natural that CGREG reports
unknown or similar.
CEREG would be the interesting one... but since the code is built around
using activated context as condition when running on LTE I think we should
use the same condition here.
Ah, so you're on one of those modems that started using CEREG instead of
CGREG for reporting LTE. So in that case I'm fine with this (maybe add
a more clarifying comment). But I suspect that we also need to consider
adding ofono_gprs_lte_status as well and taking both into account.
Regards,
-Denis
6:03 a.m.
Hi Denis,
Den mån 12 aug. 2019 kl 21:57 skrev Denis Kenzior <denkenz(a)gmail.com>:
Hi Richard,
> So I don't get this. We have CREG telling us: "we're on LTE"
and
CGREG
> telling us "Oh we don't know what our status is". Why do you
trust
one
> and not the other? Are you sure this doesn't belong in a driver
quirk
> somewhere?
>
>
> When CREG tells us we are on LTE, we will probably not even try attach
> to gprs (CGATT),
> refer to gprs_netreg_update.
> In my understanding that means that its natural that CGREG reports
> unknown or similar.
> CEREG would be the interesting one... but since the code is built around
> using activated context as condition when running on LTE I think we
should
> use the same condition here.
Ah, so you're on one of those modems that started using CEREG instead of
CGREG for reporting LTE. So in that case I'm fine with this (maybe add
a more clarifying comment). But I suspect that we also need to consider
adding ofono_gprs_lte_status as well and taking both into account.
I will refine the comment, and yes I think we need lte_status at some point.
There might be a chance we can do it without, but only when LTE is the
"other case", when more techologies are introduced we definitely need
a status per tech.
I have more in the pipeline regarding this, I'm kind of working step by step
to get LTE to work properly and with hand over to come.
--Richard
6:37 a.m.
New subject: [PATCH v2] gprs: Do not detach when running LTE and GPRS is unavailable.
From: Richard Röjfors <richard(a)puffinpack.se>
There are cases where the gprs status might updated to for instance
"unknown" while LTE is the bearer.
In that case we should not set the attach state to FALSE,
since then running LTE the conext activation reflects the attached
state.
---
src/gprs.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 9cb69d14..f7a2b739 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1660,6 +1660,15 @@ 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;
@@ -1682,8 +1691,7 @@ static void gprs_netreg_update(struct ofono_gprs *gprs)
DBG("attach: %u, driver_attached: %u", attach, gprs->driver_attached);
- if (ofono_netreg_get_technology(gprs->netreg) ==
- ACCESS_TECHNOLOGY_EUTRAN && have_read_settings(gprs))
+ if (on_lte(gprs))
/*
* For LTE we set attached status only on successful
* context activation.
@@ -2560,7 +2568,16 @@ void ofono_gprs_status_notify(struct ofono_gprs *gprs, int status)
if (status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
status != NETWORK_REGISTRATION_STATUS_ROAMING) {
- gprs_attached_update(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...
+ */
+ if (!on_lte(gprs))
+ gprs_attached_update(gprs);
return;
}
--
2.20.1
6:47 a.m.
New subject: [PATCH v2] gprs: Do not detach when running LTE and GPRS is unavailable.
Hi Richard,
On 8/12/19 3:37 PM, richard.rojfors(a)gmail.com wrote:
From: Richard Röjfors <richard(a)puffinpack.se>
There are cases where the gprs status might updated to for instance
"unknown" while LTE is the bearer.
In that case we should not set the attach state to FALSE,
since then running LTE the conext activation reflects the attached
state.
---
src/gprs.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
Applied, thanks.
Regards,
-Denis
1133
days inactive
1134
days old
7 comments
3 participants
participants (3)
-
Denis Kenzior -
Richard Röjfors -
richard.rojfors＠gmail.com