* [PATCH v2 2/3] gemalto: add PLS8 support
2017-08-30 16:30 [PATCH v2 1/3] gemalto: add PLS8 support Sebastian Arnd
@ 2017-08-30 16:30 ` Sebastian Arnd
2017-08-30 19:22 ` Denis Kenzior
2017-08-30 16:30 ` [PATCH v2 3/3] " Sebastian Arnd
2017-08-30 18:54 ` [PATCH v2 1/3] " Denis Kenzior
2 siblings, 1 reply; 7+ messages in thread
From: Sebastian Arnd @ 2017-08-30 16:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 12432 bytes --]
diff --git a/drivers/gemaltomodem/gemaltomodem.c b/drivers/gemaltomodem/gemaltomodem.c
index 91cf238..01bdb43 100644
--- a/drivers/gemaltomodem/gemaltomodem.c
+++ b/drivers/gemaltomodem/gemaltomodem.c
@@ -35,6 +35,7 @@
static int gemaltomodem_init(void)
{
gemalto_location_reporting_init();
+ gemaltowwan_gprs_context_init();
return 0;
}
@@ -42,6 +43,8 @@ static int gemaltomodem_init(void)
static void gemaltomodem_exit(void)
{
gemalto_location_reporting_exit();
+ gemaltowwan_gprs_context_exit();
+
}
OFONO_PLUGIN_DEFINE(gemaltomodem, "Gemalto modem driver", VERSION,
diff --git a/drivers/gemaltomodem/gemaltomodem.h b/drivers/gemaltomodem/gemaltomodem.h
index 7ea1e8f..3ec2380 100644
--- a/drivers/gemaltomodem/gemaltomodem.h
+++ b/drivers/gemaltomodem/gemaltomodem.h
@@ -21,5 +21,8 @@
#include <drivers/atmodem/atutil.h>
-extern void gemalto_location_reporting_init();
-extern void gemalto_location_reporting_exit();
+extern void gemalto_location_reporting_init(void);
+extern void gemalto_location_reporting_exit(void);
+extern void gemaltowwan_gprs_context_init(void);
+extern void gemaltowwan_gprs_context_exit(void);
+
diff --git a/drivers/gemaltomodem/gprs-context-wwan.c b/drivers/gemaltomodem/gprs-context-wwan.c
new file mode 100644
index 0000000..d87e12f
--- /dev/null
+++ b/drivers/gemaltomodem/gprs-context-wwan.c
@@ -0,0 +1,436 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2017 Piotr Haber, Sebastian Arnd. All rights reserved.
+ *
+ * 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 };
+static const char *cgpaddr_prefix[] = { "+CGPADDR:", NULL };
+static const char *cgcontrdp_prefix[] = { "+CGCONTRDP:", NULL };
+
+enum state {
+ STATE_IDLE,
+ STATE_ENABLING,
+ STATE_DISABLING,
+ STATE_ACTIVE,
+};
+
+enum auth_method {
+ AUTH_METHOD_NONE,
+ AUTH_METHOD_PAP,
+ AUTH_METHOD_CHAP,
+};
+
+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 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;
+ void *cb_data;
+};
+
+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) {
+ sprintf(buf, "AT^SWWAN=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 contrdp_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);
+ struct ofono_modem *modem;
+ int cid;
+ const char *interface;
+ GAtResultIter iter;
+ gboolean found = FALSE;
+
+ DBG("ok %d", ok);
+
+ if (!ok) {
+ ofono_error("Unable to get context dynamic paramerers");
+ failed_setup(gc, result, TRUE);
+ return;
+ }
+
+ /*
+ * We're not getting any parameters here as all configuration is done
+ * via dhcp.
+ */
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, "+CGCONTRDP:")) {
+ if (!g_at_result_iter_next_number(&iter, &cid))
+ goto error;
+
+ if ((unsigned int) cid == gcd->active_context)
+ found = TRUE;
+ }
+
+ if (found == FALSE)
+ goto error;
+
+ gcd->state = STATE_ACTIVE;
+
+ modem = ofono_gprs_context_get_modem(gc);
+ interface = ofono_modem_get_string(modem, "NetworkInterface");
+
+ ofono_gprs_context_set_interface(gc, interface);
+ ofono_gprs_context_set_ipv4_address(gc, gcd->address, FALSE);
+
+ CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+ return;
+
+error:
+ failed_setup(gc, NULL, TRUE);
+}
+
+static void address_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);
+ int cid;
+ const char *address;
+ char buf[64];
+ GAtResultIter iter;
+
+ DBG("ok %d", ok);
+
+ if (!ok) {
+ ofono_error("Unable to get context address");
+ failed_setup(gc, result, TRUE);
+ return;
+ }
+
+ g_at_result_iter_init(&iter, result);
+
+ if (!g_at_result_iter_next(&iter, "+CGPADDR:"))
+ goto error;
+
+ if (!g_at_result_iter_next_number(&iter, &cid))
+ goto error;
+
+ if ((unsigned int) cid != gcd->active_context)
+ goto error;
+
+ if (!g_at_result_iter_next_string(&iter, &address))
+ goto error;
+
+ strncpy(gcd->address, address, sizeof(gcd->address));
+ sprintf(buf, "AT+CGCONTRDP=%d", gcd->active_context);
+
+ if (g_at_chat_send(gcd->chat, buf, cgcontrdp_prefix,
+ contrdp_cb, gc, NULL) > 0)
+ return;
+
+error:
+ failed_setup(gc, NULL, TRUE);
+}
+
+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);
+ char buf[64];
+
+ DBG("ok %d", ok);
+
+ if (!ok) {
+ ofono_error("Unable to activate context");
+ failed_setup(gc, result, FALSE);
+ return;
+ }
+
+ sprintf(buf, "AT+CGPADDR=%u", gcd->active_context);
+ if (g_at_chat_send(gcd->chat, buf, cgpaddr_prefix,
+ address_cb, gc, NULL) > 0)
+ return;
+
+ failed_setup(gc, NULL, TRUE);
+}
+
+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[384];
+
+ DBG("ok %d", ok);
+
+ if (!ok) {
+ ofono_error("Failed to setup context");
+ failed_setup(gc, result, FALSE);
+ return;
+ }
+
+ if (gcd->username[0] && gcd->password[0])
+ sprintf(buf, "AT^SGAUTH=%u,%u,\"%s\",\"%s\"",
+ gcd->active_context, gcd->auth_method,
+ gcd->username, gcd->password);
+ else
+ sprintf(buf, "AT^SGAUTH=%u,0", gcd->active_context);
+
+ if (g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL) == 0)
+ goto error;
+
+ sprintf(buf, "AT^SWWAN=1,%u", gcd->active_context);
+
+ if (g_at_chat_send(gcd->chat, buf, none_prefix,
+ activate_cb, gc, NULL) > 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];
+ int len = 0;
+
+ DBG("cid %u", ctx->cid);
+
+ 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;
+
+ /* We only support CHAP and PAP */
+ switch (ctx->auth_method) {
+ case OFONO_GPRS_AUTH_METHOD_CHAP:
+ gcd->auth_method = AUTH_METHOD_CHAP;
+ break;
+ case OFONO_GPRS_AUTH_METHOD_PAP:
+ gcd->auth_method = AUTH_METHOD_PAP;
+ break;
+ default:
+ goto error;
+ }
+
+ switch (ctx->proto) {
+ case OFONO_GPRS_PROTO_IP:
+ len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"",
+ ctx->cid);
+ break;
+ case OFONO_GPRS_PROTO_IPV6:
+ len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV6\"",
+ ctx->cid);
+ break;
+ case OFONO_GPRS_PROTO_IPV4V6:
+ len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV4V6\"",
+ ctx->cid);
+ break;
+ }
+
+ if (ctx->apn)
+ snprintf(buf + len, sizeof(buf) - len - 3,
+ ",\"%s\"", ctx->apn);
+
+ if (g_at_chat_send(gcd->chat, buf, none_prefix,
+ setup_cb, gc, NULL) > 0)
+ return;
+
+error:
+ 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;
+
+ sprintf(buf, "AT^SWWAN=0,%u", 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;
+ 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, "NW DEACT") &&
+ !g_str_has_prefix(event, "ME DEACT") &&
+ !g_str_has_prefix(event, "PDN DEACT"))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_next_number(&iter, &cid))
+ return;
+
+ 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 vendor, void *data)
+{
+ GAtChat *chat = data;
+ struct gprs_context_data *gcd;
+
+ DBG("");
+
+ gcd = g_try_new0(struct gprs_context_data, 1);
+ if (gcd == NULL)
+ return -ENOMEM;
+
+ 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)
+{
+ 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);
+}
+
+void gemaltowwan_gprs_context_exit(void)
+{
+ ofono_gprs_context_driver_unregister(&driver);
+}
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/3] gemalto: add PLS8 support
2017-08-30 16:30 ` [PATCH v2 2/3] " Sebastian Arnd
@ 2017-08-30 19:22 ` Denis Kenzior
2017-09-04 13:28 ` Reinhard Speyerer
0 siblings, 1 reply; 7+ messages in thread
From: Denis Kenzior @ 2017-08-30 19:22 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 14287 bytes --]
Hi Sebastian,
On 08/30/2017 11:30 AM, Sebastian Arnd wrote:
> diff --git a/drivers/gemaltomodem/gemaltomodem.c b/drivers/gemaltomodem/gemaltomodem.c
> index 91cf238..01bdb43 100644
> --- a/drivers/gemaltomodem/gemaltomodem.c
> +++ b/drivers/gemaltomodem/gemaltomodem.c
> @@ -35,6 +35,7 @@
> static int gemaltomodem_init(void)
> {
> gemalto_location_reporting_init();
> + gemaltowwan_gprs_context_init();
>
> return 0;
> }
> @@ -42,6 +43,8 @@ static int gemaltomodem_init(void)
> static void gemaltomodem_exit(void)
> {
> gemalto_location_reporting_exit();
> + gemaltowwan_gprs_context_exit();
> +
No need for this extra empty line
> }
>
> OFONO_PLUGIN_DEFINE(gemaltomodem, "Gemalto modem driver", VERSION,
> diff --git a/drivers/gemaltomodem/gemaltomodem.h b/drivers/gemaltomodem/gemaltomodem.h
> index 7ea1e8f..3ec2380 100644
> --- a/drivers/gemaltomodem/gemaltomodem.h
> +++ b/drivers/gemaltomodem/gemaltomodem.h
> @@ -21,5 +21,8 @@
>
> #include <drivers/atmodem/atutil.h>
>
> -extern void gemalto_location_reporting_init();
> -extern void gemalto_location_reporting_exit();
> +extern void gemalto_location_reporting_init(void);
> +extern void gemalto_location_reporting_exit(void);
> +extern void gemaltowwan_gprs_context_init(void);
> +extern void gemaltowwan_gprs_context_exit(void);
> +
Extra empty line here as well
> diff --git a/drivers/gemaltomodem/gprs-context-wwan.c b/drivers/gemaltomodem/gprs-context-wwan.c
> new file mode 100644
> index 0000000..d87e12f
> --- /dev/null
> +++ b/drivers/gemaltomodem/gprs-context-wwan.c
> @@ -0,0 +1,436 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2017 Piotr Haber, Sebastian Arnd. All rights reserved.
> + *
> + * 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 };
> +static const char *cgpaddr_prefix[] = { "+CGPADDR:", NULL };
> +static const char *cgcontrdp_prefix[] = { "+CGCONTRDP:", NULL };
> +
> +enum state {
> + STATE_IDLE,
> + STATE_ENABLING,
> + STATE_DISABLING,
> + STATE_ACTIVE,
> +};
> +
> +enum auth_method {
> + AUTH_METHOD_NONE,
> + AUTH_METHOD_PAP,
> + AUTH_METHOD_CHAP,
> +};
> +
> +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 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;
> + void *cb_data;
> +};
> +
> +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) {
> + sprintf(buf, "AT^SWWAN=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 contrdp_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);
> + struct ofono_modem *modem;
> + int cid;
> + const char *interface;
> + GAtResultIter iter;
> + gboolean found = FALSE;
> +
> + DBG("ok %d", ok);
> +
> + if (!ok) {
> + ofono_error("Unable to get context dynamic paramerers");
> + failed_setup(gc, result, TRUE);
> + return;
> + }
> +
> + /*
> + * We're not getting any parameters here as all configuration is done
> + * via dhcp.
> + */
If you want ConnMan to obtain DNS, Netmask, Gateway, etc via DHCP, then
ofono_gprs_context_set_ipv4_address should not be used. This forces
'IPv4.Method' to be 'dhcp' instead of 'static'.
Relying on DHCP is fine, but you pay a bit of extra latency. So if your
modem provides this info via +CGCONTRDP, I would use that.
If you do decide to use DHCP only, is calling +CGPADDR & +CGCONTRDP even
necessary?
> +
> + g_at_result_iter_init(&iter, result);
> +
> + while (g_at_result_iter_next(&iter, "+CGCONTRDP:")) {
> + if (!g_at_result_iter_next_number(&iter, &cid))
> + goto error;
> +
> + if ((unsigned int) cid == gcd->active_context)
> + found = TRUE;
> + }
> +
> + if (found == FALSE)
> + goto error;
> +
> + gcd->state = STATE_ACTIVE;
> +
> + modem = ofono_gprs_context_get_modem(gc);
> + interface = ofono_modem_get_string(modem, "NetworkInterface");
> +
> + ofono_gprs_context_set_interface(gc, interface);
> + ofono_gprs_context_set_ipv4_address(gc, gcd->address, FALSE);
> +
> + CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
> + return;
> +
> +error:
> + failed_setup(gc, NULL, TRUE);
> +}
> +
> +static void address_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);
> + int cid;
> + const char *address;
> + char buf[64];
> + GAtResultIter iter;
> +
> + DBG("ok %d", ok);
> +
> + if (!ok) {
> + ofono_error("Unable to get context address");
> + failed_setup(gc, result, TRUE);
> + return;
> + }
> +
> + g_at_result_iter_init(&iter, result);
> +
> + if (!g_at_result_iter_next(&iter, "+CGPADDR:"))
> + goto error;
> +
> + if (!g_at_result_iter_next_number(&iter, &cid))
> + goto error;
> +
> + if ((unsigned int) cid != gcd->active_context)
> + goto error;
> +
> + if (!g_at_result_iter_next_string(&iter, &address))
> + goto error;
> +
> + strncpy(gcd->address, address, sizeof(gcd->address));
> + sprintf(buf, "AT+CGCONTRDP=%d", gcd->active_context);
> +
> + if (g_at_chat_send(gcd->chat, buf, cgcontrdp_prefix,
> + contrdp_cb, gc, NULL) > 0)
> + return;
> +
> +error:
> + failed_setup(gc, NULL, TRUE);
> +}
> +
> +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);
> + char buf[64];
> +
> + DBG("ok %d", ok);
> +
> + if (!ok) {
> + ofono_error("Unable to activate context");
> + failed_setup(gc, result, FALSE);
> + return;
> + }
> +
> + sprintf(buf, "AT+CGPADDR=%u", gcd->active_context);
> + if (g_at_chat_send(gcd->chat, buf, cgpaddr_prefix,
> + address_cb, gc, NULL) > 0)
> + return;
> +
> + failed_setup(gc, NULL, TRUE);
> +}
> +
> +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[384];
> +
> + DBG("ok %d", ok);
> +
> + if (!ok) {
> + ofono_error("Failed to setup context");
> + failed_setup(gc, result, FALSE);
> + return;
> + }
> +
> + if (gcd->username[0] && gcd->password[0])
> + sprintf(buf, "AT^SGAUTH=%u,%u,\"%s\",\"%s\"",
> + gcd->active_context, gcd->auth_method,
> + gcd->username, gcd->password);
> + else
> + sprintf(buf, "AT^SGAUTH=%u,0", gcd->active_context);
> +
> + if (g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL) == 0)
> + goto error;
> +
> + sprintf(buf, "AT^SWWAN=1,%u", gcd->active_context);
> +
> + if (g_at_chat_send(gcd->chat, buf, none_prefix,
> + activate_cb, gc, NULL) > 0)
> + return;
> +
> +
No double empty lines please
> +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];
> + int len = 0;
> +
> + DBG("cid %u", ctx->cid);
> +
> + 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;
> +
> + /* We only support CHAP and PAP */
> + switch (ctx->auth_method) {
> + case OFONO_GPRS_AUTH_METHOD_CHAP:
> + gcd->auth_method = AUTH_METHOD_CHAP;
> + break;
> + case OFONO_GPRS_AUTH_METHOD_PAP:
> + gcd->auth_method = AUTH_METHOD_PAP;
> + break;
> + default:
> + goto error;
> + }
> +
> + switch (ctx->proto) {
> + case OFONO_GPRS_PROTO_IP:
> + len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"",
> + ctx->cid);
> + break;
> + case OFONO_GPRS_PROTO_IPV6:
> + len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV6\"",
> + ctx->cid);
> + break;
> + case OFONO_GPRS_PROTO_IPV4V6:
> + len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV4V6\"",
> + ctx->cid);
> + break;
You don't actually handle IPv6 or dual-stack contexts up above, e.g. you
only ever set the IPv4 address...
> + }
> +
> + if (ctx->apn)
> + snprintf(buf + len, sizeof(buf) - len - 3,
> + ",\"%s\"", ctx->apn);
> +
> + if (g_at_chat_send(gcd->chat, buf, none_prefix,
> + setup_cb, gc, NULL) > 0)
> + return;
> +
> +error:
> + 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;
> +
> + sprintf(buf, "AT^SWWAN=0,%u", 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;
> + 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, "NW DEACT") &&
> + !g_str_has_prefix(event, "ME DEACT") &&
> + !g_str_has_prefix(event, "PDN DEACT"))
> + return;
> +
> + if (!g_at_result_iter_skip_next(&iter))
> + return;
> +
> + if (!g_at_result_iter_next_number(&iter, &cid))
> + return;
> +
> + 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 vendor, void *data)
> +{
> + GAtChat *chat = data;
> + struct gprs_context_data *gcd;
> +
> + DBG("");
> +
> + gcd = g_try_new0(struct gprs_context_data, 1);
> + if (gcd == NULL)
> + return -ENOMEM;
> +
> + 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)
> +{
> + 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,
PLS8 seems to be an LTE device. Do you handle network initiated bearers
or default attach bearers? If so, .read_settings should probably be
implemented.
> +};
> +
> +void gemaltowwan_gprs_context_init(void)
> +{
> + ofono_gprs_context_driver_register(&driver);
> +}
> +
> +void gemaltowwan_gprs_context_exit(void)
> +{
> + ofono_gprs_context_driver_unregister(&driver);
> +}
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/3] gemalto: add PLS8 support
2017-08-30 19:22 ` Denis Kenzior
@ 2017-09-04 13:28 ` Reinhard Speyerer
0 siblings, 0 replies; 7+ messages in thread
From: Reinhard Speyerer @ 2017-09-04 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 11678 bytes --]
Hi Denis and Sebastian,
please see my comments/suggestions below.
On Wed, Aug 30, 2017 at 02:22:47PM -0500, Denis Kenzior wrote:
> Hi Sebastian,
>
> On 08/30/2017 11:30 AM, Sebastian Arnd wrote:
> >diff --git a/drivers/gemaltomodem/gemaltomodem.c b/drivers/gemaltomodem/gemaltomodem.c
> >index 91cf238..01bdb43 100644
> >--- a/drivers/gemaltomodem/gemaltomodem.c
> >+++ b/drivers/gemaltomodem/gemaltomodem.c
> >@@ -35,6 +35,7 @@
> > static int gemaltomodem_init(void)
> > {
> > gemalto_location_reporting_init();
> >+ gemaltowwan_gprs_context_init();
> > return 0;
> > }
> >@@ -42,6 +43,8 @@ static int gemaltomodem_init(void)
> > static void gemaltomodem_exit(void)
> > {
> > gemalto_location_reporting_exit();
> >+ gemaltowwan_gprs_context_exit();
> >+
>
> No need for this extra empty line
>
> > }
> > OFONO_PLUGIN_DEFINE(gemaltomodem, "Gemalto modem driver", VERSION,
> >diff --git a/drivers/gemaltomodem/gemaltomodem.h b/drivers/gemaltomodem/gemaltomodem.h
> >index 7ea1e8f..3ec2380 100644
> >--- a/drivers/gemaltomodem/gemaltomodem.h
> >+++ b/drivers/gemaltomodem/gemaltomodem.h
> >@@ -21,5 +21,8 @@
> > #include <drivers/atmodem/atutil.h>
> >-extern void gemalto_location_reporting_init();
> >-extern void gemalto_location_reporting_exit();
> >+extern void gemalto_location_reporting_init(void);
> >+extern void gemalto_location_reporting_exit(void);
> >+extern void gemaltowwan_gprs_context_init(void);
> >+extern void gemaltowwan_gprs_context_exit(void);
> >+
>
> Extra empty line here as well
>
> >diff --git a/drivers/gemaltomodem/gprs-context-wwan.c b/drivers/gemaltomodem/gprs-context-wwan.c
> >new file mode 100644
> >index 0000000..d87e12f
> >--- /dev/null
> >+++ b/drivers/gemaltomodem/gprs-context-wwan.c
> >@@ -0,0 +1,436 @@
> >+/*
> >+ *
> >+ * oFono - Open Source Telephony
> >+ *
> >+ * Copyright (C) 2017 Piotr Haber, Sebastian Arnd. All rights reserved.
> >+ *
> >+ * 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 };
> >+static const char *cgpaddr_prefix[] = { "+CGPADDR:", NULL };
> >+static const char *cgcontrdp_prefix[] = { "+CGCONTRDP:", NULL };
> >+
> >+enum state {
> >+ STATE_IDLE,
> >+ STATE_ENABLING,
> >+ STATE_DISABLING,
> >+ STATE_ACTIVE,
> >+};
> >+
> >+enum auth_method {
> >+ AUTH_METHOD_NONE,
> >+ AUTH_METHOD_PAP,
> >+ AUTH_METHOD_CHAP,
> >+};
> >+
> >+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 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;
> >+ void *cb_data;
> >+};
> >+
> >+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) {
> >+ sprintf(buf, "AT^SWWAN=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 contrdp_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);
> >+ struct ofono_modem *modem;
> >+ int cid;
> >+ const char *interface;
> >+ GAtResultIter iter;
> >+ gboolean found = FALSE;
> >+
> >+ DBG("ok %d", ok);
> >+
> >+ if (!ok) {
> >+ ofono_error("Unable to get context dynamic paramerers");
> >+ failed_setup(gc, result, TRUE);
> >+ return;
> >+ }
> >+
> >+ /*
> >+ * We're not getting any parameters here as all configuration is done
> >+ * via dhcp.
> >+ */
>
> If you want ConnMan to obtain DNS, Netmask, Gateway, etc via DHCP,
> then ofono_gprs_context_set_ipv4_address should not be used. This
> forces 'IPv4.Method' to be 'dhcp' instead of 'static'.
>
> Relying on DHCP is fine, but you pay a bit of extra latency. So if
> your modem provides this info via +CGCONTRDP, I would use that.
>
> If you do decide to use DHCP only, is calling +CGPADDR & +CGCONTRDP
> even necessary?
>
Unfortunately the PLS8 seems to need DHCP to be used as a trigger to
pass IPv4 traffic over the network interface even if the parameters
could be obtained from AT commands.
This restriction also holds for some other Qualcomm firmwares when the
QMI network interface is used in Ethernet mode instead of Raw IP mode.
> >+
> >+ g_at_result_iter_init(&iter, result);
> >+
> >+ while (g_at_result_iter_next(&iter, "+CGCONTRDP:")) {
> >+ if (!g_at_result_iter_next_number(&iter, &cid))
> >+ goto error;
> >+
> >+ if ((unsigned int) cid == gcd->active_context)
> >+ found = TRUE;
> >+ }
> >+
> >+ if (found == FALSE)
> >+ goto error;
> >+
> >+ gcd->state = STATE_ACTIVE;
> >+
> >+ modem = ofono_gprs_context_get_modem(gc);
> >+ interface = ofono_modem_get_string(modem, "NetworkInterface");
> >+
> >+ ofono_gprs_context_set_interface(gc, interface);
> >+ ofono_gprs_context_set_ipv4_address(gc, gcd->address, FALSE);
> >+
> >+ CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
> >+ return;
> >+
> >+error:
> >+ failed_setup(gc, NULL, TRUE);
> >+}
> >+
> >+static void address_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);
> >+ int cid;
> >+ const char *address;
> >+ char buf[64];
> >+ GAtResultIter iter;
> >+
> >+ DBG("ok %d", ok);
> >+
> >+ if (!ok) {
> >+ ofono_error("Unable to get context address");
> >+ failed_setup(gc, result, TRUE);
> >+ return;
> >+ }
> >+
> >+ g_at_result_iter_init(&iter, result);
> >+
> >+ if (!g_at_result_iter_next(&iter, "+CGPADDR:"))
> >+ goto error;
> >+
> >+ if (!g_at_result_iter_next_number(&iter, &cid))
> >+ goto error;
> >+
> >+ if ((unsigned int) cid != gcd->active_context)
> >+ goto error;
> >+
> >+ if (!g_at_result_iter_next_string(&iter, &address))
> >+ goto error;
> >+
> >+ strncpy(gcd->address, address, sizeof(gcd->address));
> >+ sprintf(buf, "AT+CGCONTRDP=%d", gcd->active_context);
> >+
> >+ if (g_at_chat_send(gcd->chat, buf, cgcontrdp_prefix,
> >+ contrdp_cb, gc, NULL) > 0)
> >+ return;
> >+
> >+error:
> >+ failed_setup(gc, NULL, TRUE);
> >+}
> >+
> >+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);
> >+ char buf[64];
> >+
> >+ DBG("ok %d", ok);
> >+
> >+ if (!ok) {
> >+ ofono_error("Unable to activate context");
> >+ failed_setup(gc, result, FALSE);
> >+ return;
> >+ }
> >+
> >+ sprintf(buf, "AT+CGPADDR=%u", gcd->active_context);
> >+ if (g_at_chat_send(gcd->chat, buf, cgpaddr_prefix,
> >+ address_cb, gc, NULL) > 0)
> >+ return;
> >+
> >+ failed_setup(gc, NULL, TRUE);
> >+}
> >+
> >+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[384];
> >+
> >+ DBG("ok %d", ok);
> >+
> >+ if (!ok) {
> >+ ofono_error("Failed to setup context");
> >+ failed_setup(gc, result, FALSE);
> >+ return;
> >+ }
> >+
> >+ if (gcd->username[0] && gcd->password[0])
> >+ sprintf(buf, "AT^SGAUTH=%u,%u,\"%s\",\"%s\"",
> >+ gcd->active_context, gcd->auth_method,
> >+ gcd->username, gcd->password);
> >+ else
> >+ sprintf(buf, "AT^SGAUTH=%u,0", gcd->active_context);
> >+
> >+ if (g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL) == 0)
> >+ goto error;
> >+
> >+ sprintf(buf, "AT^SWWAN=1,%u", gcd->active_context);
> >+
> >+ if (g_at_chat_send(gcd->chat, buf, none_prefix,
> >+ activate_cb, gc, NULL) > 0)
> >+ return;
> >+
> >+
>
> No double empty lines please
>
> >+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];
> >+ int len = 0;
> >+
> >+ DBG("cid %u", ctx->cid);
> >+
> >+ 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;
> >+
> >+ /* We only support CHAP and PAP */
> >+ switch (ctx->auth_method) {
> >+ case OFONO_GPRS_AUTH_METHOD_CHAP:
> >+ gcd->auth_method = AUTH_METHOD_CHAP;
> >+ break;
> >+ case OFONO_GPRS_AUTH_METHOD_PAP:
> >+ gcd->auth_method = AUTH_METHOD_PAP;
> >+ break;
> >+ default:
> >+ goto error;
> >+ }
> >+
> >+ switch (ctx->proto) {
> >+ case OFONO_GPRS_PROTO_IP:
> >+ len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"",
> >+ ctx->cid);
> >+ break;
> >+ case OFONO_GPRS_PROTO_IPV6:
> >+ len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV6\"",
> >+ ctx->cid);
> >+ break;
> >+ case OFONO_GPRS_PROTO_IPV4V6:
> >+ len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV4V6\"",
> >+ ctx->cid);
> >+ break;
>
> You don't actually handle IPv6 or dual-stack contexts up above, e.g.
> you only ever set the IPv4 address...
My tests with a PLS8-E running firmware revision 03.017 showed
IPv6/dualstack-related problems. Please refer to
https://developer.gemalto.com/threads/ipv6dualstack-problems-pls8-e-revision-03017
for details. Perhaps it might make sense to restrict the PLS8 ofono
integration to IPv4 only for the time being until a newer PLS8
firmware becomes available.
Unless I overlooked something the patch also does not seem to handle the
\XX escaping for non-printable characters used by PLS8 with firmware
revision 03.x (and 02.x?) and AT+CSCS="GSM".
Regards,
Reinhard
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] gemalto: add PLS8 support
2017-08-30 16:30 [PATCH v2 1/3] gemalto: add PLS8 support Sebastian Arnd
2017-08-30 16:30 ` [PATCH v2 2/3] " Sebastian Arnd
@ 2017-08-30 16:30 ` Sebastian Arnd
2017-08-30 19:05 ` Denis Kenzior
2017-08-30 18:54 ` [PATCH v2 1/3] " Denis Kenzior
2 siblings, 1 reply; 7+ messages in thread
From: Sebastian Arnd @ 2017-08-30 16:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]
diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 7c33c22..17d0be9 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -64,7 +64,7 @@ static const char *pinnum_prefix[] = { "%PINNUM:", NULL };
static const char *oercn_prefix[] = { "_OERCN:", NULL };
static const char *cpinr_prefixes[] = { "+CPINR:", "+CPINRE:", NULL };
static const char *epin_prefix[] = { "*EPIN:", NULL };
-static const char *spic_prefix[] = { "+SPIC:", NULL };
+static const char *spic_prefix[] = { "+SPIC:", "^SPIC:", NULL };
static const char *pct_prefix[] = { "#PCT:", NULL };
static const char *pnnm_prefix[] = { "+PNNM:", NULL };
static const char *qpinc_prefix[] = { "+QPINC:", NULL };
@@ -875,6 +875,62 @@ error:
CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
}
+static void at_spic_gemalto_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct cb_data *cbd = user_data;
+ ofono_sim_pin_retries_cb_t cb = cbd->cb;
+ const char *final = g_at_result_final_response(result);
+ GAtResultIter iter;
+ struct ofono_error error;
+ int retries[OFONO_SIM_PASSWORD_INVALID];
+ size_t i;
+ static enum ofono_sim_password_type password_types[] = {
+ OFONO_SIM_PASSWORD_SIM_PIN,
+ OFONO_SIM_PASSWORD_SIM_PUK,
+ OFONO_SIM_PASSWORD_PHSIM_PIN,
+ OFONO_SIM_PASSWORD_PHFSIM_PUK,
+ OFONO_SIM_PASSWORD_SIM_PIN2,
+ OFONO_SIM_PASSWORD_SIM_PUK2,
+ OFONO_SIM_PASSWORD_PHNET_PIN,
+ OFONO_SIM_PASSWORD_PHNET_PUK,
+ };
+
+ decode_at_error(&error, final);
+
+ if (!ok) {
+ cb(&error, NULL, cbd->data);
+ return;
+ }
+
+ g_at_result_iter_init(&iter, result);
+
+ for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
+ retries[i] = -1;
+
+ for (i = 0; i < ARRAY_SIZE(password_types); i++) {
+ int val;
+
+ if (!g_at_result_iter_next(&iter, "^SPIC:"))
+ goto error;
+
+ if (!g_at_result_iter_next_number(&iter, &val))
+ goto error;
+
+ retries[password_types[i]] = val;
+
+ DBG("retry counter id=%d, val=%d", password_types[i],
+ retries[password_types[i]]);
+ }
+
+ cb(&error, retries, cbd->data);
+
+ return;
+
+error:
+ CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+}
+
#define AT_PCT_SET_RETRIES(retries, pin_type, value) \
retries[pin_type] = value; \
DBG("retry counter id=%d, val=%d", pin_type, value);
@@ -1104,6 +1160,18 @@ static void at_pin_retries_query(struct ofono_sim *sim,
at_spic_cb, cbd, g_free) > 0)
return;
break;
+ case OFONO_VENDOR_CINTERION:
+ if (g_at_chat_send(sd->chat, "AT^SPIC=\"SC\",0;"
+ " ^SPIC=\"SC\",1;"
+ " ^SPIC=\"PS\",0;"
+ " ^SPIC=\"PS\",1;"
+ " ^SPIC=\"P2\",0;"
+ " ^SPIC=\"P2\",1;"
+ " ^SPIC=\"PN\",0;"
+ " ^SPIC=\"PN\",1",
+ spic_prefix, at_spic_gemalto_cb, cbd, g_free) > 0)
+ return;
+ break;
case OFONO_VENDOR_TELIT:
if (g_at_chat_send(sd->chat, "AT#PCT", pct_prefix,
at_pct_cb, cbd, g_free) > 0)
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] gemalto: add PLS8 support
2017-08-30 16:30 ` [PATCH v2 3/3] " Sebastian Arnd
@ 2017-08-30 19:05 ` Denis Kenzior
0 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2017-08-30 19:05 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3688 bytes --]
Hi Sebastian,
On 08/30/2017 11:30 AM, Sebastian Arnd wrote:
> diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
> index 7c33c22..17d0be9 100644
> --- a/drivers/atmodem/sim.c
> +++ b/drivers/atmodem/sim.c
This patch should come before the plugins/gemalto.c changes...
> @@ -64,7 +64,7 @@ static const char *pinnum_prefix[] = { "%PINNUM:", NULL };
> static const char *oercn_prefix[] = { "_OERCN:", NULL };
> static const char *cpinr_prefixes[] = { "+CPINR:", "+CPINRE:", NULL };
> static const char *epin_prefix[] = { "*EPIN:", NULL };
> -static const char *spic_prefix[] = { "+SPIC:", NULL };
> +static const char *spic_prefix[] = { "+SPIC:", "^SPIC:", NULL };
Okay, so looks like both Cinterion/Gemalto and SimCom use SPIC, except
you use '^SPIC' vs '+SPIC' for SimCom. While this change is likely
harmless, I'd rather play it safe and use a separate variable. Maybe
caret_spic_prefix, or spic_gemalto_prefix?
> static const char *pct_prefix[] = { "#PCT:", NULL };
> static const char *pnnm_prefix[] = { "+PNNM:", NULL };
> static const char *qpinc_prefix[] = { "+QPINC:", NULL };
> @@ -875,6 +875,62 @@ error:
> CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> }
>
> +static void at_spic_gemalto_cb(gboolean ok, GAtResult *result,
> + gpointer user_data)
> +{
> + struct cb_data *cbd = user_data;
> + ofono_sim_pin_retries_cb_t cb = cbd->cb;
> + const char *final = g_at_result_final_response(result);
> + GAtResultIter iter;
> + struct ofono_error error;
> + int retries[OFONO_SIM_PASSWORD_INVALID];
> + size_t i;
> + static enum ofono_sim_password_type password_types[] = {
> + OFONO_SIM_PASSWORD_SIM_PIN,
> + OFONO_SIM_PASSWORD_SIM_PUK,
> + OFONO_SIM_PASSWORD_PHSIM_PIN,
> + OFONO_SIM_PASSWORD_PHFSIM_PUK,
> + OFONO_SIM_PASSWORD_SIM_PIN2,
> + OFONO_SIM_PASSWORD_SIM_PUK2,
> + OFONO_SIM_PASSWORD_PHNET_PIN,
> + OFONO_SIM_PASSWORD_PHNET_PUK,
> + };
> +
> + decode_at_error(&error, final);
> +
> + if (!ok) {
> + cb(&error, NULL, cbd->data);
> + return;
> + }
> +
> + g_at_result_iter_init(&iter, result);
> +
> + for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
> + retries[i] = -1;
> +
> + for (i = 0; i < ARRAY_SIZE(password_types); i++) {
> + int val;
> +
> + if (!g_at_result_iter_next(&iter, "^SPIC:"))
> + goto error;
> +
> + if (!g_at_result_iter_next_number(&iter, &val))
> + goto error;
> +
> + retries[password_types[i]] = val;
> +
> + DBG("retry counter id=%d, val=%d", password_types[i],
> + retries[password_types[i]]);
> + }
> +
> + cb(&error, retries, cbd->data);
> +
> + return;
> +
> +error:
> + CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> +}
> +
> #define AT_PCT_SET_RETRIES(retries, pin_type, value) \
> retries[pin_type] = value; \
> DBG("retry counter id=%d, val=%d", pin_type, value);
> @@ -1104,6 +1160,18 @@ static void at_pin_retries_query(struct ofono_sim *sim,
> at_spic_cb, cbd, g_free) > 0)
> return;
> break;
> + case OFONO_VENDOR_CINTERION:
Should we be introducing OFONO_VENDOR_GEMALTO?
> + if (g_at_chat_send(sd->chat, "AT^SPIC=\"SC\",0;"
> + " ^SPIC=\"SC\",1;"
> + " ^SPIC=\"PS\",0;"
> + " ^SPIC=\"PS\",1;"
> + " ^SPIC=\"P2\",0;"
> + " ^SPIC=\"P2\",1;"
> + " ^SPIC=\"PN\",0;"
> + " ^SPIC=\"PN\",1",
Please just use tabs for indentation here, don't mix tabs + spaces.
> + spic_prefix, at_spic_gemalto_cb, cbd, g_free) > 0)
> + return;
> + break;
> case OFONO_VENDOR_TELIT:
> if (g_at_chat_send(sd->chat, "AT#PCT", pct_prefix,
> at_pct_cb, cbd, g_free) > 0)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] gemalto: add PLS8 support
2017-08-30 16:30 [PATCH v2 1/3] gemalto: add PLS8 support Sebastian Arnd
2017-08-30 16:30 ` [PATCH v2 2/3] " Sebastian Arnd
2017-08-30 16:30 ` [PATCH v2 3/3] " Sebastian Arnd
@ 2017-08-30 18:54 ` Denis Kenzior
2 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2017-08-30 18:54 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8409 bytes --]
Hi Sebastian,
On 08/30/2017 11:30 AM, Sebastian Arnd wrote:
> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
> index 011713e..dcd8f14 100644
> --- a/plugins/gemalto.c
> +++ b/plugins/gemalto.c
> @@ -67,6 +67,7 @@ struct gemalto_data {
> GAtChat *mdm;
> struct ofono_sim *sim;
> gboolean have_sim;
> + gboolean have_net;
> struct at_util_sim_state_query *sim_state_query;
> struct gemalto_hardware_monitor *hm;
> };
> @@ -101,18 +102,31 @@ static void gemalto_debug(const char *str, void *user_data)
> ofono_info("%s%s", prefix, str);
> }
>
> -static GAtChat *open_device(const char *device)
> +static GAtChat *open_device(const char *device, char *device_debug_name)
> {
> GAtSyntax *syntax;
> GIOChannel *channel;
> GAtChat *chat;
> + GHashTable *options;
>
> DBG("Opening device %s", device);
>
> - channel = g_at_tty_open(device, NULL);
> + options = g_hash_table_new(g_str_hash, g_str_equal);
> + if (options == NULL)
> + return NULL;
> + /*
> + * The modem does not answer when the "Baud" attibute is missing
> + * The supplied value is not making any change. But the "Baud"
> + * paramenter is needed.
> + */
> + g_hash_table_insert(options, "Baud", "115200");
> +
> + channel = g_at_tty_open(device, options);
> + g_hash_table_destroy(options);
> if (channel == NULL)
> return NULL;
>
> +
We do not allow double empty lines anywhere. Even if this were allowed,
such whitespace changes are not related to the current commit and should
be submitted separately.
> syntax = g_at_syntax_new_gsm_permissive();
> chat = g_at_chat_new(channel, syntax);
> g_at_syntax_unref(syntax);
> @@ -121,6 +135,9 @@ static GAtChat *open_device(const char *device)
> if (chat == NULL)
> return NULL;
>
> + if (getenv("OFONO_AT_DEBUG"))
> + g_at_chat_set_debug(chat, gemalto_debug, device_debug_name);
> +
> return chat;
> }
>
> @@ -299,47 +316,54 @@ static int gemalto_hardware_monitor_enable(struct ofono_modem *modem)
> return 0;
> }
>
> +
No double-empty lines please
> static int gemalto_enable(struct ofono_modem *modem)
> {
> struct gemalto_data *data = ofono_modem_get_data(modem);
> - const char *app, *mdm;
> + const char *app, *mdm, *net;
>
> DBG("%p", modem);
>
> app = ofono_modem_get_string(modem, "Application");
> mdm = ofono_modem_get_string(modem, "Modem");
> + net = ofono_modem_get_string(modem, "NetworkInterface");
>
> - if (app == NULL || mdm == NULL)
> + if (net != NULL)
> + data->have_net = TRUE;
> + else
> + data->have_net = FALSE;
> +
> + if (app == NULL || (mdm == NULL && !data->have_net))
> return -EINVAL;
>
> /* Open devices */
> - data->app = open_device(app);
> + data->app = open_device(app, "App");
> if (data->app == NULL)
> return -EINVAL;
>
> - data->mdm = open_device(mdm);
> - if (data->mdm == NULL) {
> - g_at_chat_unref(data->app);
> - data->app = NULL;
> - return -EINVAL;
> - }
> -
> - if (getenv("OFONO_AT_DEBUG")) {
> - g_at_chat_set_debug(data->app, gemalto_debug, "App");
> - g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
> + /* Mdm device is only requiered when there is no net interface */
> + if (!data->have_net) {
> + data->mdm = open_device(mdm, "Mdm");
> + if (data->mdm == NULL) {
> + g_at_chat_unref(data->app);
> + data->app = NULL;
> + return -EINVAL;
> + }
With accordance to doc/coding-style.txt, item M1, please add an empty
line here
> + g_at_chat_send(data->mdm, "ATE0", none_prefix,
> + NULL, NULL, NULL);
> + g_at_chat_send(data->mdm, "AT&C0", none_prefix,
> + NULL, NULL, NULL);
> }
>
> - g_at_chat_send(data->mdm, "ATE0", none_prefix, NULL, NULL, NULL);
> - g_at_chat_send(data->app, "ATE0 +CMEE=1", none_prefix,
> - NULL, NULL, NULL);
> - g_at_chat_send(data->mdm, "AT&C0", none_prefix, NULL, NULL, NULL);
> + g_at_chat_send(data->app, "ATE0", none_prefix, NULL, NULL, NULL);
> + g_at_chat_send(data->app, "AT+CMEE=1", none_prefix, NULL, NULL, NULL);
> g_at_chat_send(data->app, "AT&C0", none_prefix, NULL, NULL, NULL);
> -
> g_at_chat_send(data->app, "AT+CFUN=4", none_prefix,
> - cfun_enable, modem, NULL);
> + cfun_enable, modem, NULL);
>
> gemalto_hardware_monitor_enable(modem);
>
> +
No double empty lines please
> return -EINPROGRESS;
> }
>
> @@ -402,7 +426,8 @@ static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
>
> DBG("modem %p %s", modem, online ? "online" : "offline");
>
> - if (g_at_chat_send(data->app, command, NULL, set_online_cb, cbd, g_free))
> + if (g_at_chat_send(data->app, command, NULL,
> + set_online_cb, cbd, g_free))
Okay, but this should really be a separate commit to specifically
address the '> 80 characters / line' issue.
> return;
>
> CALLBACK_WITH_FAILURE(cb, cbd->data);
> @@ -419,7 +444,8 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
>
> ofono_devinfo_create(modem, 0, "atmodem", data->app);
> ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
> - sim = ofono_sim_create(modem, 0, "atmodem", data->app);
> + sim = ofono_sim_create(modem, OFONO_VENDOR_CINTERION,
> + "atmodem", data->app);
>
> if (sim && data->have_sim == TRUE)
> ofono_sim_inserted_notify(sim, TRUE);
> @@ -438,7 +464,12 @@ static void gemalto_post_sim(struct ofono_modem *modem)
> ofono_sms_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
>
> gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
> - gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
> +
> + if (data->have_net)
> + gc = ofono_gprs_context_create(modem, 0,
> + "gemaltowwanmodem", data->app);
"gemaltomodem" would be better, unless you guys planning on multiple
variants?
> + else
> + gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
>
> if (gprs && gc)
> ofono_gprs_add_context(gprs, gc);
> @@ -450,7 +481,8 @@ static void gemalto_post_online(struct ofono_modem *modem)
>
> DBG("%p", modem);
>
> - ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
> + ofono_netreg_create(modem, OFONO_VENDOR_CINTERION,
> + "atmodem", data->app);
Same comment about '> 80 characters / line' as above. Separate these
chunks into a separate commit and make it first in the series.
> }
>
> static struct ofono_modem_driver gemalto_driver = {
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index aa28bcb..68c8386 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -1080,13 +1080,24 @@ static gboolean setup_gemalto(struct modem_info* modem)
> net = info->devnode;
> else if (g_strcmp0(info->subsystem, "usbmisc") == 0)
> qmi = info->devnode;
> + } else if (g_strcmp0(info->interface, "2/2/1") == 0) {
> + if (g_strcmp0(info->number, "00") == 0)
> + mdm = info->devnode;
> + else if (g_strcmp0(info->number, "02") == 0)
> + app = info->devnode;
> + else if (g_strcmp0(info->number, "04") == 0)
> + gps = info->devnode;
> + } else if (g_strcmp0(info->interface, "2/6/0") == 0 &&
> + g_strcmp0(info->number, "0a") == 0) {
Please don't mix tabs & spaces. If this doesn't fit into an 80
character line, you can get creative, e.g.:
} else if (g_strcmp0(info->interface, "2/6/0") == 0) {
if (g_strcmp0(info->number, "0a") == 0)
net = ...;
}
> + net = info->devnode;
> }
> +
This whitespace change is spurious, please remove.
> }
>
> DBG("application=%s gps=%s modem=%s network=%s qmi=%s",
> app, gps, mdm, net, qmi);
>
> - if (app == NULL || mdm == NULL)
> + if (app == NULL || (mdm == NULL && net == NULL))
> return FALSE;
>
> ofono_modem_set_string(modem->modem, "Application", app);
> @@ -1499,6 +1510,8 @@ static struct {
> { "gemalto", "option", "1e2d", "0053" },
> { "gemalto", "cdc_wdm", "1e2d", "0053" },
> { "gemalto", "qmi_wwan", "1e2d", "0053" },
> + { "gemalto", "cdc_acm", "1e2d", "0061" },
> + { "gemalto", "cdc_ether", "1e2d", "0061" },
> { "telit", "cdc_ncm", "1bc7", "0036" },
> { "telit", "cdc_acm", "1bc7", "0036" },
> { }
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 7+ messages in thread