[PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[Slava Monich]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

This prevents attached state from getting stuck at 0 like this:

1. Context deactivation is initiated over D-Bus, ctx->pending is set
2. Attached becomes FALSE, context is still marked as active
3. Attached becomes TRUE, gprs_attached_update sets GPRS_FLAG_ATTACHED_UPDATE
4. Deactivation completes, attached is 0, driver_attached is 1

Futher network status updates don't call gprs_attached_update because
driver_attached is still 1, so attached is staying 0 until we lose the
data registration again which may not happen for quite a long time.
---
 src/gprs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/gprs.c b/src/gprs.c
index 4aa00f9..5f7620b 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -135,6 +135,7 @@ struct pri_context {
 	struct ofono_gprs *gprs;
 };
 
+static void gprs_attached_update(struct ofono_gprs *gprs);
 static void gprs_netreg_update(struct ofono_gprs *gprs);
 static void gprs_deactivate_next(struct ofono_gprs *gprs);
 
@@ -946,6 +947,16 @@ static void pri_deactivate_callback(const struct ofono_error *error, void *data)
 	ofono_dbus_signal_property_changed(conn, ctx->path,
 					OFONO_CONNECTION_CONTEXT_INTERFACE,
 					"Active", DBUS_TYPE_BOOLEAN, &value);
+
+	/*
+	 * If "Attached" property was about to be signalled as TRUE but there
+	 * were still active contexts, try again to signal "Attached" property
+	 * to registered applications after active contexts have been released.
+	 */
+	if (ctx->gprs->flags & GPRS_FLAG_ATTACHED_UPDATE) {
+		ctx->gprs->flags &= ~GPRS_FLAG_ATTACHED_UPDATE;
+		gprs_attached_update(ctx->gprs);
+	}
 }
 
 static void pri_read_settings_callback(const struct ofono_error *error,
-- 
1.9.1

Re: [PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

Hi Slava,

On 11/03/2016 02:12 PM, Slava Monich wrote:
> This prevents attached state from getting stuck at 0 like this:
>
> 1. Context deactivation is initiated over D-Bus, ctx->pending is set
> 2. Attached becomes FALSE, context is still marked as active
> 3. Attached becomes TRUE, gprs_attached_update sets GPRS_FLAG_ATTACHED_UPDATE
> 4. Deactivation completes, attached is 0, driver_attached is 1
>
> Futher network status updates don't call gprs_attached_update because
> driver_attached is still 1, so attached is staying 0 until we lose the
> data registration again which may not happen for quite a long time.
> ---
>   src/gprs.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

Hi Slava,

On 11/01/2016 10:53 AM, Slava Monich wrote:
> This prevents attached state from getting stuck at 0 like this:
>
> 1. Context deactivation is initiated over D-Bus, ctx->pending is set
> 2. Attached becomes FALSE, context is still marked as active
> 3. Attached becomes TRUE, gprs_attached_update sets GPRS_FLAG_ATTACHED_UPDATE
> 4. Deactivation completes, attached is 0, driver_attached is 1
>
> Futher network status updates don't call gprs_attached_update because
> driver_attached is still 1, so attached is staying 0 until we lose the
> data registration again which may not happen for quite a long time.
> ---
>   src/gprs.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/src/gprs.c b/src/gprs.c
> index 3a4a819..3d71398 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -135,6 +135,7 @@ struct pri_context {
>   	struct ofono_gprs *gprs;
>   };
>
> +static void gprs_attached_update(struct ofono_gprs *gprs);
>   static void gprs_netreg_update(struct ofono_gprs *gprs);
>   static void gprs_deactivate_next(struct ofono_gprs *gprs);
>
> @@ -946,6 +947,16 @@ static void pri_deactivate_callback(const struct ofono_error *error, void *data)
>   	ofono_dbus_signal_property_changed(conn, ctx->path,
>   					OFONO_CONNECTION_CONTEXT_INTERFACE,
>   					"Active", DBUS_TYPE_BOOLEAN, &value);
> +
> +	/*
> +	 * If "Attached" property was about to be signalled as TRUE but there
> +	 * were still active contexts, try again to signal "Attached" property
> +	 * to registered applications after active contexts have been released.
> +	 */
> +	if (ctx->gprs && (ctx->gprs->flags & GPRS_FLAG_ATTACHED_UPDATE)) {

Can we get rid of the ctx->gprs check.  ctx->gprs is set to NULL only if:

1. Context was never added, in which case you can't call SetProperty on 
it anyway.
2. context was unregistered, in which case the driver shouldn't even be 
able to call the callback, and even if it did, we'd be crashing earlier 
since pending was already replied to.
3. gprs atom itself was unregistered

> +		ctx->gprs->flags &= ~GPRS_FLAG_ATTACHED_UPDATE;
> +		gprs_attached_update(ctx->gprs);
> +	}
>   }
>
>   static void pri_read_settings_callback(const struct ofono_error *error,
>

Regards,
-Denis

[PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[Slava Monich]

[-- Attachment #1: Type: text/plain, Size: 1738 bytes --]

This prevents attached state from getting stuck at 0 like this:

1. Context deactivation is initiated over D-Bus, ctx->pending is set
2. Attached becomes FALSE, context is still marked as active
3. Attached becomes TRUE, gprs_attached_update sets GPRS_FLAG_ATTACHED_UPDATE
4. Deactivation completes, attached is 0, driver_attached is 1

Futher network status updates don't call gprs_attached_update because
driver_attached is still 1, so attached is staying 0 until we lose the
data registration again which may not happen for quite a long time.
---
 src/gprs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/gprs.c b/src/gprs.c
index 3a4a819..3d71398 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -135,6 +135,7 @@ struct pri_context {
 	struct ofono_gprs *gprs;
 };
 
+static void gprs_attached_update(struct ofono_gprs *gprs);
 static void gprs_netreg_update(struct ofono_gprs *gprs);
 static void gprs_deactivate_next(struct ofono_gprs *gprs);
 
@@ -946,6 +947,16 @@ static void pri_deactivate_callback(const struct ofono_error *error, void *data)
 	ofono_dbus_signal_property_changed(conn, ctx->path,
 					OFONO_CONNECTION_CONTEXT_INTERFACE,
 					"Active", DBUS_TYPE_BOOLEAN, &value);
+
+	/*
+	 * If "Attached" property was about to be signalled as TRUE but there
+	 * were still active contexts, try again to signal "Attached" property
+	 * to registered applications after active contexts have been released.
+	 */
+	if (ctx->gprs && (ctx->gprs->flags & GPRS_FLAG_ATTACHED_UPDATE)) {
+		ctx->gprs->flags &= ~GPRS_FLAG_ATTACHED_UPDATE;
+		gprs_attached_update(ctx->gprs);
+	}
 }
 
 static void pri_read_settings_callback(const struct ofono_error *error,
-- 
1.9.1