Hi Denis:
> struct ofono_gprs *gprs;
> struct ofono_gprs_context *gc;
> gboolean voice;
> + gboolean online;
I don't think you really need this one, you can use the presence of gprs
atom instead.
Gprs atom is not reliable here. Because gprs will be flushed when
entering offline mode
> gboolean ndis;
> guint sim_poll_timeout;
> guint sim_poll_count;
> @@ -97,6 +98,7 @@ static int huawei_probe(struct ofono_modem *modem)
> if (data == NULL)
> return -ENOMEM;
>
> + data->online = FALSE;
> ofono_modem_set_data(modem, data);
>
> return 0;
> @@ -111,6 +113,11 @@ static void huawei_remove(struct ofono_modem
*modem)
> ofono_modem_set_data(modem, NULL);
>
> if (data->modem)
> + /*
> + * huawei_disconnect should not be called when the chat
> + * is closed initiatively
> + */
> + g_at_chat_set_disconnect_function(data->modem, NULL, NULL);
> g_at_chat_unref(data->modem);
Are you missing some parentheses around the if statement now?
Setting the disconnect function here seems fishy. We automatically set
it to NULL when GAtIO is unref-ed. Seems to me g_at_chat_unref should
be enough.
You are quite right.
>
> g_at_chat_unref(data->pcui);
> @@ -470,10 +477,7 @@ static void huawei_disconnect(gpointer user_data)
> struct ofono_modem *modem = user_data;
> struct huawei_data *data = ofono_modem_get_data(modem);
>
> - DBG("");
> -
> - if (data->gc)
> - ofono_gprs_context_remove(data->gc);
> + DBG("data->online %d", data->online);
>
> g_at_chat_unref(data->modem);
> data->modem = NULL;
> @@ -485,6 +489,13 @@ static void huawei_disconnect(gpointer user_data)
> g_at_chat_set_disconnect_function(data->modem,
> huawei_disconnect, modem);
>
> + /* reopen GPRS context channel only at online state */
> + if (data->online == FALSE)
> + return;
> +
> + if (data->gc)
> + ofono_gprs_context_remove(data->gc);
> +
> if (data->sim_state == HUAWEI_SIM_STATE_VALID ||
> data->sim_state == HUAWEI_SIM_STATE_INVALID_CS) {
> ofono_info("Reopened GPRS context channel");
> @@ -515,6 +526,11 @@ static int huawei_enable(struct ofono_modem
*modem)
>
> data->pcui = open_device(modem, "Pcui", "PCUI: ");
> if (data->pcui == NULL) {
> + /*
> + * huawei_disconnect should not be called when the chat
> + * is closed initiatively
> + */
> + g_at_chat_set_disconnect_function(data->modem, NULL, NULL);
> g_at_chat_unref(data->modem);
> data->modem = NULL;
> return -EIO;
> @@ -564,6 +580,11 @@ static int huawei_disable(struct ofono_modem
*modem)
> if (data->modem) {
> g_at_chat_cancel_all(data->modem);
> g_at_chat_unregister_all(data->modem);
> + /*
> + * huawei_disconnect should not be called when the chat
> + * is closed initiatively
> + */
> + g_at_chat_set_disconnect_function(data->modem, NULL, NULL);
> g_at_chat_unref(data->modem);
> data->modem = NULL;
> }
> @@ -579,15 +600,27 @@ static int huawei_disable(struct ofono_modem
*modem)
> return -EINPROGRESS;
> }
>
> +struct huawei_cb_data {
> + struct cb_data *cbd;
> + ofono_bool_t online;
> + struct ofono_modem *modem;
> +};
> +
> static void set_online_cb(gboolean ok, GAtResult *result, gpointer
user_data)
> {
> - struct cb_data *cbd = user_data;
> - ofono_modem_online_cb_t cb = cbd->cb;
> + struct huawei_cb_data *online_cbd = user_data;
> + ofono_modem_online_cb_t cb = online_cbd->cbd->cb;
> + struct huawei_data *data =
ofono_modem_get_data(online_cbd->modem);
>
> - if (ok)
> - CALLBACK_WITH_SUCCESS(cb, cbd->data);
> + if (ok) {
> + data->online = online_cbd->online;
> +
> + CALLBACK_WITH_SUCCESS(cb, online_cbd->cbd->data);
> +
> + g_free(online_cbd);
> + }
> else
> - CALLBACK_WITH_FAILURE(cb, cbd->data);
> + CALLBACK_WITH_FAILURE(cb, online_cbd->cbd->data);
> }
All of this can be better accomplished by using two separate callbacks.
One for online and one for offline.
agree
I suspect all you need to do is unref data->modem, set data->gc and
data->gprs to NULL in the offline callback prior to calling back into
the core.
Logically, call back only needs to set offline states
It is Huawei_disconnect's duty to decide the operation.
In theory, Huawei_disconnect may be called at any situation when io error happens.
(I observed two place, one is at ppp_disconnect the other is at offline mode CFUN=5.)
So the function must take action according to the states.
I will resent the updated patch