[PATCH] Add GPRS, mux, volume control support in SIMCOM sim900 modem module

[PATCH] Add GPRS, mux, volume control support in SIMCOM sim900 modem module

[r.r.zaripov]

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

From: Renat Zaripov <r.r.zaripov@gmail.com>

---
 plugins/sim900.c |  162 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 137 insertions(+), 25 deletions(-)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index 496faa6..f228c0e 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -29,6 +29,7 @@
 #include <glib.h>
 #include <gatchat.h>
 #include <gattty.h>
+#include <gatmux.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/plugin.h>
@@ -44,13 +45,28 @@
 #include <ofono/history.h>
 #include <ofono/log.h>
 #include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
 
 #include <drivers/atmodem/vendor.h>
 
+#define NUM_DLC 5
+
+#define SETUP_DLC   0
+#define VOICE_DLC   1
+#define NETREG_DLC  2
+#define SMS_DLC     3
+#define GPRS_DLC    4
+
+
+static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ", "GPRS: " , "Setup: "};
+
 static const char *none_prefix[] = { NULL };
 
 struct sim900_data {
-	GAtChat *modem;
+	GIOChannel *device;
+	GAtMux *mux;
+	GAtChat *dlcs[NUM_DLC];
+	guint frame_size;
 };
 
 static int sim900_probe(struct ofono_modem *modem)
@@ -76,7 +92,7 @@ static void sim900_remove(struct ofono_modem *modem)
 
 	ofono_modem_set_data(modem, NULL);
 
-	g_at_chat_unref(data->modem);
+	g_at_chat_unref(data->dlcs[SETUP_DLC]);
 
 	g_free(data);
 }
@@ -91,6 +107,7 @@ static void sim900_debug(const char *str, void *user_data)
 static GAtChat *open_device(struct ofono_modem *modem,
 				const char *key, char *debug)
 {
+	struct sim900_data *data = ofono_modem_get_data(modem);
 	const char *device;
 	GAtSyntax *syntax;
 	GIOChannel *channel;
@@ -119,10 +136,33 @@ static GAtChat *open_device(struct ofono_modem *modem,
 	if (channel == NULL)
 		return NULL;
 
+	data->device = channel;
 	syntax = g_at_syntax_new_gsm_permissive();
 	chat = g_at_chat_new(channel, syntax);
 	g_at_syntax_unref(syntax);
 
+	if (chat == NULL)
+		return NULL;
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, sim900_debug, debug);
+
+	return chat;
+}
+
+
+static GAtChat *create_chat(GIOChannel *channel, struct ofono_modem *modem,
+								char *debug)
+{
+	GAtSyntax *syntax;
+	GAtChat *chat;
+
+	if (channel == NULL)
+		return NULL;
+
+	syntax = g_at_syntax_new_gsmv1();
+	chat = g_at_chat_new(channel, syntax);
+	g_at_syntax_unref(syntax);
 	g_io_channel_unref(channel);
 
 	if (chat == NULL)
@@ -134,6 +174,65 @@ static GAtChat *open_device(struct ofono_modem *modem,
 	return chat;
 }
 
+static void setup_internal_mux(struct ofono_modem *modem)
+{
+	struct sim900_data *data = ofono_modem_get_data(modem);
+	int i;
+
+	DBG("");
+
+	data->frame_size = 128;
+
+	data->mux = g_at_mux_new_gsm0710_basic(data->device, data->frame_size);
+	if (data->mux == NULL)
+		goto error;
+
+	if (getenv("OFONO_MUX_DEBUG"))
+		g_at_mux_set_debug(data->mux, sim900_debug, "MUX: ");
+
+	if(!g_at_mux_start(data->mux)) 
+		goto error;
+
+	for (i = 0; i < NUM_DLC; i++) {
+		GIOChannel *channel = g_at_mux_create_channel(data->mux);
+
+		data->dlcs[i] = create_chat(channel, modem, dlc_prefixes[i]);
+		if (data->dlcs[i] == NULL) {
+			ofono_error("Failed to create channel");
+			goto error;
+		}
+	}
+
+	ofono_modem_set_powered(modem, TRUE);
+
+	return;
+
+error:
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct sim900_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	g_at_chat_unref(data->dlcs[SETUP_DLC]);
+	data->dlcs[SETUP_DLC] = NULL;
+
+	if (!ok)
+		goto error;
+
+	setup_internal_mux(modem);
+	
+	return;
+	
+error:
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -142,11 +241,9 @@ 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->dlcs[SETUP_DLC]);
+		data->dlcs[SETUP_DLC] = NULL;
 	}
-
-	ofono_modem_set_powered(modem, ok);
 }
 
 static int sim900_enable(struct ofono_modem *modem)
@@ -155,21 +252,22 @@ static int sim900_enable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	data->modem = open_device(modem, "Device", "Device: ");
-	if (data->modem == NULL) {
-		DBG("return -EINVAL");
+	data->dlcs[SETUP_DLC] = open_device(modem, "Device", "Setup: ");
+	if (data->dlcs[SETUP_DLC] == NULL) 
 		return -EINVAL;
-	}
 
-	g_at_chat_send(data->modem, "ATE0", NULL, NULL, NULL, NULL);
+	g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
 
 	/* For obtain correct sms service number */
-	g_at_chat_send(data->modem, "AT+CSCS=\"GSM\"", NULL,
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
 					NULL, NULL, NULL);
 
-	g_at_chat_send(data->modem, "AT+CFUN=1", none_prefix,
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=1", none_prefix,
 					cfun_enable, modem, NULL);
 
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CMUX=0,0,5,128,10,3,30,10,2", 
+		NULL, mux_setup_cb, modem, NULL);
+
 	return -EINPROGRESS;
 }
 
@@ -181,8 +279,8 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("");
 
-	g_at_chat_unref(data->modem);
-	data->modem = NULL;
+	g_at_chat_unref(data->dlcs[SETUP_DLC]);
+	data->dlcs[SETUP_DLC] = NULL;
 
 	if (ok)
 		ofono_modem_set_powered(modem, FALSE);
@@ -194,10 +292,10 @@ static int sim900_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	g_at_chat_cancel_all(data->modem);
-	g_at_chat_unregister_all(data->modem);
+	g_at_chat_cancel_all(data->dlcs[SETUP_DLC]);
+	g_at_chat_unregister_all(data->dlcs[SETUP_DLC]);
 
-	g_at_chat_send(data->modem, "AT+CFUN=4", none_prefix,
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=4", none_prefix,
 					cfun_disable, modem, NULL);
 
 	return -EINPROGRESS;
@@ -210,9 +308,9 @@ static void sim900_pre_sim(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	ofono_devinfo_create(modem, 0, "atmodem", data->modem);
+	ofono_devinfo_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
 	sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
-				data->modem);
+				data->dlcs[VOICE_DLC]);
 
 	if (sim)
 		ofono_sim_inserted_notify(sim, TRUE);
@@ -221,12 +319,25 @@ static void sim900_pre_sim(struct ofono_modem *modem)
 static void sim900_post_sim(struct ofono_modem *modem)
 {
 	struct sim900_data *data = ofono_modem_get_data(modem);
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
 
 	DBG("%p", modem);
 
-	ofono_phonebook_create(modem, 0, "atmodem", data->modem);
+	ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
 	ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
-				data->modem);
+				data->dlcs[SMS_DLC]);
+
+	gprs = ofono_gprs_create(modem, 0,		
+					"atmodem", data->dlcs[GPRS_DLC]);
+	if (gprs == NULL)
+		return;
+
+	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+					"atmodem", data->dlcs[GPRS_DLC]);
+	if (gc)
+		ofono_gprs_add_context(gprs, gc);
+
 }
 
 static void sim900_post_online(struct ofono_modem *modem)
@@ -235,9 +346,10 @@ static void sim900_post_online(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->modem);
-	ofono_ussd_create(modem, 0, "atmodem", data->modem);
-	ofono_voicecall_create(modem, 0, "atmodem", data->modem);
+	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->dlcs[VOICE_DLC]);
+	ofono_ussd_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+	ofono_voicecall_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+	ofono_call_volume_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
 }
 
 static struct ofono_modem_driver sim900_driver = {
-- 
1.7.10.4

[PATCH] Add vendor quirk for SIMCOM

[r.r.zaripov]

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

From: Renat Zaripov <r.r.zaripov@gmail.com>

---
 drivers/atmodem/gprs-context.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 3694c27..c5a0ae7 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -208,7 +208,11 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
+	if (gcd->vendor == OFONO_VENDOR_SIMCOM) 
+		memcpy(buf, "ATD*99#", 7);
+	else
+		sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
+
 	if (g_at_chat_send(gcd->chat, buf, none_prefix,
 				at_cgdata_cb, gc, NULL) > 0)
 		return;
-- 
1.7.10.4

Re: [PATCH] Add GPRS, mux, volume control support in SIMCOM sim900 modem module

[Denis Kenzior]

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

Hi Renat,

On 02/28/2013 09:43 AM, r.r.zaripov(a)gmail.com wrote:
> From: Renat Zaripov<r.r.zaripov@gmail.com>
>
> ---
>   plugins/sim900.c |  162 +++++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 137 insertions(+), 25 deletions(-)
>

Watch those trailing whitespaces:

Applying: Add GPRS, mux, volume control support in SIMCOM sim900 modem 
module
/home/denkenz/ofono-master/.git/rebase-apply/patch:118: trailing whitespace.
	if(!g_at_mux_start(data->mux))
/home/denkenz/ofono-master/.git/rebase-apply/patch:153: trailing whitespace.
	
/home/denkenz/ofono-master/.git/rebase-apply/patch:155: trailing whitespace.
	
/home/denkenz/ofono-master/.git/rebase-apply/patch:186: trailing whitespace.
	if (data->dlcs[SETUP_DLC] == NULL)
/home/denkenz/ofono-master/.git/rebase-apply/patch:202: trailing whitespace.
	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CMUX=0,0,5,128,10,3,30,10,2",
warning: squelched 1 whitespace error
fatal: 6 lines add whitespace errors.


> diff --git a/plugins/sim900.c b/plugins/sim900.c
> index 496faa6..f228c0e 100644
> --- a/plugins/sim900.c
> +++ b/plugins/sim900.c
> @@ -29,6 +29,7 @@
>   #include<glib.h>
>   #include<gatchat.h>
>   #include<gattty.h>
> +#include<gatmux.h>
>
>   #define OFONO_API_SUBJECT_TO_CHANGE
>   #include<ofono/plugin.h>
> @@ -44,13 +45,28 @@
>   #include<ofono/history.h>
>   #include<ofono/log.h>
>   #include<ofono/voicecall.h>
> +#include<ofono/call-volume.h>
>
>   #include<drivers/atmodem/vendor.h>
>
> +#define NUM_DLC 5
> +
> +#define SETUP_DLC   0
> +#define VOICE_DLC   1
> +#define NETREG_DLC  2
> +#define SMS_DLC     3
> +#define GPRS_DLC    4
> +
> +

No double empty lines please

> +static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ", "GPRS: " , "Setup: "};

This line is over 80 characters

> +
>   static const char *none_prefix[] = { NULL };
>
>   struct sim900_data {
> -	GAtChat *modem;
> +	GIOChannel *device;
> +	GAtMux *mux;
> +	GAtChat *dlcs[NUM_DLC];
> +	guint frame_size;
>   };
>
>   static int sim900_probe(struct ofono_modem *modem)
> @@ -76,7 +92,7 @@ static void sim900_remove(struct ofono_modem *modem)
>
>   	ofono_modem_set_data(modem, NULL);
>
> -	g_at_chat_unref(data->modem);
> +	g_at_chat_unref(data->dlcs[SETUP_DLC]);

Why do you only take care of 1 DLC here? You need to clean up all of 
them and the mux as well.

>
>   	g_free(data);
>   }
> @@ -91,6 +107,7 @@ static void sim900_debug(const char *str, void *user_data)
>   static GAtChat *open_device(struct ofono_modem *modem,
>   				const char *key, char *debug)
>   {
> +	struct sim900_data *data = ofono_modem_get_data(modem);
>   	const char *device;
>   	GAtSyntax *syntax;
>   	GIOChannel *channel;
> @@ -119,10 +136,33 @@ static GAtChat *open_device(struct ofono_modem *modem,
>   	if (channel == NULL)
>   		return NULL;
>
> +	data->device = channel;
>   	syntax = g_at_syntax_new_gsm_permissive();
>   	chat = g_at_chat_new(channel, syntax);
>   	g_at_syntax_unref(syntax);
>
> +	if (chat == NULL)
> +		return NULL;
> +
> +	if (getenv("OFONO_AT_DEBUG"))
> +		g_at_chat_set_debug(chat, sim900_debug, debug);
> +
> +	return chat;
> +}
> +
> +

Again please no double-empty lines

> +static GAtChat *create_chat(GIOChannel *channel, struct ofono_modem *modem,
> +								char *debug)
> +{
> +	GAtSyntax *syntax;
> +	GAtChat *chat;
> +
> +	if (channel == NULL)
> +		return NULL;
> +
> +	syntax = g_at_syntax_new_gsmv1();
> +	chat = g_at_chat_new(channel, syntax);
> +	g_at_syntax_unref(syntax);
>   	g_io_channel_unref(channel);
>
>   	if (chat == NULL)
> @@ -134,6 +174,65 @@ static GAtChat *open_device(struct ofono_modem *modem,
>   	return chat;
>   }
>
> +static void setup_internal_mux(struct ofono_modem *modem)
> +{
> +	struct sim900_data *data = ofono_modem_get_data(modem);
> +	int i;
> +
> +	DBG("");
> +
> +	data->frame_size = 128;
> +
> +	data->mux = g_at_mux_new_gsm0710_basic(data->device, data->frame_size);
> +	if (data->mux == NULL)
> +		goto error;
> +
> +	if (getenv("OFONO_MUX_DEBUG"))
> +		g_at_mux_set_debug(data->mux, sim900_debug, "MUX: ");
> +
> +	if(!g_at_mux_start(data->mux))

You're not cleaning up the mux here

> +		goto error;
> +
> +	for (i = 0; i<  NUM_DLC; i++) {
> +		GIOChannel *channel = g_at_mux_create_channel(data->mux);
> +
> +		data->dlcs[i] = create_chat(channel, modem, dlc_prefixes[i]);
> +		if (data->dlcs[i] == NULL) {
> +			ofono_error("Failed to create channel");
> +			goto error;

You are not cleaning up channels or the mux here

> +		}
> +	}
> +
> +	ofono_modem_set_powered(modem, TRUE);
> +
> +	return;
> +
> +error:
> +	ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct sim900_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	g_at_chat_unref(data->dlcs[SETUP_DLC]);
> +	data->dlcs[SETUP_DLC] = NULL;
> +
> +	if (!ok)
> +		goto error;
> +
> +	setup_internal_mux(modem);
> +	
> +	return;
> +	
> +error:
> +	ofono_modem_set_powered(modem, FALSE);

You also need to close the device IO channel.

> +}
> +
> +

And again, double empty lines :)

>   static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>   {
>   	struct ofono_modem *modem = user_data;
> @@ -142,11 +241,9 @@ 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->dlcs[SETUP_DLC]);
> +		data->dlcs[SETUP_DLC] = NULL;
>   	}
> -
> -	ofono_modem_set_powered(modem, ok);
>   }
>
>   static int sim900_enable(struct ofono_modem *modem)
> @@ -155,21 +252,22 @@ static int sim900_enable(struct ofono_modem *modem)
>
>   	DBG("%p", modem);
>
> -	data->modem = open_device(modem, "Device", "Device: ");
> -	if (data->modem == NULL) {
> -		DBG("return -EINVAL");
> +	data->dlcs[SETUP_DLC] = open_device(modem, "Device", "Setup: ");
> +	if (data->dlcs[SETUP_DLC] == NULL)
>   		return -EINVAL;
> -	}
>
> -	g_at_chat_send(data->modem, "ATE0", NULL, NULL, NULL, NULL);
> +	g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
>
>   	/* For obtain correct sms service number */
> -	g_at_chat_send(data->modem, "AT+CSCS=\"GSM\"", NULL,
> +	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
>   					NULL, NULL, NULL);
>
> -	g_at_chat_send(data->modem, "AT+CFUN=1", none_prefix,
> +	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=1", none_prefix,
>   					cfun_enable, modem, NULL);
>
> +	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CMUX=0,0,5,128,10,3,30,10,2",
> +		NULL, mux_setup_cb, modem, NULL);

Maybe you should do this from cfun_enable, and call 
ofono_modem_set_powered once the mux is setup, not before.

> +
>   	return -EINPROGRESS;
>   }
>
> @@ -181,8 +279,8 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
>
>   	DBG("");
>
> -	g_at_chat_unref(data->modem);
> -	data->modem = NULL;
> +	g_at_chat_unref(data->dlcs[SETUP_DLC]);
> +	data->dlcs[SETUP_DLC] = NULL;

You need to clean up properly here, including all chats, muxer, and 
device channel.

>
>   	if (ok)
>   		ofono_modem_set_powered(modem, FALSE);
> @@ -194,10 +292,10 @@ static int sim900_disable(struct ofono_modem *modem)
>
>   	DBG("%p", modem);
>
> -	g_at_chat_cancel_all(data->modem);
> -	g_at_chat_unregister_all(data->modem);
> +	g_at_chat_cancel_all(data->dlcs[SETUP_DLC]);
> +	g_at_chat_unregister_all(data->dlcs[SETUP_DLC]);
>
> -	g_at_chat_send(data->modem, "AT+CFUN=4", none_prefix,
> +	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=4", none_prefix,
>   					cfun_disable, modem, NULL);
>
>   	return -EINPROGRESS;
> @@ -210,9 +308,9 @@ static void sim900_pre_sim(struct ofono_modem *modem)
>
>   	DBG("%p", modem);
>
> -	ofono_devinfo_create(modem, 0, "atmodem", data->modem);
> +	ofono_devinfo_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
>   	sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> -				data->modem);
> +				data->dlcs[VOICE_DLC]);
>
>   	if (sim)
>   		ofono_sim_inserted_notify(sim, TRUE);
> @@ -221,12 +319,25 @@ static void sim900_pre_sim(struct ofono_modem *modem)
>   static void sim900_post_sim(struct ofono_modem *modem)
>   {
>   	struct sim900_data *data = ofono_modem_get_data(modem);
> +	struct ofono_gprs *gprs;
> +	struct ofono_gprs_context *gc;
>
>   	DBG("%p", modem);
>
> -	ofono_phonebook_create(modem, 0, "atmodem", data->modem);
> +	ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
>   	ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> -				data->modem);
> +				data->dlcs[SMS_DLC]);
> +
> +	gprs = ofono_gprs_create(modem, 0,		
> +					"atmodem", data->dlcs[GPRS_DLC]);
> +	if (gprs == NULL)
> +		return;
> +
> +	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
> +					"atmodem", data->dlcs[GPRS_DLC]);
> +	if (gc)
> +		ofono_gprs_add_context(gprs, gc);
> +
>   }
>
>   static void sim900_post_online(struct ofono_modem *modem)
> @@ -235,9 +346,10 @@ static void sim900_post_online(struct ofono_modem *modem)
>
>   	DBG("%p", modem);
>
> -	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->modem);
> -	ofono_ussd_create(modem, 0, "atmodem", data->modem);
> -	ofono_voicecall_create(modem, 0, "atmodem", data->modem);
> +	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->dlcs[VOICE_DLC]);
> +	ofono_ussd_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> +	ofono_voicecall_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> +	ofono_call_volume_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
>   }
>
>   static struct ofono_modem_driver sim900_driver = {

Regards,
-Denis

Re: [PATCH] Add vendor quirk for SIMCOM

[Denis Kenzior]

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

Hi Renat,

On 02/28/2013 09:43 AM, r.r.zaripov(a)gmail.com wrote:
> From: Renat Zaripov<r.r.zaripov@gmail.com>
>
> ---
>   drivers/atmodem/gprs-context.c |    6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
> index 3694c27..c5a0ae7 100644
> --- a/drivers/atmodem/gprs-context.c
> +++ b/drivers/atmodem/gprs-context.c
> @@ -208,7 +208,11 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   		return;
>   	}
>
> -	sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
> +	if (gcd->vendor == OFONO_VENDOR_SIMCOM)
> +		memcpy(buf, "ATD*99#", 7);

At lease use strcpy, since you're not copying the '\0' here actually.

> +	else
> +		sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
> +
>   	if (g_at_chat_send(gcd->chat, buf, none_prefix,
>   				at_cgdata_cb, gc, NULL)>  0)
>   		return;

Regards,
-Denis

[PATCH] Add GPRS, mux, volume control support in SIMCOM sim900 modem module

[r.r.zaripov]

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

From: Renat Zaripov <r.r.zaripov@gmail.com>

---
 plugins/sim900.c |  201 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 171 insertions(+), 30 deletions(-)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index 496faa6..5d6338c 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -25,10 +25,10 @@
 
 #include <errno.h>
 #include <stdlib.h>
-
 #include <glib.h>
 #include <gatchat.h>
 #include <gattty.h>
+#include <gatmux.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/plugin.h>
@@ -44,13 +44,27 @@
 #include <ofono/history.h>
 #include <ofono/log.h>
 #include <ofono/voicecall.h>
-
+#include <ofono/call-volume.h>
 #include <drivers/atmodem/vendor.h>
 
+#define NUM_DLC 5
+
+#define SETUP_DLC   0
+#define VOICE_DLC   1
+#define NETREG_DLC  2
+#define SMS_DLC     3
+#define GPRS_DLC    4
+
+static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
+					"GPRS: " , "Setup: "};
+
 static const char *none_prefix[] = { NULL };
 
 struct sim900_data {
-	GAtChat *modem;
+	GIOChannel *device;
+	GAtMux *mux;
+	GAtChat * dlcs[NUM_DLC];
+	guint frame_size;
 };
 
 static int sim900_probe(struct ofono_modem *modem)
@@ -76,8 +90,6 @@ static void sim900_remove(struct ofono_modem *modem)
 
 	ofono_modem_set_data(modem, NULL);
 
-	g_at_chat_unref(data->modem);
-
 	g_free(data);
 }
 
@@ -89,8 +101,9 @@ static void sim900_debug(const char *str, void *user_data)
 }
 
 static GAtChat *open_device(struct ofono_modem *modem,
-				const char *key, char *debug)
+			const char *key, char *debug)
 {
+	struct sim900_data *data = ofono_modem_get_data(modem);
 	const char *device;
 	GAtSyntax *syntax;
 	GIOChannel *channel;
@@ -119,10 +132,32 @@ static GAtChat *open_device(struct ofono_modem *modem,
 	if (channel == NULL)
 		return NULL;
 
+	data->device = channel;
 	syntax = g_at_syntax_new_gsm_permissive();
 	chat = g_at_chat_new(channel, syntax);
 	g_at_syntax_unref(syntax);
 
+	if (chat == NULL)
+		return NULL;
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, sim900_debug, debug);
+
+	return chat;
+}
+
+static GAtChat *create_chat(GIOChannel *channel, struct ofono_modem *modem,
+				char *debug)
+{
+	GAtSyntax *syntax;
+	GAtChat *chat;
+
+	if (channel == NULL)
+		return NULL;
+
+	syntax = g_at_syntax_new_gsmv1();
+	chat = g_at_chat_new(channel, syntax);
+	g_at_syntax_unref(syntax);
 	g_io_channel_unref(channel);
 
 	if (chat == NULL)
@@ -134,6 +169,96 @@ static GAtChat *open_device(struct ofono_modem *modem,
 	return chat;
 }
 
+static void shutdown_device(struct sim900_data *data)
+{
+	int i, fd;
+
+	DBG("");
+
+	for (i = 0; i < NUM_DLC; i++) {
+		if (data->dlcs[i] == NULL)
+			continue;
+
+		g_at_chat_unref(data->dlcs[i]);
+		data->dlcs[i] = NULL;
+	}
+
+	if (data->mux) {
+		g_at_mux_shutdown(data->mux);
+		g_at_mux_unref(data->mux);
+		data->mux = NULL;
+		goto done;
+	}
+
+done:
+	g_io_channel_unref(data->device);
+	data->device = NULL;
+}
+
+static void setup_internal_mux(struct ofono_modem *modem)
+{
+	struct sim900_data *data = ofono_modem_get_data(modem);
+	int i;
+
+	DBG("");
+
+	data->frame_size = 128;
+
+	data->mux = g_at_mux_new_gsm0710_basic(data->device,
+						data->frame_size);
+	if (data->mux == NULL)
+		goto error;
+
+	if (getenv("OFONO_MUX_DEBUG"))
+		g_at_mux_set_debug(data->mux, sim900_debug, "MUX: ");
+
+	if (!g_at_mux_start(data->mux)) {
+		g_at_mux_shutdown(data->mux);
+		g_at_mux_unref(data->mux);
+		goto error;
+	}
+
+	for (i = 0; i < NUM_DLC; i++) {
+		GIOChannel *channel = g_at_mux_create_channel(data->mux);
+
+		data->dlcs[i] = create_chat(channel, modem, dlc_prefixes[i]);
+		if (data->dlcs[i] == NULL) {
+			ofono_error("Failed to create channel");
+			goto error;
+		}
+	}
+
+	ofono_modem_set_powered(modem, TRUE);
+
+	return;
+
+error:
+	shutdown_device(data);
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct sim900_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	g_at_chat_unref(data->dlcs[SETUP_DLC]);
+	data->dlcs[SETUP_DLC] = NULL;
+
+	if (!ok)
+		goto error;
+
+	setup_internal_mux(modem);
+
+	return;
+
+error:
+	shutdown_device(data);
+	ofono_modem_set_powered(modem, FALSE);
+}
+
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -142,11 +267,15 @@ 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->dlcs[SETUP_DLC]);
+		data->dlcs[SETUP_DLC] = NULL;
+		ofono_modem_set_powered(modem, FALSE);
+		return;
 	}
 
-	ofono_modem_set_powered(modem, ok);
+	g_at_chat_send(data->dlcs[SETUP_DLC],
+			"AT+CMUX=0,0,5,128,10,3,30,10,2", NULL,
+			mux_setup_cb, modem, NULL);
 }
 
 static int sim900_enable(struct ofono_modem *modem)
@@ -155,25 +284,22 @@ static int sim900_enable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	data->modem = open_device(modem, "Device", "Device: ");
-	if (data->modem == NULL) {
-		DBG("return -EINVAL");
+	data->dlcs[SETUP_DLC] = open_device(modem, "Device", "Setup: ");
+	if (data->dlcs[SETUP_DLC] == NULL)
 		return -EINVAL;
-	}
 
-	g_at_chat_send(data->modem, "ATE0", NULL, NULL, NULL, NULL);
+	g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
 
 	/* For obtain correct sms service number */
-	g_at_chat_send(data->modem, "AT+CSCS=\"GSM\"", NULL,
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
 					NULL, NULL, NULL);
 
-	g_at_chat_send(data->modem, "AT+CFUN=1", none_prefix,
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=1", none_prefix,
 					cfun_enable, modem, NULL);
 
 	return -EINPROGRESS;
 }
 
-
 static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -181,8 +307,11 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("");
 
-	g_at_chat_unref(data->modem);
-	data->modem = NULL;
+	g_at_mux_shutdown(data->mux);
+	g_at_mux_unref(data->mux);
+
+	g_at_chat_unref(data->dlcs[SETUP_DLC]);
+	data->dlcs[SETUP_DLC] = NULL;
 
 	if (ok)
 		ofono_modem_set_powered(modem, FALSE);
@@ -194,12 +323,11 @@ static int sim900_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	g_at_chat_cancel_all(data->modem);
-	g_at_chat_unregister_all(data->modem);
-
-	g_at_chat_send(data->modem, "AT+CFUN=4", none_prefix,
+	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=4", none_prefix,
 					cfun_disable, modem, NULL);
 
+	shutdown_device(data);
+
 	return -EINPROGRESS;
 }
 
@@ -210,9 +338,9 @@ static void sim900_pre_sim(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	ofono_devinfo_create(modem, 0, "atmodem", data->modem);
+	ofono_devinfo_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
 	sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
-				data->modem);
+						data->dlcs[VOICE_DLC]);
 
 	if (sim)
 		ofono_sim_inserted_notify(sim, TRUE);
@@ -221,12 +349,23 @@ static void sim900_pre_sim(struct ofono_modem *modem)
 static void sim900_post_sim(struct ofono_modem *modem)
 {
 	struct sim900_data *data = ofono_modem_get_data(modem);
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
 
 	DBG("%p", modem);
 
-	ofono_phonebook_create(modem, 0, "atmodem", data->modem);
+	ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
 	ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
-				data->modem);
+						data->dlcs[SMS_DLC]);
+
+	gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+	if (gprs == NULL)
+		return;
+
+	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+					"atmodem", data->dlcs[GPRS_DLC]);
+	if (gc)
+		ofono_gprs_add_context(gprs, gc);
 }
 
 static void sim900_post_online(struct ofono_modem *modem)
@@ -235,9 +374,11 @@ static void sim900_post_online(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->modem);
-	ofono_ussd_create(modem, 0, "atmodem", data->modem);
-	ofono_voicecall_create(modem, 0, "atmodem", data->modem);
+	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM,
+			"atmodem", data->dlcs[NETREG_DLC]);
+	ofono_ussd_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+	ofono_voicecall_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+	ofono_call_volume_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
 }
 
 static struct ofono_modem_driver sim900_driver = {
-- 
1.7.10.4

Re: [PATCH] Add GPRS, mux, volume control support in SIMCOM sim900 modem module

[Denis Kenzior]

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

Hi Renat,

On 03/12/2013 09:37 AM, r.r.zaripov(a)gmail.com wrote:
> From: Renat Zaripov<r.r.zaripov@gmail.com>
>
> ---
>   plugins/sim900.c |  201 ++++++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 171 insertions(+), 30 deletions(-)
>

I applied this patch, however here are a few comments:

> @@ -119,10 +132,32 @@ static GAtChat *open_device(struct ofono_modem *modem,
>   	if (channel == NULL)
>   		return NULL;
>
> +	data->device = channel;
>   	syntax = g_at_syntax_new_gsm_permissive();
>   	chat = g_at_chat_new(channel, syntax);
>   	g_at_syntax_unref(syntax);
>
> +	if (chat == NULL)
> +		return NULL;

In this case you are not destroying the data->device object.  This is a 
potential memory leak.  I fixed this for you in commit 
b4518caa50f03b40e9f9434dc17558665bb8ee3f

> +
> +	if (getenv("OFONO_AT_DEBUG"))
> +		g_at_chat_set_debug(chat, sim900_debug, debug);
> +
> +	return chat;
> +}
> +

<snip>

> @@ -134,6 +169,96 @@ static GAtChat *open_device(struct ofono_modem *modem,
>   	return chat;
>   }
>
> +static void shutdown_device(struct sim900_data *data)
> +{
> +	int i, fd;

make --no-print-directory all-am
   CC     plugins/sim900.o
cc1: warnings being treated as errors
plugins/sim900.c: In function ‘shutdown_device’:
plugins/sim900.c:180:9: error: unused variable ‘fd’
make[1]: *** [plugins/sim900.o] Error 1
make: *** [all] Error 2

Please 'make distcheck' before submitting patches.

> +
> +	DBG("");
> +
> +	for (i = 0; i<  NUM_DLC; i++) {
> +		if (data->dlcs[i] == NULL)
> +			continue;
> +
> +		g_at_chat_unref(data->dlcs[i]);
> +		data->dlcs[i] = NULL;
> +	}
> +
> +	if (data->mux) {
> +		g_at_mux_shutdown(data->mux);
> +		g_at_mux_unref(data->mux);
> +		data->mux = NULL;
> +		goto done;

This goto seems redundant.  I got rid of this for you in commit 
548611e93948902ccffaf5568dcd31bef1d4f764.

> +	}
> +
> +done:
> +	g_io_channel_unref(data->device);
> +	data->device = NULL;
> +}
> +

<snip>

>   static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
>   {
>   	struct ofono_modem *modem = user_data;
> @@ -181,8 +307,11 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
>
>   	DBG("");
>
> -	g_at_chat_unref(data->modem);
> -	data->modem = NULL;
> +	g_at_mux_shutdown(data->mux);
> +	g_at_mux_unref(data->mux);
> +
> +	g_at_chat_unref(data->dlcs[SETUP_DLC]);
> +	data->dlcs[SETUP_DLC] = NULL;
>
>   	if (ok)
>   		ofono_modem_set_powered(modem, FALSE);


> @@ -194,12 +323,11 @@ static int sim900_disable(struct ofono_modem *modem)
>
>   	DBG("%p", modem);
>
> -	g_at_chat_cancel_all(data->modem);
> -	g_at_chat_unregister_all(data->modem);
> -
> -	g_at_chat_send(data->modem, "AT+CFUN=4", none_prefix,
> +	g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=4", none_prefix,
>   					cfun_disable, modem, NULL);
>
> +	shutdown_device(data);
> +

Calling shutdown_device here is premature.  I bet that you're never 
actually sending the CFUN=4 with this setup because you close the tty 
before that happens.  I fixed this in commit 
93ac1669a069a1bcce1ed121ca60977db53abf4e.  If you disagree, please let 
me know.

>   	return -EINPROGRESS;
>   }
>

Regards,
-Denis