Re: [PATCH_v2 3/4] huaweicdma: Add SIM creation and set_online method driver

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Guillaume,

On 11/21/2011 04:54 AM, Guillaume Zajac wrote:
> ---
>  plugins/huaweicdma.c |  283 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 276 insertions(+), 7 deletions(-)
> 
> diff --git a/plugins/huaweicdma.c b/plugins/huaweicdma.c
> index 4c83114..0fd9ef2 100644
> --- a/plugins/huaweicdma.c
> +++ b/plugins/huaweicdma.c
> @@ -37,10 +37,28 @@
>  #include <ofono/cdma-netreg.h>
>  #include <ofono/cdma-connman.h>
>  #include <ofono/log.h>
> +#include <ofono.h>
> +
> +#include <drivers/atmodem/atutil.h>
> +#include <drivers/atmodem/vendor.h>
> +
> +static const char *none_prefix[] = { NULL };
> +static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
> +
> +enum {
> +	SIM_STATE_VALID =		1,
> +	SIM_STATE_EMBEDDED =		240,
> +	SIM_STATE_NOT_EXISTENT =	255,
> +};
>  
>  struct huaweicdma_data {
>  	GAtChat *modem;
>  	GAtChat *pcui;
> +	gboolean have_sim;
> +	int sim_state;
> +	guint sysinfo_poll_source;
> +	guint sysinfo_poll_count;

You forgot to clean these up in huaweicdma_remove

> +	struct cb_data *online_cbd;
>  };
>  
>  static void huaweicdma_debug(const char *str, void *data)
> @@ -79,6 +97,122 @@ static void huaweicdma_remove(struct ofono_modem *modem)
>  	g_free(data);
>  }
>  
> +static void simst_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct huaweicdma_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	int sim_state;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "^SIMST:"))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &sim_state))
> +		return;
> +
> +	DBG("%d -> %d", data->sim_state, sim_state);
> +
> +	data->sim_state = sim_state;
> +}
> +
> +static gboolean parse_sysinfo_result(GAtResult *result, int *srv_status,
> +					int *srv_domain, int *sim_state)
> +{
> +	GAtResultIter iter;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
> +		return FALSE;
> +
> +	if (!g_at_result_iter_next_number(&iter, srv_status))
> +		return FALSE;
> +
> +	if (!g_at_result_iter_next_number(&iter, srv_domain))
> +		return FALSE;
> +
> +	if (!g_at_result_iter_skip_next(&iter))
> +		return FALSE;
> +
> +	if (!g_at_result_iter_skip_next(&iter))
> +		return FALSE;
> +
> +	if (!g_at_result_iter_next_number(&iter, sim_state))
> +		return FALSE;

This is an obvious candidate to refactor into a library

> +
> +	return TRUE;
> +}
> +
> +static void shutdown_device(struct huaweicdma_data *data)
> +{
> +	g_at_chat_cancel_all(data->modem);
> +	g_at_chat_unregister_all(data->modem);
> +
> +	g_at_chat_unref(data->modem);
> +	data->modem = NULL;
> +
> +	g_at_chat_cancel_all(data->pcui);
> +	g_at_chat_unregister_all(data->pcui);
> +
> +	g_at_chat_unref(data->pcui);
> +	data->pcui = NULL;
> +}
> +
> +static gboolean sysinfo_enable_check(gpointer user_data);
> +
> +static void sysinfo_enable_cb(gboolean ok, GAtResult *result,
> +						gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct huaweicdma_data *data = ofono_modem_get_data(modem);
> +	int srv_status, srv_domain, sim_state;
> +
> +	if (!ok)
> +		goto failure;
> +
> +	if (parse_sysinfo_result(result, &srv_status, &srv_domain,
> +						&sim_state) == FALSE)
> +		goto failure;
> +
> +	DBG("%d -> %d", data->sim_state, sim_state);
> +
> +	data->sim_state = sim_state;
> +
> +	if (sim_state == SIM_STATE_NOT_EXISTENT) {
> +		data->sysinfo_poll_count++;
> +
> +		if (data->sysinfo_poll_count > 5)
> +			goto failure;
> +
> +		data->sysinfo_poll_source = g_timeout_add_seconds(1,
> +						sysinfo_enable_check, modem);
> +		return;
> +	}
> +
> +	data->have_sim = TRUE;
> +	ofono_modem_set_powered(modem, TRUE);
> +	return;
> +
> +failure:
> +	shutdown_device(data);
> +	ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static gboolean sysinfo_enable_check(gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct huaweicdma_data *data = ofono_modem_get_data(modem);
> +
> +	data->sysinfo_poll_source = 0;
> +
> +	g_at_chat_send(data->pcui, "AT^SYSINFO", sysinfo_prefix,
> +					sysinfo_enable_cb, modem, NULL);
> +
> +	return FALSE;
> +}
> +
>  static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>  {
>  	struct ofono_modem *modem = user_data;
> @@ -87,14 +221,18 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>  	DBG("");
>  
>  	if (!ok) {
> -		g_at_chat_unref(data->modem);
> -		data->modem = NULL;
> -
> -		g_at_chat_unref(data->pcui);
> -		data->pcui = NULL;
> +		shutdown_device(data);
> +		ofono_modem_set_powered(modem, FALSE);
> +		return;
>  	}
>  
> -	ofono_modem_set_powered(modem, ok);
> +	/* Follow sim state changes */
> +	g_at_chat_register(data->pcui, "^SIMST:", simst_notify,
> +						FALSE, modem, NULL);
> +
> +	data->sysinfo_poll_count = 0;
> +
> +	sysinfo_enable_check(modem);
>  }
>  
>  static GAtChat *open_device(struct ofono_modem *modem,
> @@ -150,6 +288,8 @@ static int huaweicdma_enable(struct ofono_modem *modem)
>  	g_at_chat_send(data->modem, "ATE0 &C0 +CMEE=1", NULL, NULL, NULL, NULL);
>  	g_at_chat_send(data->pcui, "ATE0 &C0 +CMEE=1", NULL, NULL, NULL, NULL);
>  
> +	data->sim_state = SIM_STATE_NOT_EXISTENT;
> +
>  	g_at_chat_send(data->pcui, "AT+CFUN=1", NULL,
>  					cfun_enable, modem, NULL);
>  
> @@ -191,18 +331,146 @@ static int huaweicdma_disable(struct ofono_modem *modem)
>  	return -EINPROGRESS;
>  }
>  
> +
> +static gboolean sysinfo_online_check(gpointer user_data);
> +
> +static void sysinfo_online_cb(gboolean ok, GAtResult *result,
> +						gpointer user_data)
> +{
> +	struct huaweicdma_data *data = user_data;
> +	ofono_modem_online_cb_t cb = data->online_cbd->cb;
> +	int srv_status, srv_domain, sim_state;
> +
> +	if (!ok)
> +		goto failure;
> +
> +	if (parse_sysinfo_result(result, &srv_status, &srv_domain,
> +							&sim_state) == FALSE)
> +		goto failure;
> +
> +	DBG("%d -> %d", data->sim_state, sim_state);
> +
> +	data->sim_state = sim_state;
> +
> +	/* Valid service status and at minimum PS domain */
> +	if (srv_status > 0 && srv_domain > 1) {
> +		CALLBACK_WITH_SUCCESS(cb, data->online_cbd->data);
> +		goto done;
> +	}

Why are you checking this here? PS domain is a GSM specific thing and
should have no bearing.

> +
> +	switch (sim_state) {
> +	case SIM_STATE_VALID:
> +	case SIM_STATE_EMBEDDED:
> +		CALLBACK_WITH_SUCCESS(cb, data->online_cbd->data);
> +		goto done;
> +	}

Why are we not handling the EMBEDDED state elsewhere, e.g. to tell the
core that no SIM is to be expected?

> +
> +	data->sysinfo_poll_count++;
> +
> +	if (data->sysinfo_poll_count > 15)
> +		goto failure;
> +
> +	data->sysinfo_poll_source = g_timeout_add_seconds(2,
> +						sysinfo_online_check, data);
> +	return;
> +
> +failure:
> +	CALLBACK_WITH_FAILURE(cb, data->online_cbd->data);
> +
> +done:
> +	g_free(data->online_cbd);
> +	data->online_cbd = NULL;
> +}
> +
> +static gboolean sysinfo_online_check(gpointer user_data)
> +{
> +	struct huaweicdma_data *data = user_data;
> +
> +	data->sysinfo_poll_source = 0;
> +
> +	g_at_chat_send(data->pcui, "AT^SYSINFO", sysinfo_prefix,
> +					sysinfo_online_cb, data, NULL);
> +
> +	return FALSE;
> +}
> +
> +static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct huaweicdma_data *data = ofono_modem_get_data(modem);
> +
> +	if (!ok) {
> +		ofono_modem_online_cb_t cb = data->online_cbd->cb;
> +
> +		CALLBACK_WITH_FAILURE(cb, data->online_cbd->data);
> +
> +		g_free(data->online_cbd);
> +		data->online_cbd = NULL;
> +		return;
> +	}
> +
> +	data->sysinfo_poll_count = 0;
> +
> +	sysinfo_online_check(data);
> +}
> +
> +static void set_offline_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_modem_online_cb_t cb = cbd->cb;
> +	struct ofono_error error;
> +
> +	decode_at_error(&error, g_at_result_final_response(result));
> +	cb(&error, cbd->data);
> +}
> +
> +static void huaweicdma_set_online(struct ofono_modem *modem,
> +				ofono_bool_t online,
> +				ofono_modem_online_cb_t cb, void *user_data)
> +{
> +	struct huaweicdma_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("modem %p %s", modem, online ? "online" : "offline");
> +
> +	if (online == TRUE) {
> +		data->online_cbd = cb_data_new(cb, user_data);
> +
> +		if (g_at_chat_send(data->pcui, "AT+CFUN=1", none_prefix,
> +					set_online_cb, modem, NULL) > 0)
> +			return;
> +
> +		g_free(data->online_cbd);
> +		data->online_cbd = NULL;
> +	} else {
> +		struct cb_data *cbd = cb_data_new(cb, user_data);
> +
> +		if (g_at_chat_send(data->pcui, "AT+CFUN=4", none_prefix,
> +					set_offline_cb, cbd, g_free) > 0)
> +			return;
> +
> +		g_free(cbd);
> +	}
> +
> +	CALLBACK_WITH_FAILURE(cb, user_data);
> +}
> +
>  static void huaweicdma_pre_sim(struct ofono_modem *modem)
>  {
>  	struct huaweicdma_data *data = ofono_modem_get_data(modem);
> +	struct ofono_sim *sim;
>  
>  	DBG("%p", modem);
>  
>  	ofono_devinfo_create(modem, 0, "cdmamodem", data->pcui);
> +	sim = ofono_sim_create(modem, OFONO_VENDOR_HUAWEI,
> +					"cdmamodem", data->pcui);
> +
> +	if (sim && data->have_sim == TRUE)
> +		ofono_sim_inserted_notify(sim, TRUE);
>  }
>  
>  static void huaweicdma_post_sim(struct ofono_modem *modem)
>  {
> -	DBG("%p", modem);
>  }
>  
>  static void huaweicdma_post_online(struct ofono_modem *modem)
> @@ -222,6 +490,7 @@ static struct ofono_modem_driver huaweicdma_driver = {
>  	.remove		= huaweicdma_remove,
>  	.enable		= huaweicdma_enable,
>  	.disable	= huaweicdma_disable,
> +	.set_online	= huaweicdma_set_online,
>  	.pre_sim	= huaweicdma_pre_sim,
>  	.post_sim	= huaweicdma_post_sim,
>  	.post_online	= huaweicdma_post_online,

Regards,
-Denis