gemaltomodem: added Gemalto-specific gprs-context atom This atom name is not the same as the driver, because it might be necessary to add other grps-context atoms, according to the needs. The name is therefore referring to the main command used, AT^SWWAN.

---
  drivers/gemaltomodem/gemaltomodem.c      |   2 +
  drivers/gemaltomodem/gemaltomodem.h      |   3 +
  drivers/gemaltomodem/gprs-context-wwan.c | 446 +++++++++++++++++++++++++++++++
  3 files changed, 451 insertions(+)

create mode 100644 drivers/gemaltomodem/gprs-context-wwan.c

drivers/gemaltomodem/gemaltomodem.c +1 -0

index 614ca81..7afd3c1 100644
@@ -35,6 +35,7 @@
  static int gemaltomodem_init(void)
  {
  	gemalto_location_reporting_init();
+	gemaltowwan_gprs_context_init();
  	gemalto_voicecall_init();
  	return 0;
  }

drivers/gemaltomodem/gemaltomodem.c +0 -0

  	gemalto_voicecall_exit();
  }
 

drivers/gemaltomodem/gemaltomodem.h +3 -0

index 4e6ed16..f45aea9 100644
@@ -24,5 +24,8 @@
  extern void gemalto_location_reporting_init();
  extern void gemalto_location_reporting_exit();
 
+extern void gemaltowwan_gprs_context_init();
+extern void gemaltowwan_gprs_context_exit();
+
  extern void gemalto_voicecall_init();
  extern void gemalto_voicecall_exit();

drivers/gemaltomodem/gprs-context-wwan.c new file +64 -0

new file mode 100644
index 0000000..12378a3
@@ -0,0 +1,446 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Piotr Haber. All rights reserved.
+ *  Copyright (C) 2018 Sebastian Arnd. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <sys/stat.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/gprs-context.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "gemaltomodem.h"
+
+static const char *none_prefix[] = { NULL };
+
+enum state {
+	STATE_IDLE,
+	STATE_ENABLING,
+	STATE_DISABLING,
+	STATE_ACTIVE,
+};
+
+struct gprs_context_data {
+	GAtChat *chat;
+	unsigned int active_context;
+	char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
+	char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
+	enum ofono_gprs_auth_method auth_method;
+	enum state state;
+	enum ofono_gprs_proto proto;
+	char address[64];
+	char netmask[64];
+	char gateway[64];
+	char dns1[64];
+	char dns2[64];
+	ofono_gprs_context_cb_t cb;

drivers/gemaltomodem/gprs-context-wwan.c new file +7 -0

+	int len;
+	/*
+	 * 0: use cgauth
+	 * 1: use sgauth(pwd, user)
+	 * 2: use sgauth(user, pwd)
+	 */
+

drivers/gemaltomodem/gprs-context-wwan.c new file +24 -0

+	default:
+		auth_type = 0;
+		break;
+	}
+
+	if (auth_type != 0 && (!*username || !*password)) > +		return FALSE;
+
+	switch (gto_auth) {
+	case 1:
+	case 2:
+		len = snprintf(buf, buflen, "AT^SGAUTH=%d", cid);
+		break;
+	case 0:
+	default:
+		len = snprintf(buf, buflen, "AT+CGAUTH=%d", cid);
+		break;
+	}
+
+	buflen -= len;
+
+	switch(auth_type) {
+	case 0:
+
+		switch (gto_auth) {

drivers/gemaltomodem/gprs-context-wwan.c new file +116 -0

+
+	case 1:
+	case 2:
+
+		switch (gto_auth) {
+		case 1:
+			snprintf(buf+len, buflen, ",%d,\"%s\",\"%s\"",
+					auth_type, password, username);
+			break;
+		case 0:
+		case 2:
+		default:
+			snprintf(buf+len, buflen, ",%d,\"%s\",\"%s\"",
+					auth_type, username, password);
+		}
+		break;
+
+	default:
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+static void gemalto_get_cgdcont_command(struct ofono_modem *modem,
+			guint cid, enum ofono_gprs_proto proto, const char *apn,
+							char *buf, guint buflen)
+{
+	int len = snprintf(buf, buflen, "AT+CGDCONT=%u", cid);
+	buflen -= len;
+
+	if (!apn) /* it will remove the context */
+		return;
+
+	switch (proto) {
+	case OFONO_GPRS_PROTO_IPV6:
+		snprintf(buf+len, buflen, ",\"IPV6\",\"%s\"", apn);
+		break;
+	case OFONO_GPRS_PROTO_IPV4V6:
+		snprintf(buf+len, buflen, ",\"IPV4V6\",\"%s\"", apn);
+		break;
+	case OFONO_GPRS_PROTO_IP:
+	default:
+		snprintf(buf+len, buflen, ",\"IP\",\"%s\"", apn);
+		break;
+	}
+}
+
+static void failed_setup(struct ofono_gprs_context *gc,
+				GAtResult *result, gboolean deactivate)
+{
+	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+	struct ofono_error error;
+	char buf[64];
+
+	DBG("deactivate %d", deactivate);
+
+	if (deactivate == TRUE) {
+
+		if (gcd->use_wwan)
+			sprintf(buf, "AT^SWWAN=0,%u", gcd->active_context);
+		else
+			sprintf(buf, "AT+CGACT=0,%u", gcd->active_context);
+
+		g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
+	}
+
+	gcd->active_context = 0;
+	gcd->state = STATE_IDLE;
+
+	if (result == NULL) {
+		CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
+		return;
+	}
+
+	decode_at_error(&error, g_at_result_final_response(result));
+	gcd->cb(&error, gcd->cb_data);
+}
+
+static void activate_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("Unable activate context");
+		/*
+		 * We've reported sucess already, so can't just call
+		 * failed_setup we call ofono_gprs_context_deactivated instead.
+		 * Thats not a clean solution at all, but as it seems there is
+		 * no clean way to determine whether it is possible to activate
+		 * the context before issuing AT^SWWAN. A possible workaround
+		 * might be to issue AT+CGACT=1 and AT+CGACT=0 and try if that
+		 * works, before calling CALLBACK_WITH_SUCCESS.
+		 */
+		ofono_gprs_context_deactivated(gc, gcd->active_context);
+		gcd->active_context = 0;
+		gcd->state = STATE_IDLE;
+		return;
+	}
+	/* We've reported sucess already */
+}
+
+static void setup_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);
+	char buf[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+					OFONO_GPRS_MAX_PASSWORD_LENGTH +1];
+	struct ofono_modem *modem = ofono_gprs_context_get_modem(gc);
+	const char *interface;
+
+	if (!ok) {
+		ofono_error("Failed to setup context");

drivers/gemaltomodem/gprs-context-wwan.c new file +16 -0

+	}
+	/*
+	 * note that if the auth command is not ok we skip it and continue
+	 * but if the sending fails we do an error
+	 */
+
+	if (gcd->use_wwan)
+		sprintf(buf, "AT^SWWAN=1,%u", gcd->active_context);
+	else
+		sprintf(buf, "AT+CGACT=%u,1", gcd->active_context);
+
+	if (g_at_chat_send(gcd->chat, buf, none_prefix,
+					activate_cb, gc, NULL) > 0){
+
+		interface = ofono_modem_get_string(modem, "NetworkInterface");
+

drivers/gemaltomodem/gprs-context-wwan.c new file +158 -0

+
+		return;
+	}
+
+error:
+	failed_setup(gc, NULL, FALSE);
+}
+
+static void gemaltowwan_gprs_activate_primary(struct ofono_gprs_context *gc,
+				const struct ofono_gprs_primary_context *ctx,
+				ofono_gprs_context_cb_t cb, void *data)
+{
+	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+	char buf[OFONO_GPRS_MAX_APN_LENGTH + 128];
+	struct ofono_modem *modem = ofono_gprs_context_get_modem(gc);
+
+	DBG("cid %u", ctx->cid);
+
+	gcd->use_wwan = ofono_modem_get_integer(modem, "Gemalto_WWAN");
+	gcd->active_context = ctx->cid;
+	gcd->cb = cb;
+	gcd->cb_data = data;
+	memcpy(gcd->username, ctx->username, sizeof(ctx->username));
+	memcpy(gcd->password, ctx->password, sizeof(ctx->password));
+	gcd->state = STATE_ENABLING;
+	gcd->proto = ctx->proto;
+	gcd->auth_method = ctx->auth_method;
+
+	gemalto_get_cgdcont_command(modem, ctx->cid, ctx->proto, ctx->apn, buf,
+								sizeof(buf));
+
+	if (g_at_chat_send(gcd->chat, buf, none_prefix,
+				setup_cb, gc, NULL) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static void deactivate_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);
+
+	gcd->active_context = 0;
+	gcd->state = STATE_IDLE;
+
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+}
+
+static void gemaltowwan_gprs_deactivate_primary(struct ofono_gprs_context *gc,
+					unsigned int cid,
+					ofono_gprs_context_cb_t cb, void *data)
+{
+	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+	char buf[64];
+
+	DBG("cid %u", cid);
+
+	gcd->state = STATE_DISABLING;
+	gcd->cb = cb;
+	gcd->cb_data = data;
+
+	if (gcd->use_wwan)
+		sprintf(buf, "AT^SWWAN=0,%u", gcd->active_context);
+	else
+		sprintf(buf, "AT+CGACT=%u,0", gcd->active_context);
+
+	if (g_at_chat_send(gcd->chat, buf, none_prefix,
+				deactivate_cb, gc, NULL) > 0)
+		return;
+
+	CALLBACK_WITH_SUCCESS(cb, data);
+}
+
+static void cgev_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gprs_context *gc = user_data;
+	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+	const char *event;
+	int cid = 0;
+	GAtResultIter iter;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CGEV:"))
+		return;
+
+	if (!g_at_result_iter_next_unquoted_string(&iter, &event))
+		return;
+
+	if (!g_str_has_prefix(event, "ME PDN DEACT"))
+		return;
+
+	sscanf(event, "%*s %*s %*s %u", &cid);
+
+	DBG("cid %d", cid);
+
+	if ((unsigned int) cid != gcd->active_context)
+		return;
+
+	ofono_gprs_context_deactivated(gc, gcd->active_context);
+
+	gcd->active_context = 0;
+	gcd->state = STATE_IDLE;
+}
+
+static int gemaltowwan_gprs_context_probe(struct ofono_gprs_context *gc,
+					unsigned int model, void *data)
+{
+	GAtChat *chat = data;
+	struct gprs_context_data *gcd;
+	struct ofono_modem *modem = ofono_gprs_context_get_modem(gc);
+
+	DBG("");
+
+	gcd = g_try_new0(struct gprs_context_data, 1);
+	if (gcd == NULL)
+		return -ENOMEM;
+
+	if (modem)
+		gcd->use_wwan = ofono_modem_get_integer(modem, "Gemalto_WWAN");
+	gcd->chat = g_at_chat_clone(chat);
+	ofono_gprs_context_set_data(gc, gcd);
+	g_at_chat_register(chat, "+CGEV:", cgev_notify, FALSE, gc, NULL);
+
+	return 0;
+}
+
+static void gemaltowwan_gprs_context_remove(struct ofono_gprs_context *gc)
+{
+	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+
+	DBG("");
+
+	ofono_gprs_context_set_data(gc, NULL);
+
+	g_at_chat_unref(gcd->chat);
+	g_free(gcd);
+}
+
+static void gemaltowwan_gprs_detach_shutdown(struct ofono_gprs_context *gc,
+					unsigned int cid)
+{
+	DBG("cid %u", cid);
+
+	ofono_gprs_context_deactivated(gc, cid);
+}
+
+static struct ofono_gprs_context_driver driver = {
+	.name			= "gemaltowwanmodem",
+	.probe			= gemaltowwan_gprs_context_probe,
+	.remove			= gemaltowwan_gprs_context_remove,
+	.activate_primary	= gemaltowwan_gprs_activate_primary,
+	.deactivate_primary	= gemaltowwan_gprs_deactivate_primary,
+	.detach_shutdown	= gemaltowwan_gprs_detach_shutdown,
+};

Hi Giacinto,

So please fix your commit description in the next version.

Quoted 4 lines

---
  drivers/gemaltomodem/gemaltomodem.c      |   2 +
  drivers/gemaltomodem/gemaltomodem.h      |   3 +
  drivers/gemaltomodem/gprs-context-wwan.c | 446

+++++++++++++++++++++++++++++++

Review

Quoted 4 lines

  3 files changed, 451 insertions(+)

create mode 100644 drivers/gemaltomodem/gprs-context-wwan.c

(quoted diff) +0 -0

b/drivers/gemaltomodem/gemaltomodem.c

Quoted 12 lines

index 614ca81..7afd3c1 100644

drivers/gemaltomodem/gemaltomodem.c +1 -0

@@ -35,6 +35,7 @@
  static int gemaltomodem_init(void)
  {
      gemalto_location_reporting_init();
+     gemaltowwan_gprs_context_init();
      gemalto_voicecall_init();
      return 0;
  }

@@ -42,6 +43,7 @@ static int gemaltomodem_init(void)

drivers/gemaltomodem/gemaltomodem.c static int gemaltomodem_init(void) · line 46

Quoted 4 lines

drivers/gemaltomodem/gemaltomodem.c +1 -0

  static void gemaltomodem_exit(void)
  {
      gemalto_location_reporting_exit();
+     gemaltowwan_gprs_context_exit();

I'm not really a fan of the name. How many of gprs_context types do you have, and what is fundamentally different between them that they cannot be handled by the same driver?

      gemalto_voicecall_exit();

Review

Quoted 3 lines

drivers/gemaltomodem/gemaltomodem.c +0 -0

  }
 

(quoted diff) +0 -0

b/drivers/gemaltomodem/gemaltomodem.h

Review

Quoted 13 lines

index 4e6ed16..f45aea9 100644

drivers/gemaltomodem/gemaltomodem.h +3 -0

@@ -24,5 +24,8 @@
  extern void gemalto_location_reporting_init();
  extern void gemalto_location_reporting_exit();
 
+extern void gemaltowwan_gprs_context_init();
+extern void gemaltowwan_gprs_context_exit();
+
  extern void gemalto_voicecall_init();
  extern void gemalto_voicecall_exit();

(quoted diff) +0 -0

b/drivers/gemaltomodem/gprs-context-wwan.c

Quoted 64 lines

new file mode 100644
index 0000000..12378a3

drivers/gemaltomodem/gprs-context-wwan.c new file +59 -0

@@ -0,0 +1,446 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Piotr Haber. All rights reserved.
+ *  Copyright (C) 2018 Sebastian Arnd. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <sys/stat.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/gprs-context.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "gemaltomodem.h"
+
+static const char *none_prefix[] = { NULL };
+
+enum state {
+     STATE_IDLE,
+     STATE_ENABLING,
+     STATE_DISABLING,
+     STATE_ACTIVE,
+};
+
+struct gprs_context_data {
+     GAtChat *chat;
+     unsigned int active_context;
+     char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
+     char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
+     enum ofono_gprs_auth_method auth_method;
+     enum state state;
+     enum ofono_gprs_proto proto;
+     char address[64];

drivers/gemaltomodem/gprs-context-wwan.c line 69

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+     char netmask[64];
+     char gateway[64];
+     char dns1[64];
+     char dns2[64];
+     ofono_gprs_context_cb_t cb;
+     void *cb_data;
+     int use_wwan;
+};
+
+static gboolean gemalto_get_auth_command(struct ofono_modem *modem, int

cid,

drivers/gemaltomodem/gprs-context-wwan.c line 74

Quoted 5 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +5 -0

+                             enum ofono_gprs_auth_method auth_method,
+                             const char *username, const char *password,
+                             char *buf, guint buflen)
+{
+     int gto_auth = ofono_modem_get_integer(modem, "Gemalto_Auth");

Please use a CamelCase-d variable for this. E.g. GemaltoAuthType

Quoted 14 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +13 -0

+     int len;
+     /*
+      * 0: use cgauth
+      * 1: use sgauth(pwd, user)
+      * 2: use sgauth(user, pwd)
+      */
+
+     int auth_type;
+
+     switch (auth_method) {
+     case OFONO_GPRS_AUTH_METHOD_PAP:
+             auth_type = 1;
+             break;

drivers/gemaltomodem/gprs-context-wwan.c line 91

Quoted 4 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +4 -0

+     case OFONO_GPRS_AUTH_METHOD_CHAP:
+             auth_type = 2;
+             break;
+     case OFONO_GPRS_AUTH_METHOD_NONE:

So until the GPRS_AUTH_METHOD_NONE is actually introduced, you might want to just get this upstream with the existing enumeration. E.g. just assume that if username & password is empty, the auth method is none.

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 7 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +6 -0

+     default:
+             auth_type = 0;
+             break;
+     }
+
+     if (auth_type != 0 && (!*username || !*password)) > +

return FALSE;

Quoted 24 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +24 -0

+
+     switch (gto_auth) {
+     case 1:
+     case 2:
+             len = snprintf(buf, buflen, "AT^SGAUTH=%d", cid);
+             break;
+     case 0:
+     default:
+             len = snprintf(buf, buflen, "AT+CGAUTH=%d", cid);
+             break;
+     }
+
+     buflen -= len;
+
+     switch(auth_type) {
+     case 0:
+
+             switch (gto_auth) {
+             case 2:
+                     snprintf(buf+len, buflen, ",0,\"\",\"\"");
+                     break;
+             case 0:
+             case 1:
+             default:

drivers/gemaltomodem/gprs-context-wwan.c line 126

Quoted 4 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +4 -0

+                     snprintf(buf+len, buflen, ",0");
+                     break;
+             }
+             break;

This switch might be more compact as an if statement. Also the coding style is all wrong in this function. See doc/coding-style.txt item M3 in particular.

Quoted 17 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +16 -0

+
+     case 1:
+     case 2:
+
+             switch (gto_auth) {
+             case 1:
+                     snprintf(buf+len, buflen, ",%d,\"%s\",\"%s\"",
+                                     auth_type, password, username);
+                     break;
+             case 0:
+             case 2:
+             default:
+                     snprintf(buf+len, buflen, ",%d,\"%s\",\"%s\"",
+                                     auth_type, username, password);
+             }
+             break;

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+
+     default:
+             return FALSE;
+     }
+
+     return TRUE;
+}
+
+static void gemalto_get_cgdcont_command(struct ofono_modem *modem,
+                     guint cid, enum ofono_gprs_proto proto, const char

*apn,

drivers/gemaltomodem/gprs-context-wwan.c line 152

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+                                                     char *buf, guint

buflen)

Quoted 28 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +28 -0

+{
+     int len = snprintf(buf, buflen, "AT+CGDCONT=%u", cid);
+     buflen -= len;
+
+     if (!apn) /* it will remove the context */
+             return;
+
+     switch (proto) {
+     case OFONO_GPRS_PROTO_IPV6:
+             snprintf(buf+len, buflen, ",\"IPV6\",\"%s\"", apn);
+             break;
+     case OFONO_GPRS_PROTO_IPV4V6:
+             snprintf(buf+len, buflen, ",\"IPV4V6\",\"%s\"", apn);
+             break;
+     case OFONO_GPRS_PROTO_IP:
+     default:
+             snprintf(buf+len, buflen, ",\"IP\",\"%s\"", apn);
+             break;
+     }
+}
+
+static void failed_setup(struct ofono_gprs_context *gc,
+                             GAtResult *result, gboolean deactivate)
+{
+     struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+     struct ofono_error error;
+     char buf[64];
+

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+     DBG("deactivate %d", deactivate);
+
+     if (deactivate == TRUE) {
+
+             if (gcd->use_wwan)
+                     sprintf(buf, "AT^SWWAN=0,%u", gcd->active_context);
+             else
+                     sprintf(buf, "AT+CGACT=0,%u", gcd->active_context);
+
+             g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL,

NULL);

Quoted 5 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +5 -0

+     }
+
+     gcd->active_context = 0;
+     gcd->state = STATE_IDLE;
+

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+     if (result == NULL) {
+             CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
+             return;
+     }
+
+     decode_at_error(&error, g_at_result_final_response(result));
+     gcd->cb(&error, gcd->cb_data);
+}
+
+static void activate_cb(gboolean ok, GAtResult *result, gpointer

user_data)

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 11 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +11 -0

+{
+     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("Unable activate context");
+             /*
+              * We've reported sucess already, so can't just call
+              * failed_setup we call ofono_gprs_context_deactivated

instead.

drivers/gemaltomodem/gprs-context-wwan.c line 217

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+              * Thats not a clean solution at all, but as it seems

there is

drivers/gemaltomodem/gprs-context-wwan.c line 218

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+              * no clean way to determine whether it is possible to

activate

drivers/gemaltomodem/gprs-context-wwan.c line 219

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+              * the context before issuing AT^SWWAN. A possible

workaround

drivers/gemaltomodem/gprs-context-wwan.c line 220

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+              * might be to issue AT+CGACT=1 and AT+CGACT=0 and try if

that

Quoted 17 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +17 -0

+              * works, before calling CALLBACK_WITH_SUCCESS.
+              */
+             ofono_gprs_context_deactivated(gc, gcd->active_context);
+             gcd->active_context = 0;
+             gcd->state = STATE_IDLE;
+             return;
+     }
+     /* We've reported sucess already */
+}
+
+static void setup_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);
+     char buf[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+                                     OFONO_GPRS_MAX_PASSWORD_LENGTH +1];
+     struct ofono_modem *modem = ofono_gprs_context_get_modem(gc);

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+     const char *interface;
+
+     if (!ok) {
+             ofono_error("Failed to setup context");
+             failed_setup(gc, result, FALSE);
+             return;
+     }
+
+     if (gemalto_get_auth_command(modem, gcd->active_context,
+                             gcd->auth_method, gcd->username,

gcd->password,

drivers/gemaltomodem/gprs-context-wwan.c line 248

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+                                                     buf, sizeof(buf)))

{

drivers/gemaltomodem/gprs-context-wwan.c line 249

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+             if (!g_at_chat_send(gcd->chat, buf, none_prefix, NULL,

NULL,

drivers/gemaltomodem/gprs-context-wwan.c line 250

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+

NULL))

drivers/gemaltomodem/gprs-context-wwan.c line 252

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+             goto error;

indentation is wrong here?

Quoted 6 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +5 -0

+     }
+     /*
+      * note that if the auth command is not ok we skip it and continue
+      * but if the sending fails we do an error
+      */

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+
+     if (gcd->use_wwan)
+             sprintf(buf, "AT^SWWAN=1,%u", gcd->active_context);
+     else
+             sprintf(buf, "AT+CGACT=%u,1", gcd->active_context);
+
+     if (g_at_chat_send(gcd->chat, buf, none_prefix,
+                                     activate_cb, gc, NULL) > 0){
+
+             interface = ofono_modem_get_string(modem,

"NetworkInterface");

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 6 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +6 -0

+
+             ofono_gprs_context_set_interface(gc, interface);
+             ofono_gprs_context_set_ipv4_address(gc, NULL, FALSE);
+
+             /*
+              * We report sucess already here because some modules need

a

drivers/gemaltomodem/gprs-context-wwan.c line 273

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+              * DHCP request to complete the AT^SWWAN command

sucessfully

drivers/gemaltomodem/gprs-context-wwan.c line 278

Quoted 4 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +4 -0

+              */
+             gcd->state = STATE_ACTIVE;
+
+             CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);

So why are you calling SUCCESS even in the case of +CGACT? Also,
context activations via +CGACT need to be queried for static IPs, no? E.g. via +CGCONTRDP? I don't see you doing that.

Fundamentally, your context activation has succeeded or it didn't. The DHCP operation is purely local between the modem and the host. No other vendor in the world has this weird setup that you do. So I don't really know what to tell you here as this ^SWWAN interaction is fundamentally broken and it can't go upstream in this form.

Can ^SWWAN support static ip reporting instead of DHCP? That would be preferred anyway as DHCP is slow and unneeded.

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 9 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +9 -0

+
+             return;
+     }
+
+error:
+     failed_setup(gc, NULL, FALSE);
+}
+
+static void gemaltowwan_gprs_activate_primary(struct ofono_gprs_context

*gc,

drivers/gemaltomodem/gprs-context-wwan.c line 287

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+                             const struct ofono_gprs_primary_context

*ctx,

Quoted 9 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +9 -0

+                             ofono_gprs_context_cb_t cb, void *data)
+{
+     struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+     char buf[OFONO_GPRS_MAX_APN_LENGTH + 128];
+     struct ofono_modem *modem = ofono_gprs_context_get_modem(gc);
+
+     DBG("cid %u", ctx->cid);
+
+     gcd->use_wwan = ofono_modem_get_integer(modem, "Gemalto_WWAN");

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+     gcd->active_context = ctx->cid;
+     gcd->cb = cb;
+     gcd->cb_data = data;
+     memcpy(gcd->username, ctx->username, sizeof(ctx->username));
+     memcpy(gcd->password, ctx->password, sizeof(ctx->password));
+     gcd->state = STATE_ENABLING;
+     gcd->proto = ctx->proto;
+     gcd->auth_method = ctx->auth_method;
+
+     gemalto_get_cgdcont_command(modem, ctx->cid, ctx->proto, ctx->apn,

buf,

drivers/gemaltomodem/gprs-context-wwan.c line 307

Quoted 1 line

drivers/gemaltomodem/gprs-context-wwan.c new file +1 -0

+

sizeof(buf));

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 9 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +9 -0

+
+     if (g_at_chat_send(gcd->chat, buf, none_prefix,
+                             setup_cb, gc, NULL) > 0)
+             return;
+
+     CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static void deactivate_cb(gboolean ok, GAtResult *result, gpointer

user_data)

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 13 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +13 -0

+{
+     struct ofono_gprs_context *gc = user_data;
+     struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+
+     DBG("ok %d", ok);
+
+     gcd->active_context = 0;
+     gcd->state = STATE_IDLE;
+
+     CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+}
+
+static void gemaltowwan_gprs_deactivate_primary(struct

ofono_gprs_context *gc,

drivers/gemaltomodem/gprs-context-wwan.c line 331

Quoted 2 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +2 -0

+                                     unsigned int cid,
+                                     ofono_gprs_context_cb_t cb, void

*data)

Quoted 59 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +59 -0

+{
+     struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+     char buf[64];
+
+     DBG("cid %u", cid);
+
+     gcd->state = STATE_DISABLING;
+     gcd->cb = cb;
+     gcd->cb_data = data;
+
+     if (gcd->use_wwan)
+             sprintf(buf, "AT^SWWAN=0,%u", gcd->active_context);
+     else
+             sprintf(buf, "AT+CGACT=%u,0", gcd->active_context);
+
+     if (g_at_chat_send(gcd->chat, buf, none_prefix,
+                             deactivate_cb, gc, NULL) > 0)
+             return;
+
+     CALLBACK_WITH_SUCCESS(cb, data);
+}
+
+static void cgev_notify(GAtResult *result, gpointer user_data)
+{
+     struct ofono_gprs_context *gc = user_data;
+     struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+     const char *event;
+     int cid = 0;
+     GAtResultIter iter;
+
+     g_at_result_iter_init(&iter, result);
+
+     if (!g_at_result_iter_next(&iter, "+CGEV:"))
+             return;
+
+     if (!g_at_result_iter_next_unquoted_string(&iter, &event))
+             return;
+
+     if (!g_str_has_prefix(event, "ME PDN DEACT"))
+             return;
+
+     sscanf(event, "%*s %*s %*s %u", &cid);
+
+     DBG("cid %d", cid);
+
+     if ((unsigned int) cid != gcd->active_context)
+             return;
+
+     ofono_gprs_context_deactivated(gc, gcd->active_context);
+
+     gcd->active_context = 0;
+     gcd->state = STATE_IDLE;
+}
+
+static int gemaltowwan_gprs_context_probe(struct ofono_gprs_context *gc,
+                                     unsigned int model, void *data)
+{
+     GAtChat *chat = data;
+     struct gprs_context_data *gcd;

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 10 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +10 -0

+     struct ofono_modem *modem = ofono_gprs_context_get_modem(gc);
+
+     DBG("");
+
+     gcd = g_try_new0(struct gprs_context_data, 1);
+     if (gcd == NULL)
+             return -ENOMEM;
+
+     if (modem)
+             gcd->use_wwan = ofono_modem_get_integer(modem,

"Gemalto_WWAN");

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 8 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +8 -0

+     gcd->chat = g_at_chat_clone(chat);
+     ofono_gprs_context_set_data(gc, gcd);
+     g_at_chat_register(chat, "+CGEV:", cgev_notify, FALSE, gc, NULL);
+
+     return 0;
+}
+
+static void gemaltowwan_gprs_context_remove(struct ofono_gprs_context

*gc)

drivers/gemaltomodem/gprs-context-wwan.c

Quoted 12 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +12 -0

+{
+     struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+
+     DBG("");
+
+     ofono_gprs_context_set_data(gc, NULL);
+
+     g_at_chat_unref(gcd->chat);
+     g_free(gcd);
+}
+
+static void gemaltowwan_gprs_detach_shutdown(struct ofono_gprs_context

*gc,

Quoted 21 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +21 -0

+                                     unsigned int cid)
+{
+     DBG("cid %u", cid);
+
+     ofono_gprs_context_deactivated(gc, cid);
+}
+
+static struct ofono_gprs_context_driver driver = {
+     .name                   = "gemaltowwanmodem",
+     .probe                  = gemaltowwan_gprs_context_probe,
+     .remove                 = gemaltowwan_gprs_context_remove,
+     .activate_primary       = gemaltowwan_gprs_activate_primary,
+     .deactivate_primary     = gemaltowwan_gprs_deactivate_primary,
+     .detach_shutdown        = gemaltowwan_gprs_detach_shutdown,
+};
+
+void gemaltowwan_gprs_context_init(void)
+{
+     ofono_gprs_context_driver_register(&driver);
+}
+

drivers/gemaltomodem/gprs-context-wwan.c line 446

Quoted 5 lines

drivers/gemaltomodem/gprs-context-wwan.c new file +4 -0

+void gemaltowwan_gprs_context_exit(void)
+{
+     ofono_gprs_context_driver_unregister(&driver);
+}

Regards,
-Denis

Hi Denis,

Quoted 3 lines · Denis Kenzior

Hi Giacinto,

Quoted 5 lines

Quoted 4 lines

---
  drivers/gemaltomodem/gemaltomodem.c      |   2 +
  drivers/gemaltomodem/gemaltomodem.h      |   3 +
  drivers/gemaltomodem/gprs-context-wwan.c | 446

+++++++++++++++++++++++++++++++

Quoted 8 lines

Review

Quoted 4 lines

  3 files changed, 451 insertions(+)

create mode 100644 drivers/gemaltomodem/gprs-context-wwan.c

(quoted diff) +0 -0

b/drivers/gemaltomodem/gemaltomodem.c

drivers/gemaltomodem/gemaltomodem.c

Quoted 3 lines

index 614ca81..7afd3c1 100644

drivers/gemaltomodem/gemaltomodem.c +0 -0

+

Quoted 10 lines

+static gboolean gemalto_get_auth_command(struct ofono_modem *modem, int cid,

drivers/gemaltomodem/gemaltomodem.c

Quoted 2 lines

drivers/gemaltomodem/gemaltomodem.c +2 -0

+                             enum ofono_gprs_auth_method auth_method,
+                             const char *username, const char

*password,

Quoted 3 lines

drivers/gemaltomodem/gemaltomodem.c +3 -0

+                             char *buf, guint buflen)
+{
+     int gto_auth = ofono_modem_get_integer(modem, "Gemalto_Auth");

Quoted 2 lines

Please use a CamelCase-d variable for this. E.g. GemaltoAuthType

ok. This is new for this code, because I have always seen only variables with underscores, but no problem.
Personally I would prefer camelCase 'gemaltoAuthType', because it is more coherent in case of 1-part names like 'user',
but I'll do as you say.

Quoted 3 lines

Regards,
-Denis

Regards,
Giacinto

So until the GPRS_AUTH_METHOD_NONE is actually introduced, you might want to just get this upstream with the existing enumeration.  E.g. just
assume that if username & password is empty, the auth method is none.

I have pushed the new method NONE again, and I prefer to wait until it is taken.

The current solution is really a non-documented hack. Checking whether user&password are empty to change the type of authentication goes against the caller settings.
It is better to give an error so that there is a chance to have good settings.

Fundamentally, your context activation has succeeded or it didn't.  The DHCP operation is purely local between the modem and the host.  No other
vendor in the world has this weird setup that you do.  So I don't really
know what to tell you here as this ^SWWAN interaction is fundamentally broken and it can't go upstream in this form.

Can ^SWWAN support static ip reporting instead of DHCP?  That would be preferred anyway as DHCP is slow and unneeded.

This is a discussion about how Gemalto products work.

Admittedly, there are a few models, in the high-volume low-price range (already deployed),
that won't return from AT^SWWAN until a DHCP exchange is performed, as per Sebastian's comment above.

The rest (which are the large majority) of the modules would allow a 'standard' treatment, but also work with the code above.

As per DHCP vs static, we have both, depending on the module.

Just for information, I have stepped upon networks *requiring* DHCP:
in this kind of networks, an MBIM module that would normally return a static settings,
returns 0x00 in the "IPv4 configuration Available" field in of the IP_CONFIGURATION answer,
instead of the usual 0x0F (address, gateway, dns, mtu).
So, it looks like that DHCP has to be considered as a valid alternative also in this case.

We cannot change every module and every network to fit our wishes.

Back to the code, I don't see a solution at the moment other than that one. I can add an 'if' to filter only the models that need it, but cannot remove it completely.
And this code would be specific for the driver, I don't see why it couldn't go upstream.

Hi Giacinto,

Quoted 6 lines

So until the GPRS_AUTH_METHOD_NONE is actually introduced, you might want to just get this upstream with the existing enumeration. E.g. just
assume that if username & password is empty, the auth method is none.

Quoted 9 lines

I have pushed the new method NONE again, and I prefer to wait until it is taken.

The current solution is really a non-documented hack. Checking whether user&password are empty to change the type of authentication goes against the caller settings.
It is better to give an error so that there is a chance to have good settings.

The issue is that you haven't properly addressed all the points that arise from adding a new enumeration. You're touching something that essentially affects every driver and not updating these properly. So take your pick, but nothing is going upstream until either this patch fits into the old framework, or the new framework is ready...

Quoted 1 line

Fundamentally, your context activation has succeeded or it didn't.

The

Quoted 5 lines

DHCP operation is purely local between the modem and the host. No other
vendor in the world has this weird setup that you do. So I don't really
know what to tell you here as this ^SWWAN interaction is

fundamentally

Quoted 3 lines

broken and it can't go upstream in this form.

Can ^SWWAN support static ip reporting instead of DHCP? That would

be

Quoted 5 lines

preferred anyway as DHCP is slow and unneeded.

This is a discussion about how Gemalto products work.

Quoted 5 lines

Admittedly, there are a few models, in the high-volume low-price range (already deployed),
that won't return from AT^SWWAN until a DHCP exchange is performed, as per Sebastian's comment above.

Can firmware be fixed / upgraded on these?

The rest (which are the large majority) of the modules would allow a

'standard' treatment, but also work with the code above.

So why don't we enable sane ones for now and deal with the insane ones later?

Quoted 11 lines

As per DHCP vs static, we have both, depending on the module.

Just for information, I have stepped upon networks *requiring* DHCP:
in this kind of networks, an MBIM module that would normally return a static settings,
returns 0x00 in the "IPv4 configuration Available" field in of the IP_CONFIGURATION answer,
instead of the usual 0x0F (address, gateway, dns, mtu).
So, it looks like that DHCP has to be considered as a valid alternative also in this case.

This is fine. You just set the IP configuration method accordingly. We already have a few examples where the gprs_context driver might choose one or the other depending on whether support for a given command exists. This can be extended to support cases where the network requires DHCP. Which by the way is completely weird for cases such as PPP where no concept of DHCP really exists and your address has to be obtained by the time the context is activated. So I wouldn't mind some extra details on what is *actually* going on here under the hood.

Quoted 4 lines

We cannot change every module and every network to fit our wishes.

Back to the code, I don't see a solution at the moment other than that

one.

Quoted 4 lines

I can add an 'if' to filter only the models that need it, but cannot remove it completely.
And this code would be specific for the driver, I don't see why it couldn't go upstream.

So I would filter these models out from this driver. Or see if they can work with PPP based driver and we can deal with these later.

-Denis