Hi Denis,

On 01/06/2012 00:45, Denis Kenzior wrote:
Hi Guillaume,

On 05/31/2012 04:59 AM, Guillaume Zajac wrote:
---
src/gprs.c | 84 +++++++++++++++++++++++++++++++++++++++++++++---------------
  1 files changed, 63 insertions(+), 21 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index 994607d..9ec0ca1 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -48,6 +48,7 @@

  #define GPRS_FLAG_ATTACHING 0x1
  #define GPRS_FLAG_RECHECK 0x2
+#define GPRS_FLAG_ATTACHED_UPDATE 0x4

  #define SETTINGS_STORE "gprs"
  #define SETTINGS_GROUP "Settings"
@@ -1440,6 +1441,44 @@ void ofono_gprs_resume_notify(struct ofono_gprs *gprs)
      update_suspended_property(gprs, FALSE);
  }

+static gboolean check_active_contexts(struct ofono_gprs *gprs)

Name this have_active_contexts

Ok


+{
+    GSList *l;
+    struct pri_context *ctx;
+
+    for (l = gprs->contexts; l; l = l->next) {
+        ctx = l->data;
+
+        if (ctx->active == TRUE)
+            return TRUE;
+    }
+
+    return FALSE;
+}
+
+static void release_active_contexts(struct ofono_gprs *gprs)
+{
+    GSList *l;
+    struct pri_context *ctx;
+
+    for (l = gprs->contexts; l; l = l->next) {
+        struct ofono_gprs_context *gc;
+
+        ctx = l->data;
+
+        if (ctx->active == FALSE)
+            continue;
+
+        /* This context is already being messed with */
+        if (ctx->pending)
+            continue;
+
+        gc = ctx->context_driver;
+
+        gc->driver->release_primary(gc, ctx->context.cid);

You better check that this driver function is implemented

Yes



+    }
+}
+
  static void gprs_attached_update(struct ofono_gprs *gprs)
  {
      DBusConnection *conn = ofono_dbus_get_connection();
@@ -1454,30 +1493,23 @@ static void gprs_attached_update(struct ofono_gprs *gprs)
      if (attached == gprs->attached)
          return;

-    gprs->attached = attached;
-
-    if (gprs->attached == FALSE) {
-        GSList *l;
-        struct pri_context *ctx;
-
-        for (l = gprs->contexts; l; l = l->next) {
-            ctx = l->data;
-
-            if (ctx->active == FALSE)
-                continue;
-
-            pri_reset_context_settings(ctx);
-            release_context(ctx);
-
-            value = FALSE;
-            ofono_dbus_signal_property_changed(conn, ctx->path,
-                    OFONO_CONNECTION_CONTEXT_INTERFACE,
-                    "Active", DBUS_TYPE_BOOLEAN,&value);
-        }
-
+    /*
+ * If an active context is found, a PPP session might be still active + * at driver level. "Attached" = TRUE property can't be signalled to
+     * the applications registered on GPRS properties.
+     * Active contexts have to be release at driver level.
+     */
+    if (attached == FALSE) {
+        release_active_contexts(gprs);
          gprs->bearer = -1;
+    } else if (check_active_contexts(gprs) == TRUE) {
+        gprs->flags |= GPRS_FLAG_ATTACHED_UPDATE;
+        release_active_contexts(gprs);

Why do we run the release operation here, wasn't it sufficient to run it once on the Attached goes Detached transition above?

Yes you are right, if context is still active it must be being shut down.


+        return;
      }

+    gprs->attached = attached;
+
      path = __ofono_atom_get_path(gprs->atom);
      value = attached;
      ofono_dbus_signal_property_changed(conn, path,
@@ -2266,6 +2298,16 @@ void ofono_gprs_context_deactivated(struct ofono_gprs_context *gc,
                      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 (gc->gprs->flags&  GPRS_FLAG_ATTACHED_UPDATE) {
+        gc->gprs->flags&= ~GPRS_FLAG_ATTACHED_UPDATE;
+        gprs_attached_update(gc->gprs);
+    }
  }

  int ofono_gprs_context_driver_register(

Overall I like this approach, but lets get this tested really well.

Regards,
-Denis


Kind regards,
Guillaume
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono

Reply via email to