[PATCH 1/1] emit settings signal before active signal

[PATCH 1/1] emit settings signal before active signal

[Martin Xu]

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

---
 src/gprs.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index b533209..57bb794 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -374,18 +374,19 @@ static void pri_activate_callback(const struct ofono_error *error,
 	__ofono_dbus_pending_reply(&gc->pending,
 				dbus_message_new_method_return(gc->pending));
 
-	value = ctx->active;
-	ofono_dbus_signal_property_changed(conn, ctx->path,
-						DATA_CONTEXT_INTERFACE,
-						"Active", DBUS_TYPE_BOOLEAN,
-						&value);
-
 	/* If we don't have the interface, don't bother emitting any settings,
 	 * as nobody can make use of them
 	 */
 	if (interface != NULL)
 		pri_update_context_settings(ctx, interface, static_ip,
 						ip, netmask, gateway, dns);
+
+	value = ctx->active;
+	ofono_dbus_signal_property_changed(conn, ctx->path,
+						DATA_CONTEXT_INTERFACE,
+						"Active", DBUS_TYPE_BOOLEAN,
+						&value);
+
 }
 
 static void pri_deactivate_callback(const struct ofono_error *error, void *data)
-- 
1.6.1.3

Re: [PATCH 1/1] emit settings signal before active signal

[Marcel Holtmann]

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

Hi Martin

> ---
>  src/gprs.c |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)

quick question on why? I did remember that Denis and I talked about on
IRC, but I forgot what our conclusion on the order of these two signals
was. We should really document this. Do you remember? And please prepare
a patch that also puts a comment about this in the API documentation.

Regards

Marcel

RE: [PATCH 1/1] emit settings signal before active signal

[Xu, Martin]

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

Marcel:
> -----Original Message-----
> From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf
> Of Marcel Holtmann
> Sent: Sunday, November 22, 2009 2:14 AM
> To: ofono(a)ofono.org
> Subject: Re: [PATCH 1/1] emit settings signal before active signal
> 
> Hi Martin
> 
> > ---
> >  src/gprs.c |   13 +++++++------
> >  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> quick question on why? I did remember that Denis and I talked about on
> IRC, but I forgot what our conclusion on the order of these two signals
> was. We should really document this. Do you remember? And please prepare
> a patch that also puts a comment about this in the API documentation.
I can find below log

(2:02:38 AM) holtmann: Not sure if we need the Settings empty, but for completion it makes sense.
(2:03:05 AM) holtmann: We might wanna emit the Settings empty before Activated false, so that ConnMan can clean up the interface before it goes away.
(2:03:27 AM) denkenz: What's the difference?
(2:03:54 AM) denkenz: strictly speaking we shouldn't even bother for empty settings
(2:04:05 AM) denkenz: useless signal anyway
(2:04:13 AM) holtmann: Yeah. Just easier for some code path in ConnMan.
 (2:04:49 AM) denkenz: holtmann: ok, that's simple enough anyway

I cannot find the conclusion on active TRUE order. But I think it should the same with active FALSE. 
So ConnMan can setup interface before set network connected.

RE: [PATCH 1/1] emit settings signal before active signal

[Marcel Holtmann]

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

Hi Martin,

> > > ---
> > >  src/gprs.c |   13 +++++++------
> > >  1 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > quick question on why? I did remember that Denis and I talked about on
> > IRC, but I forgot what our conclusion on the order of these two signals
> > was. We should really document this. Do you remember? And please prepare
> > a patch that also puts a comment about this in the API documentation.
> I can find below log
> 
> (2:02:38 AM) holtmann: Not sure if we need the Settings empty, but for completion it makes sense.
> (2:03:05 AM) holtmann: We might wanna emit the Settings empty before Activated false, so that ConnMan can clean up the interface before it goes away.
> (2:03:27 AM) denkenz: What's the difference?
> (2:03:54 AM) denkenz: strictly speaking we shouldn't even bother for empty settings
> (2:04:05 AM) denkenz: useless signal anyway
> (2:04:13 AM) holtmann: Yeah. Just easier for some code path in ConnMan.
>  (2:04:49 AM) denkenz: holtmann: ok, that's simple enough anyway
> 
> I cannot find the conclusion on active TRUE order. But I think it should the same with active FALSE. 
> So ConnMan can setup interface before set network connected.

sounds good enough for me. Patch has been applied.

However please submit a patch that updates the documentation with the
details about the signal order.

Regards

Marcel