12:25 p.m.
Sometimes when open the data device, it may fail. If open the data device
failed, retry once one second later.
---
plugins/huawei.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/plugins/huawei.c b/plugins/huawei.c
index e791718..b888b0f 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -51,6 +51,7 @@
#include <ofono/phonebook.h>
#include <ofono/message-waiting.h>
#include <ofono/log.h>
+#include <ofono.h>
#include <drivers/atmodem/atutil.h>
#include <drivers/atmodem/vendor.h>
@@ -80,6 +81,8 @@ struct huawei_data {
gboolean ndis;
guint sim_poll_timeout;
guint sim_poll_count;
+ guint reopen_source;
+ ofono_bool_t online;
};
#define MAX_SIM_POLL_COUNT 5
@@ -107,6 +110,11 @@ static void huawei_remove(struct ofono_modem *modem)
DBG("%p", modem);
+ if (data->reopen_source > 0) {
+ g_source_remove(data->reopen_source);
+ data->reopen_source = 0;
+ }
+
ofono_modem_set_data(modem, NULL);
if (data->modem)
@@ -465,6 +473,14 @@ static GAtChat *open_device(struct ofono_modem *modem,
return chat;
}
+static void huawei_disconnect(gpointer user_data);
+
+static gboolean reopen_callback(gpointer user_data)
+{
+ huawei_disconnect(user_data);
+ return FALSE;
+}
+
static void huawei_disconnect(gpointer user_data)
{
struct ofono_modem *modem = user_data;
@@ -476,15 +492,26 @@ static void huawei_disconnect(gpointer user_data)
data->modem = NULL;
data->modem = open_device(modem, "Modem", "Modem: ");
- if (data->modem == NULL)
+ /* retry once if failed */
+ if (data->modem == NULL && data->reopen_source == 0) {
+ data->reopen_source =
+ g_timeout_add_seconds(1, reopen_callback, modem);
+
+ ofono_debug("open device failed, try to reopen it.");
return;
+ }
+
+ if (data->reopen_source > 0)
+ data->reopen_source = 0;
g_at_chat_set_disconnect_function(data->modem,
huawei_disconnect, modem);
- /* gprs_context has been destructed and needs not reopen */
- if (data->gc == NULL)
- return;
+ /* gprs_context has been destructed, if offline needs not recreate.
+ if device is online, need recreate.
+ */
+ if (data->gc == NULL && data->online == FALSE)
+ return;
ofono_gprs_context_remove(data->gc);
@@ -492,6 +519,13 @@ static void huawei_disconnect(gpointer user_data)
data->sim_state == HUAWEI_SIM_STATE_INVALID_CS) {
ofono_info("Reopened GPRS context channel");
+ if (__ofono_modem_find_atom(modem,
+ OFONO_ATOM_TYPE_GPRS) == NULL) {
+ data->gprs = ofono_gprs_create(modem,
+ OFONO_VENDOR_HUAWEI,
+ "atmodem", data->pcui);
+ }
+
data->gc = ofono_gprs_context_create(modem, 0, "atmodem",
data->modem);
@@ -559,6 +593,11 @@ static int huawei_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->sim_poll_timeout > 0) {
g_source_remove(data->sim_poll_timeout);
data->sim_poll_timeout = 0;
@@ -588,6 +627,12 @@ static void set_online_cb(gboolean ok, GAtResult *result, gpointer
user_data)
ofono_modem_online_cb_t cb = cbd->cb;
struct ofono_error error;
+ if (ok) {
+ struct huawei_data *data = cbd->user;
+
+ data->online = TRUE;
+ }
+
decode_at_error(&error, g_at_result_final_response(result));
cb(&error, cbd->data);
}
@@ -603,6 +648,7 @@ static void set_offline_cb(gboolean ok, GAtResult *result, gpointer
user_data)
data->gc = NULL;
data->gprs = NULL;
+ data->online = FALSE;
}
decode_at_error(&error, g_at_result_final_response(result));
--
1.7.5
12:41 p.m.
Hi Caiwen,
Sometimes when open the data device, it may fail. If open the data
device
failed, retry once one second later.
---
plugins/huawei.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/plugins/huawei.c b/plugins/huawei.c
index e791718..b888b0f 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -51,6 +51,7 @@
#include <ofono/phonebook.h>
#include <ofono/message-waiting.h>
#include <ofono/log.h>
+#include <ofono.h>
this include is wrong and should not be needed. the ofono.h is internal
to the oFono core and should not be used in plugins.
#include <drivers/atmodem/atutil.h>
#include <drivers/atmodem/vendor.h>
@@ -80,6 +81,8 @@ struct huawei_data {
gboolean ndis;
guint sim_poll_timeout;
guint sim_poll_count;
+ guint reopen_source;
Call this one reopen_timeout or similar. reopen_source is too generic.
+ ofono_bool_t online;
};
#define MAX_SIM_POLL_COUNT 5
@@ -107,6 +110,11 @@ static void huawei_remove(struct ofono_modem *modem)
DBG("%p", modem);
+ if (data->reopen_source > 0) {
+ g_source_remove(data->reopen_source);
+ data->reopen_source = 0;
+ }
+
ofono_modem_set_data(modem, NULL);
if (data->modem)
@@ -465,6 +473,14 @@ static GAtChat *open_device(struct ofono_modem *modem,
return chat;
}
+static void huawei_disconnect(gpointer user_data);
+
+static gboolean reopen_callback(gpointer user_data)
+{
+ huawei_disconnect(user_data);
+ return FALSE;
+}
+
static void huawei_disconnect(gpointer user_data)
{
struct ofono_modem *modem = user_data;
@@ -476,15 +492,26 @@ static void huawei_disconnect(gpointer user_data)
data->modem = NULL;
data->modem = open_device(modem, "Modem", "Modem: ");
- if (data->modem == NULL)
+ /* retry once if failed */
+ if (data->modem == NULL && data->reopen_source == 0) {
+ data->reopen_source =
+ g_timeout_add_seconds(1, reopen_callback, modem);
+
+ ofono_debug("open device failed, try to reopen it.");
return;
+ }
+
+ if (data->reopen_source > 0)
+ data->reopen_source = 0;
Using reopen_source this way is wrong. Now you are just loosing the
reference to that timeout. What are you trying to do here?
I would expect that you keep that timeout source around until either you
do not need it anymore or the timeout actually fired and then reset it.
g_at_chat_set_disconnect_function(data->modem,
huawei_disconnect, modem);
- /* gprs_context has been destructed and needs not reopen */
- if (data->gc == NULL)
- return;
+ /* gprs_context has been destructed, if offline needs not recreate.
+ if device is online, need recreate.
+ */
+ if (data->gc == NULL && data->online == FALSE)
+ return;
I think this is wrong as well. We should remove that context no matter
if online of offline, but off course you do not wanna create a new GPRS
context if you are actually offline, right?
ofono_gprs_context_remove(data->gc);
@@ -492,6 +519,13 @@ static void huawei_disconnect(gpointer user_data)
data->sim_state == HUAWEI_SIM_STATE_INVALID_CS) {
ofono_info("Reopened GPRS context channel");
+ if (__ofono_modem_find_atom(modem,
+ OFONO_ATOM_TYPE_GPRS) == NULL) {
+ data->gprs = ofono_gprs_create(modem,
+ OFONO_VENDOR_HUAWEI,
+ "atmodem", data->pcui);
+ }
+
I am not getting this change. What is it trying to fix?
data->gc = ofono_gprs_context_create(modem, 0,
"atmodem",
data->modem);
@@ -559,6 +593,11 @@ static int huawei_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->sim_poll_timeout > 0) {
g_source_remove(data->sim_poll_timeout);
data->sim_poll_timeout = 0;
@@ -588,6 +627,12 @@ static void set_online_cb(gboolean ok, GAtResult *result, gpointer
user_data)
ofono_modem_online_cb_t cb = cbd->cb;
struct ofono_error error;
+ if (ok) {
+ struct huawei_data *data = cbd->user;
+
+ data->online = TRUE;
+ }
+
decode_at_error(&error, g_at_result_final_response(result));
cb(&error, cbd->data);
}
@@ -603,6 +648,7 @@ static void set_offline_cb(gboolean ok, GAtResult *result, gpointer
user_data)
data->gc = NULL;
data->gprs = NULL;
+ data->online = FALSE;
}
decode_at_error(&error, g_at_result_final_response(result));
Regards
Marcel
1:41 p.m.
> Sometimes when open the data device, it may fail. If open the
data
device
> failed, retry once one second later.
>
> ---
> plugins/huawei.c | 54
++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/plugins/huawei.c b/plugins/huawei.c
> index e791718..b888b0f 100644
> --- a/plugins/huawei.c
> +++ b/plugins/huawei.c
> @@ -51,6 +51,7 @@
> #include <ofono/phonebook.h>
> #include <ofono/message-waiting.h>
> #include <ofono/log.h>
> +#include <ofono.h>
this include is wrong and should not be needed. the ofono.h is internal
to the oFono core and should not be used in plugins.
> #include <drivers/atmodem/atutil.h>
> #include <drivers/atmodem/vendor.h>
> @@ -80,6 +81,8 @@ struct huawei_data {
> gboolean ndis;
> guint sim_poll_timeout;
> guint sim_poll_count;
> + guint reopen_source;
Call this one reopen_timeout or similar. reopen_source is too generic.
OK, I will rename it.
> + ofono_bool_t online;
> };
>
> #define MAX_SIM_POLL_COUNT 5
> @@ -107,6 +110,11 @@ static void huawei_remove(struct ofono_modem
*modem)
>
> DBG("%p", modem);
>
> + if (data->reopen_source > 0) {
> + g_source_remove(data->reopen_source);
> + data->reopen_source = 0;
> + }
> +
> ofono_modem_set_data(modem, NULL);
>
> if (data->modem)
> @@ -465,6 +473,14 @@ static GAtChat *open_device(struct ofono_modem
*modem,
> return chat;
> }
>
> +static void huawei_disconnect(gpointer user_data);
> +
> +static gboolean reopen_callback(gpointer user_data)
> +{
> + huawei_disconnect(user_data);
> + return FALSE;
> +}
> +
> static void huawei_disconnect(gpointer user_data)
> {
> struct ofono_modem *modem = user_data;
> @@ -476,15 +492,26 @@ static void huawei_disconnect(gpointer
user_data)
> data->modem = NULL;
>
> data->modem = open_device(modem, "Modem", "Modem: ");
> - if (data->modem == NULL)
> + /* retry once if failed */
> + if (data->modem == NULL && data->reopen_source == 0) {
> + data->reopen_source =
> + g_timeout_add_seconds(1, reopen_callback, modem);
> +
> + ofono_debug("open device failed, try to reopen it.");
> return;
> + }
> +
> + if (data->reopen_source > 0)
> + data->reopen_source = 0;
Using reopen_source this way is wrong. Now you are just loosing the
reference to that timeout. What are you trying to do here?
I would expect that you keep that timeout source around until either
you
do not need it anymore or the timeout actually fired and then reset it.
Yes, only loose the reference here. In fact, here the timeout has fired.
I will move this to reopen_callback(), it should be better.
> g_at_chat_set_disconnect_function(data->modem,
> huawei_disconnect, modem);
>
> - /* gprs_context has been destructed and needs not reopen */
> - if (data->gc == NULL)
> - return;
> + /* gprs_context has been destructed, if offline needs not
recreate.
> + if device is online, need recreate.
> + */
> + if (data->gc == NULL && data->online == FALSE)
> + return;
I think this is wrong as well. We should remove that context no matter
if online of offline, but off course you do not wanna create a new GPRS
context if you are actually offline, right?
If the device is offline, the GPRS context atom has been released when set
the device to offline. That's another issue has been fixed by Martin (Need
set data->gc to NULL, because it has been release if device is offline).
It will create a new GPRS context when the device is switch to online(in
huawei_post_online()), so don't create it here if it is offline.
> ofono_gprs_context_remove(data->gc);
>
> @@ -492,6 +519,13 @@ static void huawei_disconnect(gpointer user_data)
> data->sim_state == HUAWEI_SIM_STATE_INVALID_CS) {
> ofono_info("Reopened GPRS context channel");
>
> + if (__ofono_modem_find_atom(modem,
> + OFONO_ATOM_TYPE_GPRS) == NULL) {
> + data->gprs = ofono_gprs_create(modem,
> + OFONO_VENDOR_HUAWEI,
> + "atmodem", data->pcui);
> + }
> +
I am not getting this change. What is it trying to fix?
There is one case that the GPRS atom is not created. If the device is
switched to online during the time reopen the device, due to data->modem is
NULL, GPRS atom is not created. Here insure GPRS atom is created.
> data->gc = ofono_gprs_context_create(modem, 0,
"atmodem",
> data->modem);
>
> @@ -559,6 +593,11 @@ static int huawei_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->sim_poll_timeout > 0) {
> g_source_remove(data->sim_poll_timeout);
> data->sim_poll_timeout = 0;
> @@ -588,6 +627,12 @@ static void set_online_cb(gboolean ok, GAtResult
*result, gpointer user_data)
> ofono_modem_online_cb_t cb = cbd->cb;
> struct ofono_error error;
>
> + if (ok) {
> + struct huawei_data *data = cbd->user;
> +
> + data->online = TRUE;
> + }
> +
> decode_at_error(&error, g_at_result_final_response(result));
> cb(&error, cbd->data);
> }
> @@ -603,6 +648,7 @@ static void set_offline_cb(gboolean ok, GAtResult
*result, gpointer user_data)
>
> data->gc = NULL;
> data->gprs = NULL;
> + data->online = FALSE;
> }
>
> decode_at_error(&error, g_at_result_final_response(result));
Best regards
Caiwen
1:45 p.m.
Hi Caiwen,
> I think this is wrong as well. We should remove that context no
matter
> if online of offline, but off course you do not wanna create a new GPRS
> context if you are actually offline, right?
>
If the device is offline, the GPRS context atom has been released when set
the device to offline. That's another issue has been fixed by Martin (Need
set data->gc to NULL, because it has been release if device is offline).
It will create a new GPRS context when the device is switch to online(in
huawei_post_online()), so don't create it here if it is offline.
I suggest you migrate the huawei modem driver to create the gprs atom in
post_sim state instead of post_online. It will solve many of the issues
you're trying to solve.
Regards,
-Denis
3:49 p.m.
Hi Denis,
>> I think this is wrong as well. We should remove that context
no
matter
>> if online of offline, but off course you do not wanna create a new
GPRS
>> context if you are actually offline, right?
>>
>
> If the device is offline, the GPRS context atom has been released
when set
> the device to offline. That's another issue has been fixed by Martin
(Need
> set data->gc to NULL, because it has been release if device is
offline).
>
> It will create a new GPRS context when the device is switch to
online(in
> huawei_post_online()), so don't create it here if it is offline.
>
>
I suggest you migrate the huawei modem driver to create the gprs atom
in
post_sim state instead of post_online. It will solve many of the
issues
you're trying to solve.
Thanks your suggestion.
Yes, It can help that we don't need to care whether gprs atom and gprs context atom
be repeatedly created. But if do this at post_sim state, it will beak the rule
that only enable the atom when it is available. At post_sim state GPRS is still
unavailable.
In addition, all other plug-ins do this at post_online, only do this in Huawei plug-in,
it may impact the consistency of the codes.
Best regards
Caiwen
2:21 a.m.
Hi Caiwen,
On 05/11/2011 12:49 AM, Zhang, Caiwen wrote:
Hi Denis,
>>> I think this is wrong as well. We should remove that context no
> matter
>>> if online of offline, but off course you do not wanna create a new
> GPRS
>>> context if you are actually offline, right?
>>>
>>
>> If the device is offline, the GPRS context atom has been released
> when set
>> the device to offline. That's another issue has been fixed by Martin
> (Need
>> set data->gc to NULL, because it has been release if device is
> offline).
>>
>> It will create a new GPRS context when the device is switch to
> online(in
>> huawei_post_online()), so don't create it here if it is offline.
>>
>>
>
> I suggest you migrate the huawei modem driver to create the gprs atom
> in
> post_sim state instead of post_online. It will solve many of the
> issues
> you're trying to solve.
>
Thanks your suggestion.
Yes, It can help that we don't need to care whether gprs atom and gprs context atom
be repeatedly created. But if do this at post_sim state, it will beak the rule
that only enable the atom when it is available. At post_sim state GPRS is still
unavailable.
In addition, all other plug-ins do this at post_online, only do this in Huawei plug-in,
it may impact the consistency of the codes.
I wouldn't really suggest this if I didn't think it was a good idea.
All modem drivers should be migrated to create gprs in the post_sim
state instead of post_online at some point anyway. And even if the GPRS
connections cannot be established when the modem is Offline, the
relevant settings on ConnectionManager interface can still be modified.
Regards,
-Denis
4148
days inactive
4148
days old
5 comments
4 participants
participants (4)
-
Caiwen Zhang -
Denis Kenzior -
Marcel Holtmann -
Zhang, Caiwen