[PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[Tomasz Gregorek]

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

From: Tomasz Gregorek <tomasz.gregorek@stericsson.com>

Combining mbm_disable and reopen_callback into one and removing 1 second
delay between them to avoid call to reopen_callback after modem being
disconnected.
---
 plugins/mbm.c |   41 +++++++++++------------------------------
 1 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/plugins/mbm.c b/plugins/mbm.c
index dca9bd8..d1529ac 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -66,7 +66,6 @@ struct mbm_data {
 	gboolean have_sim;
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
-	guint reopen_source;
 	enum mbm_variant variant;
 };
 
@@ -297,21 +296,26 @@ static GAtChat *create_port(const char *device)
 	return chat;
 }
 
-static void mbm_disconnect(gpointer user_data);
-
-static gboolean reopen_callback(gpointer user_data)
+static void mbm_disconnect(gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
 	struct mbm_data *data = ofono_modem_get_data(modem);
 	const char *data_dev;
 
-	data->reopen_source = 0;
+	DBG("");
+
+	if (data->gc)
+		ofono_gprs_context_remove(data->gc);
+
+	g_at_chat_unref(data->data_port);
+	data->data_port = NULL;
 
 	data_dev = ofono_modem_get_string(modem, "DataDevice");
 
 	data->data_port = create_port(data_dev);
+
 	if (data->data_port == NULL)
-		return FALSE;
+		return;
 
 	if (getenv("OFONO_AT_DEBUG"))
 		g_at_chat_set_debug(data->data_port, mbm_debug, "Data: ");
@@ -323,30 +327,12 @@ static gboolean reopen_callback(gpointer user_data)
 
 	data->gc = ofono_gprs_context_create(modem, 0,
 					"atmodem", data->data_port);
+
 	if (data->gprs && data->gc) {
 		ofono_gprs_context_set_type(data->gc,
 					OFONO_GPRS_CONTEXT_TYPE_MMS);
 		ofono_gprs_add_context(data->gprs, data->gc);
 	}
-
-	return FALSE;
-}
-
-static void mbm_disconnect(gpointer user_data)
-{
-	struct ofono_modem *modem = user_data;
-	struct mbm_data *data = ofono_modem_get_data(modem);
-
-	DBG("");
-
-	if (data->gc)
-		ofono_gprs_context_remove(data->gc);
-
-	g_at_chat_unref(data->data_port);
-	data->data_port = NULL;
-
-	/* Waiting for the +CGEV: ME DEACT might also work */
-	data->reopen_source = g_timeout_add_seconds(1, reopen_callback, modem);
 }
 
 static int mbm_enable(struct ofono_modem *modem)
@@ -425,11 +411,6 @@ static int mbm_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	if (data->reopen_source > 0) {
-		g_source_remove(data->reopen_source);
-		data->reopen_source = 0;
-	}
-
 	if (data->modem_port == NULL)
 		return 0;
 
-- 
1.7.1

Re: [PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[Marcel Holtmann]

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

Hi Tomasz,

> Combining mbm_disable and reopen_callback into one and removing 1 second
> delay between them to avoid call to reopen_callback after modem being
> disconnected.

have you actually tested this? This will not work since you can not
reopen the TTY right away. You need that delay in between.

From my previous email, I remember saying that the right fix would be to
disarm the timer within disable. Why are we not doing that?

Regards

Marcel

Re: [PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[Tomasz Gregorek]

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

Hi Marcel
I already had my solution in head and misunderstood you. I will correct and
retest it.
Br
Tomasz

2011/1/12 Marcel Holtmann <marcel@holtmann.org>

> Hi Tomasz,
>
> > Combining mbm_disable and reopen_callback into one and removing 1 second
> > delay between them to avoid call to reopen_callback after modem being
> > disconnected.
>
> have you actually tested this? This will not work since you can not
> reopen the TTY right away. You need that delay in between.
>
> From my previous email, I remember saying that the right fix would be to
> disarm the timer within disable. Why are we not doing that?
>
> Regards
>
> Marcel
>
>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1195 bytes --]

Re: [PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[Marcel Holtmann]

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

Hi Tomasz,

> I already had my solution in head and misunderstood you. I will
> correct and retest it.

please do so.

And NO top posting on this mailing list. Keep that in mind.

Regards

Marcel

Re: [PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[Sjur =?unknown-8bit?q?Br=C3=A6ndeland?=]

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

Hi Tomasz, don't worry the tone on the list is quite rough. If you
like marit or I can review tomorrow, but feel free to send patch as
well if you like. :) Sjur

2011/1/12, Tomasz Gregorek <tomasz.gregorek@gmail.com>:
> Hi Marcel
> I already had my solution in head and misunderstood you. I will correct and
> retest it.
> Br
> Tomasz
>
> 2011/1/12 Marcel Holtmann <marcel@holtmann.org>
>
>> Hi Tomasz,
>>
>> > Combining mbm_disable and reopen_callback into one and removing 1 second
>> > delay between them to avoid call to reopen_callback after modem being
>> > disconnected.
>>
>> have you actually tested this? This will not work since you can not
>> reopen the TTY right away. You need that delay in between.
>>
>> From my previous email, I remember saying that the right fix would be to
>> disarm the timer within disable. Why are we not doing that?
>>
>> Regards
>>
>> Marcel
>>
>>
>> _______________________________________________
>> ofono mailing list
>> ofono(a)ofono.org
>> http://lists.ofono.org/listinfo/ofono
>>
>

Re: [PATCH] plugins-mbm: Combining mbm_disable and reopen_callback

[Tomasz Gregorek]

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

Hi Sjur

2011/1/12 Sjur Brændeland <sjurbren@gmail.com>

> Hi Tomasz, don't worry the tone on the list is quite rough. If you
> like marit or I can review tomorrow, but feel free to send patch as
> well if you like. :) Sjur
>
> 2011/1/12, Tomasz Gregorek <tomasz.gregorek@gmail.com>:
> > Hi Marcel
> > I already had my solution in head and misunderstood you. I will correct
> and
> > retest it.
> > Br
> > Tomasz
> >
> > 2011/1/12 Marcel Holtmann <marcel@holtmann.org>
> >
> >> Hi Tomasz,
> >>
> >> > Combining mbm_disable and reopen_callback into one and removing 1
> second
> >> > delay between them to avoid call to reopen_callback after modem being
> >> > disconnected.
> >>
> >> have you actually tested this? This will not work since you can not
> >> reopen the TTY right away. You need that delay in between.
> >>
> >> From my previous email, I remember saying that the right fix would be to
> >> disarm the timer within disable. Why are we not doing that?
> >>
> >> Regards
> >>
> >> Marcel
> >>
> >>
> >> _______________________________________________
> >> ofono mailing list
> >> ofono(a)ofono.org
> >> http://lists.ofono.org/listinfo/ofono
> >>
> >
>

I don't worry ;)
Just assumed things too fast which led me to misunderstanding.
Br
Tomasz

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1985 bytes --]