lte atom auth and IP protocol

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 <[email protected]>

---
  src/lte.c | 236 +++++++++++++++++++++++++++++++++++++++++-------------
  1 file changed, 180 insertions(+), 56 deletions(-)

src/lte.c +180 -56

index 23fe8e1c..c1f6d86a 100644
@@ -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, &lte->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,
+							&lte->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,28 @@ 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;
+	const char *key;
+	const char *propstr = NULL;
 
  	DBG("%s error %d", path, error->type);
 

@@ -118,55 +173,65 @@ 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);
+	if (!g_str_equal(lte->pending_info.apn, lte->info.apn)) {
+		g_strlcpy(lte->info.apn, lte->pending_info.apn,
+					OFONO_GPRS_MAX_APN_LENGTH + 1);
+		key = LTE_APN;
+		propstr = lte->info.apn;
+	} else if (lte->pending_info.proto != lte->info.proto) {
+		lte->info.proto = lte->pending_info.proto;
+		key = LTE_PROTO;
+		propstr = gprs_proto_to_string(lte->info.proto);
+	} else if (lte->pending_info.auth_method != lte->info.auth_method) {
+		lte->info.auth_method = lte->pending_info.auth_method;
+		key = LTE_AUTH_METHOD;
+		propstr = gprs_auth_method_to_string(lte->info.auth_method);
+
+	} else if (!g_str_equal(lte->pending_info.username,
+							lte->info.username)) {
+		g_strlcpy(lte->info.username, lte->pending_info.username,
+					OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+		key = LTE_USERNAME;
+		propstr = lte->info.username;
+	} else if (!g_str_equal(lte->pending_info.password,
+							lte->info.password)) {
+		g_strlcpy(lte->info.password, lte->pending_info.password,
+					OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+		key = LTE_PASSWORD;
+		propstr = lte->info.password;
+	} else {
+		/*
+		 * this should never happen, because no property change is
+		 * checked before.
+		 * Neverthelss, in this case it will answer the D-Bus message
+		 * but emit no signal
+		 */
+		ofono_error("unexpected property change: ignored");
+		key = NULL;
+	}
+
+	reply = dbus_message_new_method_return(lte->pending);
+	__ofono_dbus_pending_reply(&lte->pending, reply);
+
+	if(!key)
+		return;
 
  	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);
+		if (!*propstr)
+			/* 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, propstr);
 
  		storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
  	}
 
-	reply = dbus_message_new_method_return(lte->pending);
-	__ofono_dbus_pending_reply(&lte->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);
-
-	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, &lte->pending_info,
-					lte_set_default_attach_info_cb, lte);
-
-	return NULL;
+					key,
+					DBUS_TYPE_STRING, &propstr);
  }
 
  static DBusMessage *lte_set_property(DBusConnection *conn,

@@ -177,6 +242,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 +265,67 @@ 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);
-	}
+	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);
+
+		lte->pending_info.proto = proto;
 
-	return __ofono_error_invalid_args(msg);
+	} 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, &lte->pending_info,
+					lte_set_default_attach_info_cb, lte);
+
+	return NULL;
  }
 
  static const GDBusMethodTable lte_methods[] = {

Better subject line might be:

lte: add protocol and auth parameters to default context

Quoted 5 lines · Giacinto Cifelli

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

Don't just copy and paste the same description into every patch. Describe _this_ patch, not the series... (The way ofono does this, with one patch per subdirectory, is sub-optimal in this regard, I know).

Something like:

Many LTE networks require user authentication, even for the default context. 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.

/Jonas

Quoted 333 lines

Co-authored-by Martin Baschin <[email protected]>

---
  src/lte.c | 236 +++++++++++++++++++++++++++++++++++++++++-------------
  1 file changed, 180 insertions(+), 56 deletions(-)

src/lte.c +180 -56

index 23fe8e1c..c1f6d86a 100644
@@ -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, &lte->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,
+                                                     &lte->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,28 @@ 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;
+     const char *key;
+     const char *propstr = NULL;
 
      DBG("%s error %d", path, error->type);
 

@@ -118,55 +173,65 @@ 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);
+     if (!g_str_equal(lte->pending_info.apn, lte->info.apn)) {
+             g_strlcpy(lte->info.apn, lte->pending_info.apn,
+                                     OFONO_GPRS_MAX_APN_LENGTH + 1);
+             key = LTE_APN;
+             propstr = lte->info.apn;
+     } else if (lte->pending_info.proto != lte->info.proto) {
+             lte->info.proto = lte->pending_info.proto;
+             key = LTE_PROTO;
+             propstr = gprs_proto_to_string(lte->info.proto);
+     } else if (lte->pending_info.auth_method != lte->info.auth_method) {
+             lte->info.auth_method = lte->pending_info.auth_method;
+             key = LTE_AUTH_METHOD;
+             propstr = gprs_auth_method_to_string(lte->info.auth_method);
+
+     } else if (!g_str_equal(lte->pending_info.username,
+                                                     lte->info.username)) {
+             g_strlcpy(lte->info.username, lte->pending_info.username,
+                                     OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+             key = LTE_USERNAME;
+             propstr = lte->info.username;
+     } else if (!g_str_equal(lte->pending_info.password,
+                                                     lte->info.password)) {
+             g_strlcpy(lte->info.password, lte->pending_info.password,
+                                     OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+             key = LTE_PASSWORD;
+             propstr = lte->info.password;
+     } else {
+             /*
+              * this should never happen, because no property change is
+              * checked before.
+              * Neverthelss, in this case it will answer the D-Bus message
+              * but emit no signal
+              */
+             ofono_error("unexpected property change: ignored");
+             key = NULL;
+     }
+
+     reply = dbus_message_new_method_return(lte->pending);
+     __ofono_dbus_pending_reply(&lte->pending, reply);
+
+     if(!key)
+             return;
 
      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);
+             if (!*propstr)
+                     /* 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, propstr);
 
              storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
      }
 
-     reply = dbus_message_new_method_return(lte->pending);
-     __ofono_dbus_pending_reply(&lte->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);
-
-     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, &lte->pending_info,
-                                     lte_set_default_attach_info_cb, lte);
-
-     return NULL;
+                                     key,
+                                     DBUS_TYPE_STRING, &propstr);
  }
 
  static DBusMessage *lte_set_property(DBusConnection *conn,

@@ -177,6 +242,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 +265,67 @@ 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);
-     }
+     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);
+
+             lte->pending_info.proto = proto;
 
-     return __ofono_error_invalid_args(msg);
+     } 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, &lte->pending_info,
+                                     lte_set_default_attach_info_cb, lte);
+
+     return NULL;
  }
 

src/lte.c static DBusMessage *lte_set_property(DBusConnection *conn, · line 331

Quoted 1 line

src/lte.c +0 -0

  static const GDBusMethodTable lte_methods[] = {

The ofono_lte_default_attach_info now handles also the protocol and the authentication method, username and password.

Co-authored-by Martin Baschin <[email protected]>

---
  drivers/atmodem/lte.c | 133 ++++++++++++++++++++++++++++++++++++++----
  1 file changed, 123 insertions(+), 10 deletions(-)

drivers/atmodem/lte.c +37 -1

index efa4e5fe..72e0a6ef 100644
@@ -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

@@ -45,10 +46,57 @@ 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;
+};
+
+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;

Quoted 1 line

+

The above isn't wrong, but I question the need for the atomic constructs.  ofono isn't a thread-safe library.

I see that there other uses of atomic accessors in the codebase, but I believe these are remnants of a past approach with other ambitions... Denis will set me right if I'm wrong about this because this mostly predates my involvement with ofono. :)

Hi Jonas,

Quoted 9 lines

Quoted 1 line

+

The above isn't wrong, but I question the need for the atomic constructs. ofono isn't a thread-safe library.

I see that there other uses of atomic accessors in the codebase, but I believe these are remnants of a past approach with other ambitions... Denis will set me right if I'm wrong about this because this mostly predates my involvement with ofono. :)

oFono was never thread safe, and as you point out, atomic operations are not really necessary. But glib uses atomics for reference counting, so we stuck to that. Also Marcel at some point evangelized that g_atomic is the one true way (TM) of implementing reference counting.

With ell we don't use atomics at all.

So given the above, I find both approaches acceptable.

Regards,
-Denis

Hi Denis & Jonas,

Quoted 18 lines · Denis Kenzior

Hi Jonas,

Quoted 9 lines

Quoted 1 line

+

The above isn't wrong, but I question the need for the atomic constructs. ofono isn't a thread-safe library.

I see that there other uses of atomic accessors in the codebase, but I believe these are remnants of a past approach with other ambitions... Denis will set me right if I'm wrong about this because this mostly predates my involvement with ofono. :)

oFono was never thread safe, and as you point out, atomic operations are not really necessary. But glib uses atomics for reference counting, so we stuck to that. Also Marcel at some point evangelized that g_atomic is the one true way (TM) of implementing reference counting.

Quoted 6 lines

With ell we don't use atomics at all.

So given the above, I find both approaches acceptable.

Regards,
-Denis

Thank you both. I think I will keep this as it is then, and change the rest. I will re-submit the series later.

Regards,
Giacinto