5:49 a.m.
would like to submit this patch for the lte atom
This patch adds in core atom:
- the protocol for the default LTE APN, ip, ipv6 and both
- the authentication handling, with 3 properties:
method, username, password
then these are used in two atoms:
- atmodem, with standard AT commands syntax
- gemaltomodem, with vendor-specific syntax
The behavior of the patch is described in the api document (part 1),
and in the core atom header (part 2).
Related to the previous version, this version:
- introduces a separate atom for Gemalto vendor-specific syntax
- implements in the core the suggestions of Denis
history of versions fixes:
- a missing return in case of error in at_lte_set_default_attach_info_cb
- discards the use of lte_driver_data in favour of its member chat
for passing among callbacks, as the persistence of the first
cannot be guaranteed, or at least controlled by the driver atom
- fixes the memory management of the object passed among callbacks
by using a reference counted structure
Giacinto Cifelli (7):
lte-api: protocol and authentication properties
lte.h: added proto and authentication handling
lte: protocol and authentication for default ctx
atmodem/atutil: shared functions for cgdcont
atmodem/lte: proto and authentication handling
gemalto/lte: Gemalto vendor lte atom
plugins/gemalto: use Gemalto lte atom
Makefile.am | 5 +-
doc/lte-api.txt | 39 +++++
drivers/atmodem/atutil.c | 49 ++++++
drivers/atmodem/atutil.h | 6 +
drivers/atmodem/lte.c | 119 ++++++++++++---
drivers/gemaltomodem/gemaltomodem.c | 2 +
drivers/gemaltomodem/gemaltomodem.h | 4 +
drivers/gemaltomodem/gemaltoutil.c | 106 +++++++++++++
drivers/gemaltomodem/gemaltoutil.h | 31 ++++
drivers/gemaltomodem/lte.c | 221 +++++++++++++++++++++++++++
include/lte.h | 5 +
plugins/gemalto.c | 16 +-
src/lte.c | 222 +++++++++++++++++++++-------
13 files changed, 749 insertions(+), 76 deletions(-)
create mode 100644 drivers/gemaltomodem/gemaltoutil.c
create mode 100644 drivers/gemaltomodem/gemaltoutil.h
create mode 100644 drivers/gemaltomodem/lte.c
--
2.17.1
5:49 a.m.
New subject: [PATCH v7 1/7] lte-api: protocol and authentication properties
added 4 properties for handling the type of context and the
authentication method, exactly like in any gprs context handling.
The properties are named after the equivalent gprs-context one, for
compatibility and uniformity.
Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
---
doc/lte-api.txt | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/doc/lte-api.txt b/doc/lte-api.txt
index 8a2a97d9..c9544f60 100644
--- a/doc/lte-api.txt
+++ b/doc/lte-api.txt
@@ -33,3 +33,42 @@ Properties string DefaultAccessPointName [readwrite]
Setting this property to an empty string clears the
default APN from the modem.
+
+ string Protocol [readwrite]
+
+ Holds the protocol for this context. Valid values
+ are: "ip", "ipv6" and "dual". Default value is
"ip".
+
+ string AuthenticationMethod [readwrite]
+
+ Sets the Method used for the authentication
+ for the default APN.
+
+ Available values are "none", "pap" and "chap".
+ Default is "none".
+
+ If the AuthenticationMethod is set to 'none' it remove
+ the authentication for the DefaultAPN.
+ In case of AuthenticationMethod 'none',
+ if the Username and Password properties are not empty,
+ the values are preserved in the properties, but they
+ are not used or transmitted to the module.
+ Conversely, if Username or Password are empty, the
+ authentication method selected internally is 'none',
+ but the property AuthenticationMethod is left unchanged.
+
+ If the default APN supports authentication and it
+ fails, then it is up to the network how to proceed.
+ In general LTE access is denied and the modem can
+ fallback to a legacy technology if capable and another
+ radio technology is available.
+
+ string Username [readwrite]
+
+ Holds the username to be used for authentication
+ purposes.
+
+ string Password [readwrite]
+
+ Holds the password to be used for authentication
+ purposes.
--
2.17.1
5:49 a.m.
New subject: [PATCH v7 2/7] lte.h: added proto and authentication handling
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties changes
Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
---
include/lte.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/lte.h b/include/lte.h
index 0f2501c0..2f12ac29 100644
--- a/include/lte.h
+++ b/include/lte.h
@@ -3,6 +3,7 @@
* oFono - Open Source Telephony
*
* Copyright (C) 2016 Endocode AG. 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
@@ -32,6 +33,10 @@ struct ofono_lte;
struct ofono_lte_default_attach_info {
char apn[OFONO_GPRS_MAX_APN_LENGTH + 1];
+ enum ofono_gprs_proto proto;
+ enum ofono_gprs_auth_method auth_method;
+ char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
+ char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
};
typedef void (*ofono_lte_cb_t)(const struct ofono_error *error, void *data);
--
2.17.1
5:49 a.m.
New subject: [PATCH v7 3/7] lte: protocol and authentication for default ctx
Many LTE networks require user authentication, even for the default
context. In particular, most of the private APNs use this facility
to add some control on top of the MNO providing the service, so that
another user of the same network cannot access the private one.
As such, we add these parameters to the default context
settings that will attempt to use when registering to the network.
The additional parameters added by this patch are: protocol, user, and
password. These are sufficient to allow to connect to networks
available to the patch author where ofono previously failed to register
to the network at all.
Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
Co-authored-by: Denis Kenzior <denis.kenzior(a)intel.com>
---
src/lte.c | 222 ++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 167 insertions(+), 55 deletions(-)
diff --git a/src/lte.c b/src/lte.c
index 23fe8e1c..fc0a3442 100644
--- a/src/lte.c
+++ b/src/lte.c
@@ -3,6 +3,7 @@
* oFono - Open Source Telephony
*
* Copyright (C) 2016 Endocode AG. 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
@@ -39,7 +40,11 @@
#define SETTINGS_STORE "lte"
#define SETTINGS_GROUP "Settings"
-#define DEFAULT_APN_KEY "DefaultAccessPointName"
+#define LTE_APN "DefaultAccessPointName"
+#define LTE_PROTO "Protocol"
+#define LTE_USERNAME "Username"
+#define LTE_PASSWORD "Password"
+#define LTE_AUTH_METHOD "AuthenticationMethod"
struct ofono_lte {
const struct ofono_lte_driver *driver;
@@ -57,6 +62,10 @@ static GSList *g_drivers = NULL;
static void lte_load_settings(struct ofono_lte *lte)
{
char *apn;
+ char *proto_str;
+ char *auth_method_str;
+ char *username;
+ char *password;
if (lte->imsi == NULL)
return;
@@ -69,19 +78,57 @@ static void lte_load_settings(struct ofono_lte *lte)
return;
}
- apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP ,
- DEFAULT_APN_KEY, NULL);
- if (apn) {
+ apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+ LTE_APN, NULL);
+ proto_str = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+ LTE_PROTO, NULL);
+ auth_method_str = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+ LTE_AUTH_METHOD, NULL);
+ username = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+ LTE_USERNAME, NULL);
+ password = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+ LTE_PASSWORD, NULL);
+ if (apn && is_valid_apn(apn))
strcpy(lte->info.apn, apn);
- g_free(apn);
- }
+
+ if (proto_str == NULL)
+ proto_str = g_strdup("ip");
+
+ /* this must have a valid default */
+ if (!gprs_proto_from_string(proto_str, <e->info.proto))
+ lte->info.proto = OFONO_GPRS_PROTO_IP;
+
+ if (auth_method_str == NULL)
+ auth_method_str = g_strdup("none");
+
+ /* this must have a valid default */
+ if (!gprs_auth_method_from_string(auth_method_str,
+ <e->info.auth_method))
+ lte->info.auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+ if (username && strlen(username) <= OFONO_GPRS_MAX_USERNAME_LENGTH)
+ strcpy(lte->info.username, username);
+
+ if (password && strlen(password) <= OFONO_GPRS_MAX_PASSWORD_LENGTH)
+ strcpy(lte->info.password, password);
+
+ g_free(apn);
+ g_free(proto_str);
+ g_free(auth_method_str);
+ g_free(username);
+ g_free(password);
}
static DBusMessage *lte_get_properties(DBusConnection *conn,
DBusMessage *msg, void *data)
{
struct ofono_lte *lte = data;
+ const char *proto = gprs_proto_to_string(lte->info.proto);
const char *apn = lte->info.apn;
+ const char* auth_method =
+ gprs_auth_method_to_string(lte->info.auth_method);
+ const char *username = lte->info.username;
+ const char *password = lte->info.password;
DBusMessage *reply;
DBusMessageIter iter;
DBusMessageIter dict;
@@ -95,20 +142,31 @@ static DBusMessage *lte_get_properties(DBusConnection *conn,
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
OFONO_PROPERTIES_ARRAY_SIGNATURE,
&dict);
- ofono_dbus_dict_append(&dict, DEFAULT_APN_KEY, DBUS_TYPE_STRING, &apn);
+ ofono_dbus_dict_append(&dict, LTE_APN, DBUS_TYPE_STRING, &apn);
+ ofono_dbus_dict_append(&dict, LTE_PROTO, DBUS_TYPE_STRING, &proto);
+ ofono_dbus_dict_append(&dict, LTE_AUTH_METHOD, DBUS_TYPE_STRING,
+ &auth_method);
+ ofono_dbus_dict_append(&dict, LTE_USERNAME, DBUS_TYPE_STRING,
+ &username);
+ ofono_dbus_dict_append(&dict, LTE_PASSWORD, DBUS_TYPE_STRING,
+ &password);
dbus_message_iter_close_container(&iter, &dict);
return reply;
}
static void lte_set_default_attach_info_cb(const struct ofono_error *error,
- void *data)
+ void *data)
{
struct ofono_lte *lte = data;
const char *path = __ofono_atom_get_path(lte->atom);
DBusConnection *conn = ofono_dbus_get_connection();
DBusMessage *reply;
- const char *apn = lte->info.apn;
+ char *key;
+ char *value;
+ const char *str;
+ DBusMessageIter iter;
+ DBusMessageIter var;
DBG("%s error %d", path, error->type);
@@ -118,55 +176,48 @@ static void lte_set_default_attach_info_cb(const struct ofono_error
*error,
return;
}
- g_strlcpy(lte->info.apn, lte->pending_info.apn,
- OFONO_GPRS_MAX_APN_LENGTH + 1);
+ /*
+ * Reparsing of the message to extract the key and value
+ * No error checking needed since we already validated pending
+ */
+ dbus_message_iter_init(lte->pending, &iter);
+ dbus_message_iter_get_basic(&iter, &str);
+ key = strdup(str);
+
+ dbus_message_iter_next(&iter);
+ dbus_message_iter_recurse(&iter, &var);
+ dbus_message_iter_get_basic(&var, &str);
+ value = strdup(str);
+
+ memcpy(<e->info, <e->pending_info, sizeof(lte->info));
+
+ reply = dbus_message_new_method_return(lte->pending);
+ __ofono_dbus_pending_reply(<e->pending, reply);
if (lte->settings) {
- if (strlen(lte->info.apn) == 0)
- /* Clear entry on empty APN. */
- g_key_file_remove_key(lte->settings, SETTINGS_GROUP,
- DEFAULT_APN_KEY, NULL);
+ /*
+ * the following code removes from storage empty APN, user, pwd
+ * for proto and auth_method, given that they always
+ * have defaults, it will not do anything.
+ */
+ if (!*value)
+ /* Clear entry on empty string. */
+ g_key_file_remove_key(lte->settings,
+ SETTINGS_GROUP, key, NULL);
else
- g_key_file_set_string(lte->settings, SETTINGS_GROUP,
- DEFAULT_APN_KEY, lte->info.apn);
+ g_key_file_set_string(lte->settings,
+ SETTINGS_GROUP, key, value);
storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
}
- reply = dbus_message_new_method_return(lte->pending);
- __ofono_dbus_pending_reply(<e->pending, reply);
-
ofono_dbus_signal_property_changed(conn, path,
OFONO_CONNECTION_CONTEXT_INTERFACE,
- DEFAULT_APN_KEY,
- DBUS_TYPE_STRING, &apn);
-}
-
-static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
- DBusConnection *conn, DBusMessage *msg,
- const char *apn)
-{
- if (lte->driver->set_default_attach_info == NULL)
- return __ofono_error_not_implemented(msg);
+ key,
+ DBUS_TYPE_STRING, &value);
- if (lte->pending)
- return __ofono_error_busy(msg);
-
- if (g_str_equal(apn, lte->info.apn))
- return dbus_message_new_method_return(msg);
-
- /* We do care about empty value: it can be used for reset. */
- if (is_valid_apn(apn) == FALSE && apn[0] != '\0')
- return __ofono_error_invalid_format(msg);
-
- lte->pending = dbus_message_ref(msg);
-
- g_strlcpy(lte->pending_info.apn, apn, OFONO_GPRS_MAX_APN_LENGTH + 1);
-
- lte->driver->set_default_attach_info(lte, <e->pending_info,
- lte_set_default_attach_info_cb, lte);
-
- return NULL;
+ g_free(value);
+ g_free(key);
}
static DBusMessage *lte_set_property(DBusConnection *conn,
@@ -177,6 +228,14 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
DBusMessageIter var;
const char *property;
const char *str;
+ enum ofono_gprs_auth_method auth_method;
+ enum ofono_gprs_proto proto;
+
+ if (lte->driver->set_default_attach_info == NULL)
+ return __ofono_error_not_implemented(msg);
+
+ if (lte->pending)
+ return __ofono_error_busy(msg);
if (!dbus_message_iter_init(msg, &iter))
return __ofono_error_invalid_args(msg);
@@ -192,16 +251,69 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
dbus_message_iter_recurse(&iter, &var);
- if (!strcmp(property, DEFAULT_APN_KEY)) {
- if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
- return __ofono_error_invalid_args(msg);
+ if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+ return __ofono_error_invalid_args(msg);
- dbus_message_iter_get_basic(&var, &str);
+ dbus_message_iter_get_basic(&var, &str);
- return lte_set_default_apn(lte, conn, msg, str);
- }
+ memcpy(<e->pending_info, <e->info, sizeof(lte->info));
+
+ if ((strcmp(property, LTE_APN) == 0)) {
+
+ if (g_str_equal(str, lte->info.apn))
+ return dbus_message_new_method_return(msg);
+
+ /* We do care about empty value: it can be used for reset. */
+ if (is_valid_apn(str) == FALSE && str[0] != '\0')
+ return __ofono_error_invalid_format(msg);
+
+ g_strlcpy(lte->pending_info.apn, str,
+ OFONO_GPRS_MAX_APN_LENGTH + 1);
+
+ } else if ((strcmp(property, LTE_PROTO) == 0)) {
+
+ if (!gprs_proto_from_string(str, &proto))
+ return __ofono_error_invalid_format(msg);
+
+ if (proto == lte->info.proto)
+ return dbus_message_new_method_return(msg);
- return __ofono_error_invalid_args(msg);
+ lte->pending_info.proto = proto;
+
+ } else if (strcmp(property, LTE_AUTH_METHOD) == 0) {
+
+ if (!gprs_auth_method_from_string(str, &auth_method))
+ return __ofono_error_invalid_format(msg);
+
+ if (auth_method == lte->info.auth_method)
+ return dbus_message_new_method_return(msg);
+
+ lte->pending_info.auth_method = auth_method;
+
+ } else if (strcmp(property, LTE_USERNAME) == 0) {
+
+ if (g_str_equal(str, lte->info.username))
+ return dbus_message_new_method_return(msg);
+
+ g_strlcpy(lte->pending_info.username, str,
+ OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+
+ } else if (strcmp(property, LTE_PASSWORD) == 0) {
+
+ if (g_str_equal(str, lte->info.password))
+ return dbus_message_new_method_return(msg);
+
+ g_strlcpy(lte->pending_info.password, str,
+ OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+
+ } else
+ return __ofono_error_invalid_args(msg);
+
+ lte->pending = dbus_message_ref(msg);
+ lte->driver->set_default_attach_info(lte, <e->pending_info,
+ lte_set_default_attach_info_cb, lte);
+
+ return NULL;
}
static const GDBusMethodTable lte_methods[] = {
--
2.17.1
5:49 a.m.
New subject: [PATCH v7 4/7] atmodem/atutil: shared functions for cgdcont
included two new functions:
at_get_auth_type: converts the ofono internal ofono_gprs_auth_method
into the value of the 3GPP 27.007 to pass to the AT command
at_get_cgdcont_command: computes the AT+CGDCONT string, standard version
---
drivers/atmodem/atutil.c | 49 ++++++++++++++++++++++++++++++++++++++++
drivers/atmodem/atutil.h | 6 +++++
2 files changed, 55 insertions(+)
diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index 6f4e8a20..dc1ef3d3 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -3,6 +3,7 @@
* oFono - Open Source Telephony
*
* Copyright (C) 2008-2011 Intel Corporation. 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
@@ -27,6 +28,7 @@
#include <gatchat.h>
#include <string.h>
#include <stdlib.h>
+#include <stdio.h>
#include <errno.h>
#define OFONO_API_SUBJECT_TO_CHANGE
@@ -654,3 +656,50 @@ int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
return ret;
}
+
+int at_get_auth_type(enum ofono_gprs_auth_method auth_method)
+{
+ switch (auth_method) {
+ case OFONO_GPRS_AUTH_METHOD_PAP:
+ return 1;
+ case OFONO_GPRS_AUTH_METHOD_CHAP:
+ return 2;
+ case OFONO_GPRS_AUTH_METHOD_NONE:
+ return 0;
+ }
+
+ return 0;
+}
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+ const char *apn)
+{
+ size_t buflen = 32 + OFONO_GPRS_MAX_APN_LENGTH + 1;
+ char *buf = g_new(char, buflen);
+ int len;
+
+ len = snprintf(buf, buflen, "AT+CGDCONT=%u", cid);
+ buflen -= len;
+
+ /*
+ * if apn is null, it will remove the context.
+ * but if apn is empty, it will create a context with empty apn
+ */
+ if (!apn)
+ goto finished;
+
+ 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:
+ snprintf(buf+len, buflen, ",\"IP\",\"%s\"", apn);
+ break;
+ }
+
+finished:
+ return buf;
+}
diff --git a/drivers/atmodem/atutil.h b/drivers/atmodem/atutil.h
index 7113a4cd..6a40d045 100644
--- a/drivers/atmodem/atutil.h
+++ b/drivers/atmodem/atutil.h
@@ -3,6 +3,7 @@
* oFono - Open Source Telephony
*
* Copyright (C) 2008-2011 Intel Corporation. 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
@@ -86,6 +87,11 @@ void at_util_sim_state_query_free(struct at_util_sim_state_query
*req);
int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
char *address, char *netmask);
+int at_get_auth_type(enum ofono_gprs_auth_method auth_method);
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+ const char *apn);
+
struct cb_data {
void *cb;
void *data;
--
2.17.1
5:49 a.m.
New subject: [PATCH v7 5/7] atmodem/lte: proto and authentication handling
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
---
drivers/atmodem/lte.c | 119 ++++++++++++++++++++++++++++++++++++------
1 file changed, 102 insertions(+), 17 deletions(-)
diff --git a/drivers/atmodem/lte.c b/drivers/atmodem/lte.c
index c4866623..b50aa0bc 100644
--- a/drivers/atmodem/lte.c
+++ b/drivers/atmodem/lte.c
@@ -3,6 +3,7 @@
* oFono - Open Source Telephony
*
* Copyright (C) 2017 Intel Corporation. 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
@@ -44,41 +45,125 @@ struct lte_driver_data {
GAtChat *chat;
};
-static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+struct lte_cbd {
+ gint ref_count; /* Ref count */
+ ofono_lte_cb_t cb;
+ void *data;
+ GAtChat *chat;
+ const struct ofono_lte_default_attach_info *info;
+ struct ofono_modem *modem;
+};
+
+static struct lte_cbd *lte_cb_data_new0(void *cb, void *data,
+ GAtChat *chat, const struct ofono_lte_default_attach_info *info)
+{
+ struct lte_cbd *cbd = g_new0(struct lte_cbd, 1);
+
+ cbd->ref_count = 1;
+ cbd->cb = cb;
+ cbd->data = data;
+ cbd->chat = chat;
+ cbd->info = info;
+
+ return cbd;
+}
+
+static inline struct lte_cbd *lte_cb_data_ref(struct lte_cbd *cbd)
+{
+ if (cbd == NULL)
+ return NULL;
+
+ g_atomic_int_inc(&cbd->ref_count);
+
+ return cbd;
+}
+
+static void lte_cb_data_unref(gpointer user_data)
+{
+ gboolean is_zero;
+ struct lte_cbd *cbd = user_data;
+
+ if (cbd == NULL)
+ return;
+
+ is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
+
+ if (is_zero == TRUE)
+ g_free(cbd);
+}
+
+static void at_lte_set_auth_cb(gboolean ok, GAtResult *result,
gpointer user_data)
{
- struct cb_data *cbd = user_data;
+ struct lte_cbd *cbd = user_data;
ofono_lte_cb_t cb = cbd->cb;
struct ofono_error error;
- DBG("ok %d", ok);
-
decode_at_error(&error, g_at_result_final_response(result));
cb(&error, cbd->data);
}
+static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct lte_cbd *cbd = user_data;
+ ofono_lte_cb_t cb = cbd->cb;
+ void *data = cbd->data;
+ struct ofono_error error;
+ char buf[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+ OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
+ size_t buflen = sizeof(buf);
+ size_t len;
+ enum ofono_gprs_auth_method auth_method;
+ int auth_type;
+
+ if (!ok) {
+ lte_cb_data_unref(cbd);
+ decode_at_error(&error, g_at_result_final_response(result));
+ cb(&error, data);
+ return;
+ }
+
+ auth_method = cbd->info->auth_method;
+
+ /* change the authentication method if the parameters are invalid */
+ if (!*cbd->info->username || !*cbd->info->password)
+ auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+ auth_type = at_get_auth_type(auth_method);
+ len = snprintf(buf, buflen, "AT+CGAUTH=0,%d",auth_type);
+ buflen -= len;
+
+ if (auth_method != OFONO_GPRS_AUTH_METHOD_NONE)
+ snprintf(buf + len, buflen, ",\"%s\",\"%s\"",
+ cbd->info->username, cbd->info->password);
+
+ cbd = lte_cb_data_ref(cbd);
+ if (g_at_chat_send(cbd->chat, buf, NULL,
+ at_lte_set_auth_cb, cbd, lte_cb_data_unref) > 0)
+ return;
+
+ lte_cb_data_unref(cbd);
+ CALLBACK_WITH_FAILURE(cb, data);
+}
+
static void at_lte_set_default_attach_info(const struct ofono_lte *lte,
const struct ofono_lte_default_attach_info *info,
ofono_lte_cb_t cb, void *data)
{
struct lte_driver_data *ldd = ofono_lte_get_data(lte);
- char buf[32 + OFONO_GPRS_MAX_APN_LENGTH + 1];
- struct cb_data *cbd = cb_data_new(cb, data);
-
- DBG("LTE config with APN: %s", info->apn);
+ struct lte_cbd *cbd = lte_cb_data_new0(cb, data, ldd->chat, info);
+ char *buf = at_get_cgdcont_command(0, info->proto, info->apn);
- if (strlen(info->apn) > 0)
- snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"IP\",\"%s\"",
- info->apn);
- else
- snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"IP\"");
-
- /* We can't do much in case of failure so don't check response. */
if (g_at_chat_send(ldd->chat, buf, NULL,
- at_lte_set_default_attach_info_cb, cbd, g_free) > 0)
- return;
+ at_lte_set_default_attach_info_cb,
+ cbd, lte_cb_data_unref) > 0)
+ goto end;
+ lte_cb_data_unref(cbd);
CALLBACK_WITH_FAILURE(cb, data);
+end:
+ g_free(buf);
}
static gboolean lte_delayed_register(gpointer user_data)
--
2.17.1
5:49 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Added a vendor-specific lte atom, for handling PDP context creation
and authentication, using the actual formats for the vendor.
The code for PDP context and authentication command formating
is in a separate file, gemaltoutil, because in the future it can be
shared with the gprs-context atom.
Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
Co-authored-by: Sebastian Arnd <sebastianarnd(a)gmail.com>
---
Makefile.am | 5 +-
drivers/gemaltomodem/gemaltomodem.c | 2 +
drivers/gemaltomodem/gemaltomodem.h | 4 +
drivers/gemaltomodem/gemaltoutil.c | 106 +++++++++++++
drivers/gemaltomodem/gemaltoutil.h | 31 ++++
drivers/gemaltomodem/lte.c | 221 ++++++++++++++++++++++++++++
6 files changed, 368 insertions(+), 1 deletion(-)
create mode 100644 drivers/gemaltomodem/gemaltoutil.c
create mode 100644 drivers/gemaltomodem/gemaltoutil.h
create mode 100644 drivers/gemaltomodem/lte.c
diff --git a/Makefile.am b/Makefile.am
index e8e4ed95..dd0bc0bb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -397,8 +397,11 @@ builtin_modules += gemaltomodem
builtin_sources += drivers/atmodem/atutil.h \
drivers/gemaltomodem/gemaltomodem.h \
drivers/gemaltomodem/gemaltomodem.c \
+ drivers/gemaltomodem/gemaltoutil.h \
+ drivers/gemaltomodem/gemaltoutil.c \
drivers/gemaltomodem/location-reporting.c \
- drivers/gemaltomodem/voicecall.c
+ drivers/gemaltomodem/voicecall.c \
+ drivers/gemaltomodem/lte.c
builtin_modules += xmm7modem
builtin_sources += drivers/atmodem/atutil.h \
diff --git a/drivers/gemaltomodem/gemaltomodem.c b/drivers/gemaltomodem/gemaltomodem.c
index 4818ac66..459c3583 100644
--- a/drivers/gemaltomodem/gemaltomodem.c
+++ b/drivers/gemaltomodem/gemaltomodem.c
@@ -37,12 +37,14 @@ static int gemaltomodem_init(void)
{
gemalto_location_reporting_init();
gemalto_voicecall_init();
+ gemalto_lte_init();
return 0;
}
static void gemaltomodem_exit(void)
{
+ gemalto_lte_exit();
gemalto_voicecall_exit();
gemalto_location_reporting_exit();
}
diff --git a/drivers/gemaltomodem/gemaltomodem.h b/drivers/gemaltomodem/gemaltomodem.h
index 27b1460e..9f285563 100644
--- a/drivers/gemaltomodem/gemaltomodem.h
+++ b/drivers/gemaltomodem/gemaltomodem.h
@@ -21,9 +21,13 @@
*/
#include <drivers/atmodem/atutil.h>
+#include "gemaltoutil.h"
extern void gemalto_location_reporting_init();
extern void gemalto_location_reporting_exit();
extern void gemalto_voicecall_init();
extern void gemalto_voicecall_exit();
+
+extern void gemalto_lte_init();
+extern void gemalto_lte_exit();
diff --git a/drivers/gemaltomodem/gemaltoutil.c b/drivers/gemaltomodem/gemaltoutil.c
new file mode 100644
index 00000000..d8c08e16
--- /dev/null
+++ b/drivers/gemaltomodem/gemaltoutil.c
@@ -0,0 +1,106 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+#include <gatchat.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/log.h>
+#include <ofono/types.h>
+
+#include "ofono.h"
+#include "gemaltomodem.h"
+
+enum gemalto_auth_option {
+ GEMALTO_AUTH_DEFAULTS = 0x00,
+ GEMALTO_AUTH_USE_SGAUTH = 0x01,
+ GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
+ GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
+};
+
+char *gemalto_get_auth_command(struct ofono_modem *modem, int cid,
+ enum ofono_gprs_auth_method auth_method,
+ const char *username, const char *password)
+{
+ int gemalto_auth = ofono_modem_get_integer(modem, "GemaltoAuthType");
+ int len;
+ int auth_type = at_get_auth_type(auth_method);
+ size_t buflen = 32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+ OFONO_GPRS_MAX_PASSWORD_LENGTH + 1;
+ char *buf = g_new(char, buflen);
+
+ /* for now. Later to consider modules where the LTE attach CID=3 */
+ if (cid==0)
+ cid=1;
+
+ if (gemalto_auth & GEMALTO_AUTH_USE_SGAUTH)
+ len = snprintf(buf, buflen, "AT^SGAUTH=%d,%d", cid, auth_type);
+ else
+ len = snprintf(buf, buflen, "AT+CGAUTH=%d,%d", cid, auth_type);
+
+ buflen -= len;
+
+ switch(auth_method) {
+ case OFONO_GPRS_AUTH_METHOD_NONE:
+
+ if (gemalto_auth & GEMALTO_AUTH_ALW_ALL_PARAMS)
+ snprintf(buf+len, buflen, ",\"\",\"\"");
+
+ break;
+ case OFONO_GPRS_AUTH_METHOD_PAP:
+ case OFONO_GPRS_AUTH_METHOD_CHAP:
+
+ if (gemalto_auth & GEMALTO_AUTH_ORDER_PWD_USR)
+ snprintf(buf+len, buflen, ",\"%s\",\"%s\"",
+ password, username);
+ else
+ snprintf(buf+len, buflen, ",\"%s\",\"%s\"",
+ username, password);
+
+ break;
+ }
+
+ return buf;
+}
+
+char *gemalto_get_cgdcont_command(struct ofono_modem *modem, guint cid,
+ enum ofono_gprs_proto proto, const char *apn)
+{
+ /*
+ * For future extension: verify if the module supports automatic
+ * context provisioning and if so, also if there is a manual override
+ * This is an ofono_modem_get_integer property
+ */
+
+ /* for now. Later to consider modules where the LTE attach CID=3 */
+ if (cid==0)
+ cid=1;
+
+ return at_get_cgdcont_command(cid, proto, apn);
+}
diff --git a/drivers/gemaltomodem/gemaltoutil.h b/drivers/gemaltomodem/gemaltoutil.h
new file mode 100644
index 00000000..d653f61d
--- /dev/null
+++ b/drivers/gemaltomodem/gemaltoutil.h
@@ -0,0 +1,31 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+struct ofono_modem;
+
+char *gemalto_get_auth_command(struct ofono_modem *modem, int cid,
+ enum ofono_gprs_auth_method auth_method,
+ const char *username, const char *password);
+
+
+char *gemalto_get_cgdcont_command(struct ofono_modem *modem, guint cid,
+ enum ofono_gprs_proto proto, const char *apn);
+
diff --git a/drivers/gemaltomodem/lte.c b/drivers/gemaltomodem/lte.c
new file mode 100644
index 00000000..1c72173f
--- /dev/null
+++ b/drivers/gemaltomodem/lte.c
@@ -0,0 +1,221 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2017 Intel Corporation. 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <ofono/modem.h>
+#include <ofono/gprs-context.h>
+#include <ofono/log.h>
+#include <ofono/lte.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "gemaltomodem.h"
+
+struct gemalto_lte_driver_data {
+ GAtChat *chat;
+};
+
+struct gemalto_lte_cbd {
+ gint ref_count; /* Ref count */
+ ofono_lte_cb_t cb;
+ void *data;
+ GAtChat *chat;
+ const struct ofono_lte_default_attach_info *info;
+ struct ofono_modem *modem;
+};
+
+static struct gemalto_lte_cbd *gemalto_lte_cb_data_new0(void *cb, void *data,
+ GAtChat *chat, const struct ofono_lte_default_attach_info *info,
+ struct ofono_modem *modem)
+{
+ struct gemalto_lte_cbd *cbd = g_new0(struct gemalto_lte_cbd, 1);
+
+ cbd->ref_count = 1;
+ cbd->cb = cb;
+ cbd->data = data;
+ cbd->chat = chat;
+ cbd->info = info;
+ cbd->modem = modem;
+
+ return cbd;
+}
+
+static inline struct gemalto_lte_cbd *gemalto_lte_cb_data_ref(
+ struct gemalto_lte_cbd *cbd)
+{
+ if (cbd == NULL)
+ return NULL;
+
+ g_atomic_int_inc(&cbd->ref_count);
+
+ return cbd;
+}
+
+static void gemalto_lte_cb_data_unref(gpointer user_data)
+{
+ gboolean is_zero;
+ struct gemalto_lte_cbd *cbd = user_data;
+
+ if (cbd == NULL)
+ return;
+
+ is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
+
+ if (is_zero == TRUE)
+ g_free(cbd);
+}
+
+static void gemalto_lte_set_auth_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct gemalto_lte_cbd *cbd = user_data;
+ ofono_lte_cb_t cb = cbd->cb;
+ struct ofono_error error;
+
+ decode_at_error(&error, g_at_result_final_response(result));
+ cb(&error, cbd->data);
+}
+
+static void gemalto_lte_set_default_attach_info_cb(gboolean ok,
+ GAtResult *result, gpointer user_data)
+{
+ struct gemalto_lte_cbd *cbd = user_data;
+ ofono_lte_cb_t cb = cbd->cb;
+ void *data = cbd->data;
+ struct ofono_error error;
+ char *buf;
+ enum ofono_gprs_auth_method auth_method;
+
+ if (!ok) {
+ gemalto_lte_cb_data_unref(cbd);
+ decode_at_error(&error, g_at_result_final_response(result));
+ cb(&error, data);
+ return;
+ }
+
+ auth_method = cbd->info->auth_method;
+
+ /* change the authentication method if the parameters are invalid */
+ if (!*cbd->info->username || !*cbd->info->password)
+ auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+ buf = gemalto_get_auth_command(cbd->modem, 0, auth_method,
+ cbd->info->username, cbd->info->password);
+ cbd = gemalto_lte_cb_data_ref(cbd);
+
+ if (g_at_chat_send(cbd->chat, buf, NULL,
+ gemalto_lte_set_auth_cb, cbd,
+ gemalto_lte_cb_data_unref) > 0)
+ goto end;
+
+ gemalto_lte_cb_data_unref(cbd);
+ CALLBACK_WITH_FAILURE(cb, data);
+end:
+ g_free(buf);
+}
+
+static void gemalto_lte_set_default_attach_info(const struct ofono_lte *lte,
+ const struct ofono_lte_default_attach_info *info,
+ ofono_lte_cb_t cb, void *data)
+{
+ struct gemalto_lte_driver_data *ldd = ofono_lte_get_data(lte);
+ struct ofono_modem *modem = ofono_lte_get_modem(lte);
+ struct gemalto_lte_cbd *cbd = gemalto_lte_cb_data_new0(cb, data,
+ ldd->chat, info, modem);
+ char *buf = gemalto_get_cgdcont_command(modem, 0, info->proto,
+ info->apn);
+
+ if (g_at_chat_send(ldd->chat, buf, NULL,
+ gemalto_lte_set_default_attach_info_cb,
+ cbd, gemalto_lte_cb_data_unref) > 0)
+ goto end;
+
+ gemalto_lte_cb_data_unref(cbd);
+ CALLBACK_WITH_FAILURE(cb, data);
+end:
+ g_free(buf);
+}
+
+static gboolean gemalto_lte_delayed_register(gpointer user_data)
+{
+ struct ofono_lte *lte = user_data;
+
+ ofono_lte_register(lte);
+
+ return FALSE;
+}
+
+static int gemalto_lte_probe(struct ofono_lte *lte, unsigned int vendor,
+ void *data)
+{
+ GAtChat *chat = data;
+ struct gemalto_lte_driver_data *ldd;
+
+ ldd = g_new0(struct gemalto_lte_driver_data, 1);
+
+ ldd->chat = g_at_chat_clone(chat);
+
+ ofono_lte_set_data(lte, ldd);
+
+ g_idle_add(gemalto_lte_delayed_register, lte);
+
+ return 0;
+}
+
+static void gemalto_lte_remove(struct ofono_lte *lte)
+{
+ struct gemalto_lte_driver_data *ldd = ofono_lte_get_data(lte);
+
+ g_at_chat_unref(ldd->chat);
+
+ ofono_lte_set_data(lte, NULL);
+
+ g_free(ldd);
+}
+
+static const struct ofono_lte_driver driver = {
+ .name = "gemaltomodem",
+ .probe = gemalto_lte_probe,
+ .remove = gemalto_lte_remove,
+ .set_default_attach_info = gemalto_lte_set_default_attach_info,
+};
+
+void gemalto_lte_init(void)
+{
+ ofono_lte_driver_register(&driver);
+}
+
+void gemalto_lte_exit(void)
+{
+ ofono_lte_driver_unregister(&driver);
+}
--
2.17.1
6:32 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Giacinto,
This looks pretty good to me... Some comments below.
On 18/10/18 20:49, Giacinto Cifelli wrote:
Added a vendor-specific lte atom, for handling PDP context creation
and authentication, using the actual formats for the vendor.
The code for PDP context and authentication command formating
is in a separate file, gemaltoutil, because in the future it can be
shared with the gprs-context atom.
Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
Co-authored-by: Sebastian Arnd <sebastianarnd(a)gmail.com>
---
Makefile.am | 5 +-
drivers/gemaltomodem/gemaltomodem.c | 2 +
drivers/gemaltomodem/gemaltomodem.h | 4 +
drivers/gemaltomodem/gemaltoutil.c | 106 +++++++++++++
drivers/gemaltomodem/gemaltoutil.h | 31 ++++
drivers/gemaltomodem/lte.c | 221 ++++++++++++++++++++++++++++
6 files changed, 368 insertions(+), 1 deletion(-)
create mode 100644 drivers/gemaltomodem/gemaltoutil.c
create mode 100644 drivers/gemaltomodem/gemaltoutil.h
create mode 100644 drivers/gemaltomodem/lte.c
diff --git a/Makefile.am b/Makefile.am
index e8e4ed95..dd0bc0bb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -397,8 +397,11 @@ builtin_modules += gemaltomodem
builtin_sources += drivers/atmodem/atutil.h \
drivers/gemaltomodem/gemaltomodem.h \
drivers/gemaltomodem/gemaltomodem.c \
+ drivers/gemaltomodem/gemaltoutil.h \
+ drivers/gemaltomodem/gemaltoutil.c \
drivers/gemaltomodem/location-reporting.c \
- drivers/gemaltomodem/voicecall.c
+ drivers/gemaltomodem/voicecall.c \
+ drivers/gemaltomodem/lte.c
builtin_modules += xmm7modem
builtin_sources += drivers/atmodem/atutil.h \
diff --git a/drivers/gemaltomodem/gemaltomodem.c b/drivers/gemaltomodem/gemaltomodem.c
index 4818ac66..459c3583 100644
--- a/drivers/gemaltomodem/gemaltomodem.c
+++ b/drivers/gemaltomodem/gemaltomodem.c
@@ -37,12 +37,14 @@ static int gemaltomodem_init(void)
{
gemalto_location_reporting_init();
gemalto_voicecall_init();
+ gemalto_lte_init();
return 0;
}
static void gemaltomodem_exit(void)
{
+ gemalto_lte_exit();
gemalto_voicecall_exit();
gemalto_location_reporting_exit();
}
diff --git a/drivers/gemaltomodem/gemaltomodem.h b/drivers/gemaltomodem/gemaltomodem.h
index 27b1460e..9f285563 100644
--- a/drivers/gemaltomodem/gemaltomodem.h
+++ b/drivers/gemaltomodem/gemaltomodem.h
@@ -21,9 +21,13 @@
*/
#include <drivers/atmodem/atutil.h>
+#include "gemaltoutil.h"
extern void gemalto_location_reporting_init();
extern void gemalto_location_reporting_exit();
extern void gemalto_voicecall_init();
extern void gemalto_voicecall_exit();
+
+extern void gemalto_lte_init();
+extern void gemalto_lte_exit();
diff --git a/drivers/gemaltomodem/gemaltoutil.c b/drivers/gemaltomodem/gemaltoutil.c
new file mode 100644
index 00000000..d8c08e16
--- /dev/null
+++ b/drivers/gemaltomodem/gemaltoutil.c
@@ -0,0 +1,106 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+#include <gatchat.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
You shouldn't need this here... only if you pull in ofono/plugin.h directly.
+#include <ofono/log.h>
+#include <ofono/types.h>
+
+#include "ofono.h"
+#include "gemaltomodem.h"
+
+enum gemalto_auth_option {
+ GEMALTO_AUTH_DEFAULTS = 0x00,
+ GEMALTO_AUTH_USE_SGAUTH = 0x01,
+ GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
+ GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
+};
Using an enumeration to name flags feels awkward. I would #define these:
#define AUTH_F_USE_SGATH (1<<0)
#define AUTH_F_SWAP_CREDENTIALS (1<<1)
#define AUTH_F_ALWAYS_ALL_PARAMS (1<<2)
Abbreviating ALWAYS to ALW is awkward...
Furthermore, considering that these flags are supposed to be set in
udevng.c or the gemalto plugin, these flags really need to be defined
elsewhere... include/gemalto.h, perhaps?
+
+char *gemalto_get_auth_command(struct ofono_modem *modem, int cid,
+ enum ofono_gprs_auth_method auth_method,
+ const char *username, const char *password)
+{
+ int gemalto_auth = ofono_modem_get_integer(modem, "GemaltoAuthType");
Can we call these flags, or authflags, or something along those lines?
Would be much clear, especially since we already have auth_method and
auth_type to confuse with gemalto_auth.
+ int len;
+ int auth_type = at_get_auth_type(auth_method);
+ size_t buflen = 32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+ OFONO_GPRS_MAX_PASSWORD_LENGTH + 1;
+ char *buf = g_new(char, buflen);
+
+ /* for now. Later to consider modules where the LTE attach CID=3 */
+ if (cid==0)
+ cid=1;
+
+ if (gemalto_auth & GEMALTO_AUTH_USE_SGAUTH)
+ len = snprintf(buf, buflen, "AT^SGAUTH=%d,%d", cid, auth_type);
Instead of declaring the auth_type variable, just call the function
directly in the parameter list. Rename the function to
at_auth_type_from_method() (or similar) so that it's obvious what it does.
(This stems from the fact that I keep losing track of the meaning of the
similarly named variables... auth_type, auth_method, gemalto_auth).
+ else
+ len = snprintf(buf, buflen, "AT+CGAUTH=%d,%d", cid, auth_type);
+
+ buflen -= len;
+
+ switch(auth_method) {
+ case OFONO_GPRS_AUTH_METHOD_NONE:
+
+ if (gemalto_auth & GEMALTO_AUTH_ALW_ALL_PARAMS)
+ snprintf(buf+len, buflen, ",\"\",\"\"");
+
+ break;
+ case OFONO_GPRS_AUTH_METHOD_PAP:
+ case OFONO_GPRS_AUTH_METHOD_CHAP:
+
+ if (gemalto_auth & GEMALTO_AUTH_ORDER_PWD_USR)
+ snprintf(buf+len, buflen, ",\"%s\",\"%s\"",
+ password, username);
+ else
+ snprintf(buf+len, buflen, ",\"%s\",\"%s\"",
+ username, password);
+
+ break;
+ }
+
+ return buf;
+}
+
+char *gemalto_get_cgdcont_command(struct ofono_modem *modem, guint cid,
+ enum ofono_gprs_proto proto, const char *apn)
+{
+ /*
+ * For future extension: verify if the module supports automatic
+ * context provisioning and if so, also if there is a manual override
+ * This is an ofono_modem_get_integer property
+ */
+
+ /* for now. Later to consider modules where the LTE attach CID=3 */
+ if (cid==0)
+ cid=1;
I'm a bit confused why cid==0 means cid should be 1... is this because
the AT network-registration atom assumes CID == 0 for the default
context? This should probably be fixed at the source, in that case, in
atmodem/network-registration.c, with a vendor quirk...???
+
+ return at_get_cgdcont_command(cid, proto, apn);
+}
diff --git a/drivers/gemaltomodem/gemaltoutil.h b/drivers/gemaltomodem/gemaltoutil.h
new file mode 100644
index 00000000..d653f61d
--- /dev/null
+++ b/drivers/gemaltomodem/gemaltoutil.h
@@ -0,0 +1,31 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+struct ofono_modem;
I think you might as well just pull in modem.h here.
+
+char *gemalto_get_auth_command(struct ofono_modem *modem, int cid,
+ enum ofono_gprs_auth_method auth_method,
This enum isn't defined either.
+ const char *username, const char *password);
+
+
+char *gemalto_get_cgdcont_command(struct ofono_modem *modem, guint cid,
+ enum ofono_gprs_proto proto, const char *apn);
+
diff --git a/drivers/gemaltomodem/lte.c b/drivers/gemaltomodem/lte.c
new file mode 100644
index 00000000..1c72173f
--- /dev/null
+++ b/drivers/gemaltomodem/lte.c
@@ -0,0 +1,221 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2017 Intel Corporation. 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <ofono/modem.h>
+#include <ofono/gprs-context.h>
+#include <ofono/log.h>
+#include <ofono/lte.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "gemaltomodem.h"
+
+struct gemalto_lte_driver_data {
+ GAtChat *chat;
+};
+
+struct gemalto_lte_cbd {
+ gint ref_count; /* Ref count */
+ ofono_lte_cb_t cb;
+ void *data;
+ GAtChat *chat;
+ const struct ofono_lte_default_attach_info *info;
+ struct ofono_modem *modem;
+};
+
+static struct gemalto_lte_cbd *gemalto_lte_cb_data_new0(void *cb, void *data,
+ GAtChat *chat, const struct ofono_lte_default_attach_info *info,
+ struct ofono_modem *modem)
+{
+ struct gemalto_lte_cbd *cbd = g_new0(struct gemalto_lte_cbd, 1);
+
+ cbd->ref_count = 1;
+ cbd->cb = cb;
+ cbd->data = data;
+ cbd->chat = chat;
+ cbd->info = info;
+ cbd->modem = modem;
+
+ return cbd;
+}
+
+static inline struct gemalto_lte_cbd *gemalto_lte_cb_data_ref(
+ struct gemalto_lte_cbd *cbd)
+{
+ if (cbd == NULL)
+ return NULL;
+
+ g_atomic_int_inc(&cbd->ref_count);
+
+ return cbd;
+}
+
+static void gemalto_lte_cb_data_unref(gpointer user_data)
+{
+ gboolean is_zero;
+ struct gemalto_lte_cbd *cbd = user_data;
+
+ if (cbd == NULL)
+ return;
+
+ is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
+
+ if (is_zero == TRUE)
+ g_free(cbd);
+}
+
+static void gemalto_lte_set_auth_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct gemalto_lte_cbd *cbd = user_data;
+ ofono_lte_cb_t cb = cbd->cb;
+ struct ofono_error error;
+
+ decode_at_error(&error, g_at_result_final_response(result));
+ cb(&error, cbd->data);
+}
+
+static void gemalto_lte_set_default_attach_info_cb(gboolean ok,
+ GAtResult *result, gpointer user_data)
+{
+ struct gemalto_lte_cbd *cbd = user_data;
+ ofono_lte_cb_t cb = cbd->cb;
+ void *data = cbd->data;
+ struct ofono_error error;
+ char *buf;
+ enum ofono_gprs_auth_method auth_method;
+
+ if (!ok) {
+ gemalto_lte_cb_data_unref(cbd);
+ decode_at_error(&error, g_at_result_final_response(result));
+ cb(&error, data);
+ return;
+ }
+
+ auth_method = cbd->info->auth_method;
+
+ /* change the authentication method if the parameters are invalid */
+ if (!*cbd->info->username || !*cbd->info->password)
+ auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+ buf = gemalto_get_auth_command(cbd->modem, 0, auth_method,
+ cbd->info->username, cbd->info->password);
+ cbd = gemalto_lte_cb_data_ref(cbd);
+
+ if (g_at_chat_send(cbd->chat, buf, NULL,
+ gemalto_lte_set_auth_cb, cbd,
+ gemalto_lte_cb_data_unref) > 0)
+ goto end;
+
+ gemalto_lte_cb_data_unref(cbd);
+ CALLBACK_WITH_FAILURE(cb, data);
+end:
+ g_free(buf);
+}
+
+static void gemalto_lte_set_default_attach_info(const struct ofono_lte *lte,
+ const struct ofono_lte_default_attach_info *info,
+ ofono_lte_cb_t cb, void *data)
+{
+ struct gemalto_lte_driver_data *ldd = ofono_lte_get_data(lte);
+ struct ofono_modem *modem = ofono_lte_get_modem(lte);
+ struct gemalto_lte_cbd *cbd = gemalto_lte_cb_data_new0(cb, data,
+ ldd->chat, info, modem);
+ char *buf = gemalto_get_cgdcont_command(modem, 0, info->proto,
+ info->apn);
+
+ if (g_at_chat_send(ldd->chat, buf, NULL,
+ gemalto_lte_set_default_attach_info_cb,
+ cbd, gemalto_lte_cb_data_unref) > 0)
+ goto end;
+
+ gemalto_lte_cb_data_unref(cbd);
+ CALLBACK_WITH_FAILURE(cb, data);
+end:
+ g_free(buf);
+}
+
+static gboolean gemalto_lte_delayed_register(gpointer user_data)
+{
+ struct ofono_lte *lte = user_data;
+
+ ofono_lte_register(lte);
+
+ return FALSE;
+}
+
+static int gemalto_lte_probe(struct ofono_lte *lte, unsigned int vendor,
+ void *data)
+{
+ GAtChat *chat = data;
+ struct gemalto_lte_driver_data *ldd;
+
+ ldd = g_new0(struct gemalto_lte_driver_data, 1);
+
+ ldd->chat = g_at_chat_clone(chat);
+
+ ofono_lte_set_data(lte, ldd);
+
+ g_idle_add(gemalto_lte_delayed_register, lte);
Why do you need to delay the registration?
+
+ return 0;
+}
+
+static void gemalto_lte_remove(struct ofono_lte *lte)
+{
+ struct gemalto_lte_driver_data *ldd = ofono_lte_get_data(lte);
+
+ g_at_chat_unref(ldd->chat);
+
+ ofono_lte_set_data(lte, NULL);
+
+ g_free(ldd);
+}
+
+static const struct ofono_lte_driver driver = {
+ .name = "gemaltomodem",
+ .probe = gemalto_lte_probe,
+ .remove = gemalto_lte_remove,
+ .set_default_attach_info = gemalto_lte_set_default_attach_info,
+};
+
+void gemalto_lte_init(void)
+{
+ ofono_lte_driver_register(&driver);
+}
+
+void gemalto_lte_exit(void)
+{
+ ofono_lte_driver_unregister(&driver);
+}
/Jonas
9:59 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
On 18/10/18 22:32, Jonas Bonn wrote:
...
> +#include <ofono/log.h>
> +#include <ofono/types.h>
> +
> +#include "ofono.h"
> +#include "gemaltomodem.h"
> +
> +enum gemalto_auth_option {
> + GEMALTO_AUTH_DEFAULTS = 0x00,
> + GEMALTO_AUTH_USE_SGAUTH = 0x01,
> + GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
> + GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
> +};
Using an enumeration to name flags feels awkward. I would #define these:
#define AUTH_F_USE_SGATH (1<<0)
#define AUTH_F_SWAP_CREDENTIALS (1<<1)
#define AUTH_F_ALWAYS_ALL_PARAMS (1<<2)
It may feel awkward but I actually find that (enum'ing flags) quite
useful for debugging with gdb - it allows the debugger to refer to those
flags by name.
Cheers,
-Slava
2:21 p.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Jonas,
thank you for taking the time to go through the code.
I haven't seen any comments on the first 5 patches. Is it possible to
apply them?
On Thu, Oct 18, 2018 at 9:32 PM Jonas Bonn <jonas(a)norrbonn.se> wrote:
Hi Giacinto,
This looks pretty good to me... Some comments below.
On 18/10/18 20:49, Giacinto Cifelli wrote:
> Added a vendor-specific lte atom, for handling PDP context creation
> and authentication, using the actual formats for the vendor.
>
> The code for PDP context and authentication command formating
> is in a separate file, gemaltoutil, because in the future it can be
> shared with the gprs-context atom.
>
> Co-authored-by: Martin Baschin <martin.baschin(a)googlemail.com>
> Co-authored-by: Sebastian Arnd <sebastianarnd(a)gmail.com>
> ---
> Makefile.am | 5 +-
> drivers/gemaltomodem/gemaltomodem.c | 2 +
> drivers/gemaltomodem/gemaltomodem.h | 4 +
> drivers/gemaltomodem/gemaltoutil.c | 106 +++++++++++++
> drivers/gemaltomodem/gemaltoutil.h | 31 ++++
> drivers/gemaltomodem/lte.c | 221 ++++++++++++++++++++++++++++
> 6 files changed, 368 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gemaltomodem/gemaltoutil.c
> create mode 100644 drivers/gemaltomodem/gemaltoutil.h
> create mode 100644 drivers/gemaltomodem/lte.c
>
> diff --git a/Makefile.am b/Makefile.am
> index e8e4ed95..dd0bc0bb 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -397,8 +397,11 @@ builtin_modules += gemaltomodem
> builtin_sources += drivers/atmodem/atutil.h \
> drivers/gemaltomodem/gemaltomodem.h \
> drivers/gemaltomodem/gemaltomodem.c \
> + drivers/gemaltomodem/gemaltoutil.h \
> + drivers/gemaltomodem/gemaltoutil.c \
> drivers/gemaltomodem/location-reporting.c \
> - drivers/gemaltomodem/voicecall.c
> + drivers/gemaltomodem/voicecall.c \
> + drivers/gemaltomodem/lte.c
>
> builtin_modules += xmm7modem
> builtin_sources += drivers/atmodem/atutil.h \
> diff --git a/drivers/gemaltomodem/gemaltomodem.c
b/drivers/gemaltomodem/gemaltomodem.c
> index 4818ac66..459c3583 100644
> --- a/drivers/gemaltomodem/gemaltomodem.c
> +++ b/drivers/gemaltomodem/gemaltomodem.c
> @@ -37,12 +37,14 @@ static int gemaltomodem_init(void)
> {
> gemalto_location_reporting_init();
> gemalto_voicecall_init();
> + gemalto_lte_init();
>
> return 0;
> }
>
> static void gemaltomodem_exit(void)
> {
> + gemalto_lte_exit();
> gemalto_voicecall_exit();
> gemalto_location_reporting_exit();
> }
> diff --git a/drivers/gemaltomodem/gemaltomodem.h
b/drivers/gemaltomodem/gemaltomodem.h
> index 27b1460e..9f285563 100644
> --- a/drivers/gemaltomodem/gemaltomodem.h
> +++ b/drivers/gemaltomodem/gemaltomodem.h
> @@ -21,9 +21,13 @@
> */
>
> #include <drivers/atmodem/atutil.h>
> +#include "gemaltoutil.h"
>
> extern void gemalto_location_reporting_init();
> extern void gemalto_location_reporting_exit();
>
> extern void gemalto_voicecall_init();
> extern void gemalto_voicecall_exit();
> +
> +extern void gemalto_lte_init();
> +extern void gemalto_lte_exit();
> diff --git a/drivers/gemaltomodem/gemaltoutil.c
b/drivers/gemaltomodem/gemaltoutil.c
> new file mode 100644
> index 00000000..d8c08e16
> --- /dev/null
> +++ b/drivers/gemaltomodem/gemaltoutil.c
> @@ -0,0 +1,106 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <glib.h>
> +#include <gatchat.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +
> +#define OFONO_API_SUBJECT_TO_CHANGE
You shouldn't need this here... only if you pull in ofono/plugin.h directly.
> +#include <ofono/log.h>
> +#include <ofono/types.h>
> +
> +#include "ofono.h"
> +#include "gemaltomodem.h"
> +
> +enum gemalto_auth_option {
> + GEMALTO_AUTH_DEFAULTS = 0x00,
> + GEMALTO_AUTH_USE_SGAUTH = 0x01,
> + GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
> + GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
> +};
Using an enumeration to name flags feels awkward. I would #define these:
#define AUTH_F_USE_SGATH (1<<0)
#define AUTH_F_SWAP_CREDENTIALS (1<<1)
#define AUTH_F_ALWAYS_ALL_PARAMS (1<<2)
I prefer enum because I have several groups of flags, so it is clearer
which values a variable can take.
And also the advantage of gdb mentioned by Slava.
Abbreviating ALWAYS to ALW is awkward...
ALW is a standard abbreviation. the 3GPP 31.102 is full of it.
But this I more willing to change, along with the shift of bits
instead of the hexa mapping.
Furthermore, considering that these flags are supposed to be set in
udevng.c or the gemalto plugin, these flags really need to be defined
elsewhere... include/gemalto.h, perhaps?
> +
> +char *gemalto_get_auth_command(struct ofono_modem *modem, int cid,
> + enum ofono_gprs_auth_method auth_method,
> + const char *username, const char *password)
> +{
> + int gemalto_auth = ofono_modem_get_integer(modem,
"GemaltoAuthType");
Can we call these flags, or authflags, or something along those lines?
Would be much clear, especially since we already have auth_method and
auth_type to confuse with gemalto_auth.
ok, I will call them GemaltoAuthFlags.
> + int len;
> + int auth_type = at_get_auth_type(auth_method);
> + size_t buflen = 32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
> + OFONO_GPRS_MAX_PASSWORD_LENGTH + 1;
> + char *buf = g_new(char, buflen);
> +
> + /* for now. Later to consider modules where the LTE attach CID=3 */
> + if (cid==0)
> + cid=1;
> +
> + if (gemalto_auth & GEMALTO_AUTH_USE_SGAUTH)
> + len = snprintf(buf, buflen, "AT^SGAUTH=%d,%d", cid,
auth_type);
Instead of declaring the auth_type variable, just call the function
directly in the parameter list. Rename the function to
at_auth_type_from_method() (or similar) so that it's obvious what it does.
(This stems from the fact that I keep losing track of the meaning of the
similarly named variables... auth_type, auth_method, gemalto_auth).
ok, good idea, this variable is only used once.
> + else
> + len = snprintf(buf, buflen, "AT+CGAUTH=%d,%d", cid,
auth_type);
> +
> + buflen -= len;
> +
> + switch(auth_method) {
> + case OFONO_GPRS_AUTH_METHOD_NONE:
> +
> + if (gemalto_auth & GEMALTO_AUTH_ALW_ALL_PARAMS)
> + snprintf(buf+len, buflen,
",\"\",\"\"");
> +
> + break;
> + case OFONO_GPRS_AUTH_METHOD_PAP:
> + case OFONO_GPRS_AUTH_METHOD_CHAP:
> +
> + if (gemalto_auth & GEMALTO_AUTH_ORDER_PWD_USR)
> + snprintf(buf+len, buflen,
",\"%s\",\"%s\"",
> + password, username);
> + else
> + snprintf(buf+len, buflen,
",\"%s\",\"%s\"",
> + username, password);
> +
> + break;
> + }
> +
> + return buf;
> +}
> +
> +char *gemalto_get_cgdcont_command(struct ofono_modem *modem, guint cid,
> + enum ofono_gprs_proto proto, const char *apn)
> +{
> + /*
> + * For future extension: verify if the module supports automatic
> + * context provisioning and if so, also if there is a manual override
> + * This is an ofono_modem_get_integer property
> + */
> +
> + /* for now. Later to consider modules where the LTE attach CID=3 */
> + if (cid==0)
> + cid=1;
I'm a bit confused why cid==0 means cid should be 1... is this because
the AT network-registration atom assumes CID == 0 for the default
context? This should probably be fixed at the source, in that case, in
atmodem/network-registration.c, with a vendor quirk...???
the 3GPP 27.007 says that the LTE attach context ID is 0, but actual
modems use other values.
For now there are around only models which use CID=1, but Gemalto also
has models with CID=3.
I will have to set another variable for this.
> +
> + return at_get_cgdcont_command(cid, proto, apn);
> +}
> diff --git a/drivers/gemaltomodem/gemaltoutil.h
b/drivers/gemaltomodem/gemaltoutil.h
> new file mode 100644
> index 00000000..d653f61d
> --- /dev/null
> +++ b/drivers/gemaltomodem/gemaltoutil.h
> @@ -0,0 +1,31 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +struct ofono_modem;
I think you might as well just pull in modem.h here.
> +
> +char *gemalto_get_auth_command(struct ofono_modem *modem, int cid,
> + enum ofono_gprs_auth_method auth_method,
This enum isn't defined either.
That's strange. I might have included the relevant headers in the c file.
I'll look into it.
> + const char *username, const char *password);
> +
> +
> +char *gemalto_get_cgdcont_command(struct ofono_modem *modem, guint cid,
> + enum ofono_gprs_proto proto, const char *apn);
> +
> diff --git a/drivers/gemaltomodem/lte.c b/drivers/gemaltomodem/lte.c
> new file mode 100644
> index 00000000..1c72173f
> --- /dev/null
> +++ b/drivers/gemaltomodem/lte.c
> @@ -0,0 +1,221 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2017 Intel Corporation. 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +
> +#include <ofono/modem.h>
> +#include <ofono/gprs-context.h>
> +#include <ofono/log.h>
> +#include <ofono/lte.h>
> +
> +#include "gatchat.h"
> +#include "gatresult.h"
> +
> +#include "gemaltomodem.h"
> +
> +struct gemalto_lte_driver_data {
> + GAtChat *chat;
> +};
> +
> +struct gemalto_lte_cbd {
> + gint ref_count; /* Ref count */
> + ofono_lte_cb_t cb;
> + void *data;
> + GAtChat *chat;
> + const struct ofono_lte_default_attach_info *info;
> + struct ofono_modem *modem;
> +};
> +
> +static struct gemalto_lte_cbd *gemalto_lte_cb_data_new0(void *cb, void *data,
> + GAtChat *chat, const struct ofono_lte_default_attach_info *info,
> + struct ofono_modem *modem)
> +{
> + struct gemalto_lte_cbd *cbd = g_new0(struct gemalto_lte_cbd, 1);
> +
> + cbd->ref_count = 1;
> + cbd->cb = cb;
> + cbd->data = data;
> + cbd->chat = chat;
> + cbd->info = info;
> + cbd->modem = modem;
> +
> + return cbd;
> +}
> +
> +static inline struct gemalto_lte_cbd *gemalto_lte_cb_data_ref(
> + struct gemalto_lte_cbd *cbd)
> +{
> + if (cbd == NULL)
> + return NULL;
> +
> + g_atomic_int_inc(&cbd->ref_count);
> +
> + return cbd;
> +}
> +
> +static void gemalto_lte_cb_data_unref(gpointer user_data)
> +{
> + gboolean is_zero;
> + struct gemalto_lte_cbd *cbd = user_data;
> +
> + if (cbd == NULL)
> + return;
> +
> + is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
> +
> + if (is_zero == TRUE)
> + g_free(cbd);
> +}
> +
> +static void gemalto_lte_set_auth_cb(gboolean ok, GAtResult *result,
> + gpointer user_data)
> +{
> + struct gemalto_lte_cbd *cbd = user_data;
> + ofono_lte_cb_t cb = cbd->cb;
> + struct ofono_error error;
> +
> + decode_at_error(&error, g_at_result_final_response(result));
> + cb(&error, cbd->data);
> +}
> +
> +static void gemalto_lte_set_default_attach_info_cb(gboolean ok,
> + GAtResult *result, gpointer user_data)
> +{
> + struct gemalto_lte_cbd *cbd = user_data;
> + ofono_lte_cb_t cb = cbd->cb;
> + void *data = cbd->data;
> + struct ofono_error error;
> + char *buf;
> + enum ofono_gprs_auth_method auth_method;
> +
> + if (!ok) {
> + gemalto_lte_cb_data_unref(cbd);
> + decode_at_error(&error, g_at_result_final_response(result));
> + cb(&error, data);
> + return;
> + }
> +
> + auth_method = cbd->info->auth_method;
> +
> + /* change the authentication method if the parameters are invalid */
> + if (!*cbd->info->username || !*cbd->info->password)
> + auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
> +
> + buf = gemalto_get_auth_command(cbd->modem, 0, auth_method,
> + cbd->info->username, cbd->info->password);
> + cbd = gemalto_lte_cb_data_ref(cbd);
> +
> + if (g_at_chat_send(cbd->chat, buf, NULL,
> + gemalto_lte_set_auth_cb, cbd,
> + gemalto_lte_cb_data_unref) > 0)
> + goto end;
> +
> + gemalto_lte_cb_data_unref(cbd);
> + CALLBACK_WITH_FAILURE(cb, data);
> +end:
> + g_free(buf);
> +}
> +
> +static void gemalto_lte_set_default_attach_info(const struct ofono_lte *lte,
> + const struct ofono_lte_default_attach_info *info,
> + ofono_lte_cb_t cb, void *data)
> +{
> + struct gemalto_lte_driver_data *ldd = ofono_lte_get_data(lte);
> + struct ofono_modem *modem = ofono_lte_get_modem(lte);
> + struct gemalto_lte_cbd *cbd = gemalto_lte_cb_data_new0(cb, data,
> + ldd->chat, info, modem);
> + char *buf = gemalto_get_cgdcont_command(modem, 0, info->proto,
> + info->apn);
> +
> + if (g_at_chat_send(ldd->chat, buf, NULL,
> + gemalto_lte_set_default_attach_info_cb,
> + cbd, gemalto_lte_cb_data_unref) > 0)
> + goto end;
> +
> + gemalto_lte_cb_data_unref(cbd);
> + CALLBACK_WITH_FAILURE(cb, data);
> +end:
> + g_free(buf);
> +}
> +
> +static gboolean gemalto_lte_delayed_register(gpointer user_data)
> +{
> + struct ofono_lte *lte = user_data;
> +
> + ofono_lte_register(lte);
> +
> + return FALSE;
> +}
> +
> +static int gemalto_lte_probe(struct ofono_lte *lte, unsigned int vendor,
> + void *data)
> +{
> + GAtChat *chat = data;
> + struct gemalto_lte_driver_data *ldd;
> +
> + ldd = g_new0(struct gemalto_lte_driver_data, 1);
> +
> + ldd->chat = g_at_chat_clone(chat);
> +
> + ofono_lte_set_data(lte, ldd);
> +
> + g_idle_add(gemalto_lte_delayed_register, lte);
Why do you need to delay the registration?
> +
> + return 0;
> +}
> +
> +static void gemalto_lte_remove(struct ofono_lte *lte)
> +{
> + struct gemalto_lte_driver_data *ldd = ofono_lte_get_data(lte);
> +
> + g_at_chat_unref(ldd->chat);
> +
> + ofono_lte_set_data(lte, NULL);
> +
> + g_free(ldd);
> +}
> +
> +static const struct ofono_lte_driver driver = {
> + .name = "gemaltomodem",
> + .probe = gemalto_lte_probe,
> + .remove = gemalto_lte_remove,
> + .set_default_attach_info = gemalto_lte_set_default_attach_info,
> +};
> +
> +void gemalto_lte_init(void)
> +{
> + ofono_lte_driver_register(&driver);
> +}
> +
> +void gemalto_lte_exit(void)
> +{
> + ofono_lte_driver_unregister(&driver);
> +}
>
/Jonas
Giacinto
5:04 p.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
On 19/10/18 05:21, Giacinto Cifelli wrote:
Hi Jonas,
thank you for taking the time to go through the code.
I haven't seen any comments on the first 5 patches. Is it possible to
apply them?
On the whole, things look pretty good. Denis applies the patches; I can
only expedite the process a bit by providing some review while he
sleeps... ;)
/Jonas
3:18 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Giacinto,
>> +char *gemalto_get_cgdcont_command(struct ofono_modem *modem,
guint cid,
>> + enum ofono_gprs_proto proto, const char *apn)
>> +{
>> + /*
>> + * For future extension: verify if the module supports automatic
>> + * context provisioning and if so, also if there is a manual override
>> + * This is an ofono_modem_get_integer property
>> + */
>> +
>> + /* for now. Later to consider modules where the LTE attach CID=3 */
>> + if (cid==0)
>> + cid=1;
>
> I'm a bit confused why cid==0 means cid should be 1... is this because
> the AT network-registration atom assumes CID == 0 for the default
> context? This should probably be fixed at the source, in that case, in
> atmodem/network-registration.c, with a vendor quirk...???
the 3GPP 27.007 says that the LTE attach context ID is 0, but actual
modems use other values.
For now there are around only models which use CID=1, but Gemalto also
has models with CID=3.
I will have to set another variable for this.
I still don't get this explanation. You have a gemalto specific
function, which is being passed the CID directly by the caller. Why is
there weird logic here to mess with the CID, just have the caller pass
in the proper CID directly.
Also note that cid 1 and 3 are by default valid CIDs for context
activations. So your 'default attach' profile can be overridden by the
core at any time. So you may want to address this by setting the cid
range appropriately.
Regards,
-Denis
3:24 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Denis,
I still don't get this explanation. You have a gemalto specific
function, which is being passed the CID directly by the caller. Why is
there weird logic here to mess with the CID, just have the caller pass
in the proper CID directly.
Also note that cid 1 and 3 are by default valid CIDs for context
activations. So your 'default attach' profile can be overridden by the
core at any time. So you may want to address this by setting the cid
range appropriately.
The function is to be called also from gprs-context, therefore passing
0 it knows it has to check whether it has to use 1 or 3 or something
else.
The cid-ranges are set in the plugin, also because I cannot use
AT+CGDCONT=? with all models.
There are some that return an answer to this command like (1-16),
which is ok, but then we have some that return (1,2,3,4,5,6), and
still other (1-10,17)...
Regards,
-Denis
Regards,
Giacinto
3:27 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Giacinto,
On 10/20/2018 11:24 AM, Giacinto Cifelli wrote:
Hi Denis,
>
> I still don't get this explanation. You have a gemalto specific
> function, which is being passed the CID directly by the caller. Why is
> there weird logic here to mess with the CID, just have the caller pass
> in the proper CID directly.
>
> Also note that cid 1 and 3 are by default valid CIDs for context
> activations. So your 'default attach' profile can be overridden by the
> core at any time. So you may want to address this by setting the cid
> range appropriately.
The function is to be called also from gprs-context, therefore passing
0 it knows it has to check whether it has to use 1 or 3 or something
else.
Sure, but fundamentally it is still gemalto specific, so why would you
call it with cid=0 anyway?
The cid-ranges are set in the plugin, also because I cannot use
AT+CGDCONT=? with all models.
There are some that return an answer to this command like (1-16),
which is ok, but then we have some that return (1,2,3,4,5,6), and
still other (1-10,17)...
That is all fine, but the logic to determine the cid belongs outside the
utility function.
Regards,
-Denis
3:29 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Denis,
On Sat, Oct 20, 2018 at 6:27 PM Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Giacinto,
On 10/20/2018 11:24 AM, Giacinto Cifelli wrote:
> Hi Denis,
>>
>> I still don't get this explanation. You have a gemalto specific
>> function, which is being passed the CID directly by the caller. Why is
>> there weird logic here to mess with the CID, just have the caller pass
>> in the proper CID directly.
>>
>> Also note that cid 1 and 3 are by default valid CIDs for context
>> activations. So your 'default attach' profile can be overridden by the
>> core at any time. So you may want to address this by setting the cid
>> range appropriately.
>
> The function is to be called also from gprs-context, therefore passing
> 0 it knows it has to check whether it has to use 1 or 3 or something
> else.
Sure, but fundamentally it is still gemalto specific, so why would you
call it with cid=0 anyway?
> The cid-ranges are set in the plugin, also because I cannot use
> AT+CGDCONT=? with all models.
> There are some that return an answer to this command like (1-16),
> which is ok, but then we have some that return (1,2,3,4,5,6), and
> still other (1-10,17)...
That is all fine, but the logic to determine the cid belongs outside the
utility function.
ok.
Regards,
-Denis
Regards,
Giacinto
3:13 a.m.
New subject: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom
Hi Jonas,
Using an enumeration to name flags feels awkward. I would #define
these:
#define AUTH_F_USE_SGATH (1<<0)
#define AUTH_F_SWAP_CREDENTIALS (1<<1)
#define AUTH_F_ALWAYS_ALL_PARAMS (1<<2)
Actually we prefer enums for enumerations with multiple related values
or ones that can be extended. It is fine to use a #define if you have a
single random value, but if you have a series of related flags, just use
an enum. This also forces one to keep naming consistent per item M11 of
doc/coding-style.txt.
The compiler can also treat enums as first class citizens, making them
available in the debugger, etc.
Abbreviating ALWAYS to ALW is awkward...
+1
> +
> +struct ofono_modem;
I think you might as well just pull in modem.h here.
This is bad advice. If the header file is only using a pointer to a
structure, a forward declaration is enough. Including the entire
modem.h file here would just be extra work for the preprocessor and
compiler, nothing more. I prefer my compilation times to be fast ;)
> +static int gemalto_lte_probe(struct ofono_lte *lte, unsigned int
vendor,
> + void *data)
> +{
> + GAtChat *chat = data;
> + struct gemalto_lte_driver_data *ldd;
> +
> + ldd = g_new0(struct gemalto_lte_driver_data, 1);
> +
> + ldd->chat = g_at_chat_clone(chat);
> +
> + ofono_lte_set_data(lte, ldd);
> +
> + g_idle_add(gemalto_lte_delayed_register, lte);
Why do you need to delay the registration?
It is not allowed to call register from within the probe method. That
is because register might trigger driver method calls, but until probe()
returns the driver isn't set. Some atoms do not do this (yet) and you
can get away with it, but it is bad practice in general.
Regards,
-Denis
5:49 a.m.
New subject: [PATCH v7 7/7] plugins/gemalto: use Gemalto lte atom
Switch from atmodem to gemaltomodem for the lte atom, in order to
benefit from the correct syntax for the AT commands.
The current models included in the Gemalto plugin only use one type
of authentication syntax, despite the atom supporting the syntax for
all Gemalto modules.
Therefore for now the variable GemaltoAuthType is set to a fixed value,
independent on the model.
---
plugins/gemalto.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 5d3c77a9..3ca9f2d6 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -63,6 +63,13 @@ static const char *none_prefix[] = { NULL };
static const char *sctm_prefix[] = { "^SCTM:", NULL };
static const char *sbv_prefix[] = { "^SBV:", NULL };
+enum auth_option {
+ GEMALTO_AUTH_DEFAULTS = 0x00,
+ GEMALTO_AUTH_USE_SGAUTH = 0x01,
+ GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
+ GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
+};
+
struct gemalto_hardware_monitor {
DBusMessage *msg;
int32_t temperature;
@@ -609,9 +616,12 @@ static void gemalto_post_sim(struct ofono_modem *modem)
ofono_call_meter_create(modem, 0, "atmodem", data->app);
ofono_call_barring_create(modem, 0, "atmodem", data->app);
- if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x))
- ofono_lte_create(modem, OFONO_VENDOR_CINTERION,
- "atmodem", data->app);
+ if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x)) {
+ ofono_modem_set_integer(modem, "GemaltoAuthType",
+ GEMALTO_AUTH_USE_SGAUTH |
+ GEMALTO_AUTH_ORDER_PWD_USR);
+ ofono_lte_create(modem, 0, "gemaltomodem", data->app);
+ }
}
static void gemalto_post_online(struct ofono_modem *modem)
--
2.17.1
6:41 a.m.
New subject: [PATCH v7 7/7] plugins/gemalto: use Gemalto lte atom
Hi Giacinto,
On 18/10/18 20:49, Giacinto Cifelli wrote:
Switch from atmodem to gemaltomodem for the lte atom, in order to
benefit from the correct syntax for the AT commands.
The current models included in the Gemalto plugin only use one type
of authentication syntax, despite the atom supporting the syntax for
all Gemalto modules.
Therefore for now the variable GemaltoAuthType is set to a fixed value,
independent on the model.
---
plugins/gemalto.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 5d3c77a9..3ca9f2d6 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -63,6 +63,13 @@ static const char *none_prefix[] = { NULL };
static const char *sctm_prefix[] = { "^SCTM:", NULL };
static const char *sbv_prefix[] = { "^SBV:", NULL };
+enum auth_option {
+ GEMALTO_AUTH_DEFAULTS = 0x00,
+ GEMALTO_AUTH_USE_SGAUTH = 0x01,
+ GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
+ GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
+};
+
struct gemalto_hardware_monitor {
DBusMessage *msg;
int32_t temperature;
@@ -609,9 +616,12 @@ static void gemalto_post_sim(struct ofono_modem *modem)
ofono_call_meter_create(modem, 0, "atmodem", data->app);
ofono_call_barring_create(modem, 0, "atmodem", data->app);
- if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x))
- ofono_lte_create(modem, OFONO_VENDOR_CINTERION,
- "atmodem", data->app);
+ if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x)) {
+ ofono_modem_set_integer(modem, "GemaltoAuthType",
+ GEMALTO_AUTH_USE_SGAUTH |
+ GEMALTO_AUTH_ORDER_PWD_USR);
+ ofono_lte_create(modem, 0, "gemaltomodem", data->app);
Why not call ofono_modem_get_string(modem, "Model") directly in the LTE
atom in order to get the flags?
/Jonas
+ }
}
static void gemalto_post_online(struct ofono_modem *modem)
2:09 p.m.
New subject: [PATCH v7 7/7] plugins/gemalto: use Gemalto lte atom
Hi Jonas,
On Thu, Oct 18, 2018 at 9:41 PM Jonas Bonn <jonas(a)norrbonn.se> wrote:
Hi Giacinto,
On 18/10/18 20:49, Giacinto Cifelli wrote:
> Switch from atmodem to gemaltomodem for the lte atom, in order to
> benefit from the correct syntax for the AT commands.
> The current models included in the Gemalto plugin only use one type
> of authentication syntax, despite the atom supporting the syntax for
> all Gemalto modules.
> Therefore for now the variable GemaltoAuthType is set to a fixed value,
> independent on the model.
> ---
> plugins/gemalto.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
> index 5d3c77a9..3ca9f2d6 100644
> --- a/plugins/gemalto.c
> +++ b/plugins/gemalto.c
> @@ -63,6 +63,13 @@ static const char *none_prefix[] = { NULL };
> static const char *sctm_prefix[] = { "^SCTM:", NULL };
> static const char *sbv_prefix[] = { "^SBV:", NULL };
>
> +enum auth_option {
> + GEMALTO_AUTH_DEFAULTS = 0x00,
> + GEMALTO_AUTH_USE_SGAUTH = 0x01,
> + GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
> + GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
> +};
> +
> struct gemalto_hardware_monitor {
> DBusMessage *msg;
> int32_t temperature;
> @@ -609,9 +616,12 @@ static void gemalto_post_sim(struct ofono_modem *modem)
> ofono_call_meter_create(modem, 0, "atmodem", data->app);
> ofono_call_barring_create(modem, 0, "atmodem", data->app);
>
> - if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x))
> - ofono_lte_create(modem, OFONO_VENDOR_CINTERION,
> - "atmodem", data->app);
> + if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x)) {
> + ofono_modem_set_integer(modem, "GemaltoAuthType",
> + GEMALTO_AUTH_USE_SGAUTH |
> + GEMALTO_AUTH_ORDER_PWD_USR);
> + ofono_lte_create(modem, 0, "gemaltomodem", data->app);
Why not call ofono_modem_get_string(modem, "Model") directly in the LTE
atom in order to get the flags?
Because it is not the atom's job.
There are other flags planned, and not all will be determined by model.
Beside, remembering to update each atom for each new modem is error-prone.
/Jonas
Regards,
Giacinto
4:57 p.m.
New subject: [PATCH v7 7/7] plugins/gemalto: use Gemalto lte atom
On 19/10/18 05:09, Giacinto Cifelli wrote:
Hi Jonas,
On Thu, Oct 18, 2018 at 9:41 PM Jonas Bonn <jonas(a)norrbonn.se> wrote:
>
> Hi Giacinto,
>
> On 18/10/18 20:49, Giacinto Cifelli wrote:
>> Switch from atmodem to gemaltomodem for the lte atom, in order to
>> benefit from the correct syntax for the AT commands.
>> The current models included in the Gemalto plugin only use one type
>> of authentication syntax, despite the atom supporting the syntax for
>> all Gemalto modules.
>> Therefore for now the variable GemaltoAuthType is set to a fixed value,
>> independent on the model.
>> ---
>> plugins/gemalto.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
>> index 5d3c77a9..3ca9f2d6 100644
>> --- a/plugins/gemalto.c
>> +++ b/plugins/gemalto.c
>> @@ -63,6 +63,13 @@ static const char *none_prefix[] = { NULL };
>> static const char *sctm_prefix[] = { "^SCTM:", NULL };
>> static const char *sbv_prefix[] = { "^SBV:", NULL };
>>
>> +enum auth_option {
>> + GEMALTO_AUTH_DEFAULTS = 0x00,
>> + GEMALTO_AUTH_USE_SGAUTH = 0x01,
>> + GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
>> + GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
>> +};
>> +
>> struct gemalto_hardware_monitor {
>> DBusMessage *msg;
>> int32_t temperature;
>> @@ -609,9 +616,12 @@ static void gemalto_post_sim(struct ofono_modem *modem)
>> ofono_call_meter_create(modem, 0, "atmodem", data->app);
>> ofono_call_barring_create(modem, 0, "atmodem", data->app);
>>
>> - if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x))
>> - ofono_lte_create(modem, OFONO_VENDOR_CINTERION,
>> - "atmodem",
data->app);
>> + if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x)) {
>> + ofono_modem_set_integer(modem, "GemaltoAuthType",
>> + GEMALTO_AUTH_USE_SGAUTH |
>> + GEMALTO_AUTH_ORDER_PWD_USR);
>> + ofono_lte_create(modem, 0, "gemaltomodem",
data->app);
>
> Why not call ofono_modem_get_string(modem, "Model") directly in the LTE
> atom in order to get the flags?
Because it is not the atom's job.
There are other flags planned, and not all will be determined by model.
Beside, remembering to update each atom for each new modem is error-prone.
It's difficult to provide patch review when part of the job involves
reading the author's mind to know whither they are going... :)
As things currently stand, the flags are determined _only_ by model and
are accessed only by gemaltoutil.c. There's no reason to go from model
to flags at such a high level. Put a model_to_flags() function in
gemaltoutil and keep everything in one place.
/Jonas
5:04 p.m.
New subject: [PATCH v7 7/7] plugins/gemalto: use Gemalto lte atom
hi,
On Fri, 19 Oct 2018, 07:57 Jonas Bonn, <jonas(a)norrbonn.se> wrote:
On 19/10/18 05:09, Giacinto Cifelli wrote:
> Hi Jonas,
>
> On Thu, Oct 18, 2018 at 9:41 PM Jonas Bonn <jonas(a)norrbonn.se> wrote:
>>
>> Hi Giacinto,
>>
>> On 18/10/18 20:49, Giacinto Cifelli wrote:
>>> Switch from atmodem to gemaltomodem for the lte atom, in order to
>>> benefit from the correct syntax for the AT commands.
>>> The current models included in the Gemalto plugin only use one type
>>> of authentication syntax, despite the atom supporting the syntax for
>>> all Gemalto modules.
>>> Therefore for now the variable GemaltoAuthType is set to a fixed value,
>>> independent on the model.
>>> ---
>>> plugins/gemalto.c | 16 +++++++++++++---
>>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
>>> index 5d3c77a9..3ca9f2d6 100644
>>> --- a/plugins/gemalto.c
>>> +++ b/plugins/gemalto.c
>>> @@ -63,6 +63,13 @@ static const char *none_prefix[] = { NULL };
>>> static const char *sctm_prefix[] = { "^SCTM:", NULL };
>>> static const char *sbv_prefix[] = { "^SBV:", NULL };
>>>
>>> +enum auth_option {
>>> + GEMALTO_AUTH_DEFAULTS = 0x00,
>>> + GEMALTO_AUTH_USE_SGAUTH = 0x01,
>>> + GEMALTO_AUTH_ORDER_PWD_USR = 0x02,
>>> + GEMALTO_AUTH_ALW_ALL_PARAMS = 0x04,
>>> +};
>>> +
>>> struct gemalto_hardware_monitor {
>>> DBusMessage *msg;
>>> int32_t temperature;
>>> @@ -609,9 +616,12 @@ static void gemalto_post_sim(struct ofono_modem
*modem)
>>> ofono_call_meter_create(modem, 0, "atmodem",
data->app);
>>> ofono_call_barring_create(modem, 0, "atmodem",
data->app);
>>>
>>> - if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x))
>>> - ofono_lte_create(modem, OFONO_VENDOR_CINTERION,
>>> - "atmodem",
data->app);
>>> + if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x)) {
>>> + ofono_modem_set_integer(modem, "GemaltoAuthType",
>>> + GEMALTO_AUTH_USE_SGAUTH |
>>> +
GEMALTO_AUTH_ORDER_PWD_USR);
>>> + ofono_lte_create(modem, 0, "gemaltomodem",
data->app);
>>
>> Why not call ofono_modem_get_string(modem, "Model") directly in the
LTE
>> atom in order to get the flags?
>
> Because it is not the atom's job.
> There are other flags planned, and not all will be determined by model.
> Beside, remembering to update each atom for each new modem is
error-prone.
It's difficult to provide patch review when part of the job involves
reading the author's mind to know whither they are going... :)
As things currently stand, the flags are determined _only_ by model and
are accessed only by gemaltoutil.c. There's no reason to go from model
to flags at such a high level. Put a model_to_flags() function in
gemaltoutil and keep everything in one place.
I think I will remove these last two patches from the series for today.
I see if I can start submitting the Gemalto plugin over the weekend.
It is planned anyway, and even if several features depend on atom support
not yet there, I can at least give the new structure.
Then I will re-submit the lte gemalto afterwards.
/Jonas
Giacinto
1429
days inactive
1431
days old
20 comments
4 participants
participants (4)
-
Denis Kenzior -
Giacinto Cifelli -
Jonas Bonn -
Slava Monich