[PATCH 3/3] xmm7modem: data channel setup for xmm7modem

[PATCH 3/3] xmm7modem: data channel setup for xmm7modem

[Antara Borwankar]

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

Added the implementation required to setup data channel in
ifxmodem gprs-context driver for xmm7modem vendor
---
 drivers/ifxmodem/gprs-context.c | 43 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 7f3628b..278d507 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -260,6 +260,22 @@ error:
 	failed_setup(gc, NULL, TRUE);
 }
 
+static void cgdata_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_gprs_context *gc = user_data;
+	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+
+	DBG("ok %d", ok);
+
+	if (!ok) {
+		ofono_error("Failed to establish session");
+		failed_setup(gc, result, TRUE);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+}
+
 static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_gprs_context *gc = user_data;
@@ -269,8 +285,11 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	const char *laddrnetmask = NULL;
 	const char *gw = NULL;
-	const char *interface;
 	const char *dns[3];
+	const char *ctrlpath;
+	char *datapath;
+	char buf[100];
+	const char *interface;
 
 	DBG("ok %d", ok);
 
@@ -327,9 +346,13 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	DBG("DNS2: %s\n", gcd->dns2);
 	DBG("Gateway: %s\n", gcd->gateway);
 
-	interface = ofono_modem_get_string(modem, "NetworkInterface");
+	ctrlpath = ofono_modem_get_string(modem, "CtrlPath");
+	interface = ofono_gprs_context_get_interface(gc);
+
+	sprintf(buf, "%s%c", ofono_modem_get_string(modem, "DataPath"),
+					interface[strlen(interface)-1]);
+	datapath = g_strdup(buf);
 
-	ofono_gprs_context_set_interface(gc, interface);
 	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
 
 	if (gcd->netmask[0])
@@ -340,7 +363,19 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
 
-	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+	sprintf(buf, "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u", ctrlpath,
+				datapath, gcd->active_context);
+	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	sprintf(buf, "AT+CGDATA=\"M-RAW_IP\",%u", gcd->active_context);
+
+	if (g_at_chat_send(gcd->chat, buf, none_prefix, cgdata_cb,
+						gc, NULL) > 0) {
+		g_free(datapath);
+		return;
+	}
+
+	g_free(datapath);
+	CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
 }
 
 static void ifx_read_settings(struct ofono_gprs_context *gc)
-- 
1.9.1

Re: [PATCH 3/3] xmm7modem: data channel setup for xmm7modem

[Denis Kenzior]

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

Hi Antara,

On 02/15/2019 05:43 AM, Antara Borwankar wrote:
> Added the implementation required to setup data channel in
> ifxmodem gprs-context driver for xmm7modem vendor
> ---
>   drivers/ifxmodem/gprs-context.c | 43 +++++++++++++++++++++++++++++++++++++----
>   1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
> index 7f3628b..278d507 100644
> --- a/drivers/ifxmodem/gprs-context.c
> +++ b/drivers/ifxmodem/gprs-context.c
> @@ -260,6 +260,22 @@ error:
>   	failed_setup(gc, NULL, TRUE);
>   }
>   
> +static void cgdata_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_gprs_context *gc = user_data;
> +	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
> +
> +	DBG("ok %d", ok);
> +
> +	if (!ok) {
> +		ofono_error("Failed to establish session");
> +		failed_setup(gc, result, TRUE);
> +		return;
> +	}
> +
> +	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
> +}
> +
>   static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   {
>   	struct ofono_gprs_context *gc = user_data;
> @@ -269,8 +285,11 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   
>   	const char *laddrnetmask = NULL;
>   	const char *gw = NULL;
> -	const char *interface;
>   	const char *dns[3];
> +	const char *ctrlpath;
> +	char *datapath;
> +	char buf[100];
> +	const char *interface;
>   
>   	DBG("ok %d", ok);
>   
> @@ -327,9 +346,13 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   	DBG("DNS2: %s\n", gcd->dns2);
>   	DBG("Gateway: %s\n", gcd->gateway);
>   
> -	interface = ofono_modem_get_string(modem, "NetworkInterface");
> +	ctrlpath = ofono_modem_get_string(modem, "CtrlPath");
> +	interface = ofono_gprs_context_get_interface(gc);
> +
> +	sprintf(buf, "%s%c", ofono_modem_get_string(modem, "DataPath"),
> +					interface[strlen(interface)-1]);

Wouldn't this fail if multiple modems are plugged in?  It seems like 
just basing this off the interface name is not enough.

See attached patches for an idea of how to handle this.  Not fully happy 
with it, so if you have other ideas please suggest.

> +	datapath = g_strdup(buf);

Why do you need this strdup?  Just make your life easier and use 
g_strdup_printf instead of sprintf above.

Or better yet, simply use a different buffer on the stack for this.

>   
> -	ofono_gprs_context_set_interface(gc, interface);
>   	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
>   
>   	if (gcd->netmask[0])
> @@ -340,7 +363,19 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   
>   	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
>   
> -	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
> +	sprintf(buf, "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u", ctrlpath,
> +				datapath, gcd->active_context);

You can free datapath here actually instead of repeating the g_free 
twice below.

> +	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
> +	sprintf(buf, "AT+CGDATA=\"M-RAW_IP\",%u", gcd->active_context);
> +
> +	if (g_at_chat_send(gcd->chat, buf, none_prefix, cgdata_cb,
> +						gc, NULL) > 0) {
> +		g_free(datapath);
> +		return;
> +	}
> +
> +	g_free(datapath);
> +	CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
>   }
>   
>   static void ifx_read_settings(struct ofono_gprs_context *gc)
> 

Regards,
-Denis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-include-Add-ofono_gprs_context_get_ordinal.patch --]
[-- Type: text/x-patch, Size: 878 bytes --]

>From 497da103a6ec0d17440fdf8d00c4f2237424da4e Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Thu, 21 Feb 2019 11:07:30 -0600
Subject: [PATCH 1/2] include: Add ofono_gprs_context_get_ordinal

---
 include/gprs-context.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/gprs-context.h b/include/gprs-context.h
index ad0872f4..8c0226ee 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -86,6 +86,7 @@ void ofono_gprs_context_set_data(struct ofono_gprs_context *gc, void *data);
 void *ofono_gprs_context_get_data(struct ofono_gprs_context *gc);
 
 struct ofono_modem *ofono_gprs_context_get_modem(struct ofono_gprs_context *gc);
+unsigned int ofono_gprs_context_get_ordinal(struct ofono_gprs_context *gc);
 
 void ofono_gprs_context_set_type(struct ofono_gprs_context *gc,
 					enum ofono_gprs_context_type type);
-- 
2.13.5


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gprs-implement-ofono_gprs_context_get_ordinal.patch --]
[-- Type: text/x-patch, Size: 1354 bytes --]

>From 8b06d2bc203ee4737620e4dfef953ed13a8bbf81 Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Thu, 21 Feb 2019 11:21:38 -0600
Subject: [PATCH 2/2] gprs: implement ofono_gprs_context_get_ordinal

---
 src/gprs.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/gprs.c b/src/gprs.c
index b0d56e1e..ba2a47cb 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -2797,6 +2797,25 @@ struct ofono_modem *ofono_gprs_context_get_modem(struct ofono_gprs_context *gc)
 	return __ofono_atom_get_modem(gc->atom);
 }
 
+/*
+ * Returns the ordinal position of this context (the order is usually the same
+ * as the sequence of ofono_gprs_add_context calls) starting from 1.  Note that
+ * if one of the context drivers fails to probe or calls
+ * ofono_gprs_context_remove, then this function will return an index that
+ * differs from the ofono_gprs_add_context order.  This should generally not
+ * happen, as the above situation generally indicates a bigger problem
+ */
+unsigned int ofono_gprs_context_get_ordinal(struct ofono_gprs_context *gc)
+{
+	if (!gc)
+		return 0;
+
+	if (!__ofono_atom_get_registered(gc->atom))
+		return 0;
+
+	return g_slist_index(gc->gprs->context_drivers, gc) + 1;
+}
+
 void ofono_gprs_context_set_type(struct ofono_gprs_context *gc,
 					enum ofono_gprs_context_type type)
 {
-- 
2.13.5